linux-kernel - Re: [PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and MBa62xx carrier board Device Trees

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d25b1447-c28b-4998-b238-92672434dc28@lunn.ch>
Date: Mon, 9 Dec 2024 15:42:52 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Matthias Schiffer <matthias.schiffer@...tq-group.com>
Cc: Nishanth Menon <nm@...com>, Vignesh Raghavendra <vigneshr@...com>,
	Tero Kristo <kristo@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Kees Cook <kees@...nel.org>, Tony Luck <tony.luck@...el.com>,
	"Guilherme G. Piccoli" <gpiccoli@...lia.com>,
	Felipe Balbi <balbi@...nel.org>,
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	linux-hardening@...r.kernel.org, Devarsh Thakkar <devarsht@...com>,
	Hari Nagalla <hnagalla@...com>, linux@...tq-group.com
Subject: Re: [PATCH v2 5/5] arm64: dts: ti: Add TQ-Systems TQMa62xx SoM and
 MBa62xx carrier board Device Trees

On Mon, Dec 09, 2024 at 02:55:31PM +0100, Matthias Schiffer wrote:
> On Mon, 2024-12-09 at 14:24 +0100, Andrew Lunn wrote:
> > 
> > > +&cpsw_port1 {
> > > +	phy-mode = "rgmii-rxid";
> > > +	phy-handle = <&cpsw3g_phy0>;
> > > +};
> > > +
> > > +&cpsw_port2 {
> > > +	phy-mode = "rgmii-rxid";
> > > +	phy-handle = <&cpsw3g_phy3>;
> > > +};
> > 
> > rgmii-rxid is very odd.
> > 
> > > +
> > > +&cpsw3g_mdio {
> > > +	status = "okay";
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&main_mdio1_pins>;
> > > +
> > > +	cpsw3g_phy0: ethernet-phy@0 {
> > > +		compatible = "ethernet-phy-ieee802.3-c22";
> > > +		reg = <0x0>;
> > > +		reset-gpios = <&main_gpio1 11 GPIO_ACTIVE_LOW>;
> > > +		reset-assert-us = <1000>;
> > > +		reset-deassert-us = <1000>;
> > > +		ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> > 
> > I guess this is the explanation.
> > 
> > What happens when you use rgmii-id, and don't have this delay here?
> > That would be normal.
> > 
> > 	Andrew
> 
> 
> This is normal for AM62-based boards, see the DTSI of the TI reference
> starterkit for example:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi#n451
> 
> With rgmii-id, both ti,rx-internal-delay and ti,tx-internal-delay should be set.
> As ti,*-internal-delay sets the delay on the PHY side, phy-mode "rgmii" is the
> one that would not use either:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ti,dp83867.yaml#n78
> 
> At the end of the day, does it really matter as long as MAC and PHY agree on the
> used mode? We copied this part of the hardware design from the TI reference
> board, and did our hardware qualification with these settings, so I think it
> makes sense to use the same phy-mode configuration.

What i try to achieve is every board uses the same configuration. The
PHY adds the delays, not the MAC. There are a few exceptions, because
a few cheap PHYs don't support delays, and so the MAC needs to add
them. But in general, any board i review, i always ask that the PHY
does the delay.

Also, don't put too much value in vendor code. Vendors don't care
about Linux has a whole, being uniform across all systems. Many
vendors do the minimum to get their stuff working, sometimes Monkeys
typing Shakespeare, and not a lot more.

I also find a lot of developers don't really understand what phy-mode
and PHY_INTERFACE_MODE_RGMII_* actually mean. phy-mode = 'rgmii' means
the board has extra long clock lines, so the MAC/PHY does not need to
add delays. rgmii-rxid means the board has an extra long rx clock
line, but a normal length tx clock line. Now, i doubt your board is
actually like this?

You want to correctly describe your hardware in DT, which i guess is
"rgmii-id". That means something, either the MAC or the PHY needs to
add delays. PHY_INTERFACE_MODE_RGMII_* is what is passed to the
PHY. To get it to add the 2ns delays, you pass
PHY_INTERFACE_MODE_RGMII_ID, and you should not need any additional
properties in DT, it should default to 2ns. If you need to tune the
delay, 2ns does not work, but you actually need 1.8ns etc, then you
can add additional parameters. But given you have
DP83867_RGMIIDCTL_2_00_NS, i doubt you need this.

	Andrew