binding for scanning a MDIO bus

binding for scanning a MDIO bus

[Alexandru Marginean]

Hi everyone,

I am looking for the proper binding to scan for a PHY on an MDIO bus 
that's not a child of the Ethernet device but otherwise is associated 
with it.  Scanning this bus should guarantee finding the correct PHY, if 
one exists.  As far as I can tell current bindings don't allow such 
association, although the code seems to support it.

The hardware that I'm using and could use such a binding is a NXP QDS 
board with PHY cards.  In particular this is a LS1028A, but the problem 
is common to the NXP QDS boards.  These cards wire MDIO up to the CPU 
through a mux.  The mux practically selects one of the slots/cards so 
the MDIO bus the PHY is on is private to the slot/card.
Each slot is also associated with an Ethernet interface, this is subject 
to serdes configuration and specifically for that I'm looking to apply a 
DT overlay.  Overlays are really impractical with the PHY cards though, 
there are several types of cards, number of slots can go up to 8 or so 
on some types of QDS boards and number of PHY card overlays that should 
be defined would blow up.  The number of overlays users would need to 
apply at boot would also go up to number of slots + 1.

The function of_mdiobus_register does scan for PHYs if 'reg' is missing 
in PHY nodes, is this code considered obsolete, is it OK to use it if 
needed but otherwise discouraged?  Any thoughts on including support for 
scanning in the binding document, like making 'reg' property in phy 
nodes optional?

For what is worth scanning is a good solution in some cases, better than 
others anyway.  I'm sure it's not just people being too lazy to set up 
'reg' that use this code.

Thank you!
Alex

Re: binding for scanning a MDIO bus

[Andrew Lunn]

On Fri, Nov 22, 2019 at 12:31:30PM +0000, Alexandru Marginean wrote:
> Hi everyone,
> 
> I am looking for the proper binding to scan for a PHY on an MDIO bus 
> that's not a child of the Ethernet device but otherwise is associated 
> with it.  Scanning this bus should guarantee finding the correct PHY, if 
> one exists.  As far as I can tell current bindings don't allow such 
> association, although the code seems to support it.
> 
> The hardware that I'm using and could use such a binding is a NXP QDS 
> board with PHY cards.  In particular this is a LS1028A, but the problem 
> is common to the NXP QDS boards.  These cards wire MDIO up to the CPU 
> through a mux.  The mux practically selects one of the slots/cards so 
> the MDIO bus the PHY is on is private to the slot/card.
> Each slot is also associated with an Ethernet interface, this is subject 
> to serdes configuration and specifically for that I'm looking to apply a 
> DT overlay.  Overlays are really impractical with the PHY cards though, 
> there are several types of cards, number of slots can go up to 8 or so 
> on some types of QDS boards and number of PHY card overlays that should 
> be defined would blow up.  The number of overlays users would need to 
> apply at boot would also go up to number of slots + 1.
> 
> The function of_mdiobus_register does scan for PHYs if 'reg' is missing 
> in PHY nodes, is this code considered obsolete, is it OK to use it if 
> needed but otherwise discouraged?  Any thoughts on including support for 
> scanning in the binding document, like making 'reg' property in phy 
> nodes optional?
> 
> For what is worth scanning is a good solution in some cases, better than 
> others anyway.  I'm sure it's not just people being too lazy to set up 
> 'reg' that use this code.

Hi Alexandru

You often see the bus registered using mdiobus_register(). That then
means a scan is performed and all phys on the bus found. The MAC
driver then uses phy_find_first() to find the first PHY on the bus.
The danger here is that the hardware design changes, somebody adds a
second PHY, and it all stops working in interesting and confusing
ways.

Would this work for you?

      Andrew

Re: binding for scanning a MDIO bus

[Alexandru Marginean]

Hi Andrew,

On 11/22/2019 4:09 PM, Andrew Lunn wrote:
> On Fri, Nov 22, 2019 at 12:31:30PM +0000, Alexandru Marginean wrote:
>> Hi everyone,
>>
>> I am looking for the proper binding to scan for a PHY on an MDIO bus
>> that's not a child of the Ethernet device but otherwise is associated
>> with it.  Scanning this bus should guarantee finding the correct PHY, if
>> one exists.  As far as I can tell current bindings don't allow such
>> association, although the code seems to support it.
>>
>> The hardware that I'm using and could use such a binding is a NXP QDS
>> board with PHY cards.  In particular this is a LS1028A, but the problem
>> is common to the NXP QDS boards.  These cards wire MDIO up to the CPU
>> through a mux.  The mux practically selects one of the slots/cards so
>> the MDIO bus the PHY is on is private to the slot/card.
>> Each slot is also associated with an Ethernet interface, this is subject
>> to serdes configuration and specifically for that I'm looking to apply a
>> DT overlay.  Overlays are really impractical with the PHY cards though,
>> there are several types of cards, number of slots can go up to 8 or so
>> on some types of QDS boards and number of PHY card overlays that should
>> be defined would blow up.  The number of overlays users would need to
>> apply at boot would also go up to number of slots + 1.
>>
>> The function of_mdiobus_register does scan for PHYs if 'reg' is missing
>> in PHY nodes, is this code considered obsolete, is it OK to use it if
>> needed but otherwise discouraged?  Any thoughts on including support for
>> scanning in the binding document, like making 'reg' property in phy
>> nodes optional?
>>
>> For what is worth scanning is a good solution in some cases, better than
>> others anyway.  I'm sure it's not just people being too lazy to set up
>> 'reg' that use this code.
> 
> Hi Alexandru
> 
> You often see the bus registered using mdiobus_register(). That then
> means a scan is performed and all phys on the bus found. The MAC
> driver then uses phy_find_first() to find the first PHY on the bus.
> The danger here is that the hardware design changes, somebody adds a
> second PHY, and it all stops working in interesting and confusing
> ways.
> 
> Would this work for you?
> 
>        Andrew
> 

How does the MAC get a reference to the mdio bus though, is there some 
way to describe this relationship in the DT?  I did say that Eth and 
mdio are associated and they are, but not in the way Eth would just know 
without looking in the DT what mdio that is.  It's only that one such 
mdio exists for a given Eth and it's safe to use, but Eth needs to be 
told where to find it and that information should come from the DT.

Mdio buses of slots/cards are defined in DT under the mdio mux.  The mux 
itself is accessed over I2C and its parent-mdio is a stand-alone device 
that is not associated with a specific Ethernet device.  And on top of 
that, based on serdes configuration, some Eth interfaces may end up on a 
different slot and for that I want to apply a DT overlay to set the 
proper Eth/mdio association.

Current code allows me to do something like this, as seen by Linux on boot:
/ {
....
	mdio-mux {
		/* slot 1 */
		mdio@4 {
			slot1_phy0: phy {
				/* 'reg' missing on purpose */
			};
		};
	};
....
};

&enetc_port0 {
	phy-handle = <&slot1_phy0>;
	phy-mode = "sgmii";
};

But the binding does not allow this, 'reg' is a required property of 
phys.  Is this kind of DT structure acceptable even if it's not 
compliant to the binding?  Assuming it's fine, any thoughts on making 
this official in the binding?  If it's not, are there alternative 
options for such a set-up?

Thanks!
Alex

Re: binding for scanning a MDIO bus

[Andrew Lunn]

> > Hi Alexandru
> > 
> > You often see the bus registered using mdiobus_register(). That then
> > means a scan is performed and all phys on the bus found. The MAC
> > driver then uses phy_find_first() to find the first PHY on the bus.
> > The danger here is that the hardware design changes, somebody adds a
> > second PHY, and it all stops working in interesting and confusing
> > ways.
> > 
> > Would this work for you?
> > 
> >        Andrew
> > 
> 
> How does the MAC get a reference to the mdio bus though, is there some 
> way to describe this relationship in the DT?  I did say that Eth and 
> mdio are associated and they are, but not in the way Eth would just know 
> without looking in the DT what mdio that is.

What i described is generally used for PCIe card, USB dongles,
etc. The MAC driver is the one registering the MDIO bus, so it has
what it needs. Such hardware is also pretty much guaranteed to only
have one PHY on the bus, so phy_find_first() is less dangerous.

> Mdio buses of slots/cards are defined in DT under the mdio mux.  The mux 
> itself is accessed over I2C and its parent-mdio is a stand-alone device 
> that is not associated with a specific Ethernet device.  And on top of 
> that, based on serdes configuration, some Eth interfaces may end up on a 
> different slot and for that I want to apply a DT overlay to set the 
> proper Eth/mdio association.
> 
> Current code allows me to do something like this, as seen by Linux on boot:
> / {
> ....
> 	mdio-mux {
> 		/* slot 1 */
> 		mdio@4 {
> 			slot1_phy0: phy {
> 				/* 'reg' missing on purpose */
> 			};
> 		};
> 	};
> ....
> };
> 
> &enetc_port0 {
> 	phy-handle = <&slot1_phy0>;
> 	phy-mode = "sgmii";
> };
> 
> But the binding does not allow this, 'reg' is a required property of 
> phys.  Is this kind of DT structure acceptable even if it's not 
> compliant to the binding?  Assuming it's fine, any thoughts on making 
> this official in the binding?  If it's not, are there alternative 
> options for such a set-up?

In principle, this is O.K. The code seems to support it, even if the
binding does not give it as an option. It get messy when you have
multiple PHYs on the bus though. And if you are using DT, you are
supposed to know what the hardware is. Since you don't seems to know
what your hardware is, you are going to spam your kernel logs with

                        /* be noisy to encourage people to set reg property */
                        dev_info(&mdio->dev, "scan phy %pOFn at address %i\n",
                                 child, addr);

which i agree with.

      Andrew

Re: binding for scanning a MDIO bus

[Alexandru Marginean]

On 11/22/2019 6:42 PM, Andrew Lunn wrote:
>>> Hi Alexandru
>>>
>>> You often see the bus registered using mdiobus_register(). That then
>>> means a scan is performed and all phys on the bus found. The MAC
>>> driver then uses phy_find_first() to find the first PHY on the bus.
>>> The danger here is that the hardware design changes, somebody adds a
>>> second PHY, and it all stops working in interesting and confusing
>>> ways.
>>>
>>> Would this work for you?
>>>
>>>         Andrew
>>>
>>
>> How does the MAC get a reference to the mdio bus though, is there some
>> way to describe this relationship in the DT?  I did say that Eth and
>> mdio are associated and they are, but not in the way Eth would just know
>> without looking in the DT what mdio that is.
> 
> What i described is generally used for PCIe card, USB dongles,
> etc. The MAC driver is the one registering the MDIO bus, so it has
> what it needs. Such hardware is also pretty much guaranteed to only
> have one PHY on the bus, so phy_find_first() is less dangerous.

I get that, it's clear how it works if it's all part of the same device, 
but that's not applicable to our QDS boards.  These are pretty much the 
opposite of the PCIe cards and dongles.  They are designed to support as 
many combinations as possible of interfaces, protocols PHYs and so on. 
What I'm trying to do is to have the infrastructure in place so that 
users of these boards don't have to rebuild both U-Boot and Linux binary 
to get an Ethernet interface running with a different PHY card.

>> Mdio buses of slots/cards are defined in DT under the mdio mux.  The mux
>> itself is accessed over I2C and its parent-mdio is a stand-alone device
>> that is not associated with a specific Ethernet device.  And on top of
>> that, based on serdes configuration, some Eth interfaces may end up on a
>> different slot and for that I want to apply a DT overlay to set the
>> proper Eth/mdio association.
>>
>> Current code allows me to do something like this, as seen by Linux on boot:
>> / {
>> ....
>> 	mdio-mux {
>> 		/* slot 1 */
>> 		mdio@4 {
>> 			slot1_phy0: phy {
>> 				/* 'reg' missing on purpose */
>> 			};
>> 		};
>> 	};
>> ....
>> };
>>
>> &enetc_port0 {
>> 	phy-handle = <&slot1_phy0>;
>> 	phy-mode = "sgmii";
>> };
>>
>> But the binding does not allow this, 'reg' is a required property of
>> phys.  Is this kind of DT structure acceptable even if it's not
>> compliant to the binding?  Assuming it's fine, any thoughts on making
>> this official in the binding?  If it's not, are there alternative
>> options for such a set-up?
> 
> In principle, this is O.K. The code seems to support it, even if the
> binding does not give it as an option. It get messy when you have
> multiple PHYs on the bus though. And if you are using DT, you are
> supposed to know what the hardware is. Since you don't seems to know
> what your hardware is, you are going to spam your kernel logs with
> 
>                          /* be noisy to encourage people to set reg property */
>                          dev_info(&mdio->dev, "scan phy %pOFn at address %i\n",
>                                   child, addr);
> 
> which i agree with. >
>        Andrew
> 

Yeah, specifically on these QDS boards we're using DT and we can't 
practically tell kernel up front what PHY is going to be present.  I 
noticed the messages, having some verbosity caused by PHY scanning is 
fine.  It's definitely causing much less pain than editing DTs to 
describe what card is plugged in in which slot.  Ironically these cards 
physically look like PCIe cards, although they are not.

OK, I'll go with PHY nodes with missing 'reg' properties.

Thanks!
Alex

Re: binding for scanning a MDIO bus

[Vladimir Oltean]

On Fri, 22 Nov 2019 at 22:43, Alexandru Marginean
<alexandru.marginean@nxp.com> wrote:
>
> On 11/22/2019 6:42 PM, Andrew Lunn wrote:
> >>> Hi Alexandru
> >>>
> >>> You often see the bus registered using mdiobus_register(). That then
> >>> means a scan is performed and all phys on the bus found. The MAC
> >>> driver then uses phy_find_first() to find the first PHY on the bus.
> >>> The danger here is that the hardware design changes, somebody adds a
> >>> second PHY, and it all stops working in interesting and confusing
> >>> ways.
> >>>
> >>> Would this work for you?
> >>>
> >>>         Andrew
> >>>
> >>
> >> How does the MAC get a reference to the mdio bus though, is there some
> >> way to describe this relationship in the DT?  I did say that Eth and
> >> mdio are associated and they are, but not in the way Eth would just know
> >> without looking in the DT what mdio that is.
> >
> > What i described is generally used for PCIe card, USB dongles,
> > etc. The MAC driver is the one registering the MDIO bus, so it has
> > what it needs. Such hardware is also pretty much guaranteed to only
> > have one PHY on the bus, so phy_find_first() is less dangerous.
>
> I get that, it's clear how it works if it's all part of the same device,
> but that's not applicable to our QDS boards.  These are pretty much the
> opposite of the PCIe cards and dongles.  They are designed to support as
> many combinations as possible of interfaces, protocols PHYs and so on.
> What I'm trying to do is to have the infrastructure in place so that
> users of these boards don't have to rebuild both U-Boot and Linux binary
> to get an Ethernet interface running with a different PHY card.
>
> >> Mdio buses of slots/cards are defined in DT under the mdio mux.  The mux
> >> itself is accessed over I2C and its parent-mdio is a stand-alone device
> >> that is not associated with a specific Ethernet device.  And on top of
> >> that, based on serdes configuration, some Eth interfaces may end up on a
> >> different slot and for that I want to apply a DT overlay to set the
> >> proper Eth/mdio association.
> >>
> >> Current code allows me to do something like this, as seen by Linux on boot:
> >> / {
> >> ....
> >>      mdio-mux {
> >>              /* slot 1 */
> >>              mdio@4 {
> >>                      slot1_phy0: phy {
> >>                              /* 'reg' missing on purpose */
> >>                      };
> >>              };
> >>      };
> >> ....
> >> };
> >>
> >> &enetc_port0 {
> >>      phy-handle = <&slot1_phy0>;
> >>      phy-mode = "sgmii";
> >> };
> >>
> >> But the binding does not allow this, 'reg' is a required property of
> >> phys.  Is this kind of DT structure acceptable even if it's not
> >> compliant to the binding?  Assuming it's fine, any thoughts on making
> >> this official in the binding?  If it's not, are there alternative
> >> options for such a set-up?
> >
> > In principle, this is O.K. The code seems to support it, even if the
> > binding does not give it as an option. It get messy when you have
> > multiple PHYs on the bus though. And if you are using DT, you are
> > supposed to know what the hardware is. Since you don't seems to know
> > what your hardware is, you are going to spam your kernel logs with
> >
> >                          /* be noisy to encourage people to set reg property */
> >                          dev_info(&mdio->dev, "scan phy %pOFn at address %i\n",
> >                                   child, addr);
> >
> > which i agree with. >
> >        Andrew
> >
>
> Yeah, specifically on these QDS boards we're using DT and we can't
> practically tell kernel up front what PHY is going to be present.  I
> noticed the messages, having some verbosity caused by PHY scanning is
> fine.  It's definitely causing much less pain than editing DTs to
> describe what card is plugged in in which slot.  Ironically these cards
> physically look like PCIe cards, although they are not.
>
> OK, I'll go with PHY nodes with missing 'reg' properties.
>
> Thanks!
> Alex

Hi Alex,

Let's say there is a QSGMII PHY on such a riser card. There is a
single SerDes lane but on the card there is a PHY chip that acts as 4
PHYs in the same package. Each MAC needs to talk to its own PHY
(separate addresses) in the package. How would your proposed bindings
identify which MAC needs to talk to which PHY to get its correct link
status?

Thanks,
-Vladimir

Re: binding for scanning a MDIO bus

[Alexandru Marginean]

On 11/22/2019 10:12 PM, Vladimir Oltean wrote:
> On Fri, 22 Nov 2019 at 22:43, Alexandru Marginean
> <alexandru.marginean@nxp.com> wrote:
>>
>> On 11/22/2019 6:42 PM, Andrew Lunn wrote:
>>>>> Hi Alexandru
>>>>>
>>>>> You often see the bus registered using mdiobus_register(). That then
>>>>> means a scan is performed and all phys on the bus found. The MAC
>>>>> driver then uses phy_find_first() to find the first PHY on the bus.
>>>>> The danger here is that the hardware design changes, somebody adds a
>>>>> second PHY, and it all stops working in interesting and confusing
>>>>> ways.
>>>>>
>>>>> Would this work for you?
>>>>>
>>>>>          Andrew
>>>>>
>>>>
>>>> How does the MAC get a reference to the mdio bus though, is there some
>>>> way to describe this relationship in the DT?  I did say that Eth and
>>>> mdio are associated and they are, but not in the way Eth would just know
>>>> without looking in the DT what mdio that is.
>>>
>>> What i described is generally used for PCIe card, USB dongles,
>>> etc. The MAC driver is the one registering the MDIO bus, so it has
>>> what it needs. Such hardware is also pretty much guaranteed to only
>>> have one PHY on the bus, so phy_find_first() is less dangerous.
>>
>> I get that, it's clear how it works if it's all part of the same device,
>> but that's not applicable to our QDS boards.  These are pretty much the
>> opposite of the PCIe cards and dongles.  They are designed to support as
>> many combinations as possible of interfaces, protocols PHYs and so on.
>> What I'm trying to do is to have the infrastructure in place so that
>> users of these boards don't have to rebuild both U-Boot and Linux binary
>> to get an Ethernet interface running with a different PHY card.
>>
>>>> Mdio buses of slots/cards are defined in DT under the mdio mux.  The mux
>>>> itself is accessed over I2C and its parent-mdio is a stand-alone device
>>>> that is not associated with a specific Ethernet device.  And on top of
>>>> that, based on serdes configuration, some Eth interfaces may end up on a
>>>> different slot and for that I want to apply a DT overlay to set the
>>>> proper Eth/mdio association.
>>>>
>>>> Current code allows me to do something like this, as seen by Linux on boot:
>>>> / {
>>>> ....
>>>>       mdio-mux {
>>>>               /* slot 1 */
>>>>               mdio@4 {
>>>>                       slot1_phy0: phy {
>>>>                               /* 'reg' missing on purpose */
>>>>                       };
>>>>               };
>>>>       };
>>>> ....
>>>> };
>>>>
>>>> &enetc_port0 {
>>>>       phy-handle = <&slot1_phy0>;
>>>>       phy-mode = "sgmii";
>>>> };
>>>>
>>>> But the binding does not allow this, 'reg' is a required property of
>>>> phys.  Is this kind of DT structure acceptable even if it's not
>>>> compliant to the binding?  Assuming it's fine, any thoughts on making
>>>> this official in the binding?  If it's not, are there alternative
>>>> options for such a set-up?
>>>
>>> In principle, this is O.K. The code seems to support it, even if the
>>> binding does not give it as an option. It get messy when you have
>>> multiple PHYs on the bus though. And if you are using DT, you are
>>> supposed to know what the hardware is. Since you don't seems to know
>>> what your hardware is, you are going to spam your kernel logs with
>>>
>>>                           /* be noisy to encourage people to set reg property */
>>>                           dev_info(&mdio->dev, "scan phy %pOFn at address %i\n",
>>>                                    child, addr);
>>>
>>> which i agree with. >
>>>         Andrew
>>>
>>
>> Yeah, specifically on these QDS boards we're using DT and we can't
>> practically tell kernel up front what PHY is going to be present.  I
>> noticed the messages, having some verbosity caused by PHY scanning is
>> fine.  It's definitely causing much less pain than editing DTs to
>> describe what card is plugged in in which slot.  Ironically these cards
>> physically look like PCIe cards, although they are not.
>>
>> OK, I'll go with PHY nodes with missing 'reg' properties.
>>
>> Thanks!
>> Alex
> 
> Hi Alex,
> 
> Let's say there is a QSGMII PHY on such a riser card. There is a
> single SerDes lane but on the card there is a PHY chip that acts as 4
> PHYs in the same package. Each MAC needs to talk to its own PHY
> (separate addresses) in the package. How would your proposed bindings
> identify which MAC needs to talk to which PHY to get its correct link
> status?
> 
> Thanks,
> -Vladimir
> 

By my proposed binding you mean the missing 'reg' property in phy nodes? 
  Looking at the of_mdiobus_register code it does support scanning for 
multiple PHYs on a single bus and it scans the addresses in order. 
Matching MACs and PHYs correctly isn't an NXP specific requirement for 
sure :)

Hardware is generally well behaved, the quad PHYs I've seen so far use 
addresses in order for the 4 interfaces.  It's common to driver the base 
address with a few PHY pins and then the PHY assigns incremental 
addresses to the other 3 interfaces.  If we ever bump into a PHY that 
doesn't assign addresses that way we're going to do something different 
for that PHY, maybe a custom DT overlay.  But if this works for all the 
others cards it's still pretty useful.

Alex