| Message ID | 20200313052252.25389-2-o.rempel@pengutronix.de |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | add TJA1102 support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success |
On 3/12/2020 10:22 PM, Oleksij Rempel wrote: > Document the NXP TJA11xx PHY bindings. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > .../devicetree/bindings/net/nxp,tja11xx.yaml | 61 +++++++++++++++++++ > 1 file changed, 61 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > new file mode 100644 > index 000000000000..42be0255512b > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > @@ -0,0 +1,61 @@ > +# SPDX-License-Identifier: GPL-2.0+ > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP TJA11xx PHY > + > +maintainers: > + - Andrew Lunn <andrew@lunn.ch> > + - Florian Fainelli <f.fainelli@gmail.com> > + - Heiner Kallweit <hkallweit1@gmail.com> > + > +description: > + Bindings for NXP TJA11xx automotive PHYs > + > +allOf: > + - $ref: ethernet-phy.yaml# > + > +patternProperties: > + "^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. There are QSGMII PHYs which have 4 PHYs embedded and AFAICT they are defined as 4 separate Ethernet PHY nodes and this would not be quite a big stretch to represent them that way compared to how they are. I would recommend doing the same thing and not bend the MDIO framework to support the registration of "nested" Ethernet PHY nodes.
> > diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > new file mode 100644 > > index 000000000000..42be0255512b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > @@ -0,0 +1,61 @@ > > +# SPDX-License-Identifier: GPL-2.0+ > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: NXP TJA11xx PHY > > + > > +maintainers: > > + - Andrew Lunn <andrew@lunn.ch> > > + - Florian Fainelli <f.fainelli@gmail.com> > > + - Heiner Kallweit <hkallweit1@gmail.com> > > + > > +description: > > + Bindings for NXP TJA11xx automotive PHYs > > + > > +allOf: > > + - $ref: ethernet-phy.yaml# > > + > > +patternProperties: > > + "^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. > > > There are QSGMII PHYs which have 4 PHYs embedded and AFAICT they are > defined as 4 separate Ethernet PHY nodes and this would not be quite a > big stretch to represent them that way compared to how they are. > > I would recommend doing the same thing and not bend the MDIO framework > to support the registration of "nested" Ethernet PHY nodes. Hi Florian The issue here is the missing PHY ID in the secondary PHY. Because of that, the secondary does not probe in the normal way. We need the primary to be involved to some degree. It needs to register it. What i'm not so clear on is if it just needs to register it, or if these sub nodes are actually needed, given the current code. Andrew
On Fri, Mar 13, 2020 at 07:10:56PM +0100, Andrew Lunn wrote: > > > diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > > new file mode 100644 > > > index 000000000000..42be0255512b > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > > @@ -0,0 +1,61 @@ > > > +# SPDX-License-Identifier: GPL-2.0+ > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: NXP TJA11xx PHY > > > + > > > +maintainers: > > > + - Andrew Lunn <andrew@lunn.ch> > > > + - Florian Fainelli <f.fainelli@gmail.com> > > > + - Heiner Kallweit <hkallweit1@gmail.com> > > > + > > > +description: > > > + Bindings for NXP TJA11xx automotive PHYs > > > + > > > +allOf: > > > + - $ref: ethernet-phy.yaml# > > > + > > > +patternProperties: > > > + "^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. > > > > > > There are QSGMII PHYs which have 4 PHYs embedded and AFAICT they are > > defined as 4 separate Ethernet PHY nodes and this would not be quite a > > big stretch to represent them that way compared to how they are. > > > > I would recommend doing the same thing and not bend the MDIO framework > > to support the registration of "nested" Ethernet PHY nodes. > > Hi Florian > > The issue here is the missing PHY ID in the secondary PHY. Because of > that, the secondary does not probe in the normal way. We need the > primary to be involved to some degree. It needs to register it. What > i'm not so clear on is if it just needs to register it, or if these > sub nodes are actually needed, given the current code. There are a bit more dependencies: - PHY0 is responsible for health monitoring. If some thing wrong, it may shut down complete chip. - We have shared reset. It make no sense to probe PHY1 before PHY0 with more controlling options will be probed - It is possible bat dangerous to use PHY1 without PHY0. Regards, Oleksij
On 3/13/2020 11:16 AM, Oleksij Rempel wrote: > On Fri, Mar 13, 2020 at 07:10:56PM +0100, Andrew Lunn wrote: >>>> diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml >>>> new file mode 100644 >>>> index 000000000000..42be0255512b >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml >>>> @@ -0,0 +1,61 @@ >>>> +# SPDX-License-Identifier: GPL-2.0+ >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: NXP TJA11xx PHY >>>> + >>>> +maintainers: >>>> + - Andrew Lunn <andrew@lunn.ch> >>>> + - Florian Fainelli <f.fainelli@gmail.com> >>>> + - Heiner Kallweit <hkallweit1@gmail.com> >>>> + >>>> +description: >>>> + Bindings for NXP TJA11xx automotive PHYs >>>> + >>>> +allOf: >>>> + - $ref: ethernet-phy.yaml# >>>> + >>>> +patternProperties: >>>> + "^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. >>> >>> >>> There are QSGMII PHYs which have 4 PHYs embedded and AFAICT they are >>> defined as 4 separate Ethernet PHY nodes and this would not be quite a >>> big stretch to represent them that way compared to how they are. >>> >>> I would recommend doing the same thing and not bend the MDIO framework >>> to support the registration of "nested" Ethernet PHY nodes. >> >> Hi Florian >> >> The issue here is the missing PHY ID in the secondary PHY. Because of >> that, the secondary does not probe in the normal way. We need the >> primary to be involved to some degree. It needs to register it. What >> i'm not so clear on is if it just needs to register it, or if these >> sub nodes are actually needed, given the current code. > > There are a bit more dependencies: > - PHY0 is responsible for health monitoring. If some thing wrong, it may > shut down complete chip. > - We have shared reset. It make no sense to probe PHY1 before PHY0 with > more controlling options will be probed > - It is possible bat dangerous to use PHY1 without PHY0. probing is a software problem though. If we want to describe the PHY package more correctly, we should be using a container node, something like this maybe: phy-package { compatible = "nxp,tja1102"; ethernet-phy@4 { reg = <4>; }; ethernet-phy@5 { reg = <5>; }; };
On Fri, Mar 13, 2020 at 11:20:35AM -0700, Florian Fainelli wrote: > > > On 3/13/2020 11:16 AM, Oleksij Rempel wrote: > > On Fri, Mar 13, 2020 at 07:10:56PM +0100, Andrew Lunn wrote: > >>>> diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > >>>> new file mode 100644 > >>>> index 000000000000..42be0255512b > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > >>>> @@ -0,0 +1,61 @@ > >>>> +# SPDX-License-Identifier: GPL-2.0+ > >>>> +%YAML 1.2 > >>>> +--- > >>>> +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>> + > >>>> +title: NXP TJA11xx PHY > >>>> + > >>>> +maintainers: > >>>> + - Andrew Lunn <andrew@lunn.ch> > >>>> + - Florian Fainelli <f.fainelli@gmail.com> > >>>> + - Heiner Kallweit <hkallweit1@gmail.com> > >>>> + > >>>> +description: > >>>> + Bindings for NXP TJA11xx automotive PHYs > >>>> + > >>>> +allOf: > >>>> + - $ref: ethernet-phy.yaml# > >>>> + > >>>> +patternProperties: > >>>> + "^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. > >>> > >>> > >>> There are QSGMII PHYs which have 4 PHYs embedded and AFAICT they are > >>> defined as 4 separate Ethernet PHY nodes and this would not be quite a > >>> big stretch to represent them that way compared to how they are. > >>> > >>> I would recommend doing the same thing and not bend the MDIO framework > >>> to support the registration of "nested" Ethernet PHY nodes. > >> > >> Hi Florian > >> > >> The issue here is the missing PHY ID in the secondary PHY. Because of > >> that, the secondary does not probe in the normal way. We need the > >> primary to be involved to some degree. It needs to register it. What > >> i'm not so clear on is if it just needs to register it, or if these > >> sub nodes are actually needed, given the current code. > > > > There are a bit more dependencies: > > - PHY0 is responsible for health monitoring. If some thing wrong, it may > > shut down complete chip. > > - We have shared reset. It make no sense to probe PHY1 before PHY0 with > > more controlling options will be probed > > - It is possible bat dangerous to use PHY1 without PHY0. > > probing is a software problem though. If we want to describe the PHY > package more correctly, we should be using a container node, something > like this maybe: > > phy-package { > compatible = "nxp,tja1102"; > > ethernet-phy@4 { > reg = <4>; > }; > > ethernet-phy@5 { > reg = <5>; > }; > }; Yes, this is almost the same as it is currently done: phy-package { reg = <4>; ethernet-phy@5 { reg = <5>; }; }; Because the primary PHY0 can be autodetected by the bus scan. But I have nothing against your suggestions. Please, some one should say the last word here, how exactly it should be implemented? Regards, Oleksij
On Fri, Mar 13, 2020 at 07:53:27PM +0100, Oleksij Rempel wrote: > On Fri, Mar 13, 2020 at 11:20:35AM -0700, Florian Fainelli wrote: > > > > > > On 3/13/2020 11:16 AM, Oleksij Rempel wrote: > > > On Fri, Mar 13, 2020 at 07:10:56PM +0100, Andrew Lunn wrote: > > >>>> diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > >>>> new file mode 100644 > > >>>> index 000000000000..42be0255512b > > >>>> --- /dev/null > > >>>> +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > >>>> @@ -0,0 +1,61 @@ > > >>>> +# SPDX-License-Identifier: GPL-2.0+ > > >>>> +%YAML 1.2 > > >>>> +--- > > >>>> +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > > >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > > >>>> + > > >>>> +title: NXP TJA11xx PHY > > >>>> + > > >>>> +maintainers: > > >>>> + - Andrew Lunn <andrew@lunn.ch> > > >>>> + - Florian Fainelli <f.fainelli@gmail.com> > > >>>> + - Heiner Kallweit <hkallweit1@gmail.com> > > >>>> + > > >>>> +description: > > >>>> + Bindings for NXP TJA11xx automotive PHYs > > >>>> + > > >>>> +allOf: > > >>>> + - $ref: ethernet-phy.yaml# > > >>>> + > > >>>> +patternProperties: > > >>>> + "^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. > > >>> > > >>> > > >>> There are QSGMII PHYs which have 4 PHYs embedded and AFAICT they are > > >>> defined as 4 separate Ethernet PHY nodes and this would not be quite a > > >>> big stretch to represent them that way compared to how they are. > > >>> > > >>> I would recommend doing the same thing and not bend the MDIO framework > > >>> to support the registration of "nested" Ethernet PHY nodes. > > >> > > >> Hi Florian > > >> > > >> The issue here is the missing PHY ID in the secondary PHY. Because of > > >> that, the secondary does not probe in the normal way. We need the > > >> primary to be involved to some degree. It needs to register it. What > > >> i'm not so clear on is if it just needs to register it, or if these > > >> sub nodes are actually needed, given the current code. > > > > > > There are a bit more dependencies: > > > - PHY0 is responsible for health monitoring. If some thing wrong, it may > > > shut down complete chip. > > > - We have shared reset. It make no sense to probe PHY1 before PHY0 with > > > more controlling options will be probed > > > - It is possible bat dangerous to use PHY1 without PHY0. > > > > probing is a software problem though. If we want to describe the PHY > > package more correctly, we should be using a container node, something > > like this maybe: > > > > phy-package { > > compatible = "nxp,tja1102"; > > > > ethernet-phy@4 { > > reg = <4>; > > }; > > > > ethernet-phy@5 { > > reg = <5>; > > }; > > }; > > Yes, this is almost the same as it is currently done: > > phy-package { > reg = <4>; > > ethernet-phy@5 { > reg = <5>; > }; > }; > > Because the primary PHY0 can be autodetected by the bus scan. > But I have nothing against your suggestions. Please, some one should say the > last word here, how exactly it should be implemented? ping, Regards, Oleksij
On 3/17/2020 4:56 AM, Oleksij Rempel wrote: > On Fri, Mar 13, 2020 at 07:53:27PM +0100, Oleksij Rempel wrote: >> On Fri, Mar 13, 2020 at 11:20:35AM -0700, Florian Fainelli wrote: >>> >>> >>> On 3/13/2020 11:16 AM, Oleksij Rempel wrote: >>>> On Fri, Mar 13, 2020 at 07:10:56PM +0100, Andrew Lunn wrote: >>>>>>> diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml >>>>>>> new file mode 100644 >>>>>>> index 000000000000..42be0255512b >>>>>>> --- /dev/null >>>>>>> +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml >>>>>>> @@ -0,0 +1,61 @@ >>>>>>> +# SPDX-License-Identifier: GPL-2.0+ >>>>>>> +%YAML 1.2 >>>>>>> +--- >>>>>>> +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# >>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>>>> + >>>>>>> +title: NXP TJA11xx PHY >>>>>>> + >>>>>>> +maintainers: >>>>>>> + - Andrew Lunn <andrew@lunn.ch> >>>>>>> + - Florian Fainelli <f.fainelli@gmail.com> >>>>>>> + - Heiner Kallweit <hkallweit1@gmail.com> >>>>>>> + >>>>>>> +description: >>>>>>> + Bindings for NXP TJA11xx automotive PHYs >>>>>>> + >>>>>>> +allOf: >>>>>>> + - $ref: ethernet-phy.yaml# >>>>>>> + >>>>>>> +patternProperties: >>>>>>> + "^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. >>>>>> >>>>>> >>>>>> There are QSGMII PHYs which have 4 PHYs embedded and AFAICT they are >>>>>> defined as 4 separate Ethernet PHY nodes and this would not be quite a >>>>>> big stretch to represent them that way compared to how they are. >>>>>> >>>>>> I would recommend doing the same thing and not bend the MDIO framework >>>>>> to support the registration of "nested" Ethernet PHY nodes. >>>>> >>>>> Hi Florian >>>>> >>>>> The issue here is the missing PHY ID in the secondary PHY. Because of >>>>> that, the secondary does not probe in the normal way. We need the >>>>> primary to be involved to some degree. It needs to register it. What >>>>> i'm not so clear on is if it just needs to register it, or if these >>>>> sub nodes are actually needed, given the current code. >>>> >>>> There are a bit more dependencies: >>>> - PHY0 is responsible for health monitoring. If some thing wrong, it may >>>> shut down complete chip. >>>> - We have shared reset. It make no sense to probe PHY1 before PHY0 with >>>> more controlling options will be probed >>>> - It is possible bat dangerous to use PHY1 without PHY0. >>> >>> probing is a software problem though. If we want to describe the PHY >>> package more correctly, we should be using a container node, something >>> like this maybe: >>> >>> phy-package { >>> compatible = "nxp,tja1102"; >>> >>> ethernet-phy@4 { >>> reg = <4>; >>> }; >>> >>> ethernet-phy@5 { >>> reg = <5>; >>> }; >>> }; >> >> Yes, this is almost the same as it is currently done: >> >> phy-package { >> reg = <4>; >> >> ethernet-phy@5 { >> reg = <5>; >> }; >> }; >> >> Because the primary PHY0 can be autodetected by the bus scan. >> But I have nothing against your suggestions. Please, some one should say the >> last word here, how exactly it should be implemented? It's not for me to decide, I was hoping the Device Tree maintainers could chime in, your current approach would certainly work although it feels visually awkward.
On Tue, Mar 17, 2020 at 12:48:47PM -0700, Florian Fainelli wrote: > > > On 3/17/2020 4:56 AM, Oleksij Rempel wrote: > > On Fri, Mar 13, 2020 at 07:53:27PM +0100, Oleksij Rempel wrote: > >> On Fri, Mar 13, 2020 at 11:20:35AM -0700, Florian Fainelli wrote: > >>> > >>> > >>> On 3/13/2020 11:16 AM, Oleksij Rempel wrote: > >>>> On Fri, Mar 13, 2020 at 07:10:56PM +0100, Andrew Lunn wrote: > >>>>>>> diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > >>>>>>> new file mode 100644 > >>>>>>> index 000000000000..42be0255512b > >>>>>>> --- /dev/null > >>>>>>> +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > >>>>>>> @@ -0,0 +1,61 @@ > >>>>>>> +# SPDX-License-Identifier: GPL-2.0+ > >>>>>>> +%YAML 1.2 > >>>>>>> +--- > >>>>>>> +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > >>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>>>>> + > >>>>>>> +title: NXP TJA11xx PHY > >>>>>>> + > >>>>>>> +maintainers: > >>>>>>> + - Andrew Lunn <andrew@lunn.ch> > >>>>>>> + - Florian Fainelli <f.fainelli@gmail.com> > >>>>>>> + - Heiner Kallweit <hkallweit1@gmail.com> > >>>>>>> + > >>>>>>> +description: > >>>>>>> + Bindings for NXP TJA11xx automotive PHYs > >>>>>>> + > >>>>>>> +allOf: > >>>>>>> + - $ref: ethernet-phy.yaml# > >>>>>>> + > >>>>>>> +patternProperties: > >>>>>>> + "^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. > >>>>>> > >>>>>> > >>>>>> There are QSGMII PHYs which have 4 PHYs embedded and AFAICT they are > >>>>>> defined as 4 separate Ethernet PHY nodes and this would not be quite a > >>>>>> big stretch to represent them that way compared to how they are. > >>>>>> > >>>>>> I would recommend doing the same thing and not bend the MDIO framework > >>>>>> to support the registration of "nested" Ethernet PHY nodes. > >>>>> > >>>>> Hi Florian > >>>>> > >>>>> The issue here is the missing PHY ID in the secondary PHY. Because of > >>>>> that, the secondary does not probe in the normal way. We need the > >>>>> primary to be involved to some degree. It needs to register it. What > >>>>> i'm not so clear on is if it just needs to register it, or if these > >>>>> sub nodes are actually needed, given the current code. > >>>> > >>>> There are a bit more dependencies: > >>>> - PHY0 is responsible for health monitoring. If some thing wrong, it may > >>>> shut down complete chip. > >>>> - We have shared reset. It make no sense to probe PHY1 before PHY0 with > >>>> more controlling options will be probed > >>>> - It is possible bat dangerous to use PHY1 without PHY0. > >>> > >>> probing is a software problem though. If we want to describe the PHY > >>> package more correctly, we should be using a container node, something > >>> like this maybe: > >>> > >>> phy-package { > >>> compatible = "nxp,tja1102"; > >>> > >>> ethernet-phy@4 { > >>> reg = <4>; > >>> }; > >>> > >>> ethernet-phy@5 { > >>> reg = <5>; > >>> }; > >>> }; > >> > >> Yes, this is almost the same as it is currently done: > >> > >> phy-package { > >> reg = <4>; > >> > >> ethernet-phy@5 { > >> reg = <5>; > >> }; > >> }; > >> > >> Because the primary PHY0 can be autodetected by the bus scan. > >> But I have nothing against your suggestions. Please, some one should say the > >> last word here, how exactly it should be implemented? > > It's not for me to decide, I was hoping the Device Tree maintainers > could chime in, your current approach would certainly work although it > feels visually awkward. Something like this is what I'd do: ethernet-phy@4 { compatible = "nxp,tja1102"; reg = <4 5>; }; Rob
On 3/20/2020 4:05 PM, Rob Herring wrote: >>>> Because the primary PHY0 can be autodetected by the bus scan. >>>> But I have nothing against your suggestions. Please, some one should say the >>>> last word here, how exactly it should be implemented? >> >> It's not for me to decide, I was hoping the Device Tree maintainers >> could chime in, your current approach would certainly work although it >> feels visually awkward. > > Something like this is what I'd do: > > ethernet-phy@4 { > compatible = "nxp,tja1102"; > reg = <4 5>; > }; But the parent (MDIO bus controller) has #address-cells = 1 and #size-cells = 0, so how can this be made to work without creating two nodes or a first node encapsulating another one?
On Sun, Mar 22, 2020 at 3:09 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > > > On 3/20/2020 4:05 PM, Rob Herring wrote: > >>>> Because the primary PHY0 can be autodetected by the bus scan. > >>>> But I have nothing against your suggestions. Please, some one should say the > >>>> last word here, how exactly it should be implemented? > >> > >> It's not for me to decide, I was hoping the Device Tree maintainers > >> could chime in, your current approach would certainly work although it > >> feels visually awkward. > > > > Something like this is what I'd do: > > > > ethernet-phy@4 { > > compatible = "nxp,tja1102"; > > reg = <4 5>; > > }; > > But the parent (MDIO bus controller) has #address-cells = 1 and > #size-cells = 0, so how can this be made to work without creating two > nodes or a first node encapsulating another one? That is the size of the address, not how many addresses there are. If the device has 2 addresses, then 2 address entries seems entirely appropriate. Rob
On Fri, Mar 13, 2020 at 12:23 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > Document the NXP TJA11xx PHY bindings. Given the discussion, I'd marked this one as "changes requested" expecting a new version to review the schema. And gmail decided to make a new thread due to the extra 'RE:'. So it fell off my radar. This schema is fundamentally broken as there's no way to match for when to apply this schema. How do we find a NXP TJA11xx PHY? I suppose we can look for 'ethernet-phy' with a child node 'ethernet-phy', but then that would apply to any phy like this one. This needs a compatible string IMO given it is non-standard. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > .../devicetree/bindings/net/nxp,tja11xx.yaml | 61 +++++++++++++++++++ > 1 file changed, 61 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > new file mode 100644 > index 000000000000..42be0255512b > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > @@ -0,0 +1,61 @@ > +# SPDX-License-Identifier: GPL-2.0+ Dual license new bindings: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP TJA11xx PHY > + > +maintainers: > + - Andrew Lunn <andrew@lunn.ch> > + - Florian Fainelli <f.fainelli@gmail.com> > + - Heiner Kallweit <hkallweit1@gmail.com> > + > +description: > + Bindings for NXP TJA11xx automotive PHYs Perhaps some information about how this phy is special. > + > +allOf: > + - $ref: ethernet-phy.yaml# Not needed here as ethernet-phy.yaml already has a 'select' condition to apply. > + > +patternProperties: > + "^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. > + > + properties: > + reg: > + minimum: 0 > + maximum: 31 > + description: > + The ID number for the child PHY. Should be +1 of parent PHY. > + > + required: > + - reg > + > +examples: > + - | > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + tja1101_phy0: ethernet-phy@4 { > + reg = <0x4>; > + }; > + }; > + - | > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + tja1102_phy0: ethernet-phy@4 { > + reg = <0x4>; > + #address-cells = <1>; > + #size-cells = <0>; These aren't documented. > + > + tja1102_phy1: ethernet-phy@5 { > + reg = <0x5>; > + }; > + }; > + }; > -- > 2.25.1 >
@Rob, thank you for the review. @David, should I send fixes or reworked initial patches? On Tue, Apr 28, 2020 at 12:30:06PM -0500, Rob Herring wrote: > On Fri, Mar 13, 2020 at 12:23 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > > Document the NXP TJA11xx PHY bindings. > > Given the discussion, I'd marked this one as "changes requested" > expecting a new version to review the schema. And gmail decided to > make a new thread due to the extra 'RE:'. So it fell off my radar. > > This schema is fundamentally broken as there's no way to match for > when to apply this schema. How do we find a NXP TJA11xx PHY? I suppose > we can look for 'ethernet-phy' with a child node 'ethernet-phy', but > then that would apply to any phy like this one. This needs a > compatible string IMO given it is non-standard. > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > .../devicetree/bindings/net/nxp,tja11xx.yaml | 61 +++++++++++++++++++ > > 1 file changed, 61 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > > > diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > new file mode 100644 > > index 000000000000..42be0255512b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml > > @@ -0,0 +1,61 @@ > > +# SPDX-License-Identifier: GPL-2.0+ > > Dual license new bindings: > > (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: NXP TJA11xx PHY > > + > > +maintainers: > > + - Andrew Lunn <andrew@lunn.ch> > > + - Florian Fainelli <f.fainelli@gmail.com> > > + - Heiner Kallweit <hkallweit1@gmail.com> > > + > > +description: > > + Bindings for NXP TJA11xx automotive PHYs > > Perhaps some information about how this phy is special. > > > + > > +allOf: > > + - $ref: ethernet-phy.yaml# > > Not needed here as ethernet-phy.yaml already has a 'select' condition to apply. > > > + > > +patternProperties: > > + "^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. > > + > > + properties: > > + reg: > > + minimum: 0 > > + maximum: 31 > > + description: > > + The ID number for the child PHY. Should be +1 of parent PHY. > > + > > + required: > > + - reg > > + > > +examples: > > + - | > > + mdio { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + tja1101_phy0: ethernet-phy@4 { > > + reg = <0x4>; > > + }; > > + }; > > + - | > > + mdio { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + tja1102_phy0: ethernet-phy@4 { > > + reg = <0x4>; > > > + #address-cells = <1>; > > + #size-cells = <0>; > > These aren't documented. > > > + > > + tja1102_phy1: ethernet-phy@5 { > > + reg = <0x5>; > > + }; > > + }; > > + }; > > -- > > 2.25.1 > > >
On 4/28/2020 9:38 PM, Oleksij Rempel wrote: > @Rob, thank you for the review. > > @David, should I send fixes or reworked initial patches? You need to send incremental patches, once David applies the patches, they are part of the git history for the trees he maintains.
diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml new file mode 100644 index 000000000000..42be0255512b --- /dev/null +++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml @@ -0,0 +1,61 @@ +# SPDX-License-Identifier: GPL-2.0+ +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/nxp,tja11xx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP TJA11xx PHY + +maintainers: + - Andrew Lunn <andrew@lunn.ch> + - Florian Fainelli <f.fainelli@gmail.com> + - Heiner Kallweit <hkallweit1@gmail.com> + +description: + Bindings for NXP TJA11xx automotive PHYs + +allOf: + - $ref: ethernet-phy.yaml# + +patternProperties: + "^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. + + properties: + reg: + minimum: 0 + maximum: 31 + description: + The ID number for the child PHY. Should be +1 of parent PHY. + + required: + - reg + +examples: + - | + mdio { + #address-cells = <1>; + #size-cells = <0>; + + tja1101_phy0: ethernet-phy@4 { + reg = <0x4>; + }; + }; + - | + mdio { + #address-cells = <1>; + #size-cells = <0>; + + tja1102_phy0: ethernet-phy@4 { + reg = <0x4>; + #address-cells = <1>; + #size-cells = <0>; + + tja1102_phy1: ethernet-phy@5 { + reg = <0x5>; + }; + }; + };
Document the NXP TJA11xx PHY bindings. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- .../devicetree/bindings/net/nxp,tja11xx.yaml | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/nxp,tja11xx.yaml