[PATCH] dt-bindings: phy: make phy-cells description a text

[PATCH] dt-bindings: phy: make phy-cells description a text

[Krzysztof Kozlowski]

The description field is a string, so using YAML inside phy-cells
description is not actually helpful.  Make it a proper text.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/phy/mediatek,tphy.yaml           | 14 ++++----
 .../bindings/phy/mediatek,xsphy.yaml          | 10 +++---
 .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 32 ++++++++-----------
 3 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
index 4b638c1d4221..bd0e4c4915ed 100644
--- a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
+++ b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
@@ -154,14 +154,12 @@ patternProperties:
       "#phy-cells":
         const: 1
         description: |
-          The cells contain the following arguments.
-
-          - description: The PHY type
-              enum:
-                - PHY_TYPE_USB2
-                - PHY_TYPE_USB3
-                - PHY_TYPE_PCIE
-                - PHY_TYPE_SATA
+          The cells contain the following arguments::
+            - The PHY type::
+              - PHY_TYPE_USB2
+              - PHY_TYPE_USB3
+              - PHY_TYPE_PCIE
+              - PHY_TYPE_SATA
 
       nvmem-cells:
         items:
diff --git a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml
index 598fd2b95c29..7262b8e184e2 100644
--- a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml
+++ b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml
@@ -100,12 +100,10 @@ patternProperties:
       "#phy-cells":
         const: 1
         description: |
-          The cells contain the following arguments.
-
-          - description: The PHY type
-              enum:
-                - PHY_TYPE_USB2
-                - PHY_TYPE_USB3
+          The cells contain the following arguments::
+            - The PHY type::
+              - PHY_TYPE_USB2
+              - PHY_TYPE_USB3
 
       # The following optional vendor properties are only for debug or HQA test
       mediatek,eye-src:
diff --git a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
index 79906519c652..7083eddb467c 100644
--- a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
+++ b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
@@ -18,25 +18,19 @@ properties:
   "#phy-cells":
     const: 4
     description: |
-      The cells contain the following arguments.
-
-      - description: The GTR lane
-        minimum: 0
-        maximum: 3
-      - description: The PHY type
-        enum:
-          - PHY_TYPE_DP
-          - PHY_TYPE_PCIE
-          - PHY_TYPE_SATA
-          - PHY_TYPE_SGMII
-          - PHY_TYPE_USB3
-      - description: The PHY instance
-        minimum: 0
-        maximum: 1 # for DP, SATA or USB
-        maximum: 3 # for PCIE or SGMII
-      - description: The reference clock number
-        minimum: 0
-        maximum: 3
+      The cells contain the following arguments::
+        - The GTR lane (minimum:: 0, maximum:: 3)
+        - The PHY type::
+            - PHY_TYPE_DP
+            - PHY_TYPE_PCIE
+            - PHY_TYPE_SATA
+            - PHY_TYPE_SGMII
+            - PHY_TYPE_USB3
+      - The PHY instance::
+          minimum:: 0
+          maximum:: 1 # for DP, SATA or USB
+          maximum:: 3 # for PCIE or SGMII
+      - The reference clock number (minimum:: 0, maximum:: 3)
 
   compatible:
     enum:
-- 
2.34.1

Re: [PATCH] dt-bindings: phy: make phy-cells description a text

[Laurent Pinchart]

Hi Krzysztof,

Thank you for the patch.

On Sun, Jun 19, 2022 at 01:33:25PM +0200, Krzysztof Kozlowski wrote:
> The description field is a string, so using YAML inside phy-cells
> description is not actually helpful.

Does it hurt though ? For xlnx,zynqmp-psgtr.yaml I wrote it that way to
prepare for a future where it could be described using a YAML schema
(but such future may never come).

> Make it a proper text.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/phy/mediatek,tphy.yaml           | 14 ++++----
>  .../bindings/phy/mediatek,xsphy.yaml          | 10 +++---
>  .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 32 ++++++++-----------
>  3 files changed, 23 insertions(+), 33 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> index 4b638c1d4221..bd0e4c4915ed 100644
> --- a/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/mediatek,tphy.yaml
> @@ -154,14 +154,12 @@ patternProperties:
>        "#phy-cells":
>          const: 1
>          description: |
> -          The cells contain the following arguments.
> -
> -          - description: The PHY type
> -              enum:
> -                - PHY_TYPE_USB2
> -                - PHY_TYPE_USB3
> -                - PHY_TYPE_PCIE
> -                - PHY_TYPE_SATA
> +          The cells contain the following arguments::
> +            - The PHY type::
> +              - PHY_TYPE_USB2
> +              - PHY_TYPE_USB3
> +              - PHY_TYPE_PCIE
> +              - PHY_TYPE_SATA
>  
>        nvmem-cells:
>          items:
> diff --git a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml
> index 598fd2b95c29..7262b8e184e2 100644
> --- a/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml
> @@ -100,12 +100,10 @@ patternProperties:
>        "#phy-cells":
>          const: 1
>          description: |
> -          The cells contain the following arguments.
> -
> -          - description: The PHY type
> -              enum:
> -                - PHY_TYPE_USB2
> -                - PHY_TYPE_USB3
> +          The cells contain the following arguments::
> +            - The PHY type::
> +              - PHY_TYPE_USB2
> +              - PHY_TYPE_USB3
>  
>        # The following optional vendor properties are only for debug or HQA test
>        mediatek,eye-src:
> diff --git a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> index 79906519c652..7083eddb467c 100644
> --- a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> +++ b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> @@ -18,25 +18,19 @@ properties:
>    "#phy-cells":
>      const: 4
>      description: |
> -      The cells contain the following arguments.
> -
> -      - description: The GTR lane
> -        minimum: 0
> -        maximum: 3
> -      - description: The PHY type
> -        enum:
> -          - PHY_TYPE_DP
> -          - PHY_TYPE_PCIE
> -          - PHY_TYPE_SATA
> -          - PHY_TYPE_SGMII
> -          - PHY_TYPE_USB3
> -      - description: The PHY instance
> -        minimum: 0
> -        maximum: 1 # for DP, SATA or USB
> -        maximum: 3 # for PCIE or SGMII
> -      - description: The reference clock number
> -        minimum: 0
> -        maximum: 3
> +      The cells contain the following arguments::
> +        - The GTR lane (minimum:: 0, maximum:: 3)
> +        - The PHY type::
> +            - PHY_TYPE_DP
> +            - PHY_TYPE_PCIE
> +            - PHY_TYPE_SATA
> +            - PHY_TYPE_SGMII
> +            - PHY_TYPE_USB3
> +      - The PHY instance::
> +          minimum:: 0
> +          maximum:: 1 # for DP, SATA or USB
> +          maximum:: 3 # for PCIE or SGMII
> +      - The reference clock number (minimum:: 0, maximum:: 3)
>  
>    compatible:
>      enum:
> -- 
> 2.34.1
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH] dt-bindings: phy: make phy-cells description a text

[Krzysztof Kozlowski]

On 19/06/2022 13:40, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> Thank you for the patch.
> 
> On Sun, Jun 19, 2022 at 01:33:25PM +0200, Krzysztof Kozlowski wrote:
>> The description field is a string, so using YAML inside phy-cells
>> description is not actually helpful.
> 
> Does it hurt though ? For xlnx,zynqmp-psgtr.yaml I wrote it that way to
> prepare for a future where it could be described using a YAML schema
> (but such future may never come).

No, it does not hurt. It is however confusing some folks and they think
schema goes into description. The description should be
readable/descriptive for humans, so if you think your approach is
better, I am perfectly fine with it.


Best regards,
Krzysztof

Re: [PATCH] dt-bindings: phy: make phy-cells description a text

[Laurent Pinchart]

On Sun, Jun 19, 2022 at 01:46:17PM +0200, Krzysztof Kozlowski wrote:
> On 19/06/2022 13:40, Laurent Pinchart wrote:
> > Hi Krzysztof,
> > 
> > Thank you for the patch.
> > 
> > On Sun, Jun 19, 2022 at 01:33:25PM +0200, Krzysztof Kozlowski wrote:
> >> The description field is a string, so using YAML inside phy-cells
> >> description is not actually helpful.
> > 
> > Does it hurt though ? For xlnx,zynqmp-psgtr.yaml I wrote it that way to
> > prepare for a future where it could be described using a YAML schema
> > (but such future may never come).
> 
> No, it does not hurt. It is however confusing some folks and they think
> schema goes into description. The description should be
> readable/descriptive for humans, so if you think your approach is
> better, I am perfectly fine with it.

I don't mind much. If you think it would be a good idea to eventually
describe the #phy-cells elements in YAML schema then I'd rather keep the
current description, if there's very little chance it will happen, I
don't mind changing it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] dt-bindings: phy: make phy-cells description a text

[Rob Herring]

On Sun, Jun 19, 2022 at 02:40:12PM +0300, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> Thank you for the patch.
> 
> On Sun, Jun 19, 2022 at 01:33:25PM +0200, Krzysztof Kozlowski wrote:
> > The description field is a string, so using YAML inside phy-cells
> > description is not actually helpful.
> 
> Does it hurt though ? 

Unfortunately, I see this a bit. It's convenient because the schema 
passes all the checks. Doh! And I usually stare at it wondering how it 
passed.

Though I probably did review this, so IDK...

> For xlnx,zynqmp-psgtr.yaml I wrote it that way to
> prepare for a future where it could be described using a YAML schema
> (but such future may never come).

There's 2 parts. There's the resolving the defines and then applying the 
schema to the cells. I actually think the latter would be easier. At 
least from a documenting standpoint, we just need to define our own 
keyword to stick the schema under. With the tools doing dtb based 
validation now, the tools already get the phandle node and get the cell 
size from the DT. It's just another step to extract the 
node's compatible, find it's schema, and get its cell format schema. Any 
volunteers?

Rob