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

[Andrew Lunn]

On Mon, May 11, 2026 at 07:44:17PM +0000, Selvamani Rajagopal wrote:
> [..]
> 
> > 
> > On Mon, May 11, 2026 at 06:19:11PM +0000, Selvamani Rajagopal wrote:
> > > - PHY loopback has to set only BMCR_LOOPBACK bit. As a result, we can't
> > > use generic, genphy_loopback.
> > > - Direct MMD calls via Clause 45 read/write avoids indirect calls.
> > 
> > The threading of the patch appears to be broken.
> 
> I am not clear. I did apply all the patches in sequence on a fresh source tree. It applied cleanly. What kind of issues I should watch out for, for the issue of "threading is broken".

Look at other patchsets on the list. All the emails are threaded
together, and if you have a sensible mail client, it should show them
as a tree, with replies extending the tree. 'git send-email' will
thread them by default, so will 'b4'.

> 
> > 
> > Also, one patch, one type of change.
> > 
> > > Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@xxxxxxxxxx>
> > > ---
> > > drivers/net/phy/ncn26000.c | 36 ++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 36 insertions(+)
> > >
> > > 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.
> 
> I just followed what is in the existing, similar 10base-t1s phy code, microchip_t1s.c. 
> Certainly, I will investigate is_c45 flag.

Look at is from a different direction. What other PHY drivers do what
you do? In general, if you do something no other drivers do, it is
often wrong.

	 Andrew