Re: [PATCH v5 2/3] hw/net/imx_fec: Allow phy not to be the first device on the mii bus.

[Jean-Christophe DUBOIS]

Le 15/06/2020 à 14:23, Peter Maydell a écrit :
> On Thu, 4 Jun 2020 at 13:39, Jean-Christophe Dubois <jcd@tribudubois.net> wrote:
> > Up to now we were allowing only one PHY device and it had to be the
> > first device on the bus.
> >
> > The i.MX6UL has 2 Ethernet devices and can therefore have several
> > PHY devices on the bus (and not necessarilly as device 0).
> >
> > This patch allows for PHY devices on 2nd, 3rd or any position.
> >
> > Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> > diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> > index eefedc252de..29e613699ee 100644
> > --- a/hw/net/imx_fec.c
> > +++ b/hw/net/imx_fec.c
> > @@ -280,11 +280,9 @@ static void imx_phy_reset(IMXFECState *s)
> >   static uint32_t imx_phy_read(IMXFECState *s, int reg)
> >   {
> >       uint32_t val;
> > +    uint32_t phy = reg / 32;
> >
> > -    if (reg > 31) {
> > -        /* we only advertise one phy */
> > -        return 0;
> > -    }
> > +    reg %= 32;
> This change means we now support multiple PHYs...

Yes, on the i.MX6UL there are 2 ENET devices but only one MDIO bus to 
acess the PHYs.

On the i.MX6UL evaluation board, PHY seems to be at offset/adress 1 and 2.

See linux/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi for details.

> >       switch (reg) {
> >       case 0:     /* Basic Control */
> > @@ -331,19 +329,18 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
> >           break;
> >       }
> >
> > -    trace_imx_phy_read(val, reg);
> > +    trace_imx_phy_read(val, phy, reg);
> ...but the only change we actually make is to trace the phy number.
> Surely if there is more than one phy then each phy needs to have
> its own state (phy_control/phy_status/phy_advertise/etc), rather
> than all these PHYs all sharing the same state under the hood?

Well there might be several PHYs on the MDIO bus but each PHY is used 
only by one ENET device. There is no plan for one ENET to use either 
PHY. It can only use one.

In Qemu (or at least in the FEC ethernet device emulator) the phy state 
is embedded in the ethernet device state.

> It also sounds from your commit message as if there isn't a
> large number of PHYs, but only perhaps two. In that case
> don't we need to fail attempts to access non-existent PHYs
> and only work with the ones which actually exist on any
> given board?

As stated on the particular i.MX6UL evaluation board there are 2 phys 
connected to the MDIO bus at adresse 1 and 2.

On another board the PHYs could be at different addresses or we might 
use only one MAC and therefore only one PHY.

So in order to be able to fail on bad PHY access we need to discriminate 
for each board which PHYs are actually present and at what address on 
the MDIO bus. We would also need to know which PHY is connected to which 
MAC.

I might have overlook something but so far there is no clear separate 
PHY and MAC implementation.

Usually the PHY is more or less part of the MAC implementation and there 
are no easy way to instantiate them separately then connect them with 
the required bus (MDIO and MAC).

I guess it might be feasible even if it sounds like overkill most of the 
time (you usually get one MAC connected to one PHY).

Is there such a qemu framework that would allow to connect multiple PHY 
to a single MDIO bus and then each PHY to a specific MAC device? Could 
you point me in the right direction ?

> thanks
> -- PMM