Message-ID: <b1ee2262-47af-3138-6fd6-782b3d821f31@nxp.com>
Date:   Fri, 22 Nov 2019 23:01:27 +0000
From:   Alexandru Marginean <alexandru.marginean@....com>
To:     Vladimir Oltean <olteanv@...il.com>
CC:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        network dev <netdev@...r.kernel.org>,
        Grygorii Strashko <grygorii.strashko@...com>
Subject: Re: binding for scanning a MDIO bus

On 11/22/2019 10:12 PM, Vladimir Oltean wrote:
> On Fri, 22 Nov 2019 at 22:43, Alexandru Marginean
> <alexandru.marginean@....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

Powered by blists - more mailing lists