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 <[email protected]>
---
.../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
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 <[email protected]>
> ---
> .../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
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
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
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