RE: [PATCH net-next v2 3/9] net: phy: ncn26000: Added PHY loopback and read/write MMD callbacks

[Selvamani Rajagopal]

> > diff --git a/drivers/net/phy/ncn26000.c b/drivers/net/phy/ncn26000.c
> > index 68b0e4647..d302e04c8 100644
> > --- a/drivers/net/phy/ncn26000.c
> > +++ b/drivers/net/phy/ncn26000.c
> > @@ -162,6 +162,39 @@ static int ncn26000_config_intr(struct phy_device *phydev)
> > return 0;
> > }
> >
> > +/* Directly accessing via Clause 45 read avoids the extra
> > + * SPI accesses required by indirect access.
> > + */
> > +static int ncn26000_read_mmd(struct phy_device *phydev, int dev, u16 reg)
> > +{
> > + struct mii_bus *bus = phydev->mdio.bus;
> > + int addr = phydev->mdio.addr;
> > +
> > + return __mdiobus_c45_read(bus, addr, dev, reg);
> > +}
> 
> This needs more explanation. Maybe you need to look into
> phydev->is_c45.

Andew,

I spent some time to understand the is_c45 flag. As far as I can see, this is the only way. See my reasons below, to see if it makes sense.

1) Current implementation of oa_tc6.c doesn't give DT node to phydev. To be precise, dev_of_node(phydev->mdio.dev) returns NULL.
    What this means is, even if we set compatible string ethernet-phy-ieee802.3-c45, phylib won't set phydev->is_c45 flag.
   To resolve this issue, we have to replace mdiobus_register with of_mdiobus_register in oa_tc6.c

2) Ceclaring the PHY ethernet-phy-ieee802.3-c45 compatible and setting the phydrv->is_c45 flag brings a new issue. All the calls 
     are routed to c45 APIs. As a result, Generic MDIO registers (like MDIO_DEVID1 / MDIO_DEVID1) won't work and PHY won't be 
     detected.

3) Since trying with phydrv->is_c45 set, is out of question, phy_read_mmd is the only way to route c45 APIs. This requires 
    read_mmd and write_mmd callbacks to be implemented, If not, this takes the path of calling mmd_phy_read which again requires
    is_c45 flag to be set. Otherwise, it goes to c22 APIs, which requires indirect access that MAC-PHY device doesn't support.

   I tried calling mdiobus_c45_read inside phydev->read_mmd callback. It locks the system. I didn't investigate this further. As a result,
   __mdiobus_c45_read (and __mdiobus_c45_write for write_mmd) is the only course of action, as far as I can understand.


> 
> Andrew