| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 6 Jan 2023 14:51:27 +0800
From: Frank <Frank.Sae@...or-comm.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Peter Geis <pgwipeout@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
xiaogang.fan@...or-comm.com, fei.zhang@...or-comm.com,
hua.sun@...or-comm.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH net-next v1 1/3] dt-bindings: net: Add Motorcomm yt8xxx
ethernet phy Driver bindings
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<yanhong.wang@...rfivetech.com>. 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
Powered by blists - more mailing lists