> 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.
