public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: phy: qcom: at803x: Use the correct bit to disable extended next page
@ 2026-04-10 17:10 Maxime Chevallier
  2026-04-11 14:10 ` Andrew Lunn
  0 siblings, 1 reply; 3+ messages in thread
From: Maxime Chevallier @ 2026-04-10 17:10 UTC (permalink / raw)
  To: Andrew Lunn, Jakub Kicinski, davem, Eric Dumazet, Paolo Abeni,
	Simon Horman, Russell King
  Cc: Maxime Chevallier, thomas.petazzoni, netdev, linux-kernel,
	linux-arm-msm

As noted in the blamed commit, the AR8035 and other PHYs from this
family advertise the Extended Next Page support by default, which may be
understood by some partners as this PHY being multi-gig capable.

The fix is to disable XNP advertising, which is done by setting bit 12
of the Auto-Negotiation Advertisement Register (MII_ADVERTISE).

The blamed commit incorrectly uses MDIO_AN_CTRL1_XNP, which is bit 13 as per
802.3 : 45.2.7.1 AN control register (Register 7.0)

BIT 12 in MII_ADVERTISE is wrapped by ADVERTISE_RESV, used by some
drivers such as the aquantia one. 802.3 Clause 28 defines bit 12 as
Extended Next Page ability, at least in recent versions of the standard.

Let's add a define for it and use it in the at803x driver.

Fixes: 3c51fa5d2afe ("net: phy: ar803x: disable extended next page bit")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
Some further driver cleanup can be done to remove the use of
ADVERTISE_RESV, but the macro is uapi now so it'll have to stay.

 drivers/net/phy/qcom/at803x.c | 2 +-
 include/uapi/linux/mii.h      | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
index 338acd11a9b6..023c1fe0cd14 100644
--- a/drivers/net/phy/qcom/at803x.c
+++ b/drivers/net/phy/qcom/at803x.c
@@ -524,7 +524,7 @@ static int at803x_config_init(struct phy_device *phydev)
 	 * behaviour but we still need to accommodate it. XNP is only needed
 	 * for 10Gbps support, so disable XNP.
 	 */
-	return phy_modify(phydev, MII_ADVERTISE, MDIO_AN_CTRL1_XNP, 0);
+	return phy_modify(phydev, MII_ADVERTISE, ADVERTISE_XNP, 0);
 }
 
 static void at803x_link_change_notify(struct phy_device *phydev)
diff --git a/include/uapi/linux/mii.h b/include/uapi/linux/mii.h
index 39f7c44baf53..61d6edad4b94 100644
--- a/include/uapi/linux/mii.h
+++ b/include/uapi/linux/mii.h
@@ -82,7 +82,8 @@
 #define ADVERTISE_100BASE4	0x0200	/* Try for 100mbps 4k packets  */
 #define ADVERTISE_PAUSE_CAP	0x0400	/* Try for pause               */
 #define ADVERTISE_PAUSE_ASYM	0x0800	/* Try for asymetric pause     */
-#define ADVERTISE_RESV		0x1000	/* Unused...                   */
+#define ADVERTISE_XNP		0x1000  /* Extended Next Page */
+#define ADVERTISE_RESV		ADVERTISE_XNP /* Used to be reserved */
 #define ADVERTISE_RFAULT	0x2000	/* Say we can detect faults    */
 #define ADVERTISE_LPACK		0x4000	/* Ack link partners response  */
 #define ADVERTISE_NPAGE		0x8000	/* Next page bit               */
-- 
2.49.0


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

* Re: [PATCH net] net: phy: qcom: at803x: Use the correct bit to disable extended next page
  2026-04-10 17:10 [PATCH net] net: phy: qcom: at803x: Use the correct bit to disable extended next page Maxime Chevallier
@ 2026-04-11 14:10 ` Andrew Lunn
  2026-04-11 16:54   ` Maxime Chevallier
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Lunn @ 2026-04-11 14:10 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: Jakub Kicinski, davem, Eric Dumazet, Paolo Abeni, Simon Horman,
	Russell King, thomas.petazzoni, netdev, linux-kernel,
	linux-arm-msm

On Fri, Apr 10, 2026 at 07:10:20PM +0200, Maxime Chevallier wrote:
> As noted in the blamed commit, the AR8035 and other PHYs from this
> family advertise the Extended Next Page support by default, which may be
> understood by some partners as this PHY being multi-gig capable.
> 
> The fix is to disable XNP advertising, which is done by setting bit 12
> of the Auto-Negotiation Advertisement Register (MII_ADVERTISE).
> 
> The blamed commit incorrectly uses MDIO_AN_CTRL1_XNP, which is bit 13 as per
> 802.3 : 45.2.7.1 AN control register (Register 7.0)
> 
> BIT 12 in MII_ADVERTISE is wrapped by ADVERTISE_RESV, used by some
> drivers such as the aquantia one. 802.3 Clause 28 defines bit 12 as
> Extended Next Page ability, at least in recent versions of the standard.

> Let's add a define for it and use it in the at803x driver.

I agree with this, it defines the C22 4.12 bit. And this is what the
at803x driver is using it for.

>  static void at803x_link_change_notify(struct phy_device *phydev)
> diff --git a/include/uapi/linux/mii.h b/include/uapi/linux/mii.h
> index 39f7c44baf53..61d6edad4b94 100644
> --- a/include/uapi/linux/mii.h
> +++ b/include/uapi/linux/mii.h
> @@ -82,7 +82,8 @@
>  #define ADVERTISE_100BASE4	0x0200	/* Try for 100mbps 4k packets  */
>  #define ADVERTISE_PAUSE_CAP	0x0400	/* Try for pause               */
>  #define ADVERTISE_PAUSE_ASYM	0x0800	/* Try for asymetric pause     */
> -#define ADVERTISE_RESV		0x1000	/* Unused...                   */
> +#define ADVERTISE_XNP		0x1000  /* Extended Next Page */
> +#define ADVERTISE_RESV		ADVERTISE_XNP /* Used to be reserved */

Should we keep ADVERTISE_RESV?

  45.2.7.6 AN advertisement register

  If the Auto-Negotiation advertisement register (register 4) is
  present, (see 28.2.4.1.3), then this register is a copy of the
  Auto-Negotiation advertisement register (register 4). In this case,
  reads to the AN advertisement register (7.16) report the value of
  the Auto-Negotiation advertisement register (register 4); writes to
  the AN advertisement register (7.16) cause a write to occur to the
  Auto-Negotiation advertisement register.

So MDIO_MMD_AN:MDIO_AN_ADVERTISE is a straight copy of MII_ADVERTISE.

        ef4_mdio_write(efx, MDIO_MMD_AN, MDIO_AN_ADVERTISE, reg);
        ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE, adv);

So ADVERTISE_XNP is just as valid in the other two drivers using
ADVERTISE_RESV. I think we should change those as well to
ADVERTISE_XNP and remove ADVERTISE_RESV?

	Andrew

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

* Re: [PATCH net] net: phy: qcom: at803x: Use the correct bit to disable extended next page
  2026-04-11 14:10 ` Andrew Lunn
@ 2026-04-11 16:54   ` Maxime Chevallier
  0 siblings, 0 replies; 3+ messages in thread
From: Maxime Chevallier @ 2026-04-11 16:54 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jakub Kicinski, davem, Eric Dumazet, Paolo Abeni, Simon Horman,
	Russell King, thomas.petazzoni, netdev, linux-kernel,
	linux-arm-msm

Hi Andrew,

On 11/04/2026 16:10, Andrew Lunn wrote:
> On Fri, Apr 10, 2026 at 07:10:20PM +0200, Maxime Chevallier wrote:
>> As noted in the blamed commit, the AR8035 and other PHYs from this
>> family advertise the Extended Next Page support by default, which may be
>> understood by some partners as this PHY being multi-gig capable.
>>
>> The fix is to disable XNP advertising, which is done by setting bit 12
>> of the Auto-Negotiation Advertisement Register (MII_ADVERTISE).
>>
>> The blamed commit incorrectly uses MDIO_AN_CTRL1_XNP, which is bit 13 as per
>> 802.3 : 45.2.7.1 AN control register (Register 7.0)
>>
>> BIT 12 in MII_ADVERTISE is wrapped by ADVERTISE_RESV, used by some
>> drivers such as the aquantia one. 802.3 Clause 28 defines bit 12 as
>> Extended Next Page ability, at least in recent versions of the standard.
> 
>> Let's add a define for it and use it in the at803x driver.
> 
> I agree with this, it defines the C22 4.12 bit. And this is what the
> at803x driver is using it for.
> 
>>  static void at803x_link_change_notify(struct phy_device *phydev)
>> diff --git a/include/uapi/linux/mii.h b/include/uapi/linux/mii.h
>> index 39f7c44baf53..61d6edad4b94 100644
>> --- a/include/uapi/linux/mii.h
>> +++ b/include/uapi/linux/mii.h
>> @@ -82,7 +82,8 @@
>>  #define ADVERTISE_100BASE4	0x0200	/* Try for 100mbps 4k packets  */
>>  #define ADVERTISE_PAUSE_CAP	0x0400	/* Try for pause               */
>>  #define ADVERTISE_PAUSE_ASYM	0x0800	/* Try for asymetric pause     */
>> -#define ADVERTISE_RESV		0x1000	/* Unused...                   */
>> +#define ADVERTISE_XNP		0x1000  /* Extended Next Page */
>> +#define ADVERTISE_RESV		ADVERTISE_XNP /* Used to be reserved */
> 
> Should we keep ADVERTISE_RESV?
> 
>   45.2.7.6 AN advertisement register
> 
>   If the Auto-Negotiation advertisement register (register 4) is
>   present, (see 28.2.4.1.3), then this register is a copy of the
>   Auto-Negotiation advertisement register (register 4). In this case,
>   reads to the AN advertisement register (7.16) report the value of
>   the Auto-Negotiation advertisement register (register 4); writes to
>   the AN advertisement register (7.16) cause a write to occur to the
>   Auto-Negotiation advertisement register.
> 
> So MDIO_MMD_AN:MDIO_AN_ADVERTISE is a straight copy of MII_ADVERTISE.
> 
>         ef4_mdio_write(efx, MDIO_MMD_AN, MDIO_AN_ADVERTISE, reg);
>         ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE, adv);
> 
> So ADVERTISE_XNP is just as valid in the other two drivers using
> ADVERTISE_RESV. I think we should change those as well to
> ADVERTISE_XNP and remove ADVERTISE_RESV?
> 
> 	Andrew

I agree with that yes and I've considered converting these drivers once
we have net merged into net-next should this patch be applied :)

That said, ADVERTISE_RESV is in uapi, is it even possible to remove it ?

I think the best we can hope for is to no longer have in-tree users of
ADVERTISE_RESV :(

Maxime


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

end of thread, other threads:[~2026-04-11 16:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-10 17:10 [PATCH net] net: phy: qcom: at803x: Use the correct bit to disable extended next page Maxime Chevallier
2026-04-11 14:10 ` Andrew Lunn
2026-04-11 16:54   ` Maxime Chevallier

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