netdev - RE: [PATCH net] dt-bindings: net: tja11xx: fix the broken binding

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:
 <PAXPR04MB8510155101205EAF77C233C7889C2@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Wed, 4 Sep 2024 01:20:08 +0000
From: Wei Fang <wei.fang@....com>
To: Andrew Lunn <andrew@...n.ch>
CC: "davem@...emloft.net" <davem@...emloft.net>, "pabeni@...hat.com"
	<pabeni@...hat.com>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, "f.fainelli@...il.com" <f.fainelli@...il.com>,
	"hkallweit1@...il.com" <hkallweit1@...il.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>
Subject: RE: [PATCH net] dt-bindings: net: tja11xx: fix the broken binding

> On Tue, Sep 03, 2024 at 02:17:04AM +0000, Wei Fang wrote:
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - ethernet-phy-id0180.dc40
> > > > +      - ethernet-phy-id0180.dd00
> > > > +      - ethernet-phy-id0180.dc80
> > > > +      - ethernet-phy-id001b.b010
> > > > +      - ethernet-phy-id001b.b031
> > >
> > > This shows the issues with using a compatible. The driver has:
> > >
> > > #define PHY_ID_TJA_1120                 0x001BB031
> > >
> > >                 PHY_ID_MATCH_MODEL(PHY_ID_TJA_1120),
> > >
> > > which means the lowest nibble is ignored. The driver will quite
> > > happy also probe for hardware using 001b.b030, 001b.b032, 001b.b033,
> > > ... 001b.b03f
> > >
> > > Given you are inside NXP, do any of these exist? Was 001b.b030 too
> > > broken it never left the QA lab? Are there any hardware issues which
> > > might result in a new silicon stepping?
> >
> > Yes, some of the revisions do exist, but the driver should be
> > compatible with these different revisions.
> >
> > For 001b.b030, I don't think it is broken, based on the latest data
> > sheet of
> > TJA1120 (Rev 0.6 26 January 2023), the PHY ID is 001b.b030. I don't
> > know why it is defined as 001b.b031 in the driver, it may be a typo.
> 
> More likely, the board Radu Pirea has does have a device with this ID.
> 
> > >
> > > Does ethernet-phy-id0180.dc41 exist? etc.
> > I think other TJA PHYs should also have different revisions.
> >
> > Because the driver ignores the lowest nibble of the PHY ID, I think it
> > is fine to define the lowest nibble of the PHY ID in these compatible
> > strings as 0, and there is no need to list all revisions. And I don't
> > know which revisions exist, because I haven't found or have no
> > permission to download some PHY data sheets. I think what I can do is
> > to modify "ethernet-phy-id001b.b031" to "ethernet-phy-id001b.b030".
> 
> You have to be careful here. Stating a compatible forces the PHY ID. So if the
> compatible is "ethernet-phy-id001b.b031", but the board actually has a
> "ethernet-phy-id001b.b030". phydev->phy_id is going to be set to 0x001bb031.
> Any behaviour in the driver which look at that revision nibble is then going to be
> wrong.
> 
> Maybe, now, today, that does not matter, because the driver never looks at the
> revision. But it does mean developers might put the wrong compatible in DT.
> And then when you do need to add code looking at the revision, it does not
> always work, because there are some boards with the wrong compatible in DT.
> 
> Listing all possible compatibles suggests to developers they need to be careful
> and use the correct value. Or add a comment in the DT bindings that not using a
> compatible is probably safer.
> 
Thanks for the suggestion, I will try my best to check more data sheets and list all
the PHY IDs I know. I can't guarantee that I can list all the PHY IDs, but I will update
it in the future when I get more information.