[net-next,v1,1/3] dt-bindings: net: Add Motorcomm yt8xxx ethernet phy Driver bindings

Commit Message

Frank Sae Jan. 5, 2023, 7:30 a.m. UTC

Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.

Signed-off-by: Frank <[e-mail]>
---
 .../bindings/net/motorcomm,yt8xxx.yaml        | 180 ++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   1 +
 3 files changed, 183 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml

Comments

Andrew Lunn Jan. 5, 2023, 1:17 p.m. UTC | #1

> +  motorcomm,rx-delay-basic:
> +    description: |
> +      Tristate, setup the basic RGMII RX Clock delay of PHY.
> +      This basic delay is fixed at 2ns (1000Mbps) or 8ns (100Mbps、10Mbps).
> +      This basic delay usually auto set by hardware according to the voltage
> +      of RXD0 pin (low = 0, turn off;   high = 1, turn on).
> +      If not exist, this delay is controlled by hardware.
> +      0: turn off;   1: turn on.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]

Why is this needed? When the MAC driver connects to the PHY, it passes
phy-mode. For RGMII, this is one of:

linux/phy.h:	PHY_INTERFACE_MODE_RGMII,
linux/phy.h:	PHY_INTERFACE_MODE_RGMII_ID,
linux/phy.h:	PHY_INTERFACE_MODE_RGMII_RXID,
linux/phy.h:	PHY_INTERFACE_MODE_RGMII_TXID,

This tells you if you need to add a delay for the RX clock line, the
TX clock line, or both. That is all you need to know for basic RGMII
delays.

> +  motorcomm,rx-delay-additional-ps:

ethernet-phy.yaml defines rx-internal-delay-ps. Please use that.

> +    description: |
> +      Setup the additional RGMII RX Clock delay of PHY defined in pico seconds.
> +      RGMII RX Clock Delay = rx-delay-basic + rx-delay-additional-ps.
> +    enum:
> +      - 0
> +      - 150
> +      - 300
> +      - 450
> +      - 600
> +      - 750
> +      - 900
> +      - 1050
> +      - 1200
> +      - 1350
> +      - 1500
> +      - 1650
> +      - 1800
> +      - 1950
> +      - 2100
> +      - 2250

Is this property mandatory? If not, please document what value is used
if it is not present.

> +
> +  motorcomm,tx-delay-ge-ps:

tx-internal-delay-ps

And please define the default.

> +  motorcomm,tx-delay-fe-ps:

So you can only set the TX delay? What is RX delay set to? Same as 1G?
I would suggest you call this motorcomm,tx-internal-delay-fe-ps, so
that it is similar to the standard tx-internal-delay-ps.

> +    description: |
> +      Setup PHY's RGMII TX Clock delay  defined in pico seconds when the speed
> +      is 100Mbps or 10Mbps.
> +    enum:
> +      - 0
> +      - 150
> +      - 300
> +      - 450
> +      - 600
> +      - 750
> +      - 900
> +      - 1050
> +      - 1200
> +      - 1350
> +      - 1500
> +      - 1650
> +      - 1800
> +      - 1950
> +      - 2100
> +      - 2250
> +
> +  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

These two i can see being useful. But everything afterwards seems like
just copy/paste from vendor SDK for things which the hardware can do,
but probably nobody ever uses. Do you have a board using any of the
following properties?

> +
> +  motorcomm,tx-clk-adj-enabled:
> +    description: |
> +      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
> +
> +  motorcomm,sds-tx-amplitude:
> +    description: |
> +      Setup the tx driver amplitude control of SerDes. Higher amplitude is
> +      helpful for long distance.
> +      0: low;   1: middle;   2: high.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2]
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    ethernet {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        ethernet-phy@5 {
> +            reg = <5>;

PHYs are on MDIO busses, so i would expect to see an MDIO bus here,
not Ethernet.

    Andrew

Frank Sae Jan. 6, 2023, 6:51 a.m. UTC | #2

Hi Andrew,

On 2023/1/5 21:17, Andrew Lunn wrote:
>> +  motorcomm,rx-delay-basic:
>> +    description: |
>> +      Tristate, setup the basic RGMII RX Clock delay of PHY.
>> +      This basic delay is fixed at 2ns (1000Mbps) or 8ns (100Mbps、10Mbps).
>> +      This basic delay usually auto set by hardware according to the voltage
>> +      of RXD0 pin (low = 0, turn off;   high = 1, turn on).
>> +      If not exist, this delay is controlled by hardware.
>> +      0: turn off;   1: turn on.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1]
> 
> Why is this needed? When the MAC driver connects to the PHY, it passes
> phy-mode. For RGMII, this is one of:

> linux/phy.h:	PHY_INTERFACE_MODE_RGMII,
> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_ID,
> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_RXID,
> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_TXID,
> 
> This tells you if you need to add a delay for the RX clock line, the
> TX clock line, or both. That is all you need to know for basic RGMII
> delays.
> 

This basic delay can be controlled by hardware or the phy-mode which
passes from MAC driver.
Default value depends on power on strapping, according to the voltage
of RXD0 pin (low = 0, turn off;   high = 1, turn on).

Add this for the case that This basic delay is controlled by hardware,
and software don't change this.

>> +  motorcomm,rx-delay-additional-ps:
> 
> ethernet-phy.yaml defines rx-internal-delay-ps. Please use that.
> 

I will fix.

>> +    description: |
>> +      Setup the additional RGMII RX Clock delay of PHY defined in pico seconds.
>> +      RGMII RX Clock Delay = rx-delay-basic + rx-delay-additional-ps.
>> +    enum:
>> +      - 0
>> +      - 150
>> +      - 300
>> +      - 450
>> +      - 600
>> +      - 750
>> +      - 900
>> +      - 1050
>> +      - 1200
>> +      - 1350
>> +      - 1500
>> +      - 1650
>> +      - 1800
>> +      - 1950
>> +      - 2100
>> +      - 2250
> 
> Is this property mandatory? If not, please document what value is used
> if it is not present.
> 

I will fix.

>> +
>> +  motorcomm,tx-delay-ge-ps:
> 
> tx-internal-delay-ps
> 
> And please define the default.
> 
>> +  motorcomm,tx-delay-fe-ps:
> 
> So you can only set the TX delay? What is RX delay set to? Same as 1G?
> I would suggest you call this motorcomm,tx-internal-delay-fe-ps, so
> that it is similar to the standard tx-internal-delay-ps.
> 

TX delay has two type: tx-delay-ge-ps for 1G and tx-delay-fe-ps for 100M.

RX delay set for 1G and 100M, but it has two type, rx-delay-basic and
rx-delay-additional-ps, RX delay = rx-delay-basic + rx-delay-additional-ps.

I will rename to  tx-internal-delay-fe-ps and  tx-internal-delay-ge-ps.

>> +    description: |
>> +      Setup PHY's RGMII TX Clock delay  defined in pico seconds when the speed
>> +      is 100Mbps or 10Mbps.
>> +    enum:
>> +      - 0
>> +      - 150
>> +      - 300
>> +      - 450
>> +      - 600
>> +      - 750
>> +      - 900
>> +      - 1050
>> +      - 1200
>> +      - 1350
>> +      - 1500
>> +      - 1650
>> +      - 1800
>> +      - 1950
>> +      - 2100
>> +      - 2250
>> +
>> +  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
> 
> These two i can see being useful. But everything afterwards seems like
> just copy/paste from vendor SDK for things which the hardware can do,
> but probably nobody ever uses. Do you have a board using any of the
> following properties?
> 

tx-clk-adj-enabled, tx-clk-10-inverted, tx-clk-100-inverted and
tx-clk-1000-inverted is used and tested by  Yanhong
Wang<[e-mail]>. They used yt8531 on
jh7110-starfive-visionfive-v2. This will provide an additional way to
adjust the tx clk delay on yt8531.

sds-tx-amplitude can be tested on my yt8531s board.

>> +
>> +  motorcomm,tx-clk-adj-enabled:
>> +    description: |
>> +      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
>> +
>> +  motorcomm,sds-tx-amplitude:
>> +    description: |
>> +      Setup the tx driver amplitude control of SerDes. Higher amplitude is
>> +      helpful for long distance.
>> +      0: low;   1: middle;   2: high.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1, 2]
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    ethernet {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        ethernet-phy@5 {
>> +            reg = <5>;
> 
> PHYs are on MDIO busses, so i would expect to see an MDIO bus here,
> not Ethernet.
> 

I will fix.

>     Andrew

Krzysztof Kozlowski Jan. 6, 2023, 8:26 a.m. UTC | #3

On 05/01/2023 08:30, Frank wrote:
> Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.
> 

Subject: drop second, redundant "Driver bindings".

> Signed-off-by: Frank <[e-mail]>

Use full first and last name. Your email suggests something more than
only "Frank".

> ---
>  .../bindings/net/motorcomm,yt8xxx.yaml        | 180 ++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  MAINTAINERS                                   |   1 +
>  3 files changed, 183 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..337a562d864c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
> @@ -0,0 +1,180 @@
> +# 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 <[e-mail]>
> +
> +description: |
> +  Bindings for MotorComm yt8xxx PHYs.

Instead describe the hardware. No need to state the obvious that these
are bindings.

> +  yt8511 will be supported later.

Bindings should be complete. Your driver support is not relevant here.

> +
> +allOf:
> +  - $ref: ethernet-phy.yaml#
> +
> +properties:
> +  motorcomm,clk-out-frequency:

Use property suffixes matching the type.

> +    description: clock output in Hertz on clock output pin.

Drop "Hertz". It should be obvious from the suffix.

> +    $ref: /schemas/types.yaml#/definitions/uint32

Drop.

Anyway, does it fit standard clock-frequency property?

> +    enum: [0, 25000000, 125000000]
> +    default: 0
> +
> +  motorcomm,rx-delay-basic:
> +    description: |
> +      Tristate, setup the basic RGMII RX Clock delay of PHY.
> +      This basic delay is fixed at 2ns (1000Mbps) or 8ns (100Mbps、10Mbps).
> +      This basic delay usually auto set by hardware according to the voltage
> +      of RXD0 pin (low = 0, turn off;   high = 1, turn on).
> +      If not exist, this delay is controlled by hardware.

I don't understand that at all. What "not exist"? There is no verb and
no subject.

The type and description are really unclear.

> +      0: turn off;   1: turn on.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]

So this is bool?

> +
> +  motorcomm,rx-delay-additional-ps:
> +    description: |
> +      Setup the additional RGMII RX Clock delay of PHY defined in pico seconds.
> +      RGMII RX Clock Delay = rx-delay-basic + rx-delay-additional-ps.
> +    enum:

Best regards,
Krzysztof

Frank Sae Jan. 6, 2023, 9:17 a.m. UTC | #4

Hi Krzysztof Kozlowski,

On 2023/1/6 16:26, Krzysztof Kozlowski wrote:
> On 05/01/2023 08:30, Frank wrote:
>> Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.
>>
> 
> Subject: drop second, redundant "Driver bindings".

Change Subject from
dt-bindings: net: Add Motorcomm yt8xxx ethernet phy Driver bindings
to
dt-bindings: net: Add Motorcomm yt8xxx ethernet phy
?

> 
>> Signed-off-by: Frank <[e-mail]>
> 
> Use full first and last name. Your email suggests something more than
> only "Frank".
> 

OK , I will use  Frank.Sae <[e-mail]>

>> ---
>>  .../bindings/net/motorcomm,yt8xxx.yaml        | 180 ++++++++++++++++++
>>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>>  MAINTAINERS                                   |   1 +
>>  3 files changed, 183 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..337a562d864c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> @@ -0,0 +1,180 @@
>> +# 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 <[e-mail]>
>> +
>> +description: |
>> +  Bindings for MotorComm yt8xxx PHYs.
> 
> Instead describe the hardware. No need to state the obvious that these
> are bindings.
> 

I will fix.

>> +  yt8511 will be supported later.
> 
> Bindings should be complete. Your driver support is not relevant here.

I will fix.

> 
>> +
>> +allOf:
>> +  - $ref: ethernet-phy.yaml#
>> +
>> +properties:
>> +  motorcomm,clk-out-frequency:
> 
> Use property suffixes matching the type.
> 
>> +    description: clock output in Hertz on clock output pin.
> 
> Drop "Hertz". It should be obvious from the suffix.
> 
>> +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> Drop.
> 
> Anyway, does it fit standard clock-frequency property?
> 
>> +    enum: [0, 25000000, 125000000]
>> +    default: 0
>> +

Yes, I will fix.

>> +  motorcomm,rx-delay-basic:
>> +    description: |
>> +      Tristate, setup the basic RGMII RX Clock delay of PHY.
>> +      This basic delay is fixed at 2ns (1000Mbps) or 8ns (100Mbps、10Mbps).
>> +      This basic delay usually auto set by hardware according to the voltage
>> +      of RXD0 pin (low = 0, turn off;   high = 1, turn on).
>> +      If not exist, this delay is controlled by hardware.
> 
> I don't understand that at all. What "not exist"? There is no verb and
> no subject.
> 
> The type and description are really unclear.
> 
>> +      0: turn off;   1: turn on.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1]
> 
> So this is bool?
> 

This basic delay can be controlled by hardware or dts.

Default value depends on power on strapping, according to the voltage
of RXD0 pin (low = 0, turn off;   high = 1, turn on).

"not exist" means that This basic delay is controlled by hardware,
and software don't change this.

I will fix.

>> +
>> +  motorcomm,rx-delay-additional-ps:
>> +    description: |
>> +      Setup the additional RGMII RX Clock delay of PHY defined in pico seconds.
>> +      RGMII RX Clock Delay = rx-delay-basic + rx-delay-additional-ps.
>> +    enum:
> 
> Best regards,
> Krzysztof

Krzysztof Kozlowski Jan. 6, 2023, 9:25 a.m. UTC | #5

On 06/01/2023 10:17, Frank.Sae wrote:
> Hi Krzysztof Kozlowski,
> 
> On 2023/1/6 16:26, Krzysztof Kozlowski wrote:
>> On 05/01/2023 08:30, Frank wrote:
>>> Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.
>>>
>>
>> Subject: drop second, redundant "Driver bindings".
> 
> Change Subject from
> dt-bindings: net: Add Motorcomm yt8xxx ethernet phy Driver bindings
> to
> dt-bindings: net: Add Motorcomm yt8xxx ethernet phy
> ?

Yes.

> 
>>
>>> Signed-off-by: Frank <[e-mail]>
>>
>> Use full first and last name. Your email suggests something more than
>> only "Frank".
>>
> 
> OK , I will use  Frank.Sae <[e-mail]>

Without "dot" between parts of name.

> 
>>> ---
>>>  .../bindings/net/motorcomm,yt8xxx.yaml        | 180 ++++++++++++++++++
>>>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>>>  MAINTAINERS                                   |   1 +
Best regards,
Krzysztof

Andrew Lunn Jan. 6, 2023, 1:55 p.m. UTC | #6

> > Why is this needed? When the MAC driver connects to the PHY, it passes
> > phy-mode. For RGMII, this is one of:
> 
> > linux/phy.h:	PHY_INTERFACE_MODE_RGMII,
> > linux/phy.h:	PHY_INTERFACE_MODE_RGMII_ID,
> > linux/phy.h:	PHY_INTERFACE_MODE_RGMII_RXID,
> > linux/phy.h:	PHY_INTERFACE_MODE_RGMII_TXID,
> > 
> > This tells you if you need to add a delay for the RX clock line, the
> > TX clock line, or both. That is all you need to know for basic RGMII
> > delays.
> > 
> 
> This basic delay can be controlled by hardware or the phy-mode which
> passes from MAC driver.
> Default value depends on power on strapping, according to the voltage
> of RXD0 pin (low = 0, turn off;   high = 1, turn on).
> 
> Add this for the case that This basic delay is controlled by hardware,
> and software don't change this.

You should always do what phy-mode contains. Always. We have had
problems in the past where a PHY driver ignored the phy-mode, and left
the PHY however it was strapped. Which worked. But developers put the
wrong phy-mode value in DT. Then somebody had a board which actually
required that the DT value really did work, because the strapping was
wrong. So the driver was fixed to respect the PHY mode, made that
board work, and broke all the other boards which had the wrong
phy-mode in DT.

If the user want the driver to leave the mode alone, use the
strapping, they should use PHY_INTERFACE_MODE_NA. It is not well
documented, but it is used in a few places. However, i don't recommend
it.

> >> +  motorcomm,tx-delay-fe-ps:
> > 
> > So you can only set the TX delay? What is RX delay set to? Same as 1G?
> > I would suggest you call this motorcomm,tx-internal-delay-fe-ps, so
> > that it is similar to the standard tx-internal-delay-ps.
> > 
> 
> TX delay has two type: tx-delay-ge-ps for 1G and tx-delay-fe-ps for 100M.
> 
> RX delay set for 1G and 100M, but it has two type, rx-delay-basic and
> rx-delay-additional-ps, RX delay = rx-delay-basic + rx-delay-additional-ps.
> 
> I will rename to  tx-internal-delay-fe-ps and  tx-internal-delay-ge-ps.

So you can set the TX delay for 1G and Fast, but RX delay has a single
setting for both 1G and Fast? Have you seen boards what actually need
different TX delays like this?

Just because the hardware supports something does not mean Linux needs
to support it. Unless there is a real need for it. So i would suggest
your drop this DT property, and set the Fast delay to the same as the
1G delay. If any board actually requires this in the future, the
property can be added then.

> 
> > These two i can see being useful. But everything afterwards seems like
> > just copy/paste from vendor SDK for things which the hardware can do,
> > but probably nobody ever uses. Do you have a board using any of the
> > following properties?
> > 
> 
> tx-clk-adj-enabled, tx-clk-10-inverted, tx-clk-100-inverted and
> tx-clk-1000-inverted is used and tested by  Yanhong
> Wang<[e-mail]>. They used yt8531 on
> jh7110-starfive-visionfive-v2. This will provide an additional way to
> adjust the tx clk delay on yt8531.

O.K. So they are used with a real board. Can we reduce this down to
tx-clk-inverted? Have you ever seen a board which only needs the
invert for one speed and not the others? To me, that would be a very
odd design.

> sds-tx-amplitude can be tested on my yt8531s board.

Does the board break with the default value? Just because you can test
it on your RDK does not mean anybody will ever use it.

   Andrew

Andrew Lunn Jan. 6, 2023, 1:58 p.m. UTC | #7

> >> +  motorcomm,rx-delay-basic:
> >> +    description: |
> >> +      Tristate, setup the basic RGMII RX Clock delay of PHY.
> >> +      This basic delay is fixed at 2ns (1000Mbps) or 8ns (100Mbps、10Mbps).
> >> +      This basic delay usually auto set by hardware according to the voltage
> >> +      of RXD0 pin (low = 0, turn off;   high = 1, turn on).
> >> +      If not exist, this delay is controlled by hardware.
> > 
> > I don't understand that at all. What "not exist"? There is no verb and
> > no subject.
> > 
> > The type and description are really unclear.
> > 
> >> +      0: turn off;   1: turn on.
> >> +    $ref: /schemas/types.yaml#/definitions/uint32
> >> +    enum: [0, 1]
> > 
> > So this is bool?
> > 
> 
> This basic delay can be controlled by hardware or dts.
> 
> Default value depends on power on strapping, according to the voltage
> of RXD0 pin (low = 0, turn off;   high = 1, turn on).
> 
> "not exist" means that This basic delay is controlled by hardware,
> and software don't change this.

As i said in my other reply, this is not how Linux controls this. This
property should be removed.

	 Andrew

Frank Sae Jan. 11, 2023, 9:20 a.m. UTC | #8

Hi Andrew,

On 2023/1/6 21:55, Andrew Lunn wrote:
>>> Why is this needed? When the MAC driver connects to the PHY, it passes
>>> phy-mode. For RGMII, this is one of:
>>
>>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII,
>>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_ID,
>>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_RXID,
>>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_TXID,
>>>
>>> This tells you if you need to add a delay for the RX clock line, the
>>> TX clock line, or both. That is all you need to know for basic RGMII
>>> delays.
>>>
>>
>> This basic delay can be controlled by hardware or the phy-mode which
>> passes from MAC driver.
>> Default value depends on power on strapping, according to the voltage
>> of RXD0 pin (low = 0, turn off;   high = 1, turn on).
>>
>> Add this for the case that This basic delay is controlled by hardware,
>> and software don't change this.
> 
> You should always do what phy-mode contains. Always. We have had
> problems in the past where a PHY driver ignored the phy-mode, and left
> the PHY however it was strapped. Which worked. But developers put the
> wrong phy-mode value in DT. Then somebody had a board which actually
> required that the DT value really did work, because the strapping was
> wrong. So the driver was fixed to respect the PHY mode, made that
> board work, and broke all the other boards which had the wrong
> phy-mode in DT.
> 
> If the user want the driver to leave the mode alone, use the
> strapping, they should use PHY_INTERFACE_MODE_NA. It is not well
> documented, but it is used in a few places. However, i don't recommend
> it.
> 

RX delay = rx-delay-basic (0ns or 1.9ns) + x-delay-additional-ps
(N*150ps, N = 0 ~ 15)
 If rx-delay-basic is removed and controlled by phy-mode.
 when phy-mode is  rgmii-id or rgmii-rxid, RX delay is 1.9ns + N*150ps.
 But sometimes 1.9ns is still too big, we just need  0ns + N*150ps.

For this case, can we do like following ?
rx-internal-delay-ps:
    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: 0
 rx-internal-delay-ps is 0ns + N*150ps and  1.9ns + N*150ps.
 And check whether need rx-delay-basic (1.9ns) by the val of
rx-internal-delay-ps?

>>>> +  motorcomm,tx-delay-fe-ps:
>>>
>>> So you can only set the TX delay? What is RX delay set to? Same as 1G?
>>> I would suggest you call this motorcomm,tx-internal-delay-fe-ps, so
>>> that it is similar to the standard tx-internal-delay-ps.
>>>
>>
>> TX delay has two type: tx-delay-ge-ps for 1G and tx-delay-fe-ps for 100M.
>>
>> RX delay set for 1G and 100M, but it has two type, rx-delay-basic and
>> rx-delay-additional-ps, RX delay = rx-delay-basic + rx-delay-additional-ps.
>>
>> I will rename to  tx-internal-delay-fe-ps and  tx-internal-delay-ge-ps.
> 
> So you can set the TX delay for 1G and Fast, but RX delay has a single
> setting for both 1G and Fast? Have you seen boards what actually need
> different TX delays like this?
> 
> Just because the hardware supports something does not mean Linux needs
> to support it. Unless there is a real need for it. So i would suggest
> your drop this DT property, and set the Fast delay to the same as the
> 1G delay. If any board actually requires this in the future, the
> property can be added then.
> 
>>
>>> These two i can see being useful. But everything afterwards seems like
>>> just copy/paste from vendor SDK for things which the hardware can do,
>>> but probably nobody ever uses. Do you have a board using any of the
>>> following properties?
>>>
>>
>> tx-clk-adj-enabled, tx-clk-10-inverted, tx-clk-100-inverted and
>> tx-clk-1000-inverted is used and tested by  Yanhong
>> Wang<[e-mail]>. They used yt8531 on
>> jh7110-starfive-visionfive-v2. This will provide an additional way to
>> adjust the tx clk delay on yt8531.
> 
> O.K. So they are used with a real board. Can we reduce this down to
> tx-clk-inverted? Have you ever seen a board which only needs the
> invert for one speed and not the others? To me, that would be a very
> odd design.
> 

We can't reduce this down to tx-clk-inverted.
There are two mac and two yt8531 on their board. Each of yt8531 need
different config in DTS. They need adjust tx clk delay in
link_change_notify callback function according to current speed.

 They configured tx-clk-xxxx-inverted like this :

    speed     GMAC0             GMAC1
    1000M      1                  0		tx-clk-1000-inverted
    100M       1                  1		tx-clk-100-inverted
    10M       0/1                0/1     	tx-clk-10-inverted

Can we put tx-clk-adj-enabled, tx-clk-10-inverted, tx-clk-100-inverted
and tx-clk-1000-inverted in tx-clk-10-inverted like bit in byte?

tx-clk-10-inverted = tx-clk-adj-enabled (bit0),
tx-clk-10-inverted(bit1), tx-clk-100-inverted(bit1) and
tx-clk-1000-inverted(bit2).

>> sds-tx-amplitude can be tested on my yt8531s board.
> 
> Does the board break with the default value? Just because you can test
> it on your RDK does not mean anybody will ever use it.
> 
>    Andrew

Andrew Lunn Jan. 11, 2023, 1:07 p.m. UTC | #9

> RX delay = rx-delay-basic (0ns or 1.9ns) + x-delay-additional-ps
> (N*150ps, N = 0 ~ 15)
>  If rx-delay-basic is removed and controlled by phy-mode.
>  when phy-mode is  rgmii-id or rgmii-rxid, RX delay is 1.9ns + N*150ps.
>  But sometimes 1.9ns is still too big, we just need  0ns + N*150ps.
> 
> For this case, can we do like following ?
> rx-internal-delay-ps:
>     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: 0
>  rx-internal-delay-ps is 0ns + N*150ps and  1.9ns + N*150ps.
>  And check whether need rx-delay-basic (1.9ns) by the val of
> rx-internal-delay-ps?

Nothing says delays are only positive. So you could have rgmii-id or
rgmii-rxid and a rx-internal-delay-ps of -150, if you need less than
1.9ns.

As i said, rx-internal-delay-ps is used to fine tune the delay.

> We can't reduce this down to tx-clk-inverted.
> There are two mac and two yt8531 on their board. Each of yt8531 need
> different config in DTS. They need adjust tx clk delay in
> link_change_notify callback function according to current speed.
> 
>  They configured tx-clk-xxxx-inverted like this :
> 
>     speed     GMAC0             GMAC1
>     1000M      1                  0		tx-clk-1000-inverted
>     100M       1                  1		tx-clk-100-inverted
>     10M       0/1                0/1     	tx-clk-10-inverted

What MAC is this? It seems very oddly designed, getting close to
broken. I've not seen any other MAC/PHY combination need anything like
this. 

> Can we put tx-clk-adj-enabled, tx-clk-10-inverted, tx-clk-100-inverted
> and tx-clk-1000-inverted in tx-clk-10-inverted like bit in byte?

No, they are individual boolean properties, so should be kept as they
are. But i really think somebody should be looking deep into the MAC
design to understand why it is like this, and if the MAC can sort out
this mess itself.

	Andrew

Frank Sae Jan. 16, 2023, 2:49 a.m. UTC | #10

Hi Andrew,

On 2023/1/11 21:07, Andrew Lunn wrote:
>> RX delay = rx-delay-basic (0ns or 1.9ns) + x-delay-additional-ps
>> (N*150ps, N = 0 ~ 15)
>>  If rx-delay-basic is removed and controlled by phy-mode.
>>  when phy-mode is  rgmii-id or rgmii-rxid, RX delay is 1.9ns + N*150ps.
>>  But sometimes 1.9ns is still too big, we just need  0ns + N*150ps.
>>
>> For this case, can we do like following ?
>> rx-internal-delay-ps:
>>     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: 0
>>  rx-internal-delay-ps is 0ns + N*150ps and  1.9ns + N*150ps.
>>  And check whether need rx-delay-basic (1.9ns) by the val of
>> rx-internal-delay-ps?
> 
> Nothing says delays are only positive. So you could have rgmii-id or
> rgmii-rxid and a rx-internal-delay-ps of -150, if you need less than
> 1.9ns.
> 
> As i said, rx-internal-delay-ps is used to fine tune the delay.

The standard type of rx-internal-delay-ps is uint32-array, so it can't
be -150.

https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

 "-ps$":
    $ref: types.yaml#/definitions/uint32-array
    description: picosecond


Can we used rx-internal-delay-ps with int32 type?
like this:
  rx-internal-delay-ps:
    $ref: /schemas/types.yaml#/definitions/int32
    enum: [ -1900, -1750, -1600, -1450, -1300, -1150, -1000, -850, -700,
-550, -400, -250, -100, 0, 50, 150, 200, 300, 350, 450, 600, 750, 900,
1050, 1200, 1350, 1500, 1650, 1800, 1950, 2100, 2250 ]
    default: 0


> 
>> We can't reduce this down to tx-clk-inverted.
>> There are two mac and two yt8531 on their board. Each of yt8531 need
>> different config in DTS. They need adjust tx clk delay in
>> link_change_notify callback function according to current speed.
>>
>>  They configured tx-clk-xxxx-inverted like this :
>>
>>     speed     GMAC0             GMAC1
>>     1000M      1                  0		tx-clk-1000-inverted
>>     100M       1                  1		tx-clk-100-inverted
>>     10M       0/1                0/1     	tx-clk-10-inverted
> 
> What MAC is this? It seems very oddly designed, getting close to
> broken. I've not seen any other MAC/PHY combination need anything like
> this. 
> 
>> Can we put tx-clk-adj-enabled, tx-clk-10-inverted, tx-clk-100-inverted
>> and tx-clk-1000-inverted in tx-clk-10-inverted like bit in byte?
> 
> No, they are individual boolean properties, so should be kept as they
> are. But i really think somebody should be looking deep into the MAC
> design to understand why it is like this, and if the MAC can sort out
> this mess itself.
> 
> 	Andrew

Tanks. We will remove tx-clk-xxxx-inverted and tx-clk-adj-enabled.

Andrew Lunn Jan. 17, 2023, 1:16 p.m. UTC | #11

> Hi Andrew, i'm an engineer from StarFive Technology Co.
> 
> This configuration is mainly to adapt to VF2 with JH7110 SoC.
> This is a defect in the design of JH7110. Only the basic delay 
> configuration of PHY can't meet the delay needs of JH7110,
> must with the configuration of tx-clk-x-inverted together can
> gmac work normally. Otherwise, gmac cannot work normally at 
> different rates. JH7110 has been taped and cannot be modified 
> in design, so it can only be corrected on software.

So you have a choice of one PHY, since i don't know of any other PHY
with this capability. I hope this is well documented, since PHYs tend
to be considered interchangeable, use whatever is currently the
cheapest. And i assume you will fix the MAC for the next version of
the silicon?

So all these clock invert properties are fine.

I will take another look at the delay property..

  Andrew

Andrew Lunn Jan. 18, 2023, 1:40 p.m. UTC | #12

On Wed, Jan 11, 2023 at 05:20:18PM +0800, Frank.Sae wrote:
> Hi Andrew,
> 
> On 2023/1/6 21:55, Andrew Lunn wrote:
> >>> Why is this needed? When the MAC driver connects to the PHY, it passes
> >>> phy-mode. For RGMII, this is one of:
> >>
> >>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII,
> >>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_ID,
> >>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_RXID,
> >>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_TXID,
> >>>
> >>> This tells you if you need to add a delay for the RX clock line, the
> >>> TX clock line, or both. That is all you need to know for basic RGMII
> >>> delays.
> >>>
> >>
> >> This basic delay can be controlled by hardware or the phy-mode which
> >> passes from MAC driver.
> >> Default value depends on power on strapping, according to the voltage
> >> of RXD0 pin (low = 0, turn off;   high = 1, turn on).
> >>
> >> Add this for the case that This basic delay is controlled by hardware,
> >> and software don't change this.
> > 
> > You should always do what phy-mode contains. Always. We have had
> > problems in the past where a PHY driver ignored the phy-mode, and left
> > the PHY however it was strapped. Which worked. But developers put the
> > wrong phy-mode value in DT. Then somebody had a board which actually
> > required that the DT value really did work, because the strapping was
> > wrong. So the driver was fixed to respect the PHY mode, made that
> > board work, and broke all the other boards which had the wrong
> > phy-mode in DT.
> > 
> > If the user want the driver to leave the mode alone, use the
> > strapping, they should use PHY_INTERFACE_MODE_NA. It is not well
> > documented, but it is used in a few places. However, i don't recommend
> > it.
> > 
> 
> RX delay = rx-delay-basic (0ns or 1.9ns) + x-delay-additional-ps
> (N*150ps, N = 0 ~ 15)
>  If rx-delay-basic is removed and controlled by phy-mode.
>  when phy-mode is  rgmii-id or rgmii-rxid, RX delay is 1.9ns + N*150ps.
>  But sometimes 1.9ns is still too big, we just need  0ns + N*150ps.
> 
> For this case, can we do like following ?
> rx-internal-delay-ps:
>     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: 0
>  rx-internal-delay-ps is 0ns + N*150ps and  1.9ns + N*150ps.
>  And check whether need rx-delay-basic (1.9ns) by the val of
> rx-internal-delay-ps?

Please take a look at phy_get_internal_delay() and the drivers which
use it.

    Andrew

Frank Sae Jan. 19, 2023, 5:56 a.m. UTC | #13

Hi Andrew,

On 2023/1/18 21:40, Andrew Lunn wrote:
> On Wed, Jan 11, 2023 at 05:20:18PM +0800, Frank.Sae wrote:
>> Hi Andrew,
>>
>> On 2023/1/6 21:55, Andrew Lunn wrote:
>>>>> Why is this needed? When the MAC driver connects to the PHY, it passes
>>>>> phy-mode. For RGMII, this is one of:
>>>>
>>>>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII,
>>>>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_ID,
>>>>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_RXID,
>>>>> linux/phy.h:	PHY_INTERFACE_MODE_RGMII_TXID,
>>>>>
>>>>> This tells you if you need to add a delay for the RX clock line, the
>>>>> TX clock line, or both. That is all you need to know for basic RGMII
>>>>> delays.
>>>>>
>>>>
>>>> This basic delay can be controlled by hardware or the phy-mode which
>>>> passes from MAC driver.
>>>> Default value depends on power on strapping, according to the voltage
>>>> of RXD0 pin (low = 0, turn off;   high = 1, turn on).
>>>>
>>>> Add this for the case that This basic delay is controlled by hardware,
>>>> and software don't change this.
>>>
>>> You should always do what phy-mode contains. Always. We have had
>>> problems in the past where a PHY driver ignored the phy-mode, and left
>>> the PHY however it was strapped. Which worked. But developers put the
>>> wrong phy-mode value in DT. Then somebody had a board which actually
>>> required that the DT value really did work, because the strapping was
>>> wrong. So the driver was fixed to respect the PHY mode, made that
>>> board work, and broke all the other boards which had the wrong
>>> phy-mode in DT.
>>>
>>> If the user want the driver to leave the mode alone, use the
>>> strapping, they should use PHY_INTERFACE_MODE_NA. It is not well
>>> documented, but it is used in a few places. However, i don't recommend
>>> it.
>>>
>>
>> RX delay = rx-delay-basic (0ns or 1.9ns) + x-delay-additional-ps
>> (N*150ps, N = 0 ~ 15)
>>  If rx-delay-basic is removed and controlled by phy-mode.
>>  when phy-mode is  rgmii-id or rgmii-rxid, RX delay is 1.9ns + N*150ps.
>>  But sometimes 1.9ns is still too big, we just need  0ns + N*150ps.
>>
>> For this case, can we do like following ?
>> rx-internal-delay-ps:
>>     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: 0
>>  rx-internal-delay-ps is 0ns + N*150ps and  1.9ns + N*150ps.
>>  And check whether need rx-delay-basic (1.9ns) by the val of
>> rx-internal-delay-ps?
> 
> Please take a look at phy_get_internal_delay() and the drivers which
> use it.
> 
>     Andrew

 Thanks. But it may be not suitable.

rx-internal-delay-ps has two part:
0ns + N*150ps =
0,150,300,450,600,750,900,1050,1200,1350,1500,1650,1800,1950,2100,2250

1.9ns + N*150ps =
1900,2050,2200,2350,2500,2650,2800,2950,3100,3250,3400,3550,3700,3850,4000,4150

The problem is "1900,2050,2200" is less than "2250".

If I take this two parts in one sorted table, there will be three
tables, one for tx-internal-delay-ps, one for rx-internal-delay-ps and
one for the rx index to reg(delay value and 1.9ns on or off) value.

So we tend to use the following methods.

#define YT8521_CCR_RXC_DLY_1_900_NS		1900

#define YT8521_RC1R_RGMII_0_000_NS		0
#define YT8521_RC1R_RGMII_0_150_NS		1
...
#define	YT8521_RC1R_RGMII_2_250_NS		15

struct ytphy_cfg_reg_map {
	u32 cfg;
	u32 reg;
};

static const struct ytphy_cfg_reg_map ytphy_rgmii_delays[] = {
	/* for tx delay / rx delay with YT8521_CCR_RXC_DLY_EN is not set. */
	{ 0,	YT8521_RC1R_RGMII_0_000_NS },
	{ 150,	YT8521_RC1R_RGMII_0_150_NS },
	...
	{ 2250,	YT8521_RC1R_RGMII_2_250_NS },

	/* only for rx delay with YT8521_CCR_RXC_DLY_EN is set. */
	{ 0    + YT8521_CCR_RXC_DLY_1_900_NS,YT8521_RC1R_RGMII_0_000_NS },
	{ 150  + YT8521_CCR_RXC_DLY_1_900_NS,YT8521_RC1R_RGMII_0_150_NS },
	...
	{ 2250 + YT8521_CCR_RXC_DLY_1_900_NS,YT8521_RC1R_RGMII_2_250_NS }
};


static u32 ytphy_get_delay_reg_value(struct phy_device *phydev,
				     const char *prop_name,
				     const struct ytphy_cfg_reg_map *tbl,
				     int tb_size,
				     u16 *rxc_dly_en,
				     u32 dflt)
{
	struct device_node *node = phydev->mdio.dev.of_node;
	int tb_size_half = tb_size / 2;
	u32 val;
	int i;

	if (of_property_read_u32(node, prop_name, &val))
		return dflt;

	/* when rxc_dly_en is NULL, it is get the delay for tx, only half of
	 * tb_size is valid.
	 */
	if (!rxc_dly_en)
		tb_size = tb_size_half;

	for (i = 0; i < tb_size; i++) {
		if (tbl[i].cfg == val) {
			if (rxc_dly_en && i < tb_size_half)
				*rxc_dly_en = 0;
			return tbl[i].reg;
		}
	}

	phydev_warn(phydev, "Unsupported value %d for %s using default (%u)\n",
		    val, prop_name, dflt);
	return dflt;
}

static int ytphy_rgmii_clk_delay_config(struct phy_device *phydev)
{
	int tb_size = ARRAY_SIZE(ytphy_rgmii_delays);
	u16 rxc_dly_en = YT8521_CCR_RXC_DLY_EN;
	u32 rx_reg, tx_reg;
	u16 mask, val = 0;
	int ret;

	rx_reg = ytphy_get_delay_reg_value(phydev, "rx-internal-delay-ps",
					   ytphy_rgmii_delays, tb_size,
					   &rxc_dly_en,
					   YT8521_RC1R_RGMII_0_000_NS);
	tx_reg = ytphy_get_delay_reg_value(phydev, "tx-internal-delay-ps",
					   ytphy_rgmii_delays, tb_size, NULL,
					   YT8521_RC1R_RGMII_0_150_NS);

	switch (phydev->interface) {
	case PHY_INTERFACE_MODE_RGMII:
		rxc_dly_en = 0;
		break;
	case PHY_INTERFACE_MODE_RGMII_RXID:
		val |= FIELD_PREP(YT8521_RC1R_RX_DELAY_MASK, rx_reg);
		break;
	case PHY_INTERFACE_MODE_RGMII_TXID:
		rxc_dly_en = 0;
		val |= FIELD_PREP(YT8521_RC1R_GE_TX_DELAY_MASK, tx_reg);
		break;
	case PHY_INTERFACE_MODE_RGMII_ID:
		val |= FIELD_PREP(YT8521_RC1R_RX_DELAY_MASK, rx_reg) |
		       FIELD_PREP(YT8521_RC1R_GE_TX_DELAY_MASK, tx_reg);
		break;
	default: /* do not support other modes */
		return -EOPNOTSUPP;
	}

	ret = ytphy_modify_ext(phydev, YT8521_CHIP_CONFIG_REG,
			       YT8521_CCR_RXC_DLY_EN, rxc_dly_en);
	if (ret < 0)
		return ret;

	/* Generally, it is not necessary to adjust YT8521_RC1R_FE_TX_DELAY */
	mask = YT8521_RC1R_RX_DELAY_MASK | YT8521_RC1R_GE_TX_DELAY_MASK;
	return ytphy_modify_ext(phydev, YT8521_RGMII_CONFIG1_REG, mask, val);
}

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..337a562d864c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
@@ -0,0 +1,180 @@ 
+# 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 <[e-mail]>
+
+description: |
+  Bindings for MotorComm yt8xxx PHYs.
+  yt8511 will be supported later.
+
+allOf:
+  - $ref: ethernet-phy.yaml#
+
+properties:
+  motorcomm,clk-out-frequency:
+    description: clock output in Hertz on clock output pin.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 25000000, 125000000]
+    default: 0
+
+  motorcomm,rx-delay-basic:
+    description: |
+      Tristate, setup the basic RGMII RX Clock delay of PHY.
+      This basic delay is fixed at 2ns (1000Mbps) or 8ns (100Mbps、10Mbps).
+      This basic delay usually auto set by hardware according to the voltage
+      of RXD0 pin (low = 0, turn off;   high = 1, turn on).
+      If not exist, this delay is controlled by hardware.
+      0: turn off;   1: turn on.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+
+  motorcomm,rx-delay-additional-ps:
+    description: |
+      Setup the additional RGMII RX Clock delay of PHY defined in pico seconds.
+      RGMII RX Clock Delay = rx-delay-basic + rx-delay-additional-ps.
+    enum:
+      - 0
+      - 150
+      - 300
+      - 450
+      - 600
+      - 750
+      - 900
+      - 1050
+      - 1200
+      - 1350
+      - 1500
+      - 1650
+      - 1800
+      - 1950
+      - 2100
+      - 2250
+
+  motorcomm,tx-delay-ge-ps:
+    description: |
+      Setup PHY's RGMII TX Clock delay defined in pico seconds when the speed
+      is 1000Mbps.
+    enum:
+      - 0
+      - 150
+      - 300
+      - 450
+      - 600
+      - 750
+      - 900
+      - 1050
+      - 1200
+      - 1350
+      - 1500
+      - 1650
+      - 1800
+      - 1950
+      - 2100
+      - 2250
+
+  motorcomm,tx-delay-fe-ps:
+    description: |
+      Setup PHY's RGMII TX Clock delay  defined in pico seconds when the speed
+      is 100Mbps or 10Mbps.
+    enum:
+      - 0
+      - 150
+      - 300
+      - 450
+      - 600
+      - 750
+      - 900
+      - 1050
+      - 1200
+      - 1350
+      - 1500
+      - 1650
+      - 1800
+      - 1950
+      - 2100
+      - 2250
+
+  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: |
+      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
+
+  motorcomm,sds-tx-amplitude:
+    description: |
+      Setup the tx driver amplitude control of SerDes. Higher amplitude is
+      helpful for long distance.
+      0: low;   1: middle;   2: high.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2]
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    ethernet {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        ethernet-phy@5 {
+            reg = <5>;
+
+            motorcomm,clk-out-frequency = <0>;
+            #motorcomm,rx-delay-basic = <1>;
+            motorcomm,rx-delay-additional-ps = <0>;
+            motorcomm,tx-delay-fe-ps = <2250>;
+            motorcomm,tx-delay-ge-ps = <150>;
+
+            motorcomm,keep-pll-enabled;
+            motorcomm,auto-sleep-disabled;
+            motorcomm,sds-tx-amplitude = <1>;
+        };
+    };
+  - |
+    ethernet {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        phy-mode = "rgmii-id";
+        ethernet-phy@5 {
+            reg = <5>;
+
+            motorcomm,clk-out-frequency = <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 70ffb3780621..8d19157e85b7 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -845,6 +845,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 7f0b7181e60a..a1e714980154 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14161,6 +14161,7 @@  M:	Peter Geis <[e-mail]>
 M:	Frank <[e-mail]>
 L:	[e-mail]
 S:	Maintained
+F:	Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
 F:	drivers/net/phy/motorcomm.c
 
 MOXA SMARTIO/INDUSTIO/INTELLIO SERIAL CARD