[PATCH devicetree 0/2] Common PHY to YAML + tx-amplitude property

[PATCH devicetree 0/2] Common PHY to YAML + tx-amplitude property

[Marek Behún]

This converts common PHY binding to YAML and adds property for
specifying PHY transmit amplitude.

Marek Behún (2):
  dt-bindings: phy: Convert generic PHY provider binding to YAML
  dt-bindings: phy: Add `tx-amplitude-microvolt` property binding

 .../devicetree/bindings/phy/phy-bindings.txt  | 73 +----------------
 .../devicetree/bindings/phy/phy.yaml          | 81 +++++++++++++++++++
 2 files changed, 82 insertions(+), 72 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/phy.yaml

-- 
2.32.0

[PATCH devicetree 1/2] dt-bindings: phy: Convert generic PHY provider binding to YAML

[Marek Behún]

Convert phy-bindings.txt to YAML. This creates binding only for PHY
provider, since PHY consumer binding is in dtschema. Consumer binding
example is provided.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 .../devicetree/bindings/phy/phy-bindings.txt  | 73 +------------------
 .../devicetree/bindings/phy/phy.yaml          | 59 +++++++++++++++
 2 files changed, 60 insertions(+), 72 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt b/Documentation/devicetree/bindings/phy/phy-bindings.txt
index c4eb38902533..ded3cf18352d 100644
--- a/Documentation/devicetree/bindings/phy/phy-bindings.txt
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -1,72 +1 @@
-This document explains only the device tree data binding. For general
-information about PHY subsystem refer to Documentation/driver-api/phy/phy.rst
-
-PHY device node
-===============
-
-Required Properties:
-#phy-cells:	Number of cells in a PHY specifier;  The meaning of all those
-		cells is defined by the binding for the phy node. The PHY
-		provider can use the values in cells to find the appropriate
-		PHY.
-
-Optional Properties:
-phy-supply:	Phandle to a regulator that provides power to the PHY. This
-		regulator will be managed during the PHY power on/off sequence.
-
-For example:
-
-phys: phy {
-    compatible = "xxx";
-    reg = <...>;
-    .
-    .
-    #phy-cells = <1>;
-    .
-    .
-};
-
-That node describes an IP block (PHY provider) that implements 2 different PHYs.
-In order to differentiate between these 2 PHYs, an additional specifier should be
-given while trying to get a reference to it.
-
-PHY user node
-=============
-
-Required Properties:
-phys : the phandle for the PHY device (used by the PHY subsystem; not to be
-       confused with the Ethernet specific 'phy' and 'phy-handle' properties,
-       see Documentation/devicetree/bindings/net/ethernet.txt for these)
-phy-names : the names of the PHY corresponding to the PHYs present in the
-	    *phys* phandle
-
-Example 1:
-usb1: usb_otg_ss@xxx {
-    compatible = "xxx";
-    reg = <xxx>;
-    .
-    .
-    phys = <&usb2_phy>, <&usb3_phy>;
-    phy-names = "usb2phy", "usb3phy";
-    .
-    .
-};
-
-This node represents a controller that uses two PHYs, one for usb2 and one for
-usb3.
-
-Example 2:
-usb2: usb_otg_ss@xxx {
-    compatible = "xxx";
-    reg = <xxx>;
-    .
-    .
-    phys = <&phys 1>;
-    phy-names = "usbphy";
-    .
-    .
-};
-
-This node represents a controller that uses one of the PHYs of the PHY provider
-device defined previously. Note that the phy handle has an additional specifier
-"1" to differentiate between the two PHYs.
+This file has moved to ./phy.yaml.
diff --git a/Documentation/devicetree/bindings/phy/phy.yaml b/Documentation/devicetree/bindings/phy/phy.yaml
new file mode 100644
index 000000000000..a5b6b04cff5b
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common PHY Provider Generic Binding
+
+description:
+  This is the generic binding for common PHYs providers.
+
+maintainers:
+  - Kishon Vijay Abraham I <kishon@ti.com>
+  - Vinod Koul <vkoul@kernel.org>
+
+select:
+  required:
+    - '#phy-cells'
+
+properties:
+  '#phy-cells':
+    $ref: '/schemas/types.yaml#/definitions/uint32'
+    description:
+      Number of cells in a PHY specifier. The meaning of all those cells is
+      defined by the binding for the PHY node. The PHY provider can use the
+      values in cells to find the appropriate PHY.
+    maximum: 16
+
+  phy-supply:
+    description:
+      Phandle to a regulator that provides power to the PHY. This regulator
+      will be managed during the PHY power on/off sequence.
+
+required:
+  - '#phy-cells'
+
+additionalProperties: true
+
+examples:
+  - |
+    phy_regulator: regulator {
+      compatible = "regulator-fixed";
+      regulator-name = "phy-regulator";
+    };
+
+    phy: phy {
+      #phy-cells = <1>;
+      phy-supply = <&phy_regulator>;
+    };
+
+    ethernet-controller {
+      phys = <&phy 0>;
+      phy-names = "ethphy";
+    };
+
+    usb {
+      phys = <&phy 1>, <&phy 2>;
+      phy-names = "usb2phy", "usb3phy";
+    };
-- 
2.32.0

Re: [PATCH devicetree 1/2] dt-bindings: phy: Convert generic PHY provider binding to YAML

[Rob Herring]

On Wed, Dec 15, 2021 at 12:34:31AM +0100, Marek Behún wrote:
> Convert phy-bindings.txt to YAML. This creates binding only for PHY
> provider, since PHY consumer binding is in dtschema. Consumer binding
> example is provided.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  .../devicetree/bindings/phy/phy-bindings.txt  | 73 +------------------
>  .../devicetree/bindings/phy/phy.yaml          | 59 +++++++++++++++
>  2 files changed, 60 insertions(+), 72 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy.yaml

We already have phy-provider.yaml and phy-consumer.yaml in dtschema 
repo. Add what's missing there (anything copied from here will need to 
be relicensed).

Rob

[PATCH devicetree 2/2] dt-bindings: phy: Add `tx-amplitude-microvolt` property binding

[Marek Behún]

Common PHYs often have the possibility to specify peak-to-peak voltage
on the differential pair - the default voltage sometimes needs to be
changed for a particular board.

Add properties `tx-amplitude-microvolt` and
`tx-amplitude-microvolt-names` for this purpose. The second property is
needed to specify

Example usage with only one voltage (it will be used for all supported
PHY modes, the `tx-amplitude-microvolt-names` property is not needed in
this case):

  tx-amplitude-microvolt = <915000>;

Example usage with voltages for multiple modes:

  tx-amplitude-microvolt = <915000>, <1100000>, <1200000>;
  tx-amplitude-microvolt-names = "2500base-x", "usb", "pcie";

Signed-off-by: Marek Behún <kabel@kernel.org>
---

I wanted to constrain the values allowed in the
`tx-amplitude-microvolt-names` property to:
- ethernet SerDes modes (sgmii, qsgmii, 10gbase-r, 2500base-x, ...)
- PCIe modes (pattern: ^pcie[1-6]?$)
- USB modes (pattern: ^usb((-host|-device|-otg)?-(ls|fs|hs|ss|ss\+|4))?$)
- DisplayPort modes (pattern: ^dp(-rbr|-hbr[23]?|-uhbr-(10|13.5|20))?$)
- Camera modes (mipi-dphy, mipi-dphy-univ, mipi-dphy-v2.5-univ)
- Storage modes (sata, ufs-hs, ufs-hs-a, ufs-hs-b)

But was unable to. The '-names' suffix implies string-array type, and
string-array type does not allow to specify a type for all items in a
simple way, i.e.:
  items:
    enum:
      - sgmii
      - sata
      - usb
      ...
Such constraint does work when changing ethernet controller's
`phy-connection-type` to array, see
https://lore.kernel.org/netdev/20211123164027.15618-2-kabel@kernel.org/

But it seems that string-array type prohibits this.

---
 .../devicetree/bindings/phy/phy.yaml          | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy.yaml b/Documentation/devicetree/bindings/phy/phy.yaml
index a5b6b04cff5b..8915065cf6c2 100644
--- a/Documentation/devicetree/bindings/phy/phy.yaml
+++ b/Documentation/devicetree/bindings/phy/phy.yaml
@@ -31,9 +31,29 @@ properties:
       Phandle to a regulator that provides power to the PHY. This regulator
       will be managed during the PHY power on/off sequence.
 
+  tx-amplitude-microvolt:
+    description:
+      Transmit amplitude voltages in microvolts, peak-to-peak. If this property
+      contains multiple values for various PHY modes, the
+      'tx-amplitude-microvolt-names' property must be provided and contain
+      corresponding mode names.
+
+  tx-amplitude-microvolt-names:
+    description: |
+      Names of the modes corresponding to voltages in the
+      'tx-amplitude-microvolt' property. Required only if multiple voltages
+      are provided.
+
+      If a value of 'default' is provided, the system should use it for any PHY
+      mode that is otherwise not defined here. If 'default' is not provided, the
+      system should use manufacturer default value.
+
 required:
   - '#phy-cells'
 
+dependencies:
+  tx-amplitude-microvolt-names: [ tx-amplitude-microvolt ]
+
 additionalProperties: true
 
 examples:
@@ -46,6 +66,8 @@ examples:
     phy: phy {
       #phy-cells = <1>;
       phy-supply = <&phy_regulator>;
+      tx-amplitude-microvolt = <915000>, <1100000>, <1200000>;
+      tx-amplitude-microvolt-names = "2500base-x", "usb-hs", "usb-ss";
     };
 
     ethernet-controller {
-- 
2.32.0

Re: [PATCH devicetree 2/2] dt-bindings: phy: Add `tx-amplitude-microvolt` property binding

[Andrew Lunn]

On Wed, Dec 15, 2021 at 12:34:32AM +0100, Marek Behún wrote:
> Common PHYs often have the possibility to specify peak-to-peak voltage
> on the differential pair - the default voltage sometimes needs to be
> changed for a particular board.

Hi Marek

Common PHYs are not the only user of this. Ethernet PHYs can also use
it, as well as SERDESes embedded within Ethernet switches.

That is why i suggested these properties go into something like
serdes.yaml. That can then be included into Common PHY, Ethernet PHYs,
switch drivers etc.

Please could you make such a split?

       Andrew

Re: [PATCH devicetree 2/2] dt-bindings: phy: Add `tx-amplitude-microvolt` property binding

[Marek Behún]

On Wed, 15 Dec 2021 11:55:00 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Wed, Dec 15, 2021 at 12:34:32AM +0100, Marek Behún wrote:
> > Common PHYs often have the possibility to specify peak-to-peak voltage
> > on the differential pair - the default voltage sometimes needs to be
> > changed for a particular board.  
> 
> Hi Marek
> 
> Common PHYs are not the only user of this. Ethernet PHYs can also use
> it, as well as SERDESes embedded within Ethernet switches.
> 
> That is why i suggested these properties go into something like
> serdes.yaml. That can then be included into Common PHY, Ethernet PHYs,
> switch drivers etc.
> 
> Please could you make such a split?
> 
>        Andrew

Hi Andrew,

and where (into which directory) should this serdes.yaml file go?

My idea was to put the properties into common PHY and then refer to
them from other places, so for example this would be put into ethernet
PHY binding:

  serdes-tx-amplitude-microvolt:
    $ref: '/schemas/phy/phy.yaml#/properties/tx-amplitude-microvolt'

Marek

Re: [PATCH devicetree 2/2] dt-bindings: phy: Add `tx-amplitude-microvolt` property binding

[Andrew Lunn]

> Hi Andrew,
> 
> and where (into which directory) should this serdes.yaml file go?
> 
> My idea was to put the properties into common PHY and then refer to
> them from other places, so for example this would be put into ethernet
> PHY binding:
> 
>   serdes-tx-amplitude-microvolt:
>     $ref: '/schemas/phy/phy.yaml#/properties/tx-amplitude-microvolt'

I don't know much about yaml. If that works, that is fine by me. But
we should probably check with Rob this is best practice.

   Andrew

Re: [PATCH devicetree 2/2] dt-bindings: phy: Add `tx-amplitude-microvolt` property binding

[Rob Herring]

On Wed, Dec 15, 2021 at 06:22:22PM +0100, Marek Behún wrote:
> On Wed, 15 Dec 2021 11:55:00 +0100
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > On Wed, Dec 15, 2021 at 12:34:32AM +0100, Marek Behún wrote:
> > > Common PHYs often have the possibility to specify peak-to-peak voltage
> > > on the differential pair - the default voltage sometimes needs to be
> > > changed for a particular board.  
> > 
> > Hi Marek
> > 
> > Common PHYs are not the only user of this. Ethernet PHYs can also use
> > it, as well as SERDESes embedded within Ethernet switches.
> > 
> > That is why i suggested these properties go into something like
> > serdes.yaml. That can then be included into Common PHY, Ethernet PHYs,
> > switch drivers etc.
> > 
> > Please could you make such a split?
> > 
> >        Andrew
> 
> Hi Andrew,
> 
> and where (into which directory) should this serdes.yaml file go?
> 
> My idea was to put the properties into common PHY and then refer to
> them from other places, so for example this would be put into ethernet
> PHY binding:
> 
>   serdes-tx-amplitude-microvolt:

Why do you need a different name here? 

>     $ref: '/schemas/phy/phy.yaml#/properties/tx-amplitude-microvolt'

This is a pattern I try to avoid generally. Put what's common in 1 
schema file and then reference all of it from the top level.

Rob

Re: [PATCH devicetree 2/2] dt-bindings: phy: Add `tx-amplitude-microvolt` property binding

[Rob Herring]

On Wed, Dec 15, 2021 at 12:34:32AM +0100, Marek Behún wrote:
> Common PHYs often have the possibility to specify peak-to-peak voltage
> on the differential pair - the default voltage sometimes needs to be
> changed for a particular board.

I can envision needing this, but I can't say that I've seen custom 
properties being proposed for this purpose.

> 
> Add properties `tx-amplitude-microvolt` and
> `tx-amplitude-microvolt-names` for this purpose. The second property is
> needed to specify

Is the amplitude peak to peak? You just said it was, but perhaps make 
the property name more clearly defined: tx-p2p-microvolt

> 
> Example usage with only one voltage (it will be used for all supported
> PHY modes, the `tx-amplitude-microvolt-names` property is not needed in
> this case):
> 
>   tx-amplitude-microvolt = <915000>;
> 
> Example usage with voltages for multiple modes:
> 
>   tx-amplitude-microvolt = <915000>, <1100000>, <1200000>;
>   tx-amplitude-microvolt-names = "2500base-x", "usb", "pcie";

I'm not wild about the -names, but I think outside of ethernet most 
cases will only be 1 entry.

For a phy provider with multiple phys, what if each one needs a 
different voltage (for the same mode)?

> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
> 
> I wanted to constrain the values allowed in the
> `tx-amplitude-microvolt-names` property to:
> - ethernet SerDes modes (sgmii, qsgmii, 10gbase-r, 2500base-x, ...)
> - PCIe modes (pattern: ^pcie[1-6]?$)
> - USB modes (pattern: ^usb((-host|-device|-otg)?-(ls|fs|hs|ss|ss\+|4))?$)
> - DisplayPort modes (pattern: ^dp(-rbr|-hbr[23]?|-uhbr-(10|13.5|20))?$)
> - Camera modes (mipi-dphy, mipi-dphy-univ, mipi-dphy-v2.5-univ)
> - Storage modes (sata, ufs-hs, ufs-hs-a, ufs-hs-b)
> 
> But was unable to. The '-names' suffix implies string-array type, and
> string-array type does not allow to specify a type for all items in a
> simple way, i.e.:
>   items:
>     enum:
>       - sgmii
>       - sata
>       - usb
>       ...

Works here: Documentation/devicetree/bindings/arm/samsung/pmu.yaml:56

The requirement is you need to constrain the size with maxItems. It can 
be a 'should be enough for anyone' value.

> Such constraint does work when changing ethernet controller's
> `phy-connection-type` to array, see
> https://lore.kernel.org/netdev/20211123164027.15618-2-kabel@kernel.org/
> 
> But it seems that string-array type prohibits this.
> 
> ---
>  .../devicetree/bindings/phy/phy.yaml          | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy.yaml b/Documentation/devicetree/bindings/phy/phy.yaml
> index a5b6b04cff5b..8915065cf6c2 100644
> --- a/Documentation/devicetree/bindings/phy/phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/phy.yaml
> @@ -31,9 +31,29 @@ properties:
>        Phandle to a regulator that provides power to the PHY. This regulator
>        will be managed during the PHY power on/off sequence.
>  
> +  tx-amplitude-microvolt:
> +    description:
> +      Transmit amplitude voltages in microvolts, peak-to-peak. If this property
> +      contains multiple values for various PHY modes, the
> +      'tx-amplitude-microvolt-names' property must be provided and contain
> +      corresponding mode names.
> +
> +  tx-amplitude-microvolt-names:
> +    description: |
> +      Names of the modes corresponding to voltages in the
> +      'tx-amplitude-microvolt' property. Required only if multiple voltages
> +      are provided.
> +
> +      If a value of 'default' is provided, the system should use it for any PHY
> +      mode that is otherwise not defined here. If 'default' is not provided, the
> +      system should use manufacturer default value.
> +
>  required:
>    - '#phy-cells'
>  
> +dependencies:
> +  tx-amplitude-microvolt-names: [ tx-amplitude-microvolt ]
> +
>  additionalProperties: true
>  
>  examples:
> @@ -46,6 +66,8 @@ examples:
>      phy: phy {
>        #phy-cells = <1>;
>        phy-supply = <&phy_regulator>;
> +      tx-amplitude-microvolt = <915000>, <1100000>, <1200000>;
> +      tx-amplitude-microvolt-names = "2500base-x", "usb-hs", "usb-ss";
>      };
>  
>      ethernet-controller {
> -- 
> 2.32.0
> 
> 

Re: [PATCH devicetree 2/2] dt-bindings: phy: Add `tx-amplitude-microvolt` property binding

[Marek Behún]

Hello Rob,

On Wed, 15 Dec 2021 15:58:40 -0600
Rob Herring <robh@kernel.org> wrote:

> On Wed, Dec 15, 2021 at 12:34:32AM +0100, Marek Behún wrote:
> > Common PHYs often have the possibility to specify peak-to-peak voltage
> > on the differential pair - the default voltage sometimes needs to be
> > changed for a particular board.  
> 
> I can envision needing this, but I can't say that I've seen custom 
> properties being proposed for this purpose.
> 
> > 
> > Add properties `tx-amplitude-microvolt` and
> > `tx-amplitude-microvolt-names` for this purpose. The second property is
> > needed to specify  
> 
> Is the amplitude peak to peak? You just said it was, but perhaps make 
> the property name more clearly defined: tx-p2p-microvolt

Yes, it is peak to peak.

> > 
> > Example usage with only one voltage (it will be used for all supported
> > PHY modes, the `tx-amplitude-microvolt-names` property is not needed in
> > this case):
> > 
> >   tx-amplitude-microvolt = <915000>;
> > 
> > Example usage with voltages for multiple modes:
> > 
> >   tx-amplitude-microvolt = <915000>, <1100000>, <1200000>;
> >   tx-amplitude-microvolt-names = "2500base-x", "usb", "pcie";  
> 
> I'm not wild about the -names, but I think outside of ethernet most 
> cases will only be 1 entry.
> 
> For a phy provider with multiple phys, what if each one needs a 
> different voltage (for the same mode)?

For such a provider I think the best way would be to have the different
PHYs each have a subnode:
  phy-provider {
    phy@0 {
      tx-p2p-microvolt = ...;
    };
  }
> 
> > 
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > ---
> > 
> > I wanted to constrain the values allowed in the
> > `tx-amplitude-microvolt-names` property to:
> > - ethernet SerDes modes (sgmii, qsgmii, 10gbase-r, 2500base-x, ...)
> > - PCIe modes (pattern: ^pcie[1-6]?$)
> > - USB modes (pattern: ^usb((-host|-device|-otg)?-(ls|fs|hs|ss|ss\+|4))?$)
> > - DisplayPort modes (pattern: ^dp(-rbr|-hbr[23]?|-uhbr-(10|13.5|20))?$)
> > - Camera modes (mipi-dphy, mipi-dphy-univ, mipi-dphy-v2.5-univ)
> > - Storage modes (sata, ufs-hs, ufs-hs-a, ufs-hs-b)
> > 
> > But was unable to. The '-names' suffix implies string-array type, and
> > string-array type does not allow to specify a type for all items in a
> > simple way, i.e.:
> >   items:
> >     enum:
> >       - sgmii
> >       - sata
> >       - usb
> >       ...  
> 
> Works here: Documentation/devicetree/bindings/arm/samsung/pmu.yaml:56
> 
> The requirement is you need to constrain the size with maxItems. It can 
> be a 'should be enough for anyone' value.

Thx.

Marek