Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2 3/9] net: phy: ncn26000: Added PHY loopback and read/write MMD callbacks
@ 2026-05-11 18:19 Selvamani Rajagopal
  2026-05-11 19:23 ` Andrew Lunn
  0 siblings, 1 reply; 4+ messages in thread
From: Selvamani Rajagopal @ 2026-05-11 18:19 UTC (permalink / raw)
  To: Piergiorgio Beruto, andrew@lunn.ch, hkallweit1@gmail.com,
	linux@armlinux.org.uk, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

- 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.

Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
---
 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);
+}
+
+/* Directly accessing via Clause 45 write avoids the extra
+ * SPI accesses required by indirect access.
+ */
+static int ncn26000_write_mmd(struct phy_device *phydev, int dev,
+			      u16 reg, u16 val)
+{
+	struct mii_bus *bus = phydev->mdio.bus;
+	int addr = phydev->mdio.addr;
+
+	return __mdiobus_c45_write(bus, addr, dev, reg, val);
+}
+
+static int ncn26000_loopback(struct phy_device *phydev, bool enable, int speed)
+{
+	u16 mask = BMCR_LOOPBACK;
+	u16 ctl = 0;
+
+	if (enable)
+		ctl = BMCR_LOOPBACK;
+	return phy_modify(phydev, MII_BMCR, mask, ctl);
+}
+
 static struct phy_driver ncn26000_driver[] = {
 	{
 		PHY_ID_MATCH_MODEL(PHY_ID_NCN26000),
@@ -176,6 +209,9 @@ static struct phy_driver ncn26000_driver[] = {
 		.set_plca_cfg          = ncn26000_plca_set_cfg,
 		.get_plca_status       = genphy_c45_plca_get_status,
 		.soft_reset            = genphy_soft_reset,
+		.set_loopback          = ncn26000_loopback,
+		.read_mmd              = ncn26000_read_mmd,
+		.write_mmd             = ncn26000_write_mmd,
 	},
 };
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next v2 3/9] net: phy: ncn26000: Added PHY loopback and read/write MMD callbacks
  2026-05-11 18:19 [PATCH net-next v2 3/9] net: phy: ncn26000: Added PHY loopback and read/write MMD callbacks Selvamani Rajagopal
@ 2026-05-11 19:23 ` Andrew Lunn
  2026-05-11 19:44   ` Selvamani Rajagopal
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Lunn @ 2026-05-11 19:23 UTC (permalink / raw)
  To: Selvamani Rajagopal
  Cc: Piergiorgio Beruto, hkallweit1@gmail.com, linux@armlinux.org.uk,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

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.

Also, one patch, one type of change.

> Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
> ---
>  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.

	Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH net-next v2 3/9] net: phy: ncn26000: Added PHY loopback and read/write MMD callbacks
  2026-05-11 19:23 ` Andrew Lunn
@ 2026-05-11 19:44   ` Selvamani Rajagopal
  2026-05-11 20:28     ` Andrew Lunn
  0 siblings, 1 reply; 4+ messages in thread
From: Selvamani Rajagopal @ 2026-05-11 19:44 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Piergiorgio Beruto, hkallweit1@gmail.com, linux@armlinux.org.uk,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

[..]

> 
> 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".

> 
> Also, one patch, one type of change.
> 
> > Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
> > ---
> > 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.

> 
> Andrew


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next v2 3/9] net: phy: ncn26000: Added PHY loopback and read/write MMD callbacks
  2026-05-11 19:44   ` Selvamani Rajagopal
@ 2026-05-11 20:28     ` Andrew Lunn
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Lunn @ 2026-05-11 20:28 UTC (permalink / raw)
  To: Selvamani Rajagopal
  Cc: Piergiorgio Beruto, hkallweit1@gmail.com, linux@armlinux.org.uk,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

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@onsemi.com>
> > > ---
> > > 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-11 20:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-11 18:19 [PATCH net-next v2 3/9] net: phy: ncn26000: Added PHY loopback and read/write MMD callbacks Selvamani Rajagopal
2026-05-11 19:23 ` Andrew Lunn
2026-05-11 19:44   ` Selvamani Rajagopal
2026-05-11 20:28     ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox