devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
@ 2025-05-13 14:39 Vladimir Zapolskiy
  2025-05-13 14:48 ` Conor Dooley
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Vladimir Zapolskiy @ 2025-05-13 14:39 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
	Bryan O'Donoghue, Mauro Carvalho Chehab, Hans Verkuil
  Cc: linux-media, linux-arm-msm, linux-phy, devicetree

Add dt-binding schema for the CAMSS CSIPHY IPs, which provides
MIPI C-/D-PHY interfaces on Qualcomm SoCs.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 .../devicetree/bindings/phy/qcom,csiphy.yaml  | 110 ++++++++++++++++++
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,csiphy.yaml

diff --git a/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
new file mode 100644
index 000000000000..ef712c5442ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,csiphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm CSI PHY
+
+maintainers:
+  - Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
+
+description: |
+  Qualcomm SoCs equipped with a number of MIPI CSI PHY IPs, which
+  supports D-PHY or C-PHY interfaces to camera sensors.
+
+properties:
+  compatible:
+    enum:
+      - qcom,sm8250-csiphy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 2
+
+  interrupts:
+    maxItems: 1
+
+  vdda-csi-0p9-supply:
+    description: Voltage supply, 0.9V
+
+  vdda-csi-1p2-supply:
+    description: Voltage supply, 1.2V
+
+  '#phy-cells':
+    const: 0
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    description: CAMSS CSIPHY input port
+
+    patternProperties:
+      "^endpoint@[0-1]$":
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            minItems: 1
+            maxItems: 4
+
+          bus-type:
+            enum:
+              - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
+              - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
+
+        required:
+          - data-lanes
+
+    oneOf:
+      - required:
+          - endpoint
+      - required:
+          - endpoint@0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+  - '#phy-cells'
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sm8250-csiphy
+    then:
+      required:
+        - vdda-csi-0p9-supply
+        - vdda-csi-1p2-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,camcc-sm8250.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    phy@ac6e000 {
+      compatible = "qcom,sm8250-csiphy";
+      reg = <0x0ac6e000 0x1000>;
+      clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
+               <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
+      interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
+      vdda-csi-0p9-supply = <&vreg_l5a_0p88>;
+      vdda-csi-1p2-supply = <&vreg_l9a_1p2>;
+      #phy-cells = <0>;
+
+      port {
+        csiphy_in: endpoint {
+          data-lanes = <1 2 3 4>;
+          remote-endpoint = <&sensor_out>;
+        };
+      };
+    };
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-05-13 14:39 [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
@ 2025-05-13 14:48 ` Conor Dooley
  2025-05-13 15:31 ` Rob Herring (Arm)
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2025-05-13 14:48 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
	Bryan O'Donoghue, Mauro Carvalho Chehab, Hans Verkuil,
	linux-media, linux-arm-msm, linux-phy, devicetree

[-- Attachment #1: Type: text/plain, Size: 3647 bytes --]

On Tue, May 13, 2025 at 05:39:18PM +0300, Vladimir Zapolskiy wrote:
> Add dt-binding schema for the CAMSS CSIPHY IPs, which provides
> MIPI C-/D-PHY interfaces on Qualcomm SoCs.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

RFC but nothing here about what in particular is RFC about this patch.
What specifically are you looking for comments about?

> ---
>  .../devicetree/bindings/phy/qcom,csiphy.yaml  | 110 ++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> new file mode 100644
> index 000000000000..ef712c5442ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,csiphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm CSI PHY
> +
> +maintainers:
> +  - Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> +
> +description: |
> +  Qualcomm SoCs equipped with a number of MIPI CSI PHY IPs, which
> +  supports D-PHY or C-PHY interfaces to camera sensors.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sm8250-csiphy
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  vdda-csi-0p9-supply:
> +    description: Voltage supply, 0.9V
> +
> +  vdda-csi-1p2-supply:
> +    description: Voltage supply, 1.2V
> +
> +  '#phy-cells':
> +    const: 0
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    description: CAMSS CSIPHY input port
> +
> +    patternProperties:
> +      "^endpoint@[0-1]$":
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            minItems: 1
> +            maxItems: 4
> +
> +          bus-type:
> +            enum:
> +              - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> +              - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
> +
> +        required:
> +          - data-lanes
> +
> +    oneOf:
> +      - required:
> +          - endpoint
> +      - required:
> +          - endpoint@0
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - interrupts
> +  - '#phy-cells'
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8250-csiphy
> +    then:
> +      required:
> +        - vdda-csi-0p9-supply
> +        - vdda-csi-1p2-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,camcc-sm8250.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    phy@ac6e000 {
> +      compatible = "qcom,sm8250-csiphy";
> +      reg = <0x0ac6e000 0x1000>;
> +      clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
> +               <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
> +      interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
> +      vdda-csi-0p9-supply = <&vreg_l5a_0p88>;
> +      vdda-csi-1p2-supply = <&vreg_l9a_1p2>;
> +      #phy-cells = <0>;
> +
> +      port {
> +        csiphy_in: endpoint {
> +          data-lanes = <1 2 3 4>;
> +          remote-endpoint = <&sensor_out>;
> +        };
> +      };
> +    };
> -- 
> 2.45.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-05-13 14:39 [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
  2025-05-13 14:48 ` Conor Dooley
@ 2025-05-13 15:31 ` Rob Herring (Arm)
  2025-05-14  0:59 ` Dmitry Baryshkov
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Rob Herring (Arm) @ 2025-05-13 15:31 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Kishon Vijay Abraham I, Mauro Carvalho Chehab, Hans Verkuil,
	Krzysztof Kozlowski, Robert Foss, devicetree, linux-arm-msm,
	linux-phy, Todor Tomov, Conor Dooley, linux-media,
	Bryan O'Donoghue, Vinod Koul


On Tue, 13 May 2025 17:39:18 +0300, Vladimir Zapolskiy wrote:
> Add dt-binding schema for the CAMSS CSIPHY IPs, which provides
> MIPI C-/D-PHY interfaces on Qualcomm SoCs.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  .../devicetree/bindings/phy/qcom,csiphy.yaml  | 110 ++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml: port: Missing additionalProperties/unevaluatedProperties constraint

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250513143918.2572689-1-vladimir.zapolskiy@linaro.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-05-13 14:39 [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
  2025-05-13 14:48 ` Conor Dooley
  2025-05-13 15:31 ` Rob Herring (Arm)
@ 2025-05-14  0:59 ` Dmitry Baryshkov
  2025-05-14  8:17 ` Vinod Koul
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Dmitry Baryshkov @ 2025-05-14  0:59 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Robert Foss, Todor Tomov,
	Bryan O'Donoghue, Mauro Carvalho Chehab, Hans Verkuil,
	linux-media, linux-arm-msm, linux-phy, devicetree

On Tue, May 13, 2025 at 05:39:18PM +0300, Vladimir Zapolskiy wrote:
> Add dt-binding schema for the CAMSS CSIPHY IPs, which provides
> MIPI C-/D-PHY interfaces on Qualcomm SoCs.

Are these currently a part of the main camss block? How do you plan to
handle backwards compatibility?

> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  .../devicetree/bindings/phy/qcom,csiphy.yaml  | 110 ++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-05-13 14:39 [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
                   ` (2 preceding siblings ...)
  2025-05-14  0:59 ` Dmitry Baryshkov
@ 2025-05-14  8:17 ` Vinod Koul
  2025-05-14 10:25 ` Krzysztof Kozlowski
  2025-05-14 10:29 ` Bryan O'Donoghue
  5 siblings, 0 replies; 9+ messages in thread
From: Vinod Koul @ 2025-05-14  8:17 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Robert Foss, Todor Tomov, Bryan O'Donoghue,
	Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-arm-msm,
	linux-phy, devicetree

On 13-05-25, 17:39, Vladimir Zapolskiy wrote:
> Add dt-binding schema for the CAMSS CSIPHY IPs, which provides
> MIPI C-/D-PHY interfaces on Qualcomm SoCs.

This should come with driver support as well. pls post driver as well
with it

-- 
~Vinod

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-05-13 14:39 [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
                   ` (3 preceding siblings ...)
  2025-05-14  8:17 ` Vinod Koul
@ 2025-05-14 10:25 ` Krzysztof Kozlowski
  2025-05-14 19:30   ` Vladimir Zapolskiy
  2025-05-14 10:29 ` Bryan O'Donoghue
  5 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-14 10:25 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Robert Foss,
	Todor Tomov, Bryan O'Donoghue, Mauro Carvalho Chehab,
	Hans Verkuil
  Cc: linux-media, linux-arm-msm, linux-phy, devicetree

On 13/05/2025 16:39, Vladimir Zapolskiy wrote:
> Add dt-binding schema for the CAMSS CSIPHY IPs, which provides
> MIPI C-/D-PHY interfaces on Qualcomm SoCs.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  .../devicetree/bindings/phy/qcom,csiphy.yaml  | 110 ++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom,csiphy.yaml


Looks like not tested, so limited review follows.

Filename matching compatible.

> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> new file mode 100644
> index 000000000000..ef712c5442ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml

Please post the driver or any other user. Or explain why this is RFC or
what you expect here from us.


> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,csiphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm CSI PHY

SM8250 ?

> +
> +maintainers:
> +  - Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  Qualcomm SoCs equipped with a number of MIPI CSI PHY IPs, which
> +  supports D-PHY or C-PHY interfaces to camera sensors.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sm8250-csiphy
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2

Need to list the items instead

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  vdda-csi-0p9-supply:
> +    description: Voltage supply, 0.9V
> +
> +  vdda-csi-1p2-supply:
> +    description: Voltage supply, 1.2V
> +
> +  '#phy-cells':
> +    const: 0
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    description: CAMSS CSIPHY input port
> +
> +    patternProperties:
> +      "^endpoint@[0-1]$":

Keep consistent quotes, either " or '

> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            minItems: 1
> +            maxItems: 4
> +
> +          bus-type:
> +            enum:
> +              - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> +              - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
> +
> +        required:
> +          - data-lanes
> +
> +    oneOf:
> +      - required:
> +          - endpoint
> +      - required:
> +          - endpoint@0
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - interrupts
> +  - '#phy-cells'
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8250-csiphy
> +    then:
> +      required:
> +        - vdda-csi-0p9-supply
> +        - vdda-csi-1p2-supply

This makes no sense - it is only sm8250 - so this if is always true.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-05-13 14:39 [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
                   ` (4 preceding siblings ...)
  2025-05-14 10:25 ` Krzysztof Kozlowski
@ 2025-05-14 10:29 ` Bryan O'Donoghue
  5 siblings, 0 replies; 9+ messages in thread
From: Bryan O'Donoghue @ 2025-05-14 10:29 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Robert Foss,
	Todor Tomov, Mauro Carvalho Chehab, Hans Verkuil
  Cc: linux-media, linux-arm-msm, linux-phy, devicetree

On 13/05/2025 15:39, Vladimir Zapolskiy wrote:
> Add dt-binding schema for the CAMSS CSIPHY IPs, which provides
> MIPI C-/D-PHY interfaces on Qualcomm SoCs.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>   .../devicetree/bindings/phy/qcom,csiphy.yaml  | 110 ++++++++++++++++++
>   1 file changed, 110 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> new file mode 100644
> index 000000000000..ef712c5442ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,csiphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm CSI PHY
> +
> +maintainers:
> +  - Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> +
> +description: |
> +  Qualcomm SoCs equipped with a number of MIPI CSI PHY IPs, which
> +  supports D-PHY or C-PHY interfaces to camera sensors.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sm8250-csiphy
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  vdda-csi-0p9-supply:
> +    description: Voltage supply, 0.9V
> +
> +  vdda-csi-1p2-supply:
> +    description: Voltage supply, 1.2V
> +
> +  '#phy-cells':
> +    const: 0
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    description: CAMSS CSIPHY input port
> +
> +    patternProperties:
> +      "^endpoint@[0-1]$":
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            minItems: 1
> +            maxItems: 4
> +
> +          bus-type:
> +            enum:
> +              - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> +              - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
> +
> +        required:
> +          - data-lanes
> +
> +    oneOf:
> +      - required:
> +          - endpoint
> +      - required:
> +          - endpoint@0
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - interrupts
> +  - '#phy-cells'
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8250-csiphy
> +    then:
> +      required:
> +        - vdda-csi-0p9-supply
> +        - vdda-csi-1p2-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,camcc-sm8250.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    phy@ac6e000 {
> +      compatible = "qcom,sm8250-csiphy";
> +      reg = <0x0ac6e000 0x1000>;
> +      clocks = <&camcc CAM_CC_CSIPHY2_CLK>,
> +               <&camcc CAM_CC_CSI2PHYTIMER_CLK>;
> +      interrupts = <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>;
> +      vdda-csi-0p9-supply = <&vreg_l5a_0p88>;
> +      vdda-csi-1p2-supply = <&vreg_l9a_1p2>;
> +      #phy-cells = <0>;
> +
> +      port {
> +        csiphy_in: endpoint {
> +          data-lanes = <1 2 3 4>;
> +          remote-endpoint = <&sensor_out>;
> +        };
> +      };
> +    };

I have something similar in the csiphy rewrite I've been doing.

Lets sync up IRL to discuss.

---
bod

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-05-14 10:25 ` Krzysztof Kozlowski
@ 2025-05-14 19:30   ` Vladimir Zapolskiy
  2025-05-15  9:16     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 9+ messages in thread
From: Vladimir Zapolskiy @ 2025-05-14 19:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Robert Foss,
	Todor Tomov, Bryan O'Donoghue, Mauro Carvalho Chehab,
	Hans Verkuil
  Cc: linux-media, linux-arm-msm, linux-phy, devicetree

Hello Krzysztof.

On 5/14/25 13:25, Krzysztof Kozlowski wrote:
> On 13/05/2025 16:39, Vladimir Zapolskiy wrote:
>> Add dt-binding schema for the CAMSS CSIPHY IPs, which provides
>> MIPI C-/D-PHY interfaces on Qualcomm SoCs.
>>
>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>>   .../devicetree/bindings/phy/qcom,csiphy.yaml  | 110 ++++++++++++++++++
>>   1 file changed, 110 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> 
> 
> Looks like not tested, so limited review follows.
> 
> Filename matching compatible.
> 

Thank you for the review, the change is deliberately tagged as RFC.

I read this review comment as the displayed generic compatible 'qcom,csiphy'
shall be added to the list of compatibles.

>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
>> new file mode 100644
>> index 000000000000..ef712c5442ec
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
> 
> Please post the driver or any other user. Or explain why this is RFC or
> what you expect here from us.
> 

The CSIPHY driver agnostic CAMSS changes are on the linux-media list [1], the CSIPHY
driver specific changes will be added on top of these changes, however I believe
it makes sense to review these two different CAMSS changesets independently.

Here the RFC tag is given explicitly to get change reviews for the dt binding
documentation part, and the first user is the example embedded into the change.

>> @@ -0,0 +1,110 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/phy/qcom,csiphy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm CSI PHY
> 
> SM8250 ?
> 

It's supposed to be a generic device tree binding, and it covers SM8250
CAMSS CSIPHY IP as well, which could be quite handly for testing/review.

>> +
>> +maintainers:
>> +  - Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> +
>> +description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 

Ack.

>> +  Qualcomm SoCs equipped with a number of MIPI CSI PHY IPs, which
>> +  supports D-PHY or C-PHY interfaces to camera sensors.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,sm8250-csiphy
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 2
> 
> Need to list the items instead
> 

Ack.

>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  vdda-csi-0p9-supply:
>> +    description: Voltage supply, 0.9V
>> +
>> +  vdda-csi-1p2-supply:
>> +    description: Voltage supply, 1.2V
>> +
>> +  '#phy-cells':
>> +    const: 0
>> +
>> +  port:
>> +    $ref: /schemas/graph.yaml#/$defs/port-base
>> +    description: CAMSS CSIPHY input port
>> +
>> +    patternProperties:
>> +      "^endpoint@[0-1]$":
> 
> Keep consistent quotes, either " or '
> 

Ack.

>> +        $ref: /schemas/media/video-interfaces.yaml#
>> +        unevaluatedProperties: false
>> +
>> +        properties:
>> +          data-lanes:
>> +            minItems: 1
>> +            maxItems: 4
>> +
>> +          bus-type:
>> +            enum:
>> +              - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
>> +              - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
>> +
>> +        required:
>> +          - data-lanes
>> +
>> +    oneOf:
>> +      - required:
>> +          - endpoint
>> +      - required:
>> +          - endpoint@0
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - interrupts
>> +  - '#phy-cells'
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sm8250-csiphy
>> +    then:
>> +      required:
>> +        - vdda-csi-0p9-supply
>> +        - vdda-csi-1p2-supply
> 
> This makes no sense - it is only sm8250 - so this if is always true.
> 

Ack, thank you for the review comments.

--
Best wishes,
Vladimir

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs
  2025-05-14 19:30   ` Vladimir Zapolskiy
@ 2025-05-15  9:16     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-15  9:16 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Robert Foss,
	Todor Tomov, Bryan O'Donoghue, Mauro Carvalho Chehab,
	Hans Verkuil
  Cc: linux-media, linux-arm-msm, linux-phy, devicetree

On 14/05/2025 21:30, Vladimir Zapolskiy wrote:
> Hello Krzysztof.
> 
> On 5/14/25 13:25, Krzysztof Kozlowski wrote:
>> On 13/05/2025 16:39, Vladimir Zapolskiy wrote:
>>> Add dt-binding schema for the CAMSS CSIPHY IPs, which provides
>>> MIPI C-/D-PHY interfaces on Qualcomm SoCs.
>>>
>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>> ---
>>>   .../devicetree/bindings/phy/qcom,csiphy.yaml  | 110 ++++++++++++++++++
>>>   1 file changed, 110 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
>>
>>
>> Looks like not tested, so limited review follows.
>>
>> Filename matching compatible.
>>
> 
> Thank you for the review, the change is deliberately tagged as RFC.
> 
> I read this review comment as the displayed generic compatible 'qcom,csiphy'
> shall be added to the list of compatibles.

No. The comment is about filename. You must rename the filename to match
the compatible. How this could mean anything else?

> 
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
>>> new file mode 100644
>>> index 000000000000..ef712c5442ec
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/qcom,csiphy.yaml
>>
>> Please post the driver or any other user. Or explain why this is RFC or
>> what you expect here from us.
>>
> 
> The CSIPHY driver agnostic CAMSS changes are on the linux-media list [1], the CSIPHY
> driver specific changes will be added on top of these changes, however I believe
> it makes sense to review these two different CAMSS changesets independently.

Do not introduce your own rules. It is ALWAYS expected to post binding
and its driver user together.

> 
> Here the RFC tag is given explicitly to get change reviews for the dt binding
> documentation part, and the first user is the example embedded into the change.
> 
>>> @@ -0,0 +1,110 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/phy/qcom,csiphy.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm CSI PHY
>>
>> SM8250 ?
>>
> 
> It's supposed to be a generic device tree binding, and it covers SM8250
> CAMSS CSIPHY IP as well, which could be quite handly for testing/review.

It's not generic. It's specific to SM8250.



Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-05-15  9:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-13 14:39 [RFC PATCH] dt-bindings: phy: Add Qualcomm MIPI C-/D-PHY schema for CSIPHY IPs Vladimir Zapolskiy
2025-05-13 14:48 ` Conor Dooley
2025-05-13 15:31 ` Rob Herring (Arm)
2025-05-14  0:59 ` Dmitry Baryshkov
2025-05-14  8:17 ` Vinod Koul
2025-05-14 10:25 ` Krzysztof Kozlowski
2025-05-14 19:30   ` Vladimir Zapolskiy
2025-05-15  9:16     ` Krzysztof Kozlowski
2025-05-14 10:29 ` Bryan O'Donoghue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).