Re: [PATCH v2 01/14] dt-bindings: phy: Add MIPI CSI PHY for i.MX8Q

On Mon, Feb 10, 2025 at 11:10:13PM +0200, Laurent Pinchart wrote:

On Thu, Feb 06, 2025 at 04:48:55PM -0500, Frank Li wrote:

quoted

On Thu, Feb 06, 2025 at 11:18:08PM +0200, Laurent Pinchart wrote:

quoted

On Wed, Feb 05, 2025 at 12:18:10PM -0500, Frank Li wrote:

quoted

Add MIPI CSI phy binding doc for i.MX8QXP, i.MX8QM and i.MX8ULP.

s/CSI/CSI-2/ in the subject line, here and below.
s/phy/PHY/

quoted

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v1 to v2
- Add missed fsl,imx8qm-mipi-cphy, which failback to fsl,imx8qxp-mipi-cphy
- Move reg to required. Previous 8ulp use fsl,offset in downstream version.
which should be reg. So move it to required
---
 .../bindings/phy/fsl,imx8qxp-mipi-cphy.yaml        | 57 ++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8qxp-mipi-cphy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8qxp-mipi-cphy.yaml
new file mode 100644
index 0000000000000..7335b9262d0e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8qxp-mipi-cphy.yaml

@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/fsl,imx8qxp-mipi-cphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX8 SoC MIPI CSI PHY
+
+maintainers:
+  - Frank Li <Frank.Li@nxp.com>
+
+properties:
+  "#phy-cells":
+    const: 0
+
+  compatible:
+    oneOf:
+      - enum:
+          - fsl,imx8qxp-mipi-cphy
+          - fsl,imx8ulp-mipi-cphy
+      - items:
+          - const: fsl,imx8qm-mipi-cphy
+          - const: fsl,imx8qxp-mipi-cphy

Why are those called cphy when, as far as I can tell from the
documentation, they are D-PHYs ? Does that stand for *C*SI PHY ?

There are already have D-PHYS for MIPI display phy binding. cphy just means
for camera PHY.

Ah OK. I would probably have gone for *-mipi-dphy-rx then, but I'm OK
with the proposed "cphy". Explaining this in the description would be
useful.

quoted

quoted

I find
it slightly confusing, but not so much that I'd ask for a change. It's
just a name at the end of the day.

Apart from that the binding looks fairly OK. Except maybe from the fact
that this device is not a PHY :-( It has two PHY control registers, but
the rest seems related to the glue logic at the output of the CSI-2
receiver. I wonder if we should go the syscon route.

Do you means use phandle to syscon node in csi-2 driver? Actually this
ways is not perferred by device tree team because it should be exported
as what actual function, such as PHY or RESET by use standard interface.

We met similar case at other substream.

I don't like syscon much either, but in this specific case I'm not sure
what else we could do. This device really aggregates some control over
the PHY and over the glue logic at the output of the CSI-2 controller.
Modelling it as "just a PHY" will cause problem as soon as you'll want
to configure the other parameters.

I think your comments is quite good and it's one hack. I check register
layout of csr at imx8qm/imx8qxp. It is dedicated for each csi2 moudule.
This glue layer should be second 'reg' resource, which direct control in
csi2's driver.

v3 should be simpler than this version.
https://lore.kernel.org/imx/20250210-8qxp_camera-v3-0-324f5105accc@nxp.com/T/#t (local)

Frank

quoted

quoted

quoted

+
+  reg:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - "#phy-cells"
+  - compatible
+  - reg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx8qxp-mipi-cphy
+    then:
+      required:
+        - power-domains
+
+additionalProperties: false
+
+examples:
+  - |
+    phy@58221000 {
+            compatible = "fsl,imx8qxp-mipi-cphy";
+            reg = <0x58221000 0x10000>;
+            #phy-cells = <0>;
+            power-domains = <&pd 0>;
+    };
+

--
Regards,

Laurent Pinchart

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy