> -----Original Message----- > From: Andrew Lunn <andrew@xxxxxxx> > Sent: 2024年8月27日 20:35 > To: Wei Fang <wei.fang@xxxxxxx> > Cc: Rob Herring <robh@xxxxxxxxxx>; davem@xxxxxxxxxxxxx; > edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; > krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; f.fainelli@xxxxxxxxx; > hkallweit1@xxxxxxxxx; linux@xxxxxxxxxxxxxxx; Andrei Botila (OSS) > <andrei.botila@xxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v3 net-next 1/2] dt-bindings: net: tja11xx: add > "nxp,phy-output-refclk" property > > > > This binding is completely broken. I challenge you to make it report any > errors. > > > Those issues need to be addressed before you add more properties. > > > > > Sorry, I'm not sure I fully understand what you mean, do you mean I > > need to move the "nxp,rmii-refclk-in" property out of the patternProperties? > > Just like below? > > +properties: > > + nxp,rmii-refclk-in: > > + type: boolean > > + description: | > > + The REF_CLK is provided for both transmitted and received data > > + in RMII mode. This clock signal is provided by the PHY and is > > + typically derived from an external 25MHz crystal. Alternatively, > > + a 50MHz clock signal generated by an external oscillator can be > > + connected to pin REF_CLK. A third option is to connect a 25MHz > > + clock to pin CLK_IN_OUT. So, the REF_CLK should be configured > > + as input or output according to the actual circuit connection. > > + If present, indicates that the REF_CLK will be configured as > > + interface reference clock input when RMII mode enabled. > > + If not present, the REF_CLK will be configured as interface > > + reference clock output when RMII mode enabled. > > + Only supported on TJA1100 and TJA1101. > > > > patternProperties: > > "^ethernet-phy@[0-9a-f]+$": > > @@ -32,28 +71,6 @@ patternProperties: > > description: > > The ID number for the child PHY. Should be +1 of parent PHY. > > > > - nxp,rmii-refclk-in: > > - type: boolean > > - description: | > > - The REF_CLK is provided for both transmitted and received data > > - in RMII mode. This clock signal is provided by the PHY and is > > - typically derived from an external 25MHz crystal. Alternatively, > > - a 50MHz clock signal generated by an external oscillator can be > > - connected to pin REF_CLK. A third option is to connect a 25MHz > > - clock to pin CLK_IN_OUT. So, the REF_CLK should be configured > > - as input or output according to the actual circuit connection. > > - If present, indicates that the REF_CLK will be configured as > > - interface reference clock input when RMII mode enabled. > > - If not present, the REF_CLK will be configured as interface > > - reference clock output when RMII mode enabled. > > - Only supported on TJA1100 and TJA1101. > > > > > If you want/need custom properties, then you must have a compatible > string. > > > > > I looked at the binding documentation of other PHYs and there doesn't > > seem to be any precedent for doing this. Is this a newly added dt-binding > rule? > > > > There is another question. For PHY, usually its compatible string is > > either "ethernet-phy-ieee802.3-c45" or "ethernet-phy-ieee802.3-c22". > > If I want to add a custom property to TJA11xx PHY, can I use these > > generic compatible strings? As shown below: > > This is where we get into the differences between how the kernel actually > works, and how the tools work. The kernel does not need a compatible, it > reads the ID registers and uses that to load the driver. You can optionally have > a compatible with the contents of the ID registers, and that will force the > kernel to ignore the ID in the hardware and load a specific driver. > > The DT tools however require a compatible in order to match the node in the > blob to the binding in a .yaml file. Without the compatible, the binding is not > imposed, which is why you will never see an error. > > So in the example, include a compatible, using the real ID. > > For a real DT blob, you need to decide if you want to include a compatible or > not. The downside is that it forces the ID. It is not unknown for board > manufacturers to replace a PHY with another pin compatible PHY. Without a > compatible, the kernel will load the correct driver, based on the ID. With a > compatible it will keep using the same driver, which is probably wrong for the > hardware. > > Does the PHY use the lower nibble to indicate the revision? Using a compatible > will also override the revision. So the driver cannot even trust the revision if > there is a compatible. > Many thanks for the detailed explanation, currently both nxp-tja11xx and nxp-c45-tja11xx drivers use PHY_ID_MATCH_MODEL() to match the PHY driver, so the lower nibble is ignored.
