Message-ID:
 <PAXPR04MB85103AF88D92A3B08AA7D71788952@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Wed, 28 Aug 2024 07:35:51 +0000
From: Wei Fang <wei.fang@....com>
To: Andrew Lunn <andrew@...n.ch>
CC: Rob Herring <robh@...nel.org>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"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>, "linux@...linux.org.uk"
	<linux@...linux.org.uk>, "Andrei Botila (OSS)" <andrei.botila@....nxp.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 v3 net-next 1/2] dt-bindings: net: tja11xx: add
 "nxp,phy-output-refclk" property

> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: 2024年8月27日 20:35
> To: Wei Fang <wei.fang@....com>
> Cc: Rob Herring <robh@...nel.org>; davem@...emloft.net;
> edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com;
> krzk+dt@...nel.org; conor+dt@...nel.org; f.fainelli@...il.com;
> hkallweit1@...il.com; linux@...linux.org.uk; Andrei Botila (OSS)
> <andrei.botila@....nxp.com>; netdev@...r.kernel.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; imx@...ts.linux.dev
> 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.

Powered by blists - more mailing lists