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

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

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

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

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

On Wed, 15 Dec 2021 11:55:00 +0100
Andrew Lunn [off-list ref] wrote:

On Wed, Dec 15, 2021 at 12:34:32AM +0100, Marek Behún wrote:

quoted

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

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

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

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.

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 [off-list ref] wrote:

quoted

On Wed, Dec 15, 2021 at 12:34:32AM +0100, Marek Behún wrote:

quoted

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

Hello Rob,

On Wed, 15 Dec 2021 15:58:40 -0600
Rob Herring [off-list ref] wrote:

On Wed, Dec 15, 2021 at 12:34:32AM +0100, Marek Behún wrote:

quoted

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.

quoted

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.

quoted

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 = ...;
    };
  }

quoted

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