| Message ID | 20200505104215.8975-1-o.rempel@pengutronix.de |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v1] dt-bindings: net: nxp,tja11xx: rework validation support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Tue, May 05, 2020 at 12:42:15PM +0200, Oleksij Rempel wrote: > To properly identify this node, we need to use ethernet-phy-id0180.dc80. > And add missing required properties. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > .../devicetree/bindings/net/nxp,tja11xx.yaml | 55 ++++++++++++------- > 1 file changed, 35 insertions(+), 20 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > index 42be0255512b3..cc322107a24a2 100644 > --- a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > @@ -1,4 +1,4 @@ > -# SPDX-License-Identifier: GPL-2.0+ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > %YAML 1.2 > --- > $id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > @@ -12,44 +12,59 @@ maintainers: > - Heiner Kallweit <hkallweit1@gmail.com> > > description: > - Bindings for NXP TJA11xx automotive PHYs > + Bindings for the NXP TJA1102 automotive PHY. This is a dual PHY package where > + only the first PHY has global configuration register and HW health > + monitoring. > > -allOf: > - - $ref: ethernet-phy.yaml# > +properties: > + compatible: > + const: ethernet-phy-id0180.dc80 > + description: ethernet-phy-id0180.dc80 used for TJA1102 PHY > + > + reg: > + minimum: 0 > + maximum: 14 > + description: > + The PHY address of the parent PHY. Hi Oleksij reg is normally 0 to 31, since that is the address range for MDIO. Did you use 14 here because of what strapping allows? > +required: > + - compatible > + - reg > + - '#address-cells' > + - '#size-cells' So we have two different meanings of 'required' here. One meaning is the code requires it. compatible is not required, the driver will correctly be bind to the device based on its ID registers. Is reg also required by the code? The second meaning is about keeping the yaml verifier happy. It seems like compatible is needed for the verifier. Is reg also required? We do recommend having reg, but the generic code does not require it. Andrew
On Tue, May 05, 2020 at 04:01:27PM +0200, Andrew Lunn wrote: > On Tue, May 05, 2020 at 12:42:15PM +0200, Oleksij Rempel wrote: > > To properly identify this node, we need to use ethernet-phy-id0180.dc80. > > And add missing required properties. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > .../devicetree/bindings/net/nxp,tja11xx.yaml | 55 ++++++++++++------- > > 1 file changed, 35 insertions(+), 20 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > index 42be0255512b3..cc322107a24a2 100644 > > --- a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > @@ -1,4 +1,4 @@ > > -# SPDX-License-Identifier: GPL-2.0+ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > %YAML 1.2 > > --- > > $id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > > @@ -12,44 +12,59 @@ maintainers: > > - Heiner Kallweit <hkallweit1@gmail.com> > > > > description: > > - Bindings for NXP TJA11xx automotive PHYs > > + Bindings for the NXP TJA1102 automotive PHY. This is a dual PHY package where > > + only the first PHY has global configuration register and HW health > > + monitoring. > > > > -allOf: > > - - $ref: ethernet-phy.yaml# > > +properties: > > + compatible: > > + const: ethernet-phy-id0180.dc80 > > + description: ethernet-phy-id0180.dc80 used for TJA1102 PHY > > + > > + reg: > > + minimum: 0 > > + maximum: 14 > > + description: > > + The PHY address of the parent PHY. > > Hi Oleksij > > reg is normally 0 to 31, since that is the address range for MDIO. > Did you use 14 here because of what strapping allows? Yes. Only BITs 1:3 are configurable. BIT(0) is always 0 for the PHY0 and 1 for the PHY1 > > +required: > > + - compatible > > + - reg > > + - '#address-cells' > > + - '#size-cells' > > So we have two different meanings of 'required' here. > > One meaning is the code requires it. compatible is not required, the > driver will correctly be bind to the device based on its ID registers. > Is reg also required by the code? > > The second meaning is about keeping the yaml verifier happy. It seems > like compatible is needed for the verifier. Is reg also required? We > do recommend having reg, but the generic code does not require it. reg is used by: tja1102_p0_probe() tja1102_p1_register() of_mdio_parse_addr() But this is required for the slave PHY. I assume the reg can be optional for the master PHY. Should I? Regards, Oleksij
> > Hi Oleksij > > > > reg is normally 0 to 31, since that is the address range for MDIO. > > Did you use 14 here because of what strapping allows? > > Yes. Only BITs 1:3 are configurable. BIT(0) is always 0 for the PHY0 and 1 > for the PHY1 O.K. good. > > > +required: > > > + - compatible > > > + - reg > > > + - '#address-cells' > > > + - '#size-cells' > > > > So we have two different meanings of 'required' here. > > > > One meaning is the code requires it. compatible is not required, the > > driver will correctly be bind to the device based on its ID registers. > > Is reg also required by the code? > > > > The second meaning is about keeping the yaml verifier happy. It seems > > like compatible is needed for the verifier. Is reg also required? We > > do recommend having reg, but the generic code does not require it. > > reg is used by: > tja1102_p0_probe() > tja1102_p1_register() > of_mdio_parse_addr() > > But this is required for the slave PHY. I assume the reg can be > optional for the master PHY. Should I? It is recommended to have a reg value. So lets leave it as is for the moment. If anybody really does need it to be optional, we can change it later. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Tue, May 05, 2020 at 04:01:27PM +0200, Andrew Lunn wrote: > On Tue, May 05, 2020 at 12:42:15PM +0200, Oleksij Rempel wrote: > > To properly identify this node, we need to use ethernet-phy-id0180.dc80. > > And add missing required properties. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > .../devicetree/bindings/net/nxp,tja11xx.yaml | 55 ++++++++++++------- > > 1 file changed, 35 insertions(+), 20 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > index 42be0255512b3..cc322107a24a2 100644 > > --- a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > @@ -1,4 +1,4 @@ > > -# SPDX-License-Identifier: GPL-2.0+ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > %YAML 1.2 > > --- > > $id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > > @@ -12,44 +12,59 @@ maintainers: > > - Heiner Kallweit <hkallweit1@gmail.com> > > > > description: > > - Bindings for NXP TJA11xx automotive PHYs > > + Bindings for the NXP TJA1102 automotive PHY. This is a dual PHY package where > > + only the first PHY has global configuration register and HW health > > + monitoring. > > > > -allOf: > > - - $ref: ethernet-phy.yaml# > > +properties: > > + compatible: > > + const: ethernet-phy-id0180.dc80 > > + description: ethernet-phy-id0180.dc80 used for TJA1102 PHY > > + > > + reg: > > + minimum: 0 > > + maximum: 14 > > + description: > > + The PHY address of the parent PHY. > > Hi Oleksij > > reg is normally 0 to 31, since that is the address range for MDIO. > Did you use 14 here because of what strapping allows? > > > +required: > > + - compatible > > + - reg > > + - '#address-cells' > > + - '#size-cells' > > So we have two different meanings of 'required' here. > > One meaning is the code requires it. compatible is not required, the > driver will correctly be bind to the device based on its ID registers. > Is reg also required by the code? > > The second meaning is about keeping the yaml verifier happy. It seems > like compatible is needed for the verifier. Is reg also required? We > do recommend having reg, but the generic code does not require it. Well, you have to be able to match a discoverable device to a DT node. Unless you only have one thing on the bus (how would you know though, they're discoverable?), that's with reg. And if you need to say turn on a regulator for the device to be discovered, then you need compatible to know how to do that. So either don't describe the device in DT because you can discover everything or you describe it in DT with both 'compatible' and 'reg'. MDIO is not special. Rob
On Tue, 5 May 2020 12:42:15 +0200, Oleksij Rempel wrote: > To properly identify this node, we need to use ethernet-phy-id0180.dc80. > And add missing required properties. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > .../devicetree/bindings/net/nxp,tja11xx.yaml | 55 ++++++++++++------- > 1 file changed, 35 insertions(+), 20 deletions(-) > Reviewed-by: Rob Herring <robh@kernel.org>
diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml index 42be0255512b3..cc322107a24a2 100644 --- a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml @@ -1,4 +1,4 @@ -# SPDX-License-Identifier: GPL-2.0+ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 --- $id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# @@ -12,44 +12,59 @@ maintainers: - Heiner Kallweit <hkallweit1@gmail.com> description: - Bindings for NXP TJA11xx automotive PHYs + Bindings for the NXP TJA1102 automotive PHY. This is a dual PHY package where + only the first PHY has global configuration register and HW health + monitoring. -allOf: - - $ref: ethernet-phy.yaml# +properties: + compatible: + const: ethernet-phy-id0180.dc80 + description: ethernet-phy-id0180.dc80 used for TJA1102 PHY + + reg: + minimum: 0 + maximum: 14 + description: + The PHY address of the parent PHY. + + '#address-cells': + description: number of address cells for the MDIO bus + const: 1 + + '#size-cells': + description: number of size cells on the MDIO bus + const: 0 patternProperties: - "^ethernet-phy@[0-9a-f]+$": + "^ethernet-phy@[0-9a-f]$": type: object - description: | - Some packages have multiple PHYs. Secondary PHY should be defines as - subnode of the first (parent) PHY. + description: + Integrated PHY node properties: reg: - minimum: 0 - maximum: 31 + minimum: 1 + maximum: 15 description: - The ID number for the child PHY. Should be +1 of parent PHY. + The PHY address of the slave PHY. Should be +1 of parent PHY. required: - reg -examples: - - | - mdio { - #address-cells = <1>; - #size-cells = <0>; +required: + - compatible + - reg + - '#address-cells' + - '#size-cells' - tja1101_phy0: ethernet-phy@4 { - reg = <0x4>; - }; - }; +examples: - | mdio { #address-cells = <1>; #size-cells = <0>; tja1102_phy0: ethernet-phy@4 { + compatible = "ethernet-phy-id0180.dc80"; reg = <0x4>; #address-cells = <1>; #size-cells = <0>;
To properly identify this node, we need to use ethernet-phy-id0180.dc80. And add missing required properties. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- .../devicetree/bindings/net/nxp,tja11xx.yaml | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-)