[PATCH] dt-bindings: phy: samsung,ufs-phy: match clock items

[PATCH] dt-bindings: phy: samsung,ufs-phy: match clock items

[Chanho Park]

Below error is detected from dtbs_check. exynos7-ufs-phy is required
symbol clocks otherwise only PLL ref clock is required.

clock-names: ['ref_clk'] is too short

Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Chanho Park <chanho61.park@samsung.com>
---
 .../bindings/phy/samsung,ufs-phy.yaml         | 47 +++++++++++++++----
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
index 8da99461e817..3b04f31d9f21 100644
--- a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
@@ -27,18 +27,12 @@ properties:
       - const: phy-pma
 
   clocks:
-    items:
-      - description: PLL reference clock
-      - description: symbol clock for input symbol ( rx0-ch0 symbol clock)
-      - description: symbol clock for input symbol ( rx1-ch1 symbol clock)
-      - description: symbol clock for output symbol ( tx0 symbol clock)
+    minItems: 1
+    maxItems: 4
 
   clock-names:
-    items:
-      - const: ref_clk
-      - const: rx1_symbol_clk
-      - const: rx0_symbol_clk
-      - const: tx0_symbol_clk
+    minItems: 1
+    maxItems: 4
 
   samsung,pmu-syscon:
     $ref: '/schemas/types.yaml#/definitions/phandle-array'
@@ -53,6 +47,39 @@ properties:
       It can be phandle/offset pair. The second cell which can represent an
       offset is optional.
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: samsung,exynos7-ufs-phy
+
+    then:
+      properties:
+        clocks:
+          items:
+            - description: PLL reference clock
+            - description: symbol clock for input symbol ( rx0-ch0 symbol clock)
+            - description: symbol clock for input symbol ( rx1-ch1 symbol clock)
+            - description: symbol clock for output symbol ( tx0 symbol clock)
+
+        clock-names:
+          items:
+            - const: ref_clk
+            - const: rx1_symbol_clk
+            - const: rx0_symbol_clk
+            - const: tx0_symbol_clk
+
+    else:
+      properties:
+        clocks:
+          items:
+            - description: PLL reference clock
+
+        clock-names:
+          items:
+            - const: ref_clk
+
 required:
   - "#phy-cells"
   - compatible
-- 
2.37.0

Re: [PATCH] dt-bindings: phy: samsung,ufs-phy: match clock items

[Krzysztof Kozlowski]

On 07/07/2022 02:55, Chanho Park wrote:
> Below error is detected from dtbs_check. exynos7-ufs-phy is required
> symbol clocks otherwise only PLL ref clock is required.
> 
> clock-names: ['ref_clk'] is too short

Thank you for your patch. There is something to discuss/improve.

> 
> Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  .../bindings/phy/samsung,ufs-phy.yaml         | 47 +++++++++++++++----
>  1 file changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> index 8da99461e817..3b04f31d9f21 100644
> --- a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> @@ -27,18 +27,12 @@ properties:
>        - const: phy-pma
>  
>    clocks:
> -    items:
> -      - description: PLL reference clock
> -      - description: symbol clock for input symbol ( rx0-ch0 symbol clock)
> -      - description: symbol clock for input symbol ( rx1-ch1 symbol clock)
> -      - description: symbol clock for output symbol ( tx0 symbol clock)
> +    minItems: 1
> +    maxItems: 4
>  
>    clock-names:
> -    items:
> -      - const: ref_clk
> -      - const: rx1_symbol_clk
> -      - const: rx0_symbol_clk
> -      - const: tx0_symbol_clk
> +    minItems: 1
> +    maxItems: 4
>  
>    samsung,pmu-syscon:
>      $ref: '/schemas/types.yaml#/definitions/phandle-array'
> @@ -53,6 +47,39 @@ properties:
>        It can be phandle/offset pair. The second cell which can represent an
>        offset is optional.
>  
> +allOf:

The allOf block should go after "required" block.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos7-ufs-phy
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: PLL reference clock
> +            - description: symbol clock for input symbol ( rx0-ch0 symbol clock)
> +            - description: symbol clock for input symbol ( rx1-ch1 symbol clock)
> +            - description: symbol clock for output symbol ( tx0 symbol clock)

While moving drop space after '('.

> +
> +        clock-names:
> +          items:
> +            - const: ref_clk
> +            - const: rx1_symbol_clk
> +            - const: rx0_symbol_clk
> +            - const: tx0_symbol_clk
> +
> +    else:
> +      properties:
> +        clocks:
> +          items:
> +            - description: PLL reference clock
> +
> +        clock-names:
> +          items:
> +            - const: ref_clk
> +
>  required:
>    - "#phy-cells"
>    - compatible


Best regards,
Krzysztof

RE: [PATCH] dt-bindings: phy: samsung,ufs-phy: match clock items

[Chanho Park]

> > Below error is detected from dtbs_check. exynos7-ufs-phy is required
> > symbol clocks otherwise only PLL ref clock is required.
> >
> > clock-names: ['ref_clk'] is too short
> 
> Thank you for your patch. There is something to discuss/improve.

Thanks for your review :) 

> 
> >
> > Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> > Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> > ---
> >  .../bindings/phy/samsung,ufs-phy.yaml         | 47 +++++++++++++++----
> >  1 file changed, 37 insertions(+), 10 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> > b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> > index 8da99461e817..3b04f31d9f21 100644
> > --- a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> > @@ -27,18 +27,12 @@ properties:
> >        - const: phy-pma
> >
> >    clocks:
> > -    items:
> > -      - description: PLL reference clock
> > -      - description: symbol clock for input symbol ( rx0-ch0 symbol
> clock)
> > -      - description: symbol clock for input symbol ( rx1-ch1 symbol
> clock)
> > -      - description: symbol clock for output symbol ( tx0 symbol clock)
> > +    minItems: 1
> > +    maxItems: 4
> >
> >    clock-names:
> > -    items:
> > -      - const: ref_clk
> > -      - const: rx1_symbol_clk
> > -      - const: rx0_symbol_clk
> > -      - const: tx0_symbol_clk
> > +    minItems: 1
> > +    maxItems: 4
> >
> >    samsung,pmu-syscon:
> >      $ref: '/schemas/types.yaml#/definitions/phandle-array'
> > @@ -53,6 +47,39 @@ properties:
> >        It can be phandle/offset pair. The second cell which can represent
> an
> >        offset is optional.
> >
> > +allOf:
> 
> The allOf block should go after "required" block.

I wrote the block after required block but I changed the order by referring
https://elixir.bootlin.com/linux/v5.18-rc2/source/Documentation/devicetree/bindings/clock/samsung,exynos7885-clock.yaml#L53
I'll correct the order.

> 
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: samsung,exynos7-ufs-phy
> > +
> > +    then:
> > +      properties:
> > +        clocks:
> > +          items:
> > +            - description: PLL reference clock
> > +            - description: symbol clock for input symbol ( rx0-ch0 symbol
> clock)
> > +            - description: symbol clock for input symbol ( rx1-ch1 symbol
> clock)
> > +            - description: symbol clock for output symbol ( tx0
> > + symbol clock)
> 
> While moving drop space after '('.

I'll drop the spaces.

Best Regards,
Chanho Park

Re: [PATCH] dt-bindings: phy: samsung,ufs-phy: match clock items

[Krzysztof Kozlowski]

On 07/07/2022 08:23, Chanho Park wrote:
>>> Below error is detected from dtbs_check. exynos7-ufs-phy is required
>>> symbol clocks otherwise only PLL ref clock is required.
>>>
>>> clock-names: ['ref_clk'] is too short
>>
>> Thank you for your patch. There is something to discuss/improve.
> 
> Thanks for your review :) 
> 
>>
>>>
>>> Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
>>> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
>>> ---
>>>  .../bindings/phy/samsung,ufs-phy.yaml         | 47 +++++++++++++++----
>>>  1 file changed, 37 insertions(+), 10 deletions(-)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
>>> b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
>>> index 8da99461e817..3b04f31d9f21 100644
>>> --- a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
>>> @@ -27,18 +27,12 @@ properties:
>>>        - const: phy-pma
>>>
>>>    clocks:
>>> -    items:
>>> -      - description: PLL reference clock
>>> -      - description: symbol clock for input symbol ( rx0-ch0 symbol
>> clock)
>>> -      - description: symbol clock for input symbol ( rx1-ch1 symbol
>> clock)
>>> -      - description: symbol clock for output symbol ( tx0 symbol clock)
>>> +    minItems: 1
>>> +    maxItems: 4
>>>
>>>    clock-names:
>>> -    items:
>>> -      - const: ref_clk
>>> -      - const: rx1_symbol_clk
>>> -      - const: rx0_symbol_clk
>>> -      - const: tx0_symbol_clk
>>> +    minItems: 1
>>> +    maxItems: 4
>>>
>>>    samsung,pmu-syscon:
>>>      $ref: '/schemas/types.yaml#/definitions/phandle-array'
>>> @@ -53,6 +47,39 @@ properties:
>>>        It can be phandle/offset pair. The second cell which can represent
>> an
>>>        offset is optional.
>>>
>>> +allOf:
>>
>> The allOf block should go after "required" block.
> 
> I wrote the block after required block but I changed the order by referring
> https://elixir.bootlin.com/linux/v5.18-rc2/source/Documentation/devicetree/bindings/clock/samsung,exynos7885-clock.yaml#L53
> I'll correct the order.

Yeah, I put not the recommended order. I need to find another example
for giving to people :)


Best regards,
Krzysztof