public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Mohd Ayaan Anwar <mohd.anwar@oss•qualcomm.com>
To: Vladimir Oltean <olteanv@gmail•com>
Cc: "Russell King (Oracle)" <linux@armlinux•org.uk>,
	"Andrew Lunn" <andrew@lunn•ch>,
	"Heiner Kallweit" <hkallweit1@gmail•com>,
	"Alexandre Torgue" <alexandre.torgue@foss•st.com>,
	"Alexis Lothoré" <alexis.lothore@bootlin•com>,
	"Andrew Lunn" <andrew+netdev@lunn•ch>,
	"Boon Khai Ng" <boon.khai.ng@altera•com>,
	"Daniel Machon" <daniel.machon@microchip•com>,
	"David S. Miller" <davem@davemloft•net>,
	"Eric Dumazet" <edumazet@google•com>,
	"Furong Xu" <0x1207@gmail•com>,
	"Jacob Keller" <jacob.e.keller@intel•com>,
	"Jakub Kicinski" <kuba@kernel•org>,
	"Jan Petrous (OSS)" <jan.petrous@oss•nxp.com>,
	linux-arm-kernel@lists•infradead.org,
	linux-stm32@st-md-mailman•stormreply.com,
	"Maxime Chevallier" <maxime.chevallier@bootlin•com>,
	"Maxime Coquelin" <mcoquelin.stm32@gmail•com>,
	netdev@vger•kernel.org, "Paolo Abeni" <pabeni@redhat•com>,
	"Simon Horman" <horms@kernel•org>,
	"Yu-Chun Lin" <eleanor15x@gmail•com>
Subject: Re: [PATCH net-next 0/3] net: stmmac: phylink PCS conversion part 3 (dodgy stuff)
Date: Mon, 3 Nov 2025 20:17:20 +0530	[thread overview]
Message-ID: <aQjAeCNGA2cjNXy6@oss.qualcomm.com> (raw)
In-Reply-To: <20251103121353.dbnalfub5mzwad62@skbuf>

On Mon, Nov 03, 2025 at 02:13:53PM +0200, Vladimir Oltean wrote:
> On Mon, Nov 03, 2025 at 11:43:23AM +0000, Russell King (Oracle) wrote:
> > On Mon, Nov 03, 2025 at 04:50:03PM +0530, Mohd Ayaan Anwar wrote:
> > > On Mon, Nov 03, 2025 at 12:48:20PM +0200, Vladimir Oltean wrote:
> > > > 
> > > > As Russell partially pointed out, there are several assumptions in the
> > > > Aquantia PHY driver and in phylink, three of them being that:
> > > > - rate matching is only supported for PHY_INTERFACE_MODE_10GBASER and
> > > >   PHY_INTERFACE_MODE_2500BASEX (thus not PHY_INTERFACE_MODE_SGMII)
> > > > - if phy_get_rate_matching() returns RATE_MATCH_NONE for an interface,
> > > >   pl->phy_state.rate_matching will also be RATE_MATCH_NONE when using
> > > >   that interface
> > > > - if rate matching is used, the PHY is configured to use it for all
> > > >   media speeds <= phylink_interface_max_speed(link_state.interface)
> > > > 
> > > > Those assumptions are not validated very well against the ground truth
> > > > from the PHY provisioning, so the next step would be for us to see that
> > > > directly.
> > > > 
> > > > Please turn this print from aqr_gen2_read_global_syscfg() into something
> > > > visible in dmesg, i.e. by replacing phydev_dbg() with phydev_info():
> > > > 
> > > > 		phydev_dbg(phydev,
> > > > 			   "Media speed %d uses host interface %s with %s\n",
> > > > 			   syscfg->speed, phy_modes(syscfg->interface),
> > > > 			   syscfg->rate_adapt == AQR_RATE_ADAPT_NONE ? "no rate adaptation" :
> > > > 			   syscfg->rate_adapt == AQR_RATE_ADAPT_PAUSE ? "rate adaptation through flow control" :
> > > > 			   syscfg->rate_adapt == AQR_RATE_ADAPT_USX ? "rate adaptation through symbol replication" :
> > > > 			   "unrecognized rate adaptation type");
> > > 
> > > Thanks. Looks like rate adaptation is only provisioned for 10M, which
> > > matches my observation where phylink passes the exact speeds for
> > > 100/1000/2500 but 1000 for 10M.
> > 
> > Hmm, I wonder what the PHY is doing for that then. stmmac will be
> > programmed to read the Cisco SGMII in-band control word, and use
> > that to determine whether symbol replication for slower speeds is
> > being used.
> > 
> > If AQR115C is indicating 10M in the in-band control word, but is
> > actually operating the link at 1G speed, things are not going to
> > work, and I would say the PHY is broken to be doing that. The point
> > of the SGMII in-band control word is to tell the MAC about the
> > required symbol replication on the link for transmitting the slower
> > data rates over the link.
> > 
> > stmmac unfortunately doesn't give access to the raw Cisco SGMII
> > in-band control word. However, reading register 0xf8 bits 31:16 for
> > dwmac4, or register 0xd8 bits 15:0 for dwmac1000 will give this
> > information. In that bitfield, bits 2:1 give the speed. 2 = 1G,
> > 1 = 100M, 0 = 10M.
> 
> It might be Linux who is forcing the AQR115C into the nonsensical
> behaviour of advertising 10M in the SGMII control word while
> simultanously forcing the PHY MII to operate at 1G with flow control
> for the 10M media speed.
> 
> We don't control the latter, but we do control the former:
> aqr_gen2_config_inband(), if given modes == LINK_INBAND_ENABLE, will
> enable in-band for all media speeds that use PHY_INTERFACE_MODE_SGMII.
> Regardless of how the PHY was provisioned for each media speed, and
> especially regardless of rate matching settings, this function will
> uniformly set the same in-band enabled/disabled setting for all media
> speeds using the same host interface.
> 
> If dwmac_integrated_pcs_inband_caps(), as per Russell's patch 1/3,
> reports LINK_INBAND_ENABLE | LINK_INBAND_DISABLE, and if
> aqr_gen2_inband_caps() also reports LINK_INBAND_ENABLE | LINK_INBAND_DISABLE,
> then we're giving phylink_pcs_neg_mode() all the tools it needs to shoot
> itself in the foot, and select LINK_INBAND_ENABLE.
> 
> The judgement call in the Aquantia PHY driver was mine, as documented in
> commit 5d59109d47c0 ("net: phy: aquantia: report and configure in-band
> autoneg capabilities"). The idea being that the configuration would have
> been unsupportable anyway given the question that the framework asks:
> "does the PHY use in-band for SGMII, or does it not?"
> 
> Assuming the configuration at 10Mbps wasn't always broken, there's only
> one way to know how it was supposed to work: more dumping of the initial
> provisioning, prior to our modification in aqr_gen2_config_inband().
> Ayaan, please re-print the same info with this new untested patch applied.
> I am going to assume that in-band autoneg isn't enabled in the unmodified
> provisioning, at least for 10M.
> 
> Russell's request for the integrated PCS status is also a good parallel
> confirmation that yes, we've entered a mode where the PHY advertises
> SGMII replication at 10M.

> From b91162e5dae8e20b477999c4f2fcdb98c219d663 Mon Sep 17 00:00:00 2001
> From: Vladimir Oltean <vladimir.oltean@nxp•com>
> Date: Mon, 3 Nov 2025 14:03:55 +0200
> Subject: [PATCH] net: phy: aquantia: add inband setting to the
>  aqr_gen2_read_global_syscfg() print
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp•com>
> ---
>  drivers/net/phy/aquantia/aquantia_main.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
> index 41f3676c7f1e..f06b7b51bb7d 100644
> --- a/drivers/net/phy/aquantia/aquantia_main.c
> +++ b/drivers/net/phy/aquantia/aquantia_main.c
> @@ -839,6 +839,7 @@ static int aqr_gen2_read_global_syscfg(struct phy_device *phydev)
>  
>  	for (i = 0; i < AQR_NUM_GLOBAL_CFG; i++) {
>  		struct aqr_global_syscfg *syscfg = &priv->global_cfg[i];
> +		bool inband;
>  
>  		syscfg->speed = aqr_global_cfg_regs[i].speed;
>  
> @@ -849,6 +850,7 @@ static int aqr_gen2_read_global_syscfg(struct phy_device *phydev)
>  
>  		serdes_mode = FIELD_GET(VEND1_GLOBAL_CFG_SERDES_MODE, val);
>  		rate_adapt = FIELD_GET(VEND1_GLOBAL_CFG_RATE_ADAPT, val);
> +		inband = FIELD_GET(VEND1_GLOBAL_CFG_AUTONEG_ENA, val);
>  
>  		switch (serdes_mode) {
>  		case VEND1_GLOBAL_CFG_SERDES_MODE_XFI:
> @@ -896,12 +898,13 @@ static int aqr_gen2_read_global_syscfg(struct phy_device *phydev)
>  		}
>  
>  		phydev_dbg(phydev,
> -			   "Media speed %d uses host interface %s with %s\n",
> +			   "Media speed %d uses host interface %s with %s, inband %s\n",
>  			   syscfg->speed, phy_modes(syscfg->interface),
>  			   syscfg->rate_adapt == AQR_RATE_ADAPT_NONE ? "no rate adaptation" :
>  			   syscfg->rate_adapt == AQR_RATE_ADAPT_PAUSE ? "rate adaptation through flow control" :
>  			   syscfg->rate_adapt == AQR_RATE_ADAPT_USX ? "rate adaptation through symbol replication" :
> -			   "unrecognized rate adaptation type");
> +			   "unrecognized rate adaptation type",
> +			   str_enabled_disabled(inband));
>  	}
>  
>  	return 0;
> -- 
> 2.43.0
> 

Here are the logs when I boot up with a 10M link:

[   10.743044] Aquantia AQR115C stmmac-0:08: Media speed 10 uses host interface sgmii with rate adaptation through flow control, inband enabled
[   10.757965] Aquantia AQR115C stmmac-0:08: Media speed 100 uses host interface sgmii with no rate adaptation, inband enabled
[   10.769857] Aquantia AQR115C stmmac-0:08: Media speed 1000 uses host interface sgmii with no rate adaptation, inband enabled
[   10.781840] Aquantia AQR115C stmmac-0:08: Media speed 2500 uses host interface 2500base-x with no rate adaptation, inband disabled
[   10.794346] Aquantia AQR115C stmmac-0:08: Media speed 5000 uses host interface 10gbase-r with rate adaptation through flow control, inband disabled
[   10.808358] Aquantia AQR115C stmmac-0:08: Media speed 10000 uses host interface 10gbase-r with no rate adaptation, inband disabled
[   10.827242] qcom-ethqos 23040000.ethernet eth1: PHY stmmac-0:08 uses interfaces 4,23,27, validating 23
[   10.836812] qcom-ethqos 23040000.ethernet eth1:  interface 23 (2500base-x) rate match pause supports 0-7,9,13-14,47
[   10.836817] qcom-ethqos 23040000.ethernet eth1: PHY [stmmac-0:08] driver [Aquantia AQR115C] (irq=318)
[   10.836819] qcom-ethqos 23040000.ethernet eth1: phy: 2500base-x setting supported 0000000,00000000,00008000,000062ff advertising 0000000,00000000,00008000,000062ff
[   10.851865] qcom-ethqos 23040000.ethernet eth1: Enabling Safety Features
[   10.882611] qcom-ethqos 23040000.ethernet eth1: IEEE 1588-2008 Advanced Timestamp supported
[   10.895207] qcom-ethqos 23040000.ethernet eth1: registered PTP clock
[   10.902334] qcom-ethqos 23040000.ethernet eth1: configuring for phy/2500base-x link mode
[   10.910654] qcom-ethqos 23040000.ethernet eth1: major config, requested phy/2500base-x
[   10.918790] qcom-ethqos 23040000.ethernet eth1: has pcs = true
[   10.924787] qcom-ethqos 23040000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
[   10.933809] qcom-ethqos 23040000.ethernet eth1: major config, active phy/outband/2500base-x
[   10.942388] qcom-ethqos 23040000.ethernet eth1: phylink_mac_config: mode=phy/2500base-x/none adv=0000000,00000000,00000000,00000000 pause=00
[   10.966344] qcom-ethqos 23040000.ethernet eth1: phy link down 2500base-x/Unknown/Unknown/none/off/nolpi
[   12.819779] qcom-ethqos 23040000.ethernet eth1: phy link up sgmii/10Mbps/Half/pause/off/nolpi
[   12.825947] stmmac_pcs: Link Down
[   12.829539] qcom-ethqos 23040000.ethernet eth1: major config, requested phy/sgmii
[   12.831998] stmmac_pcs: Link Down
[   12.839683] qcom-ethqos 23040000.ethernet eth1: has pcs = true
[   12.843123] stmmac_pcs: Link Down
[   12.849121] qcom-ethqos 23040000.ethernet eth1: interface sgmii inband modes: pcs=03 phy=03
[   12.852546] stmmac_pcs: Link Down
[   12.861109] qcom-ethqos 23040000.ethernet eth1: major config, active phy/outband/sgmii
[   12.864535] stmmac_pcs: Link Down
[   12.872724] qcom-ethqos 23040000.ethernet eth1: phylink_mac_config: mode=phy/sgmii/pause adv=0000000,00000000,00000000,00000000 pause=00
[   12.876094] stmmac_pcs: Link Down
[   12.891394] qcom-ethqos 23040000.ethernet eth1: ethqos_configure_sgmii : Speed = 1000
[   12.892094] stmmac_pcs: Link Down
[   12.900109] dwmac: PCS configuration changed from phylink by glue, please report: 0x00040000 -> 0x00041200
[   12.903555] stmmac_pcs: Link Up
[   12.913473] qcom-ethqos 23040000.ethernet eth1: Link is Up - 10Mbps/Half - flow control off
[   12.916679] stmmac_pcs: Link Down
[   12.928659] stmmac_pcs: ANE process completed
[   12.933133] stmmac_pcs: Link Up

Although unrelated, I found it weird that the link comes up in half
duplex mode for 10M. To enable full duplex, I have to manually do it via
ethtool. I will try to connect a different link partner tomorrow, just
to rule out any issues on the other end.

	Ayaan


  reply	other threads:[~2025-11-03 14:47 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-25 20:47 [PATCH net-next 0/3] net: stmmac: phylink PCS conversion part 3 (dodgy stuff) Russell King (Oracle)
2025-10-25 20:48 ` [PATCH net-next 1/3] net: stmmac: configure AN control according to phylink Russell King (Oracle)
2025-10-25 20:48 ` [PATCH net-next 2/3] net: stmmac: report PCS configuration changes Russell King (Oracle)
2025-10-25 20:48 ` [PATCH net-next 3/3] net: stmmac: add support specifying PCS supported interfaces Russell King (Oracle)
2025-10-28 10:16   ` Maxime Chevallier
2025-10-28 10:35     ` Russell King (Oracle)
2025-10-28 10:40       ` Russell King (Oracle)
2025-10-28 11:26       ` Maxime Chevallier
2025-10-28 21:12 ` [PATCH net-next 0/3] net: stmmac: phylink PCS conversion part 3 (dodgy stuff) Mohd Ayaan Anwar
2025-10-29  9:22   ` Russell King (Oracle)
2025-10-30 13:20     ` Mohd Ayaan Anwar
2025-10-30 15:19       ` Russell King (Oracle)
2025-10-30 15:22         ` Russell King (Oracle)
2025-11-03  8:58           ` Mohd Ayaan Anwar
2025-11-03  9:52             ` Russell King (Oracle)
2025-11-03 10:18               ` Mohd Ayaan Anwar
2025-11-03 10:47                 ` Russell King (Oracle)
2025-11-03 10:48                 ` Vladimir Oltean
2025-11-03 11:20                   ` Mohd Ayaan Anwar
2025-11-03 11:43                     ` Russell King (Oracle)
2025-11-03 12:13                       ` Vladimir Oltean
2025-11-03 14:47                         ` Mohd Ayaan Anwar [this message]
2025-11-03 17:15                           ` Russell King (Oracle)
2025-11-03 17:02                         ` Russell King (Oracle)
2025-11-03 12:17                       ` Mohd Ayaan Anwar
2025-11-03 17:13                         ` Russell King (Oracle)
2025-11-05 15:46         ` Mohd Ayaan Anwar
2025-11-05 18:12           ` Russell King (Oracle)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aQjAeCNGA2cjNXy6@oss.qualcomm.com \
    --to=mohd.anwar@oss$(echo .)qualcomm.com \
    --cc=0x1207@gmail$(echo .)com \
    --cc=alexandre.torgue@foss$(echo .)st.com \
    --cc=alexis.lothore@bootlin$(echo .)com \
    --cc=andrew+netdev@lunn$(echo .)ch \
    --cc=andrew@lunn$(echo .)ch \
    --cc=boon.khai.ng@altera$(echo .)com \
    --cc=daniel.machon@microchip$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=edumazet@google$(echo .)com \
    --cc=eleanor15x@gmail$(echo .)com \
    --cc=hkallweit1@gmail$(echo .)com \
    --cc=horms@kernel$(echo .)org \
    --cc=jacob.e.keller@intel$(echo .)com \
    --cc=jan.petrous@oss$(echo .)nxp.com \
    --cc=kuba@kernel$(echo .)org \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    --cc=linux-stm32@st-md-mailman$(echo .)stormreply.com \
    --cc=linux@armlinux$(echo .)org.uk \
    --cc=maxime.chevallier@bootlin$(echo .)com \
    --cc=mcoquelin.stm32@gmail$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=olteanv@gmail$(echo .)com \
    --cc=pabeni@redhat$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox