From: Michael Riesch <michael.riesch@collabora.com>

The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
Add the variant and allow for the additional reset.

Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
---
 .../bindings/phy/rockchip-inno-csi-dphy.yaml       | 60 ++++++++++++++++++++--
 1 file changed, 55 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
index 5ac994b3c0aa..6755738b13ee 100644
--- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
+++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
@@ -21,6 +21,7 @@ properties:
       - rockchip,rk3326-csi-dphy
       - rockchip,rk3368-csi-dphy
       - rockchip,rk3568-csi-dphy
+      - rockchip,rk3588-csi-dphy
 
   reg:
     maxItems: 1
@@ -39,18 +40,49 @@ properties:
     maxItems: 1
 
   resets:
-    items:
-      - description: exclusive PHY reset line
+    minItems: 1
+    maxItems: 2
 
   reset-names:
-    items:
-      - const: apb
+    minItems: 1
+    maxItems: 2
 
   rockchip,grf:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
       Some additional phy settings are access through GRF regs.
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - rockchip,px30-csi-dphy
+              - rockchip,rk1808-csi-dphy
+              - rockchip,rk3326-csi-dphy
+              - rockchip,rk3368-csi-dphy
+              - rockchip,rk3568-csi-dphy
+    then:
+      properties:
+        resets:
+          items:
+            - description: exclusive PHY reset line
+
+        reset-names:
+          items:
+            - const: apb
+
+      required:
+        - reset-names
+    else:
+      properties:
+        resets:
+          minItems: 2
+
+        reset-names:
+          minItems: 2
+
 required:
   - compatible
   - reg
@@ -59,7 +91,6 @@ required:
   - '#phy-cells'
   - power-domains
   - resets
-  - reset-names
   - rockchip,grf
 
 additionalProperties: false
@@ -78,3 +109,22 @@ examples:
         reset-names = "apb";
         rockchip,grf = <&grf>;
     };
+  - |
+    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
+    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        csi_dphy0: phy@fedc0000 {
+            compatible = "rockchip,rk3588-csi-dphy";
+            reg = <0x0 0xfedc0000 0x0 0x8000>;
+            clocks = <&cru PCLK_CSIPHY0>;
+            clock-names = "pclk";
+            #phy-cells = <0>;
+            resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
+            rockchip,grf = <&csidphy0_grf>;
+            status = "disabled";
+        };
+    };

-- 
2.39.5

Hi,

I'm (unfortunately) not seeing any @rock-chips.com recipients ...

On Tue Jun 17, 2025 at 10:54 AM CEST, Michael Riesch via B4 Relay wrote:
> From: Michael Riesch <michael.riesch@collabora.com>
>
> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
> Add the variant and allow for the additional reset.
>
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> ---
>  .../bindings/phy/rockchip-inno-csi-dphy.yaml       | 60 ++++++++++++++++++++--
>  1 file changed, 55 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> index 5ac994b3c0aa..6755738b13ee 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> @@ -21,6 +21,7 @@ properties:
>        - rockchip,rk3326-csi-dphy
>        - rockchip,rk3368-csi-dphy
>        - rockchip,rk3568-csi-dphy
> +      - rockchip,rk3588-csi-dphy
>  
>    reg:
>      maxItems: 1
> @@ -39,18 +40,49 @@ properties:
>      maxItems: 1
>  
>    resets:
> -    items:
> -      - description: exclusive PHY reset line
> +    minItems: 1
> +    maxItems: 2
>  
>    reset-names:
> -    items:
> -      - const: apb
> +    minItems: 1
> +    maxItems: 2
>  
>    rockchip,grf:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description:
>        Some additional phy settings are access through GRF regs.
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - rockchip,px30-csi-dphy
> +              - rockchip,rk1808-csi-dphy
> +              - rockchip,rk3326-csi-dphy
> +              - rockchip,rk3368-csi-dphy
> +              - rockchip,rk3568-csi-dphy
> +    then:
> +      properties:
> +        resets:
> +          items:
> +            - description: exclusive PHY reset line
> +
> +        reset-names:
> +          items:
> +            - const: apb
> +
> +      required:
> +        - reset-names
> +    else:
> +      properties:
> +        resets:
> +          minItems: 2
> +
> +        reset-names:
> +          minItems: 2
> +
>  required:
>    - compatible
>    - reg
> @@ -59,7 +91,6 @@ required:
>    - '#phy-cells'
>    - power-domains
>    - resets
> -  - reset-names
>    - rockchip,grf
>  
>  additionalProperties: false
> @@ -78,3 +109,22 @@ examples:
>          reset-names = "apb";
>          rockchip,grf = <&grf>;
>      };
> +  - |
> +    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> +    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        csi_dphy0: phy@fedc0000 {
> +            compatible = "rockchip,rk3588-csi-dphy";
> +            reg = <0x0 0xfedc0000 0x0 0x8000>;
> +            clocks = <&cru PCLK_CSIPHY0>;
> +            clock-names = "pclk";
> +            #phy-cells = <0>;
> +            resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
> +            rockchip,grf = <&csidphy0_grf>;
> +            status = "disabled";
> +        };
> +    };

... which could hopefully tell us what the value is/should be for the
*required* 'power-domains' property, which is missing in this example.
IOW: the binding example is invalid according to its own binding.
(btw: you can drop the 'csi_dphy0' label)

And hopefully also for rk3568 so we can add it to rk356x-base.dtsi and
you can add it in patch 5 where it's also missing.

Grepping for "csi-dphy" in arch/arm*/boot/dts/rockchip returns:
- px30.dtsi
- rk356x-base.dtsi

With this patch set applied, we'd have a 3rd result: rk3588-base.dtsi

For all the listed compatibles, it's only actually defined in px30.dtsi.

Cheers (and sorry),
  Diederik

Hi Diederik,

Thanks for your comments!

On 6/17/25 16:12, Diederik de Haas wrote:
> Hi,
> 
> I'm (unfortunately) not seeing any @rock-chips.com recipients ...

Oops, I meant to include at least Kever, but forgot to do it. Will do in v2.

Cc: Kever

> 
> On Tue Jun 17, 2025 at 10:54 AM CEST, Michael Riesch via B4 Relay wrote:
>> From: Michael Riesch <michael.riesch@collabora.com>
>>
>> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
>> Add the variant and allow for the additional reset.
>>
>> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
>> ---
>>  .../bindings/phy/rockchip-inno-csi-dphy.yaml       | 60 ++++++++++++++++++++--
>>  1 file changed, 55 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> index 5ac994b3c0aa..6755738b13ee 100644
>> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> @@ -21,6 +21,7 @@ properties:
>>        - rockchip,rk3326-csi-dphy
>>        - rockchip,rk3368-csi-dphy
>>        - rockchip,rk3568-csi-dphy
>> +      - rockchip,rk3588-csi-dphy
>>  
>>    reg:
>>      maxItems: 1
>> @@ -39,18 +40,49 @@ properties:
>>      maxItems: 1
>>  
>>    resets:
>> -    items:
>> -      - description: exclusive PHY reset line
>> +    minItems: 1
>> +    maxItems: 2
>>  
>>    reset-names:
>> -    items:
>> -      - const: apb
>> +    minItems: 1
>> +    maxItems: 2
>>  
>>    rockchip,grf:
>>      $ref: /schemas/types.yaml#/definitions/phandle
>>      description:
>>        Some additional phy settings are access through GRF regs.
>>  
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - rockchip,px30-csi-dphy
>> +              - rockchip,rk1808-csi-dphy
>> +              - rockchip,rk3326-csi-dphy
>> +              - rockchip,rk3368-csi-dphy
>> +              - rockchip,rk3568-csi-dphy
>> +    then:
>> +      properties:
>> +        resets:
>> +          items:
>> +            - description: exclusive PHY reset line
>> +
>> +        reset-names:
>> +          items:
>> +            - const: apb
>> +
>> +      required:
>> +        - reset-names
>> +    else:
>> +      properties:
>> +        resets:
>> +          minItems: 2
>> +
>> +        reset-names:
>> +          minItems: 2
>> +
>>  required:
>>    - compatible
>>    - reg
>> @@ -59,7 +91,6 @@ required:
>>    - '#phy-cells'
>>    - power-domains
>>    - resets
>> -  - reset-names
>>    - rockchip,grf
>>  
>>  additionalProperties: false
>> @@ -78,3 +109,22 @@ examples:
>>          reset-names = "apb";
>>          rockchip,grf = <&grf>;
>>      };
>> +  - |
>> +    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
>> +    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
>> +
>> +    soc {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        csi_dphy0: phy@fedc0000 {
>> +            compatible = "rockchip,rk3588-csi-dphy";
>> +            reg = <0x0 0xfedc0000 0x0 0x8000>;
>> +            clocks = <&cru PCLK_CSIPHY0>;
>> +            clock-names = "pclk";
>> +            #phy-cells = <0>;
>> +            resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
>> +            rockchip,grf = <&csidphy0_grf>;
>> +            status = "disabled";
>> +        };
>> +    };
> 
> ... which could hopefully tell us what the value is/should be for the
> *required* 'power-domains' property, which is missing in this example.
> IOW: the binding example is invalid according to its own binding.

Huh, indeed. Hm. Why didn't make dt_binding_check warn me about that?!

TRM Part 1, p. 1097 states that HDMI_CSI_DPHY is in the ALIVE(PD_BUS)
power domain. With some creativity one can interpret that the CSI DPHY
is always on anyways. @Kever: Could you please elaborate on that?

> (btw: you can drop the 'csi_dphy0' label)

Will do.

> 
> And hopefully also for rk3568 so we can add it to rk356x-base.dtsi and
> you can add it in patch 5 where it's also missing.

I recall a similar discussion [0]. In the RK3568 the CSIPHY is in the
ALIVE power domain.

Note that the PHY must not be confused with the HOST blocks, which are
the MIPI CSI-2 receivers.

I guess the correct solution is to make power-domains optional. Further
input welcome, though.

Best regards,
Michael

> 
> Grepping for "csi-dphy" in arch/arm*/boot/dts/rockchip returns:
> - px30.dtsi
> - rk356x-base.dtsi
> 
> With this patch set applied, we'd have a 3rd result: rk3588-base.dtsi
> 
> For all the listed compatibles, it's only actually defined in px30.dtsi.
> 
> Cheers (and sorry),
>   Diederik

[0] https://lore.kernel.org/all/D4QNJ85V43NU.YD01E8AB4116@cknow.org/

On Wed, Jun 18, 2025 at 09:45:32AM +0200, Michael Riesch wrote:
> Hi Diederik,
> 
> Thanks for your comments!
> 
> On 6/17/25 16:12, Diederik de Haas wrote:
> > Hi,
> > 
> > I'm (unfortunately) not seeing any @rock-chips.com recipients ...
> 
> Oops, I meant to include at least Kever, but forgot to do it. Will do in v2.
> 
> Cc: Kever
> 
> > 
> > On Tue Jun 17, 2025 at 10:54 AM CEST, Michael Riesch via B4 Relay wrote:
> >> From: Michael Riesch <michael.riesch@collabora.com>
> >>
> >> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
> >> Add the variant and allow for the additional reset.
> >>
> >> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> >> ---
> >>  .../bindings/phy/rockchip-inno-csi-dphy.yaml       | 60 ++++++++++++++++++++--
> >>  1 file changed, 55 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> >> index 5ac994b3c0aa..6755738b13ee 100644
> >> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> >> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> >> @@ -21,6 +21,7 @@ properties:
> >>        - rockchip,rk3326-csi-dphy
> >>        - rockchip,rk3368-csi-dphy
> >>        - rockchip,rk3568-csi-dphy
> >> +      - rockchip,rk3588-csi-dphy
> >>  
> >>    reg:
> >>      maxItems: 1
> >> @@ -39,18 +40,49 @@ properties:
> >>      maxItems: 1
> >>  
> >>    resets:
> >> -    items:
> >> -      - description: exclusive PHY reset line
> >> +    minItems: 1
> >> +    maxItems: 2
> >>  
> >>    reset-names:
> >> -    items:
> >> -      - const: apb
> >> +    minItems: 1
> >> +    maxItems: 2
> >>  
> >>    rockchip,grf:
> >>      $ref: /schemas/types.yaml#/definitions/phandle
> >>      description:
> >>        Some additional phy settings are access through GRF regs.
> >>  
> >> +allOf:
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            enum:
> >> +              - rockchip,px30-csi-dphy
> >> +              - rockchip,rk1808-csi-dphy
> >> +              - rockchip,rk3326-csi-dphy
> >> +              - rockchip,rk3368-csi-dphy
> >> +              - rockchip,rk3568-csi-dphy
> >> +    then:
> >> +      properties:
> >> +        resets:
> >> +          items:
> >> +            - description: exclusive PHY reset line
> >> +
> >> +        reset-names:
> >> +          items:
> >> +            - const: apb
> >> +
> >> +      required:
> >> +        - reset-names
> >> +    else:
> >> +      properties:
> >> +        resets:
> >> +          minItems: 2
> >> +
> >> +        reset-names:
> >> +          minItems: 2

You have to define the names. Ideally, at the top level and then keep 
this part like this.

> >> +
> >>  required:
> >>    - compatible
> >>    - reg
> >> @@ -59,7 +91,6 @@ required:
> >>    - '#phy-cells'
> >>    - power-domains
> >>    - resets
> >> -  - reset-names
> >>    - rockchip,grf
> >>  
> >>  additionalProperties: false
> >> @@ -78,3 +109,22 @@ examples:
> >>          reset-names = "apb";
> >>          rockchip,grf = <&grf>;
> >>      };
> >> +  - |
> >> +    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> >> +    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> >> +
> >> +    soc {
> >> +        #address-cells = <2>;
> >> +        #size-cells = <2>;
> >> +
> >> +        csi_dphy0: phy@fedc0000 {
> >> +            compatible = "rockchip,rk3588-csi-dphy";
> >> +            reg = <0x0 0xfedc0000 0x0 0x8000>;
> >> +            clocks = <&cru PCLK_CSIPHY0>;
> >> +            clock-names = "pclk";
> >> +            #phy-cells = <0>;
> >> +            resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
> >> +            rockchip,grf = <&csidphy0_grf>;
> >> +            status = "disabled";
> >> +        };
> >> +    };
> > 
> > ... which could hopefully tell us what the value is/should be for the
> > *required* 'power-domains' property, which is missing in this example.
> > IOW: the binding example is invalid according to its own binding.
> 
> Huh, indeed. Hm. Why didn't make dt_binding_check warn me about that?!

You disabled the node, what do you want us to check?

Rob

On 17/06/2025 10:54, Michael Riesch via B4 Relay wrote:
> From: Michael Riesch <michael.riesch@collabora.com>
> 
> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
> Add the variant and allow for the additional reset.

No names for the new resets on the RK3588 ?

Neil

> 
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> ---
>   .../bindings/phy/rockchip-inno-csi-dphy.yaml       | 60 ++++++++++++++++++++--
>   1 file changed, 55 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> index 5ac994b3c0aa..6755738b13ee 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> @@ -21,6 +21,7 @@ properties:
>         - rockchip,rk3326-csi-dphy
>         - rockchip,rk3368-csi-dphy
>         - rockchip,rk3568-csi-dphy
> +      - rockchip,rk3588-csi-dphy
>   
>     reg:
>       maxItems: 1
> @@ -39,18 +40,49 @@ properties:
>       maxItems: 1
>   
>     resets:
> -    items:
> -      - description: exclusive PHY reset line
> +    minItems: 1
> +    maxItems: 2
>   
>     reset-names:
> -    items:
> -      - const: apb
> +    minItems: 1
> +    maxItems: 2
>   
>     rockchip,grf:
>       $ref: /schemas/types.yaml#/definitions/phandle
>       description:
>         Some additional phy settings are access through GRF regs.
>   
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - rockchip,px30-csi-dphy
> +              - rockchip,rk1808-csi-dphy
> +              - rockchip,rk3326-csi-dphy
> +              - rockchip,rk3368-csi-dphy
> +              - rockchip,rk3568-csi-dphy
> +    then:
> +      properties:
> +        resets:
> +          items:
> +            - description: exclusive PHY reset line
> +
> +        reset-names:
> +          items:
> +            - const: apb
> +
> +      required:
> +        - reset-names
> +    else:
> +      properties:
> +        resets:
> +          minItems: 2
> +
> +        reset-names:
> +          minItems: 2
> +
>   required:
>     - compatible
>     - reg
> @@ -59,7 +91,6 @@ required:
>     - '#phy-cells'
>     - power-domains
>     - resets
> -  - reset-names
>     - rockchip,grf
>   
>   additionalProperties: false
> @@ -78,3 +109,22 @@ examples:
>           reset-names = "apb";
>           rockchip,grf = <&grf>;
>       };
> +  - |
> +    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> +    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        csi_dphy0: phy@fedc0000 {
> +            compatible = "rockchip,rk3588-csi-dphy";
> +            reg = <0x0 0xfedc0000 0x0 0x8000>;
> +            clocks = <&cru PCLK_CSIPHY0>;
> +            clock-names = "pclk";
> +            #phy-cells = <0>;
> +            resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
> +            rockchip,grf = <&csidphy0_grf>;
> +            status = "disabled";
> +        };
> +    };
>

Hi Neil,

Thanks for your comments!

On 6/17/25 11:31, neil.armstrong@linaro.org wrote:
> On 17/06/2025 10:54, Michael Riesch via B4 Relay wrote:
>> From: Michael Riesch <michael.riesch@collabora.com>
>>
>> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
>> Add the variant and allow for the additional reset.
> 
> No names for the new resets on the RK3588 ?

I left the names away because TBH I don't see the value in them (in that
case).

Downstream uses reset-names = "srst_csiphy0", "srst_p_csiphy0"; and
there is no better description. One could guess that the second reset
corresponds to "apb" but this is just guessing and we would still have
to guess/find a proper name for the first reset.

Amazingly the mainline driver does not seem to do anything with the
resets (unless I overlooked some implicit magic). Downstream does a
simple reset_control_{assert,deassert} before configuring the PHY. Now
if the different resets are handled in bulk mode, does it really make
sense to address each reset individually?

Best regards,
Michael

> 
> Neil
> 
>>
>> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
>> ---
>>   .../bindings/phy/rockchip-inno-csi-dphy.yaml       | 60 ++++++++++++
>> ++++++++--
>>   1 file changed, 55 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-
>> dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-
>> dphy.yaml
>> index 5ac994b3c0aa..6755738b13ee 100644
>> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
>> @@ -21,6 +21,7 @@ properties:
>>         - rockchip,rk3326-csi-dphy
>>         - rockchip,rk3368-csi-dphy
>>         - rockchip,rk3568-csi-dphy
>> +      - rockchip,rk3588-csi-dphy
>>       reg:
>>       maxItems: 1
>> @@ -39,18 +40,49 @@ properties:
>>       maxItems: 1
>>       resets:
>> -    items:
>> -      - description: exclusive PHY reset line
>> +    minItems: 1
>> +    maxItems: 2
>>       reset-names:
>> -    items:
>> -      - const: apb
>> +    minItems: 1
>> +    maxItems: 2
>>       rockchip,grf:
>>       $ref: /schemas/types.yaml#/definitions/phandle
>>       description:
>>         Some additional phy settings are access through GRF regs.
>>   +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - rockchip,px30-csi-dphy
>> +              - rockchip,rk1808-csi-dphy
>> +              - rockchip,rk3326-csi-dphy
>> +              - rockchip,rk3368-csi-dphy
>> +              - rockchip,rk3568-csi-dphy
>> +    then:
>> +      properties:
>> +        resets:
>> +          items:
>> +            - description: exclusive PHY reset line
>> +
>> +        reset-names:
>> +          items:
>> +            - const: apb
>> +
>> +      required:
>> +        - reset-names
>> +    else:
>> +      properties:
>> +        resets:
>> +          minItems: 2
>> +
>> +        reset-names:
>> +          minItems: 2
>> +
>>   required:
>>     - compatible
>>     - reg
>> @@ -59,7 +91,6 @@ required:
>>     - '#phy-cells'
>>     - power-domains
>>     - resets
>> -  - reset-names
>>     - rockchip,grf
>>     additionalProperties: false
>> @@ -78,3 +109,22 @@ examples:
>>           reset-names = "apb";
>>           rockchip,grf = <&grf>;
>>       };
>> +  - |
>> +    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
>> +    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
>> +
>> +    soc {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        csi_dphy0: phy@fedc0000 {
>> +            compatible = "rockchip,rk3588-csi-dphy";
>> +            reg = <0x0 0xfedc0000 0x0 0x8000>;
>> +            clocks = <&cru PCLK_CSIPHY0>;
>> +            clock-names = "pclk";
>> +            #phy-cells = <0>;
>> +            resets = <&cru SRST_CSIPHY0>, <&cru SRST_P_CSIPHY0>;
>> +            rockchip,grf = <&csidphy0_grf>;
>> +            status = "disabled";
>> +        };
>> +    };
>>
> 

Am Mittwoch, 18. Juni 2025, 08:32:25 Mitteleuropäische Sommerzeit schrieb Michael Riesch:
> Hi Neil,
> 
> Thanks for your comments!
> 
> On 6/17/25 11:31, neil.armstrong@linaro.org wrote:
> > On 17/06/2025 10:54, Michael Riesch via B4 Relay wrote:
> >> From: Michael Riesch <michael.riesch@collabora.com>
> >>
> >> The Rockchip RK3588 variant of the CSI-2 DPHY features two reset lines.
> >> Add the variant and allow for the additional reset.
> > 
> > No names for the new resets on the RK3588 ?
> 
> I left the names away because TBH I don't see the value in them (in that
> case).
> 
> Downstream uses reset-names = "srst_csiphy0", "srst_p_csiphy0"; and
> there is no better description. One could guess that the second reset
> corresponds to "apb" but this is just guessing and we would still have
> to guess/find a proper name for the first reset.
> 
> Amazingly the mainline driver does not seem to do anything with the
> resets (unless I overlooked some implicit magic). Downstream does a
> simple reset_control_{assert,deassert} before configuring the PHY. Now
> if the different resets are handled in bulk mode, does it really make
> sense to address each reset individually?

it might not make sense now, but possibly in the future?

A binding and the attached devicetrees are meant to be "forever", i.e. a
new kernel _should_ support all those old devicetrees you throw at it - 
if they conform to (at some time) established bindings.

So while all drivers might not need the specific resets now, you don't
know what quirks you'll have discovered in two years ;-)

And instead of trying to update the binding and then carrying both the
new and the fallback code for the old binding around, why not do it now.

Then when you find a need for a specific reset, things magically just work.


Heiko