diff mbox series

[net-next,v3,1/5] dt-bindings: net: Add Motorcomm yt8xxx ethernet phy

Message ID 20230130063539.3700-2-Frank.Sae@motor-comm.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series add dts for yt8521 and yt8531s, add driver for yt8531 | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/apply fail Patch does not apply to net-next

Commit Message

Frank Sae Jan. 30, 2023, 6:35 a.m. UTC
Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.
 
Signed-off-by: Frank Sae <Frank.Sae@motor-comm.com>
---
 .../bindings/net/motorcomm,yt8xxx.yaml        | 102 ++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   1 +
 3 files changed, 105 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml

Comments

Andrew Lunn Jan. 30, 2023, 2:03 p.m. UTC | #1
On Mon, Jan 30, 2023 at 02:35:35PM +0800, Frank Sae wrote:
>  Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.
>  
> Signed-off-by: Frank Sae <Frank.Sae@motor-comm.com>
> ---
>  .../bindings/net/motorcomm,yt8xxx.yaml        | 102 ++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  MAINTAINERS                                   |   1 +
>  3 files changed, 105 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> new file mode 100644
> index 000000000000..8527576c15b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/motorcomm,yt8xxx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MotorComm yt8xxx Ethernet PHY
> +
> +maintainers:
> +  - frank sae <frank.sae@motor-comm.com>
> +
> +allOf:
> +  - $ref: ethernet-phy.yaml#
> +
> +properties:
> +  rx-internal-delay-ps:
> +    description: |
> +      RGMII RX Clock Delay used only when PHY operates in RGMII mode with
> +      internal delay (phy-mode is 'rgmii-id' or 'rgmii-rxid') in pico-seconds.
> +    enum: [ 0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500, 1650,
> +            1800, 1900, 1950, 2050, 2100, 2200, 2250, 2350, 2500, 2650, 2800,
> +            2950, 3100, 3250, 3400, 3550, 3700, 3850, 4000, 4150 ]
> +    default: 1950

Ah! There has been a misunderstand. Yes, this changes does make sense, but ....

> +
> +  tx-internal-delay-ps:
> +    description: |
> +      RGMII TX Clock Delay used only when PHY operates in RGMII mode with
> +      internal delay (phy-mode is 'rgmii-id' or 'rgmii-txid') in pico-seconds.
> +    enum: [ 0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500, 1650, 1800,
> +            1950, 2100, 2250 ]
> +    default: 150

... i was actually trying to say this 150 is odd. Why is this not
1950?

	Andrew
Rob Herring (Arm) Jan. 30, 2023, 10:41 p.m. UTC | #2
On Mon, Jan 30, 2023 at 02:35:35PM +0800, Frank Sae wrote:
>  Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.

Bindings are for h/w devices, not drivers.

>  
> Signed-off-by: Frank Sae <Frank.Sae@motor-comm.com>
> ---
>  .../bindings/net/motorcomm,yt8xxx.yaml        | 102 ++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  MAINTAINERS                                   |   1 +
>  3 files changed, 105 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> new file mode 100644
> index 000000000000..8527576c15b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/motorcomm,yt8xxx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MotorComm yt8xxx Ethernet PHY
> +
> +maintainers:
> +  - frank sae <frank.sae@motor-comm.com>
> +
> +allOf:
> +  - $ref: ethernet-phy.yaml#

This schema doesn't work because there is no way to determine whether to 
apply it or not. You need a compatible for your phy.

Rob
Frank Sae Jan. 31, 2023, 2:21 a.m. UTC | #3
Hi Andrew,

On 2023/1/30 22:03, Andrew Lunn wrote:
> On Mon, Jan 30, 2023 at 02:35:35PM +0800, Frank Sae wrote:
>>  Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.
>>  
>> Signed-off-by: Frank Sae <Frank.Sae@motor-comm.com>
>> ---
>>  .../bindings/net/motorcomm,yt8xxx.yaml        | 102 ++++++++++++++++++
>>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>>  MAINTAINERS                                   |   1 +
>>  3 files changed, 105 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> new file mode 100644
>> index 000000000000..8527576c15b3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> @@ -0,0 +1,102 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/motorcomm,yt8xxx.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MotorComm yt8xxx Ethernet PHY
>> +
>> +maintainers:
>> +  - frank sae <frank.sae@motor-comm.com>
>> +
>> +allOf:
>> +  - $ref: ethernet-phy.yaml#
>> +
>> +properties:
>> +  rx-internal-delay-ps:
>> +    description: |
>> +      RGMII RX Clock Delay used only when PHY operates in RGMII mode with
>> +      internal delay (phy-mode is 'rgmii-id' or 'rgmii-rxid') in pico-seconds.
>> +    enum: [ 0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500, 1650,
>> +            1800, 1900, 1950, 2050, 2100, 2200, 2250, 2350, 2500, 2650, 2800,
>> +            2950, 3100, 3250, 3400, 3550, 3700, 3850, 4000, 4150 ]
>> +    default: 1950
> 
> Ah! There has been a misunderstand. Yes, this changes does make sense, but > 
>> +
>> +  tx-internal-delay-ps:
>> +    description: |
>> +      RGMII TX Clock Delay used only when PHY operates in RGMII mode with
>> +      internal delay (phy-mode is 'rgmii-id' or 'rgmii-txid') in pico-seconds.
>> +    enum: [ 0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500, 1650, 1800,
>> +            1950, 2100, 2250 ]
>> +    default: 150
> 
> ... i was actually trying to say this 150 is odd. Why is this not
> 1950?

 Tx-delay is usually adjusted by the mac (~ 2ns).
 So here is only fine-turn for the tx-delay.

> 
> 	Andrew
Frank Sae Jan. 31, 2023, 3:19 a.m. UTC | #4
Hi Rob,

On 2023/1/31 06:41, Rob Herring wrote:
> On Mon, Jan 30, 2023 at 02:35:35PM +0800, Frank Sae wrote:
>>  Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.
> 
> Bindings are for h/w devices, not drivers.
> 
>>  
>> Signed-off-by: Frank Sae <Frank.Sae@motor-comm.com>
>> ---
>>  .../bindings/net/motorcomm,yt8xxx.yaml        | 102 ++++++++++++++++++
>>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>>  MAINTAINERS                                   |   1 +
>>  3 files changed, 105 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> new file mode 100644
>> index 000000000000..8527576c15b3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> @@ -0,0 +1,102 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/motorcomm,yt8xxx.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MotorComm yt8xxx Ethernet PHY
>> +
>> +maintainers:
>> +  - frank sae <frank.sae@motor-comm.com>
>> +
>> +allOf:
>> +  - $ref: ethernet-phy.yaml#
> 
> This schema doesn't work because there is no way to determine whether to 
> apply it or not. You need a compatible for your phy.
> 

 I don't understand how to add a compatible for my phy.

compatible:
    oneOf:
      - enum:
          - fsl,imx25-fec
          - fsl,imx27-fec

compatible usually like this, but it is usually for mac or mdio.


phy id list:
YT8511	0x0000010a
YT8521	0x0000011A
YT8531  0x4f51e91b

motorcomm,yt8xxx.yaml

    mdio0 {
        ...
        ethernet-phy@5 {
            compatible = "ethernet-phy-id0000.010a",
                         "ethernet-phy-id0000.011a",
                         "ethernet-phy-id4f51.e91b",
                         "ethernet-phy-ieee802.3-c22";
            reg = <5>;
            ...
        };
    };

 Should i add compatible like this ? 
 If above is error, please give me an example, thanks!

> Rob
Andrew Lunn Jan. 31, 2023, 1:33 p.m. UTC | #5
> phy id list:
> YT8511	0x0000010a
> YT8521	0x0000011A
> YT8531  0x4f51e91b

The first two are clearly invalid, the OUI part is zero. So i would
not list those. Just use the last one.

In the binding part, you need something like

properties:
  compatible:
    const: ethernet-phy-id4f51.e91b
    description: Only needed for DT lint tools

What i don't know is how this will work in combination with the
compatibles gained from ethernet-phy.yaml. You might need to follow
the same structure, include a oneOf:

In the example part, you would do something like:
> 
>     mdio0 {
>         ...
>         ethernet-phy@5 {
	      # Only needed to make DT lint tools work. Do not copy/paste
	      # into real DTS files.
>             compatible = "ethernet-phy-id4f51.e91b";
>             reg = <5>;
>             ...
>         };
>     };

I don't think there are any examples to follow because the kernel
does not need any of this, is probes using the IDs. Listing
compatibles like this is purely for the DT tools, which is why i put
in the comments about not copy/pasting it to real DT blobs.

   Andrew
Andrew Lunn Jan. 31, 2023, 1:42 p.m. UTC | #6
> >> +properties:
> >> +  rx-internal-delay-ps:
> >> +    description: |
> >> +      RGMII RX Clock Delay used only when PHY operates in RGMII mode with
> >> +      internal delay (phy-mode is 'rgmii-id' or 'rgmii-rxid') in pico-seconds.
> >> +    enum: [ 0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500, 1650,
> >> +            1800, 1900, 1950, 2050, 2100, 2200, 2250, 2350, 2500, 2650, 2800,
> >> +            2950, 3100, 3250, 3400, 3550, 3700, 3850, 4000, 4150 ]
> >> +    default: 1950
> > 
> > Ah! There has been a misunderstand. Yes, this changes does make sense, but > 
> >> +
> >> +  tx-internal-delay-ps:
> >> +    description: |
> >> +      RGMII TX Clock Delay used only when PHY operates in RGMII mode with
> >> +      internal delay (phy-mode is 'rgmii-id' or 'rgmii-txid') in pico-seconds.
> >> +    enum: [ 0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500, 1650, 1800,
> >> +            1950, 2100, 2250 ]
> >> +    default: 150
> > 
> > ... i was actually trying to say this 150 is odd. Why is this not
> > 1950?
> 
>  Tx-delay is usually adjusted by the mac (~ 2ns).
>  So here is only fine-turn for the tx-delay.

In general, in Linux, this is not true. The PHY inserts both
delays. Yes, you can have the MAC insert the delay, but it then means
the MAC needs to modify phy-mode to indicate it has inserted the
delay, changing rgmii-id to rmgmii-rxid when it calls
phy_connect_*(). And few MAC drivers get this correct. So i would
avoid this. Default to 1950. And if there is a device which needs a
fine tune, it can use 2100, or 2250 in its DTS file.

     Andrew
Krzysztof Kozlowski Jan. 31, 2023, 6:56 p.m. UTC | #7
On 30/01/2023 07:35, Frank Sae wrote:
>  Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.
>  
> Signed-off-by: Frank Sae <Frank.Sae@motor-comm.com>
> ---

Some of my comments from your v2 still apply. This was sent before I
replied to v2, so it's okay - but fix the points I asked.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
new file mode 100644
index 000000000000..8527576c15b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
@@ -0,0 +1,102 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/motorcomm,yt8xxx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MotorComm yt8xxx Ethernet PHY
+
+maintainers:
+  - frank sae <frank.sae@motor-comm.com>
+
+allOf:
+  - $ref: ethernet-phy.yaml#
+
+properties:
+  rx-internal-delay-ps:
+    description: |
+      RGMII RX Clock Delay used only when PHY operates in RGMII mode with
+      internal delay (phy-mode is 'rgmii-id' or 'rgmii-rxid') in pico-seconds.
+    enum: [ 0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500, 1650,
+            1800, 1900, 1950, 2050, 2100, 2200, 2250, 2350, 2500, 2650, 2800,
+            2950, 3100, 3250, 3400, 3550, 3700, 3850, 4000, 4150 ]
+    default: 1950
+
+  tx-internal-delay-ps:
+    description: |
+      RGMII TX Clock Delay used only when PHY operates in RGMII mode with
+      internal delay (phy-mode is 'rgmii-id' or 'rgmii-txid') in pico-seconds.
+    enum: [ 0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500, 1650, 1800,
+            1950, 2100, 2250 ]
+    default: 150
+
+  motorcomm,clk-out-frequency-hz:
+    description: clock output on clock output pin.
+    enum: [0, 25000000, 125000000]
+    default: 0
+
+  motorcomm,keep-pll-enabled:
+    description: |
+      If set, keep the PLL enabled even if there is no link. Useful if you
+      want to use the clock output without an ethernet link.
+    type: boolean
+
+  motorcomm,auto-sleep-disabled:
+    description: |
+      If set, PHY will not enter sleep mode and close AFE after unplug cable
+      for a timer.
+    type: boolean
+
+  motorcomm,tx-clk-adj-enabled:
+    description: |
+      This configuration is mainly to adapt to VF2 with JH7110 SoC.
+      Useful if you want to use tx-clk-xxxx-inverted to adj the delay of tx clk.
+    type: boolean
+
+  motorcomm,tx-clk-10-inverted:
+    description: |
+      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
+      Transmit PHY Clock delay train configuration when speed is 10Mbps.
+    type: boolean
+
+  motorcomm,tx-clk-100-inverted:
+    description: |
+      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
+      Transmit PHY Clock delay train configuration when speed is 100Mbps.
+    type: boolean
+
+  motorcomm,tx-clk-1000-inverted:
+    description: |
+      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
+      Transmit PHY Clock delay train configuration when speed is 1000Mbps.
+    type: boolean
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    mdio0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        phy-mode = "rgmii-id";
+        ethernet-phy@4 {
+            reg = <4>;
+            rx-internal-delay-ps = <2100>;
+            tx-internal-delay-ps = <150>;
+            motorcomm,clk-out-frequency-hz = <0>;
+            motorcomm,keep-pll-enabled;
+            motorcomm,auto-sleep-disabled;
+        };
+    };
+  - |
+    mdio0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        phy-mode = "rgmii";
+        ethernet-phy@5 {
+            reg = <5>;
+            motorcomm,clk-out-frequency-hz = <125000000>;
+            motorcomm,keep-pll-enabled;
+            motorcomm,auto-sleep-disabled;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 161766b1de50..99bb8594753c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -847,6 +847,8 @@  patternProperties:
     description: Moortec Semiconductor Ltd.
   "^mosaixtech,.*":
     description: Mosaix Technologies, Inc.
+  "^motorcomm,.*":
+    description: MotorComm, Inc.
   "^motorola,.*":
     description: Motorola, Inc.
   "^moxa,.*":
diff --git a/MAINTAINERS b/MAINTAINERS
index f82dd8d43c2b..e2f9d935ce8d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14163,6 +14163,7 @@  M:	Peter Geis <pgwipeout@gmail.com>
 M:	Frank <Frank.Sae@motor-comm.com>
 L:	netdev@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
 F:	drivers/net/phy/motorcomm.c
 
 MOXA SMARTIO/INDUSTIO/INTELLIO SERIAL CARD