linux-kernel - Re: [PATCH v2 3/4] dt-bindings: net: mdio: add phy-id-read-needs-reset property

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: <aPh9IzFNZx8FD1hR@debianbuilder>
Date: Wed, 22 Oct 2025 08:43:47 +0200
From: Buday Csaba <buday.csaba@...lan.hu>
To: Rob Herring <robh@...nel.org>
CC: Andrew Lunn <andrew@...n.ch>, 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>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Florian Fainelli
	<f.fainelli@...il.com>, <netdev@...r.kernel.org>,
	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 3/4] dt-bindings: net: mdio: add
 phy-id-read-needs-reset property

On Tue, Oct 21, 2025 at 03:10:23PM -0500, Rob Herring wrote:
> On Wed, Oct 15, 2025 at 03:45:02PM +0200, Buday Csaba wrote:
> > Some Ethernet PHYs require a hard reset before accessing their MDIO
> > registers. When the ID is not provided by a compatible string,
> > reading the PHY ID may fail on such devices.
> > 
> > This patch introduces a new device tree property called
> > `phy-id-read-needs-reset`, which can be used to hard reset the
> > PHY before attempting to read its ID via MDIO.
> > 
> > Signed-off-by: Buday Csaba <buday.csaba@...lan.hu>
> > ---
> >  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > index 2ec2d9fda..b570f8038 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > @@ -215,6 +215,14 @@ properties:
> >        Delay after the reset was deasserted in microseconds. If
> >        this property is missing the delay will be skipped.
> >  
> > +  phy-id-read-needs-reset:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description:
> > +      Some PHYs require a hard reset before accessing MDIO registers.
> > +      This workaround allows auto-detection of the PHY ID in such cases.
> > +      When the PHY ID is provided with the 'compatible' string, setting
> > +      this property has no effect.
> 
> If the phy is listed in DT, then it should have a compatible. Therefore, 
> you don't need this property.
> 
> Rob
> 

Yes, it has a compatible, but most of the time it is just
"ethernet-phy-ieee802.e-c22" (or c45), and the specific ID is not provided.

Doing a quick search: of all the DTs only 126 specify a chip ID.
There are 198 matches for "ethernet-phy-ieee802.3-c22" only.

The current description reads: 
"
      - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$"
        description:
          If the PHY reports an incorrect ID (or none at all) then [...]
"

This description tells me, that we are only encouraged to hardcode the 
chip ID in the DT, when it is read incorrectly.

The PHY we are having trouble with reports its ID perfectly, once properly
initialized: clock established and hard reset asserted after that.

I only wish to implement this condition. Andrew had a concern, that if we
unconditionally assert the reset, that may lead breaking compatibility with
some existing systems, so my best idea was to come up with a DT property
that can be used to flag this.

I also wish to emphasize: there are 43 DTs out there still using the
deprecated "phy-reset-gpios" property of the fec driver, probably for the
same reason: there is no replacement for it. Because that property does
reset the PHY before the ID read, but it has all kind of other problems.
The worst is that reset gpio handle is tossed after the initialization,
see fec_reset_phy().

If we ever want to truly deprecate that, there must be some alternative
logic to implement the same funcionality.

Csaba