netdev - Re: [net,PATCH] net: phy: dp83869: fix STRAP

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: <3b5e2a79-186e-4c92-9bef-51fdd34af351@lunn.ch>
Date: Mon, 27 Oct 2025 00:48:40 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Marek Vasut <marek.vasut@...lbox.org>
Cc: netdev@...r.kernel.org, Thanh Quan <thanh.quan.xn@...esas.com>,
	Hai Pham <hai.pham.ud@...esas.com>,
	"David S. Miller" <davem@...emloft.net>,
	Dan Murphy <dmurphy@...com>, Eric Dumazet <edumazet@...gle.com>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Russell King <linux@...linux.org.uk>,
	linux-renesas-soc@...r.kernel.org
Subject: Re: [net,PATCH] net: phy: dp83869: fix STRAP_OPMODE bitmask

On Sat, Oct 25, 2025 at 05:09:17AM +0200, Marek Vasut wrote:
> On 10/24/25 2:24 AM, Andrew Lunn wrote:
> 
> Hello Andrew,
> 
> > On Fri, Oct 24, 2025 at 12:39:45AM +0200, Marek Vasut wrote:
> > > From: Thanh Quan <thanh.quan.xn@...esas.com>
> > > 
> > > According to the TI DP83869HM datasheet Revision D (June 2025), section
> > > 7.6.1.41 STRAP_STS Register, the STRAP_OPMODE bitmask is bit [11:9].
> > > Fix this.
> > 
> > It is a good idea to state in the commit message what the bad
> > behaviour is which the patch fixes. Somebody looking through the
> > patches can then decide if they need to cherry-pick the patch into
> > their dead vendor tree, etc.
> > 
> > Please add to the commit message what issue you where seeing which
> > made you create this patch.
> 
> In short, on the hardware I use, the interface to the PHY is SGMII, but the
> driver is confused into thinking it is RGMII based on the STRAP_STS register
> content, and misconfigures the PHY for RGMII.
> 
> I plan to extend the commit message this way. I tried to cover all the bases
> there, so people can decide whether the are affected or not. Is this
> understandable or is it maybe too much ?
> 
> "
> net: phy: dp83869: fix STRAP_OPMODE bitmask
> 
> According to the TI DP83869HM datasheet Revision D (June 2025), section
> 7.6.1.41 STRAP_STS Register, the STRAP_OPMODE bitmask is bit [11:9].
> Fix this.
> 
> In case the PHY is auto-detected via PHY ID registers, or not described
> in DT, or, in case the PHY is described in DT but the optional DT property
> "ti,op-mode" is not present, then the driver reads out the PHY functional
> mode (RGMII, SGMII, ...) from hardware straps.
> 
> Currently, all upstream users of this PHY specify both DT compatible string
> "ethernet-phy-id2000.a0f1" and ti,op-mode = <DP83869_RGMII_COPPER_ETHERNET>
> property, therefore it seems no upstream users are affected by this bug.
> 
> The driver currently interprets bits [2:0] of STRAP_STS register as PHY
> functional mode. Those bits are controlled by ANEG_DIS, ANEGSEL_0 straps
> and an always-zero reserved bit. Systems that use RGMII-to-Copper functional
> mode are unlikely to disable auto-negotiation via ANEG_DIS strap, or change
> auto-negotiation behavior via ANEGSEL_0 strap. Therefore, even with this bug
> in place, the STRAP_STS register content is likely going to be interpreted
> by the driver as RGMII-to-Copper mode.
> 
> However, for a system with PHY functional mode strapping set to other mode
> than RGMII-to-Copper, the driver is likely to misinterpret the strapping
> as RGMII-to-Copper and misconfigure the PHY.
> 
> For example, on a system with SGMII-to-Copper strapping, the STRAP_STS
> register reads as 0x0c20, but the PHY ends up being configured for
> incompatible RGMII-to-Copper mode.
> "

This is good. It nice to have lots of details, cause and effect, even
if its a silly topo bug.

Please add my Reviewed-by to the next version.

   Andrew