netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Alexander Wilhelm <alexander.wilhelm@westermo.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: Aquantia PHY in OCSGMII mode?
Date: Fri, 1 Aug 2025 15:02:14 +0100	[thread overview]
Message-ID: <aIzI5roBAaRgzXxH@shell.armlinux.org.uk> (raw)
In-Reply-To: <20250801130420.m3fbqlvtzbdo5e5d@skbuf>

On Fri, Aug 01, 2025 at 04:04:20PM +0300, Vladimir Oltean wrote:
> On Fri, Aug 01, 2025 at 01:23:44PM +0100, Russell King (Oracle) wrote:
> > It looks like memac_select_pcs() and memac_prepare() fail to
> > handle 2500BASEX despite memac_initialization() suggesting the
> > SGMII PCS supports 2500BASEX.
> 
> Thanks for pointing this out, it seems to be a regression introduced by
> commit 5d93cfcf7360 ("net: dpaa: Convert to phylink").
> 
> If there are no other volunteers, I can offer to submit a patch if
> Alexander confirms this fixes his setup.
> 
> > It would also be good if the driver can also use
> > pcs->supported_interfaces which states which modes the PCS layer
> > supports as well.
> 
> The current algorithm in lynx_pcs_create() is too optimistic and
> advertises host interfaces which the PCS may not actually support.
> 
> static const phy_interface_t lynx_interfaces[] = {
> 	PHY_INTERFACE_MODE_SGMII,
> 	PHY_INTERFACE_MODE_QSGMII,
> 	PHY_INTERFACE_MODE_1000BASEX,
> 	PHY_INTERFACE_MODE_2500BASEX,
> 	PHY_INTERFACE_MODE_10GBASER,
> 	PHY_INTERFACE_MODE_USXGMII,
> };
> 
> 	for (i = 0; i < ARRAY_SIZE(lynx_interfaces); i++)
> 		__set_bit(lynx_interfaces[i], lynx->pcs.supported_interfaces);
> 
> I am concerned that if we add logic to the MAC driver which does:
> 
> 		phy_interface_or(config->supported_interfaces,
> 				 config->supported_interfaces,
> 				 pcs->supported_interfaces);
> 
> then we depart from the physical reality of the board and may end up
> accepting a host interface which we should have rejected.
> 
> There is downstream code which refines lynx_pcs_create() to this:
> 
> 	/* In case we have access to the SerDes phy/lane, then ask the SerDes
> 	 * driver what interfaces are supported based on the current PLL
> 	 * configuration.
> 	 */
> 	for (int i = 0; i < ARRAY_SIZE(lynx_interfaces); i++) {
> 		phy_interface_t iface = lynx_interfaces[i];
> 
> 		err = phy_validate(lynx->serdes[PRIMARY_LANE],
> 				   PHY_MODE_ETHERNET, iface, NULL);
> 		if (err)
> 			continue;
> 
> 		__set_bit(iface, supported_interfaces);
> 	}
> 
> but the infrastructure (the SerDes driver) is currently lacking upstream.

It looks like the SerDes driver is managed by the MAC (it validates
each mode against the serdes PHY driver's validate function - serdes
being mac_dev->fman_mac->serdes. If this SerDes doesn't exist, then
only mac_dev->phy_if is supported.

So, I don't think there's any need for the Lynx to reach out to the
SerDes in mainline as it currently stands.

As the SerDes also dictates which modes and is managed by fman, I'd
suggest for mainline that the code needs to implement the following
pseudocode:

	config->supported_interfaces = mac_support |
				(pcs->supported_interfaces &
				serdes_supported_interfaces);

rather than the simple "or pcs->supported_interfaces into the
supported bitmap" that we can do in other drivers.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2025-08-01 14:02 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-31 14:59 Aquantia PHY in OCSGMII mode? Alexander Wilhelm
2025-07-31 15:14 ` Andrew Lunn
2025-07-31 16:02   ` Russell King (Oracle)
2025-08-01  5:44     ` Alexander Wilhelm
2025-08-04 14:53       ` Andrew Lunn
2025-07-31 17:16 ` Vladimir Oltean
2025-07-31 19:26   ` Russell King (Oracle)
2025-08-01  5:50     ` Alexander Wilhelm
2025-08-01 11:01     ` Vladimir Oltean
2025-08-01 11:54       ` Alexander Wilhelm
2025-08-01 11:58         ` Russell King (Oracle)
2025-08-01 12:06           ` Alexander Wilhelm
2025-08-01 12:23             ` Russell King (Oracle)
2025-08-01 12:36               ` Alexander Wilhelm
2025-08-01 13:04               ` Vladimir Oltean
2025-08-01 14:02                 ` Russell King (Oracle) [this message]
2025-08-01 14:37                   ` Vladimir Oltean
2025-08-04  6:17                 ` Alexander Wilhelm
2025-08-04 10:01                   ` Vladimir Oltean
2025-08-04 13:01                     ` Alexander Wilhelm
2025-08-04 13:41                       ` Vladimir Oltean
2025-08-04 14:47                         ` Alexander Wilhelm
2025-08-04 16:00                           ` Vladimir Oltean
2025-08-04 16:02                             ` Vladimir Oltean
2025-08-05  7:59                               ` Alexander Wilhelm
2025-08-05 10:20                                 ` Vladimir Oltean
2025-08-05 12:44                                   ` Alexander Wilhelm
2025-08-06 14:58                                     ` Vladimir Oltean
2025-08-07  5:56                                       ` Alexander Wilhelm
2025-08-27  5:57                                       ` Alexander Wilhelm
2025-08-27  7:31                                         ` Vladimir Oltean
2025-08-27  8:41                                           ` Alexander Wilhelm
2025-08-27  8:47                                             ` Russell King (Oracle)
2025-08-27  9:03                                               ` Alexander Wilhelm
2025-08-27  9:13                                                 ` Russell King (Oracle)
2025-08-28  9:28                                                   ` Vladimir Oltean
2025-08-27  8:08                                         ` Russell King (Oracle)
2025-08-27  8:32                                           ` Alexander Wilhelm
2025-08-27  8:45                                             ` Russell King (Oracle)
2025-08-04 14:22                       ` Russell King (Oracle)
2025-08-04 14:51                         ` Alexander Wilhelm
2025-08-04 14:56                         ` Vladimir Oltean
2025-08-01 11:13     ` Vladimir Oltean
2025-08-01  5:53   ` Alexander Wilhelm

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=aIzI5roBAaRgzXxH@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=alexander.wilhelm@westermo.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).