linux-kernel - Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b98d6bab-d3bb-44d2-9dbf-623dfb673671@lunn.ch>
Date: Tue, 29 Apr 2025 18:56:08 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Chen-Yu Tsai <wens@...e.org>
Cc: Jernej Škrabec <jernej.skrabec@...il.com>,
	Andre Przywara <andre.przywara@....com>, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org, samuel@...lland.org,
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] arm64: dts: allwinner: h6: Add OrangePi 3 LTS DTS

> > The RX path looks O.K. RGMII-RXID means the PHY should be adding the
> > 2ns delay. The allwinner,rx-delay-ps = <0> should be redundant, that
> > should be the driver default. And there are no properties in the PHY
> > node about RX. All good.
> 
> The default action when the property is missing is to leave the hardware
> settings alone. I admit this doesn't match the bindings.

Please submit a patch fix the binding.

> > TX is the problem. The allwinner,tx-delay-ps = <700> causes the MAC to
> > add 700ps delay, and 'rgmii-rxid' means the PHY should not add any
> > delay. But 700ps is too low. It should be around 2000ps. So something
> > else is adding a delay, or the 700ps is not really 700ps.
> 
> Anything is possible. As was raised in a previous reply, it's possible
> instead of extending the delay range, the decreased the step size and
> added more steps. The problem is we don't really know.

By poking around with other configuration knobs, i hope we can
determine if this 700ps actually is 700ps.

> > You say the PHY is a YT8531C. These PHYs also accept
> > rx-internal-delay-ps and tx-internal-delay-ps properties in their DT
> > node.
> >
> > Try setting 'rgmii-id', allwinner,tx-delay-ps = <0>, and both
> > rx-internal-delay-ps and tx-internal-delay-ps in the PHY node to 1950.
> > If that does not work, try other values in the PHY node.
> 
> I don't get why we should ignore the strappings instead of using them
> as reference or even truth.

If you don't want the PHY to be reprogrammed pass:

* @PHY_INTERFACE_MODE_NA: Not Applicable - don't touch

rather than one of

 * @PHY_INTERFACE_MODE_RGMII: Reduced gigabit media-independent interface
 * @PHY_INTERFACE_MODE_RGMII_ID: RGMII with Internal RX+TX delay
 * @PHY_INTERFACE_MODE_RGMII_RXID: RGMII with Internal RX delay
 * @PHY_INTERFACE_MODE_RGMII_TXID: RGMII with Internal TX delay

However, if we do pass one of these RGMII modes, i expect the PHY to
follow them.

> If the strappings worked correctly w/ the
> generic PHY driver (that doesn't know how to configure the delay mode
> on the PHY side), isn't it working as intended?

The generic PHY driver is there as a fallback, for when the real PHY
driver is missing. It is nice if it works, but it often does not in
current systems.  What we really care about is that the real PHY
driver works, and the system as a whole follows the DT bindings.

	Andrew