* [PATCH RFC 00/10] net: phylink: improve PHY validation
@ 2023-11-22 15:30 Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 01/10] net: phy: add possible interfaces Russell King (Oracle)
` (11 more replies)
0 siblings, 12 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:30 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
Hi,
One of the issues which has concerned me about the rate matching
implenentation that we have is that phy_get_rate_matching() returns
whether rate matching will be used for a particular interface, and we
enquire only for one interface.
Aquantia PHYs can be programmed with the rate matching and interface
mode settings on a per-media speed basis using the per-speed vendor 1
global configuration registers.
Thus, it is possible for the PHY to be configured to use rate matching
for 10G, 5G, 2.5G with 10GBASE-R, and then SGMII for the remaining
speeds. Therefore, it clearly doesn't make sense to enquire about rate
matching for just one interface mode.
Also, PHYs that change their interfaces are handled sub-optimally, in
that we validate all the interface modes that the host supports, rather
than the interface modes that the PHY will use.
This patch series changes the way we validate PHYs, but in order to do
so, we need to know exactly which interface modes will be used by the
PHY. So that phylib can convey this information, we add
"possible_interfaces" to struct phy_device.
possible_interfaces is to be filled in by a phylib driver once the PHY
is configured (in other words in the PHYs .config_init method) with the
interface modes that it will switch between. This then allows users of
phylib to know which interface modes will be used by the PHY.
This allows us to solve both these issues: where possible_interfaces is
provided, we can validate which ethtool link modes can be supported by
looking at which interface modes that both the PHY and host support,
and request rate matching information for each mode.
This should improve the accuracy of the validation.
drivers/net/phy/aquantia/aquantia.h | 5 +
drivers/net/phy/aquantia/aquantia_main.c | 76 +++++++++++-
drivers/net/phy/bcm84881.c | 12 ++
drivers/net/phy/marvell10g.c | 203 ++++++++++++++++++++-----------
drivers/net/phy/phy_device.c | 2 +
drivers/net/phy/phylink.c | 177 +++++++++++++++++++--------
include/linux/phy.h | 3 +
7 files changed, 353 insertions(+), 125 deletions(-)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH RFC net-next 01/10] net: phy: add possible interfaces
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
@ 2023-11-22 15:31 ` Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 02/10] net: phy: marvell10g: table driven mactype decode Russell King (Oracle)
` (10 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:31 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
Add a possible_interfaces member to struct phy_device to indicate which
interfaces a clause 45 PHY may switch between depending on the media.
This must be populated by the PHY driver by the time the .config_init()
method completes according to the PHYs host-side configuration.
For example, the Marvell 88x3310 PHY can switch between 10GBASE-R,
5GBASE-R, 2500BASE-X, and SGMII on the host side depending on the media
side speed, so all these interface modes are set in the
possible_interfaces member.
This allows phylib users (such as phylink) to know in advance which
interface modes to expect, which allows them to appropriately restrict
the advertised link modes according to the capabilities of other parts
of the link.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy_device.c | 2 ++
include/linux/phy.h | 3 +++
2 files changed, 5 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2ce74593d6e4..6494e66c5e10 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1247,6 +1247,8 @@ int phy_init_hw(struct phy_device *phydev)
if (ret < 0)
return ret;
+ phy_interface_zero(phydev->possible_interfaces);
+
if (phydev->drv->config_init) {
ret = phydev->drv->config_init(phydev);
if (ret < 0)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e5f1f41e399c..6e7ebcc50b85 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -605,6 +605,8 @@ struct macsec_ops;
* @irq_rerun: Flag indicating interrupts occurred while PHY was suspended,
* requiring a rerun of the interrupt handler after resume
* @interface: enum phy_interface_t value
+ * @possible_interfaces: bitmap if interface modes that the attached PHY
+ * will switch between depending on media speed.
* @skb: Netlink message for cable diagnostics
* @nest: Netlink nest used for cable diagnostics
* @ehdr: nNtlink header for cable diagnostics
@@ -674,6 +676,7 @@ struct phy_device {
u32 dev_flags;
phy_interface_t interface;
+ DECLARE_PHY_INTERFACE_MASK(possible_interfaces);
/*
* forced speed & duplex (no autoneg)
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC net-next 02/10] net: phy: marvell10g: table driven mactype decode
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 01/10] net: phy: add possible interfaces Russell King (Oracle)
@ 2023-11-22 15:31 ` Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 03/10] net: phy: marvell10g: fill in possible_interfaces Russell King (Oracle)
` (9 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:31 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni, Marek Beh__n
Replace the code-based mactype decode with a table driven approach.
This will allow us to fill in the possible_interfaces cleanly.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/marvell10g.c | 197 +++++++++++++++++++++--------------
1 file changed, 120 insertions(+), 77 deletions(-)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index d4bb90d76881..a880b3375dee 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -141,13 +141,21 @@ enum {
MV_V2_TEMP_UNKNOWN = 0x9600, /* unknown function */
};
+struct mv3310_mactype {
+ bool valid;
+ bool fixed_interface;
+ phy_interface_t interface_10g;
+};
+
struct mv3310_chip {
bool (*has_downshift)(struct phy_device *phydev);
void (*init_supported_interfaces)(unsigned long *mask);
int (*get_mactype)(struct phy_device *phydev);
int (*set_mactype)(struct phy_device *phydev, int mactype);
int (*select_mactype)(unsigned long *interfaces);
- int (*init_interface)(struct phy_device *phydev, int mactype);
+
+ const struct mv3310_mactype *mactypes;
+ size_t n_mactypes;
#ifdef CONFIG_HWMON
int (*hwmon_read_temp_reg)(struct phy_device *phydev);
@@ -156,11 +164,10 @@ struct mv3310_chip {
struct mv3310_priv {
DECLARE_BITMAP(supported_interfaces, PHY_INTERFACE_MODE_MAX);
+ const struct mv3310_mactype *mactype;
u32 firmware_ver;
bool has_downshift;
- bool rate_match;
- phy_interface_t const_interface;
struct device *hwmon_dev;
char *hwmon_name;
@@ -702,71 +709,99 @@ static int mv3310_select_mactype(unsigned long *interfaces)
return -1;
}
-static int mv2110_init_interface(struct phy_device *phydev, int mactype)
-{
- struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
-
- priv->rate_match = false;
-
- if (mactype == MV_PMA_21X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH)
- priv->rate_match = true;
-
- if (mactype == MV_PMA_21X0_PORT_CTRL_MACTYPE_USXGMII)
- priv->const_interface = PHY_INTERFACE_MODE_USXGMII;
- else if (mactype == MV_PMA_21X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH)
- priv->const_interface = PHY_INTERFACE_MODE_10GBASER;
- else if (mactype == MV_PMA_21X0_PORT_CTRL_MACTYPE_5GBASER ||
- mactype == MV_PMA_21X0_PORT_CTRL_MACTYPE_5GBASER_NO_SGMII_AN)
- priv->const_interface = PHY_INTERFACE_MODE_NA;
- else
- return -EINVAL;
-
- return 0;
-}
-
-static int mv3310_init_interface(struct phy_device *phydev, int mactype)
-{
- struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
-
- priv->rate_match = false;
-
- if (mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH ||
- mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI_RATE_MATCH ||
- mactype == MV_V2_3310_PORT_CTRL_MACTYPE_XAUI_RATE_MATCH)
- priv->rate_match = true;
-
- if (mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_USXGMII)
- priv->const_interface = PHY_INTERFACE_MODE_USXGMII;
- else if (mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH ||
- mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_NO_SGMII_AN ||
- mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER)
- priv->const_interface = PHY_INTERFACE_MODE_10GBASER;
- else if (mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI_RATE_MATCH ||
- mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI)
- priv->const_interface = PHY_INTERFACE_MODE_RXAUI;
- else if (mactype == MV_V2_3310_PORT_CTRL_MACTYPE_XAUI_RATE_MATCH ||
- mactype == MV_V2_3310_PORT_CTRL_MACTYPE_XAUI)
- priv->const_interface = PHY_INTERFACE_MODE_XAUI;
- else
- return -EINVAL;
-
- return 0;
-}
-
-static int mv3340_init_interface(struct phy_device *phydev, int mactype)
-{
- struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
- int err = 0;
-
- priv->rate_match = false;
+static const struct mv3310_mactype mv2110_mactypes[] = {
+ [MV_PMA_21X0_PORT_CTRL_MACTYPE_USXGMII] = {
+ .valid = true,
+ .fixed_interface = true,
+ .interface_10g = PHY_INTERFACE_MODE_USXGMII,
+ },
+ [MV_PMA_21X0_PORT_CTRL_MACTYPE_5GBASER] = {
+ .valid = true,
+ .interface_10g = PHY_INTERFACE_MODE_NA,
+ },
+ [MV_PMA_21X0_PORT_CTRL_MACTYPE_5GBASER_NO_SGMII_AN] = {
+ .valid = true,
+ .interface_10g = PHY_INTERFACE_MODE_NA,
+ },
+ [MV_PMA_21X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH] = {
+ .valid = true,
+ .fixed_interface = true,
+ .interface_10g = PHY_INTERFACE_MODE_10GBASER,
+ },
+};
- if (mactype == MV_V2_3340_PORT_CTRL_MACTYPE_RXAUI_NO_SGMII_AN)
- priv->const_interface = PHY_INTERFACE_MODE_RXAUI;
- else
- err = mv3310_init_interface(phydev, mactype);
+static const struct mv3310_mactype mv3310_mactypes[] = {
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI] = {
+ .valid = true,
+ .interface_10g = PHY_INTERFACE_MODE_RXAUI,
+ },
+ [MV_V2_3310_PORT_CTRL_MACTYPE_XAUI_RATE_MATCH] = {
+ .valid = true,
+ .fixed_interface = true,
+ .interface_10g = PHY_INTERFACE_MODE_XAUI,
+ },
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI_RATE_MATCH] = {
+ .valid = true,
+ .fixed_interface = true,
+ .interface_10g = PHY_INTERFACE_MODE_RXAUI,
+ },
+ [MV_V2_3310_PORT_CTRL_MACTYPE_XAUI] = {
+ .valid = true,
+ .interface_10g = PHY_INTERFACE_MODE_XAUI,
+ },
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER] = {
+ .valid = true,
+ .interface_10g = PHY_INTERFACE_MODE_10GBASER,
+ },
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_NO_SGMII_AN] = {
+ .valid = true,
+ .interface_10g = PHY_INTERFACE_MODE_10GBASER,
+ },
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH] = {
+ .valid = true,
+ .fixed_interface = true,
+ .interface_10g = PHY_INTERFACE_MODE_10GBASER,
+ },
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_USXGMII] = {
+ .valid = true,
+ .fixed_interface = true,
+ .interface_10g = PHY_INTERFACE_MODE_USXGMII,
+ },
+};
- return err;
-}
+static const struct mv3310_mactype mv3340_mactypes[] = {
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI] = {
+ .valid = true,
+ .interface_10g = PHY_INTERFACE_MODE_RXAUI,
+ },
+ [MV_V2_3340_PORT_CTRL_MACTYPE_RXAUI_NO_SGMII_AN] = {
+ .valid = true,
+ .interface_10g = PHY_INTERFACE_MODE_RXAUI,
+ },
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI_RATE_MATCH] = {
+ .valid = true,
+ .fixed_interface = true,
+ .interface_10g = PHY_INTERFACE_MODE_RXAUI,
+ },
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER] = {
+ .valid = true,
+ .interface_10g = PHY_INTERFACE_MODE_10GBASER,
+ },
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_NO_SGMII_AN] = {
+ .valid = true,
+ .interface_10g = PHY_INTERFACE_MODE_10GBASER,
+ },
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH] = {
+ .valid = true,
+ .fixed_interface = true,
+ .interface_10g = PHY_INTERFACE_MODE_10GBASER,
+ },
+ [MV_V2_33X0_PORT_CTRL_MACTYPE_USXGMII] = {
+ .valid = true,
+ .fixed_interface = true,
+ .interface_10g = PHY_INTERFACE_MODE_USXGMII,
+ },
+};
static int mv3310_config_init(struct phy_device *phydev)
{
@@ -803,12 +838,13 @@ static int mv3310_config_init(struct phy_device *phydev)
if (mactype < 0)
return mactype;
- err = chip->init_interface(phydev, mactype);
- if (err) {
+ if (mactype >= chip->n_mactypes || !chip->mactypes[mactype].valid) {
phydev_err(phydev, "MACTYPE configuration invalid\n");
- return err;
+ return -EINVAL;
}
+ priv->mactype = &chip->mactypes[mactype];
+
/* Enable EDPD mode - saving 600mW */
err = mv3310_set_edpd(phydev, ETHTOOL_PHY_EDPD_DFLT_TX_MSECS);
if (err)
@@ -935,9 +971,8 @@ static void mv3310_update_interface(struct phy_device *phydev)
*
* In USXGMII mode the PHY interface mode is also fixed.
*/
- if (priv->rate_match ||
- priv->const_interface == PHY_INTERFACE_MODE_USXGMII) {
- phydev->interface = priv->const_interface;
+ if (priv->mactype->fixed_interface) {
+ phydev->interface = priv->mactype->interface_10g;
return;
}
@@ -949,7 +984,7 @@ static void mv3310_update_interface(struct phy_device *phydev)
*/
switch (phydev->speed) {
case SPEED_10000:
- phydev->interface = priv->const_interface;
+ phydev->interface = priv->mactype->interface_10g;
break;
case SPEED_5000:
phydev->interface = PHY_INTERFACE_MODE_5GBASER;
@@ -1163,7 +1198,9 @@ static const struct mv3310_chip mv3310_type = {
.get_mactype = mv3310_get_mactype,
.set_mactype = mv3310_set_mactype,
.select_mactype = mv3310_select_mactype,
- .init_interface = mv3310_init_interface,
+
+ .mactypes = mv3310_mactypes,
+ .n_mactypes = ARRAY_SIZE(mv3310_mactypes),
#ifdef CONFIG_HWMON
.hwmon_read_temp_reg = mv3310_hwmon_read_temp_reg,
@@ -1176,7 +1213,9 @@ static const struct mv3310_chip mv3340_type = {
.get_mactype = mv3310_get_mactype,
.set_mactype = mv3310_set_mactype,
.select_mactype = mv3310_select_mactype,
- .init_interface = mv3340_init_interface,
+
+ .mactypes = mv3340_mactypes,
+ .n_mactypes = ARRAY_SIZE(mv3340_mactypes),
#ifdef CONFIG_HWMON
.hwmon_read_temp_reg = mv3310_hwmon_read_temp_reg,
@@ -1188,7 +1227,9 @@ static const struct mv3310_chip mv2110_type = {
.get_mactype = mv2110_get_mactype,
.set_mactype = mv2110_set_mactype,
.select_mactype = mv2110_select_mactype,
- .init_interface = mv2110_init_interface,
+
+ .mactypes = mv2110_mactypes,
+ .n_mactypes = ARRAY_SIZE(mv2110_mactypes),
#ifdef CONFIG_HWMON
.hwmon_read_temp_reg = mv2110_hwmon_read_temp_reg,
@@ -1200,7 +1241,9 @@ static const struct mv3310_chip mv2111_type = {
.get_mactype = mv2110_get_mactype,
.set_mactype = mv2110_set_mactype,
.select_mactype = mv2110_select_mactype,
- .init_interface = mv2110_init_interface,
+
+ .mactypes = mv2110_mactypes,
+ .n_mactypes = ARRAY_SIZE(mv2110_mactypes),
#ifdef CONFIG_HWMON
.hwmon_read_temp_reg = mv2110_hwmon_read_temp_reg,
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC net-next 03/10] net: phy: marvell10g: fill in possible_interfaces
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 01/10] net: phy: add possible interfaces Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 02/10] net: phy: marvell10g: table driven mactype decode Russell King (Oracle)
@ 2023-11-22 15:31 ` Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 04/10] net: phy: bcm84881: " Russell King (Oracle)
` (8 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:31 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni, Marek Beh__n
Fill in the possible_interfaces member according to the selected
mactype mode.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/marvell10g.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index a880b3375dee..ad43e280930c 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -803,6 +803,22 @@ static const struct mv3310_mactype mv3340_mactypes[] = {
},
};
+static void mv3310_fill_possible_interfaces(struct phy_device *phydev)
+{
+ struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
+ unsigned long *possible = phydev->possible_interfaces;
+ const struct mv3310_mactype *mactype = priv->mactype;
+
+ if (mactype->interface_10g != PHY_INTERFACE_MODE_NA)
+ __set_bit(priv->mactype->interface_10g, possible);
+
+ if (!mactype->fixed_interface) {
+ __set_bit(PHY_INTERFACE_MODE_5GBASER, possible);
+ __set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
+ __set_bit(PHY_INTERFACE_MODE_SGMII, possible);
+ }
+}
+
static int mv3310_config_init(struct phy_device *phydev)
{
struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
@@ -845,6 +861,8 @@ static int mv3310_config_init(struct phy_device *phydev)
priv->mactype = &chip->mactypes[mactype];
+ mv3310_fill_possible_interfaces(phydev);
+
/* Enable EDPD mode - saving 600mW */
err = mv3310_set_edpd(phydev, ETHTOOL_PHY_EDPD_DFLT_TX_MSECS);
if (err)
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC net-next 04/10] net: phy: bcm84881: fill in possible_interfaces
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
` (2 preceding siblings ...)
2023-11-22 15:31 ` [PATCH RFC net-next 03/10] net: phy: marvell10g: fill in possible_interfaces Russell King (Oracle)
@ 2023-11-22 15:31 ` Russell King (Oracle)
2023-11-22 22:11 ` Florian Fainelli
2023-11-22 15:31 ` [PATCH RFC net-next 05/10] net: phy: aquantia: fill in possible_interfaces for AQR113C Russell King (Oracle)
` (7 subsequent siblings)
11 siblings, 1 reply; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:31 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
Fill in the possible_interfaces member. This PHY driver only supports
a single configuration found on SFPs.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/bcm84881.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c
index 9717a1626f3f..f1d47c264058 100644
--- a/drivers/net/phy/bcm84881.c
+++ b/drivers/net/phy/bcm84881.c
@@ -29,8 +29,19 @@ static int bcm84881_wait_init(struct phy_device *phydev)
100000, 2000000, false);
}
+static void bcm84881_fill_possible_interfaces(struct phy_device *phydev)
+{
+ unsigned long *possible = phydev->possible_interfaces;
+
+ __set_bit(PHY_INTERFACE_MODE_SGMII, possible);
+ __set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
+ __set_bit(PHY_INTERFACE_MODE_10GBASER, possible);
+}
+
static int bcm84881_config_init(struct phy_device *phydev)
{
+ bcm84881_fill_possible_interfaces(phydev);
+
switch (phydev->interface) {
case PHY_INTERFACE_MODE_SGMII:
case PHY_INTERFACE_MODE_2500BASEX:
@@ -39,6 +50,7 @@ static int bcm84881_config_init(struct phy_device *phydev)
default:
return -ENODEV;
}
+
return 0;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC net-next 05/10] net: phy: aquantia: fill in possible_interfaces for AQR113C
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
` (3 preceding siblings ...)
2023-11-22 15:31 ` [PATCH RFC net-next 04/10] net: phy: bcm84881: " Russell King (Oracle)
@ 2023-11-22 15:31 ` Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 06/10] net: phylink: split out per-interface validation Russell King (Oracle)
` (6 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:31 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
Fill in the possible_interfaces bitmap for AQR113C so phylink knows
which interface modes will be used by the PHY.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/aquantia/aquantia.h | 5 ++
drivers/net/phy/aquantia/aquantia_main.c | 76 +++++++++++++++++++++++-
2 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
index 9ed38972abdb..1c19ae74ad2b 100644
--- a/drivers/net/phy/aquantia/aquantia.h
+++ b/drivers/net/phy/aquantia/aquantia.h
@@ -47,6 +47,11 @@
#define VEND1_GLOBAL_CFG_5G 0x031e
#define VEND1_GLOBAL_CFG_10G 0x031f
/* ...and now the fields */
+#define VEND1_GLOBAL_CFG_SERDES_MODE GENMASK(2, 0)
+#define VEND1_GLOBAL_CFG_SERDES_MODE_XFI 0
+#define VEND1_GLOBAL_CFG_SERDES_MODE_SGMII 3
+#define VEND1_GLOBAL_CFG_SERDES_MODE_OCSGMII 4
+#define VEND1_GLOBAL_CFG_SERDES_MODE_XFI5G 6
#define VEND1_GLOBAL_CFG_RATE_ADAPT GENMASK(8, 7)
#define VEND1_GLOBAL_CFG_RATE_ADAPT_NONE 0
#define VEND1_GLOBAL_CFG_RATE_ADAPT_USX 1
diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index cc4a97741c4a..97a2fafa15ca 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -656,6 +656,80 @@ static int aqr107_resume(struct phy_device *phydev)
return aqr107_wait_processor_intensive_op(phydev);
}
+static const u16 aqr_global_cfg_regs[] = {
+ VEND1_GLOBAL_CFG_10M,
+ VEND1_GLOBAL_CFG_100M,
+ VEND1_GLOBAL_CFG_1G,
+ VEND1_GLOBAL_CFG_2_5G,
+ VEND1_GLOBAL_CFG_5G,
+ VEND1_GLOBAL_CFG_10G
+};
+
+static int aqr107_fill_interface_modes(struct phy_device *phydev)
+{
+ unsigned long *possible = phydev->possible_interfaces;
+ unsigned int serdes_mode, rate_adapt;
+ phy_interface_t interface;
+ int i, val;
+
+ /* Walk the media-speed configuration registers to determine which
+ * host-side serdes modes may be used by the PHY depending on the
+ * negotiated media speed.
+ */
+ for (i = 0; i < ARRAY_SIZE(aqr_global_cfg_regs); i++) {
+ val = phy_read_mmd(phydev, MDIO_MMD_VEND1,
+ aqr_global_cfg_regs[i]);
+ if (val < 0)
+ return val;
+
+ serdes_mode = FIELD_GET(VEND1_GLOBAL_CFG_SERDES_MODE, val);
+ rate_adapt = FIELD_GET(VEND1_GLOBAL_CFG_RATE_ADAPT, val);
+
+ switch (serdes_mode) {
+ case VEND1_GLOBAL_CFG_SERDES_MODE_XFI:
+ if (rate_adapt == VEND1_GLOBAL_CFG_RATE_ADAPT_USX)
+ interface = PHY_INTERFACE_MODE_USXGMII;
+ else
+ interface = PHY_INTERFACE_MODE_10GBASER;
+ break;
+
+ case VEND1_GLOBAL_CFG_SERDES_MODE_XFI5G:
+ interface = PHY_INTERFACE_MODE_5GBASER;
+ break;
+
+ case VEND1_GLOBAL_CFG_SERDES_MODE_OCSGMII:
+ interface = PHY_INTERFACE_MODE_2500BASEX;
+ break;
+
+ case VEND1_GLOBAL_CFG_SERDES_MODE_SGMII:
+ interface = PHY_INTERFACE_MODE_SGMII;
+ break;
+
+ default:
+ phydev_warn(phydev, "unrecognised serdes mode %u\n",
+ serdes_mode);
+ interface = PHY_INTERFACE_MODE_NA;
+ break;
+ }
+
+ if (interface != PHY_INTERFACE_MODE_NA)
+ __set_bit(interface, possible);
+ }
+
+ return 0;
+}
+
+static int aqr113c_config_init(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = aqr107_config_init(phydev);
+ if (ret < 0)
+ return ret;
+
+ return aqr107_fill_interface_modes(phydev);
+}
+
static int aqr107_probe(struct phy_device *phydev)
{
int ret;
@@ -794,7 +868,7 @@ static struct phy_driver aqr_driver[] = {
.name = "Aquantia AQR113C",
.probe = aqr107_probe,
.get_rate_matching = aqr107_get_rate_matching,
- .config_init = aqr107_config_init,
+ .config_init = aqr113c_config_init,
.config_aneg = aqr_config_aneg,
.config_intr = aqr_config_intr,
.handle_interrupt = aqr_handle_interrupt,
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC net-next 06/10] net: phylink: split out per-interface validation
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
` (4 preceding siblings ...)
2023-11-22 15:31 ` [PATCH RFC net-next 05/10] net: phy: aquantia: fill in possible_interfaces for AQR113C Russell King (Oracle)
@ 2023-11-22 15:31 ` Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 07/10] net: phylink: pass PHY into phylink_validate_one() Russell King (Oracle)
` (5 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:31 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
Split out the internals of phylink_validate_mask() to make the code
easier to read.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phylink.c | 42 ++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 12 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index c276f9482f78..11dd743141d5 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -689,26 +689,44 @@ static int phylink_validate_mac_and_pcs(struct phylink *pl,
return phylink_is_empty_linkmode(supported) ? -EINVAL : 0;
}
+static void phylink_validate_one(struct phylink *pl,
+ const unsigned long *supported,
+ const struct phylink_link_state *state,
+ phy_interface_t interface,
+ unsigned long *accum_supported,
+ unsigned long *accum_advertising)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_supported);
+ struct phylink_link_state tmp_state;
+
+ linkmode_copy(tmp_supported, supported);
+
+ tmp_state = *state;
+ tmp_state.interface = interface;
+
+ if (!phylink_validate_mac_and_pcs(pl, tmp_supported, &tmp_state)) {
+ phylink_dbg(pl, " interface %u (%s) rate match %s supports %*pbl\n",
+ interface, phy_modes(interface),
+ phy_rate_matching_to_str(tmp_state.rate_matching),
+ __ETHTOOL_LINK_MODE_MASK_NBITS, tmp_supported);
+
+ linkmode_or(accum_supported, accum_supported, tmp_supported);
+ linkmode_or(accum_advertising, accum_advertising,
+ tmp_state.advertising);
+ }
+}
+
static int phylink_validate_mask(struct phylink *pl, unsigned long *supported,
struct phylink_link_state *state,
const unsigned long *interfaces)
{
__ETHTOOL_DECLARE_LINK_MODE_MASK(all_adv) = { 0, };
__ETHTOOL_DECLARE_LINK_MODE_MASK(all_s) = { 0, };
- __ETHTOOL_DECLARE_LINK_MODE_MASK(s);
- struct phylink_link_state t;
int interface;
- for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX) {
- linkmode_copy(s, supported);
-
- t = *state;
- t.interface = interface;
- if (!phylink_validate_mac_and_pcs(pl, s, &t)) {
- linkmode_or(all_s, all_s, s);
- linkmode_or(all_adv, all_adv, t.advertising);
- }
- }
+ for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX)
+ phylink_validate_one(pl, supported, state, interface,
+ all_s, all_adv);
linkmode_copy(supported, all_s);
linkmode_copy(state->advertising, all_adv);
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC net-next 07/10] net: phylink: pass PHY into phylink_validate_one()
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
` (5 preceding siblings ...)
2023-11-22 15:31 ` [PATCH RFC net-next 06/10] net: phylink: split out per-interface validation Russell King (Oracle)
@ 2023-11-22 15:31 ` Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 08/10] net: phylink: pass PHY into phylink_validate_mask() Russell King (Oracle)
` (4 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:31 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
Pass the phy (if any) into phylink_validate_one() so that we can
validate each interface with its rate matching setting.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phylink.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 11dd743141d5..52414af5c93f 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -689,7 +689,7 @@ static int phylink_validate_mac_and_pcs(struct phylink *pl,
return phylink_is_empty_linkmode(supported) ? -EINVAL : 0;
}
-static void phylink_validate_one(struct phylink *pl,
+static void phylink_validate_one(struct phylink *pl, struct phy_device *phy,
const unsigned long *supported,
const struct phylink_link_state *state,
phy_interface_t interface,
@@ -704,6 +704,9 @@ static void phylink_validate_one(struct phylink *pl,
tmp_state = *state;
tmp_state.interface = interface;
+ if (phy)
+ tmp_state.rate_matching = phy_get_rate_matching(phy, interface);
+
if (!phylink_validate_mac_and_pcs(pl, tmp_supported, &tmp_state)) {
phylink_dbg(pl, " interface %u (%s) rate match %s supports %*pbl\n",
interface, phy_modes(interface),
@@ -725,7 +728,7 @@ static int phylink_validate_mask(struct phylink *pl, unsigned long *supported,
int interface;
for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX)
- phylink_validate_one(pl, supported, state, interface,
+ phylink_validate_one(pl, NULL, supported, state, interface,
all_s, all_adv);
linkmode_copy(supported, all_s);
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC net-next 08/10] net: phylink: pass PHY into phylink_validate_mask()
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
` (6 preceding siblings ...)
2023-11-22 15:31 ` [PATCH RFC net-next 07/10] net: phylink: pass PHY into phylink_validate_one() Russell King (Oracle)
@ 2023-11-22 15:31 ` Russell King (Oracle)
2023-11-22 15:32 ` [PATCH RFC net-next 09/10] net: phylink: split out PHY validation from phylink_bringup_phy() Russell King (Oracle)
` (3 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:31 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
Pass the phy (if any) into phylink_validate_mask() so that we can
validate each interface with its rate matching setting.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phylink.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 52414af5c93f..ac48a1db9979 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -719,7 +719,8 @@ static void phylink_validate_one(struct phylink *pl, struct phy_device *phy,
}
}
-static int phylink_validate_mask(struct phylink *pl, unsigned long *supported,
+static int phylink_validate_mask(struct phylink *pl, struct phy_device *phy,
+ unsigned long *supported,
struct phylink_link_state *state,
const unsigned long *interfaces)
{
@@ -728,7 +729,7 @@ static int phylink_validate_mask(struct phylink *pl, unsigned long *supported,
int interface;
for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX)
- phylink_validate_one(pl, NULL, supported, state, interface,
+ phylink_validate_one(pl, phy, supported, state, interface,
all_s, all_adv);
linkmode_copy(supported, all_s);
@@ -743,7 +744,8 @@ static int phylink_validate(struct phylink *pl, unsigned long *supported,
const unsigned long *interfaces = pl->config->supported_interfaces;
if (state->interface == PHY_INTERFACE_MODE_NA)
- return phylink_validate_mask(pl, supported, state, interfaces);
+ return phylink_validate_mask(pl, NULL, supported, state,
+ interfaces);
if (!test_bit(state->interface, interfaces))
return -EINVAL;
@@ -3179,7 +3181,8 @@ static int phylink_sfp_config_optical(struct phylink *pl)
/* For all the interfaces that are supported, reduce the sfp_support
* mask to only those link modes that can be supported.
*/
- ret = phylink_validate_mask(pl, pl->sfp_support, &config, interfaces);
+ ret = phylink_validate_mask(pl, NULL, pl->sfp_support, &config,
+ interfaces);
if (ret) {
phylink_err(pl, "unsupported SFP module: validation with support %*pb failed\n",
__ETHTOOL_LINK_MODE_MASK_NBITS, support);
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC net-next 09/10] net: phylink: split out PHY validation from phylink_bringup_phy()
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
` (7 preceding siblings ...)
2023-11-22 15:31 ` [PATCH RFC net-next 08/10] net: phylink: pass PHY into phylink_validate_mask() Russell King (Oracle)
@ 2023-11-22 15:32 ` Russell King (Oracle)
2023-11-22 15:32 ` [PATCH RFC net-next 10/10] net: phylink: use the PHY's possible_interfaces if populated Russell King (Oracle)
` (2 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:32 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
When bringing up a PHY, we need to work out which ethtool link modes it
should support and advertise. Clause 22 PHYs operate in a single
interface mode, which can be easily dealt with. However, clause 45 PHYs
tend to switch interface mode depending on the media. We need more
flexible validation at this point, so this patch splits out that code
in preparation to changing it.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phylink.c | 56 ++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 25 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index ac48a1db9979..39d85e47422e 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1760,6 +1760,35 @@ static void phylink_phy_change(struct phy_device *phydev, bool up)
phylink_pause_to_str(pl->phy_state.pause));
}
+static int phylink_validate_phy(struct phylink *pl, struct phy_device *phy,
+ unsigned long *supported,
+ struct phylink_link_state *state)
+{
+ /* Check whether we would use rate matching for the proposed interface
+ * mode.
+ */
+ state->rate_matching = phy_get_rate_matching(phy, state->interface);
+
+ /* Clause 45 PHYs may switch their Serdes lane between, e.g. 10GBASE-R,
+ * 5GBASE-R, 2500BASE-X and SGMII if they are not using rate matching.
+ * For some interface modes (e.g. RXAUI, XAUI and USXGMII) switching
+ * their Serdes is either unnecessary or not reasonable.
+ *
+ * For these which switch interface modes, we really need to know which
+ * interface modes the PHY supports to properly work out which ethtool
+ * linkmodes can be supported. For now, as a work-around, we validate
+ * against all interface modes, which may lead to more ethtool link
+ * modes being advertised than are actually supported.
+ */
+ if (phy->is_c45 && state->rate_matching == RATE_MATCH_NONE &&
+ state->interface != PHY_INTERFACE_MODE_RXAUI &&
+ state->interface != PHY_INTERFACE_MODE_XAUI &&
+ state->interface != PHY_INTERFACE_MODE_USXGMII)
+ state->interface = PHY_INTERFACE_MODE_NA;
+
+ return phylink_validate(pl, supported, state);
+}
+
static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
phy_interface_t interface)
{
@@ -1780,32 +1809,9 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
memset(&config, 0, sizeof(config));
linkmode_copy(supported, phy->supported);
linkmode_copy(config.advertising, phy->advertising);
+ config.interface = interface;
- /* Check whether we would use rate matching for the proposed interface
- * mode.
- */
- config.rate_matching = phy_get_rate_matching(phy, interface);
-
- /* Clause 45 PHYs may switch their Serdes lane between, e.g. 10GBASE-R,
- * 5GBASE-R, 2500BASE-X and SGMII if they are not using rate matching.
- * For some interface modes (e.g. RXAUI, XAUI and USXGMII) switching
- * their Serdes is either unnecessary or not reasonable.
- *
- * For these which switch interface modes, we really need to know which
- * interface modes the PHY supports to properly work out which ethtool
- * linkmodes can be supported. For now, as a work-around, we validate
- * against all interface modes, which may lead to more ethtool link
- * modes being advertised than are actually supported.
- */
- if (phy->is_c45 && config.rate_matching == RATE_MATCH_NONE &&
- interface != PHY_INTERFACE_MODE_RXAUI &&
- interface != PHY_INTERFACE_MODE_XAUI &&
- interface != PHY_INTERFACE_MODE_USXGMII)
- config.interface = PHY_INTERFACE_MODE_NA;
- else
- config.interface = interface;
-
- ret = phylink_validate(pl, supported, &config);
+ ret = phylink_validate_phy(pl, phy, supported, &config);
if (ret) {
phylink_warn(pl, "validation of %s with support %*pb and advertisement %*pb failed: %pe\n",
phy_modes(config.interface),
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC net-next 10/10] net: phylink: use the PHY's possible_interfaces if populated
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
` (8 preceding siblings ...)
2023-11-22 15:32 ` [PATCH RFC net-next 09/10] net: phylink: split out PHY validation from phylink_bringup_phy() Russell King (Oracle)
@ 2023-11-22 15:32 ` Russell King (Oracle)
2023-11-24 10:42 ` [PATCH RFC 00/10] net: phylink: improve PHY validation Jie Luo
2023-11-24 12:25 ` [PATCH net-next " Russell King (Oracle)
11 siblings, 0 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-22 15:32 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
Some PHYs such as Aquantia, Broadcom 84881, and Marvell 88X33x0 can
switch between a set of interface types depending on the negotiated
media speed, or can use rate adaption for some or all of these
interface types.
We currently assume that these are Clause 45 PHYs that are configured
not to use a specific set of interface modes, which has worked so far,
but is just a work-around. In this workaround, we validate using all
interfaces that the MAC supports, which can lead to extra modes being
advertised that can not be supported.
To properly address this, switch to using the newly introduced PHY
possible_interfaces bitmap which indicates which interface modes will
be used by the PHY as configured. We calculate the union of the PHY's
possible interfaces and MACs supported interfaces, checking that is
non-empty. If the PHY is on a SFP, we further reduce the set by those
which can be used on a SFP module, again checking that is non-empty.
Finally, we validate the subset of interfaces, taking account of
whether rate matching will be used for each individual interface mode.
This becomes independent of whether the PHY is clause 22 or clause 45.
It is encouraged that all PHYs that switch interface modes or use
rate matching should populate phydev->possible_interfaces.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phylink.c | 67 +++++++++++++++++++++++++++++++--------
1 file changed, 54 insertions(+), 13 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 39d85e47422e..48d3bd3e9fc7 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -121,6 +121,19 @@ do { \
})
#endif
+static const phy_interface_t phylink_sfp_interface_preference[] = {
+ PHY_INTERFACE_MODE_25GBASER,
+ PHY_INTERFACE_MODE_USXGMII,
+ PHY_INTERFACE_MODE_10GBASER,
+ PHY_INTERFACE_MODE_5GBASER,
+ PHY_INTERFACE_MODE_2500BASEX,
+ PHY_INTERFACE_MODE_SGMII,
+ PHY_INTERFACE_MODE_1000BASEX,
+ PHY_INTERFACE_MODE_100BASEX,
+};
+
+static DECLARE_PHY_INTERFACE_MASK(phylink_sfp_interfaces);
+
/**
* phylink_set_port_modes() - set the port type modes in the ethtool mask
* @mask: ethtool link mode mask
@@ -1764,6 +1777,47 @@ static int phylink_validate_phy(struct phylink *pl, struct phy_device *phy,
unsigned long *supported,
struct phylink_link_state *state)
{
+ DECLARE_PHY_INTERFACE_MASK(interfaces);
+
+ /* If the PHY provides a bitmap of the interfaces it will be using
+ * depending on the negotiated media speeds, use this to validate
+ * which ethtool link modes can be used.
+ */
+ if (!phy_interface_empty(phy->possible_interfaces)) {
+ /* We only care about the union of the PHY's interfaces and
+ * those which the host supports.
+ */
+ phy_interface_and(interfaces, phy->possible_interfaces,
+ pl->config->supported_interfaces);
+
+ if (phy_interface_empty(interfaces)) {
+ phylink_err(pl, "PHY has no common interfaces\n");
+ return -EINVAL;
+ }
+
+ if (phy_on_sfp(phy)) {
+ /* If the PHY is on a SFP, limit the interfaces to
+ * those that can be used with a SFP module.
+ */
+ phy_interface_and(interfaces, interfaces,
+ phylink_sfp_interfaces);
+
+ if (phy_interface_empty(interfaces)) {
+ phylink_err(pl, "SFP PHY's possible interfaces becomes empty\n");
+ return -EINVAL;
+ }
+ }
+
+ phylink_dbg(pl, "PHY %s uses interfaces %*pbl, validating %*pbl\n",
+ phydev_name(phy),
+ (int)PHY_INTERFACE_MODE_MAX,
+ phy->possible_interfaces,
+ (int)PHY_INTERFACE_MODE_MAX, interfaces);
+
+ return phylink_validate_mask(pl, phy, supported, state,
+ interfaces);
+ }
+
/* Check whether we would use rate matching for the proposed interface
* mode.
*/
@@ -3032,19 +3086,6 @@ static void phylink_sfp_detach(void *upstream, struct sfp_bus *bus)
pl->netdev->sfp_bus = NULL;
}
-static const phy_interface_t phylink_sfp_interface_preference[] = {
- PHY_INTERFACE_MODE_25GBASER,
- PHY_INTERFACE_MODE_USXGMII,
- PHY_INTERFACE_MODE_10GBASER,
- PHY_INTERFACE_MODE_5GBASER,
- PHY_INTERFACE_MODE_2500BASEX,
- PHY_INTERFACE_MODE_SGMII,
- PHY_INTERFACE_MODE_1000BASEX,
- PHY_INTERFACE_MODE_100BASEX,
-};
-
-static DECLARE_PHY_INTERFACE_MASK(phylink_sfp_interfaces);
-
static phy_interface_t phylink_choose_sfp_interface(struct phylink *pl,
const unsigned long *intf)
{
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH RFC net-next 04/10] net: phy: bcm84881: fill in possible_interfaces
2023-11-22 15:31 ` [PATCH RFC net-next 04/10] net: phy: bcm84881: " Russell King (Oracle)
@ 2023-11-22 22:11 ` Florian Fainelli
0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2023-11-22 22:11 UTC (permalink / raw)
To: Russell King (Oracle), Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Marek Behún, netdev,
Paolo Abeni
[-- Attachment #1: Type: text/plain, Size: 312 bytes --]
On 11/22/2023 7:31 AM, Russell King (Oracle) wrote:
> Fill in the possible_interfaces member. This PHY driver only supports
> a single configuration found on SFPs.
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
--
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC 00/10] net: phylink: improve PHY validation
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
` (9 preceding siblings ...)
2023-11-22 15:32 ` [PATCH RFC net-next 10/10] net: phylink: use the PHY's possible_interfaces if populated Russell King (Oracle)
@ 2023-11-24 10:42 ` Jie Luo
2023-11-24 12:25 ` [PATCH net-next " Russell King (Oracle)
11 siblings, 0 replies; 14+ messages in thread
From: Jie Luo @ 2023-11-24 10:42 UTC (permalink / raw)
To: Russell King (Oracle), Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
On 11/22/2023 11:30 PM, Russell King (Oracle) wrote:
> Hi,
>
> One of the issues which has concerned me about the rate matching
> implenentation that we have is that phy_get_rate_matching() returns
> whether rate matching will be used for a particular interface, and we
> enquire only for one interface.
>
> Aquantia PHYs can be programmed with the rate matching and interface
> mode settings on a per-media speed basis using the per-speed vendor 1
> global configuration registers.
>
> Thus, it is possible for the PHY to be configured to use rate matching
> for 10G, 5G, 2.5G with 10GBASE-R, and then SGMII for the remaining
> speeds. Therefore, it clearly doesn't make sense to enquire about rate
> matching for just one interface mode.
>
> Also, PHYs that change their interfaces are handled sub-optimally, in
> that we validate all the interface modes that the host supports, rather
> than the interface modes that the PHY will use.
>
> This patch series changes the way we validate PHYs, but in order to do
> so, we need to know exactly which interface modes will be used by the
> PHY. So that phylib can convey this information, we add
> "possible_interfaces" to struct phy_device.
>
> possible_interfaces is to be filled in by a phylib driver once the PHY
> is configured (in other words in the PHYs .config_init method) with the
> interface modes that it will switch between. This then allows users of
> phylib to know which interface modes will be used by the PHY.
>
> This allows us to solve both these issues: where possible_interfaces is
> provided, we can validate which ethtool link modes can be supported by
> looking at which interface modes that both the PHY and host support,
> and request rate matching information for each mode.
>
> This should improve the accuracy of the validation.
>
> drivers/net/phy/aquantia/aquantia.h | 5 +
> drivers/net/phy/aquantia/aquantia_main.c | 76 +++++++++++-
> drivers/net/phy/bcm84881.c | 12 ++
> drivers/net/phy/marvell10g.c | 203 ++++++++++++++++++++-----------
> drivers/net/phy/phy_device.c | 2 +
> drivers/net/phy/phylink.c | 177 +++++++++++++++++++--------
> include/linux/phy.h | 3 +
> 7 files changed, 353 insertions(+), 125 deletions(-)
Tested on qca808x PHY, the interface mode switched between sgmii and
2500base-x.
Tested-by: Luo Jie <quic_luoj@quicinc.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net-next 00/10] net: phylink: improve PHY validation
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
` (10 preceding siblings ...)
2023-11-24 10:42 ` [PATCH RFC 00/10] net: phylink: improve PHY validation Jie Luo
@ 2023-11-24 12:25 ` Russell King (Oracle)
11 siblings, 0 replies; 14+ messages in thread
From: Russell King (Oracle) @ 2023-11-24 12:25 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Florian Fainelli, Jakub Kicinski, Marek Behún,
netdev, Paolo Abeni
Hi,
One of the issues which has concerned me about the rate matching
implenentation that we have is that phy_get_rate_matching() returns
whether rate matching will be used for a particular interface, and we
enquire only for one interface.
Aquantia PHYs can be programmed with the rate matching and interface
mode settings on a per-media speed basis using the per-speed vendor 1
global configuration registers.
Thus, it is possible for the PHY to be configured to use rate matching
for 10G, 5G, 2.5G with 10GBASE-R, and then SGMII for the remaining
speeds. Therefore, it clearly doesn't make sense to enquire about rate
matching for just one interface mode.
Also, PHYs that change their interfaces are handled sub-optimally, in
that we validate all the interface modes that the host supports, rather
than the interface modes that the PHY will use.
This patch series changes the way we validate PHYs, but in order to do
so, we need to know exactly which interface modes will be used by the
PHY. So that phylib can convey this information, we add
"possible_interfaces" to struct phy_device.
possible_interfaces is to be filled in by a phylib driver once the PHY
is configured (in other words in the PHYs .config_init method) with the
interface modes that it will switch between. This then allows users of
phylib to know which interface modes will be used by the PHY.
This allows us to solve both these issues: where possible_interfaces is
provided, we can validate which ethtool link modes can be supported by
looking at which interface modes that both the PHY and host support,
and request rate matching information for each mode.
This should improve the accuracy of the validation.
Sending this out again without RFC as Jie Luo will need it for the
QCA8084 changes. No changes except to add the attributations already
received. Thanks!
drivers/net/phy/aquantia/aquantia.h | 5 +
drivers/net/phy/aquantia/aquantia_main.c | 76 +++++++++++-
drivers/net/phy/bcm84881.c | 12 ++
drivers/net/phy/marvell10g.c | 203 ++++++++++++++++++++-----------
drivers/net/phy/phy_device.c | 2 +
drivers/net/phy/phylink.c | 177 +++++++++++++++++++--------
include/linux/phy.h | 3 +
7 files changed, 353 insertions(+), 125 deletions(-)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-11-24 12:26 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-22 15:30 [PATCH RFC 00/10] net: phylink: improve PHY validation Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 01/10] net: phy: add possible interfaces Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 02/10] net: phy: marvell10g: table driven mactype decode Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 03/10] net: phy: marvell10g: fill in possible_interfaces Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 04/10] net: phy: bcm84881: " Russell King (Oracle)
2023-11-22 22:11 ` Florian Fainelli
2023-11-22 15:31 ` [PATCH RFC net-next 05/10] net: phy: aquantia: fill in possible_interfaces for AQR113C Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 06/10] net: phylink: split out per-interface validation Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 07/10] net: phylink: pass PHY into phylink_validate_one() Russell King (Oracle)
2023-11-22 15:31 ` [PATCH RFC net-next 08/10] net: phylink: pass PHY into phylink_validate_mask() Russell King (Oracle)
2023-11-22 15:32 ` [PATCH RFC net-next 09/10] net: phylink: split out PHY validation from phylink_bringup_phy() Russell King (Oracle)
2023-11-22 15:32 ` [PATCH RFC net-next 10/10] net: phylink: use the PHY's possible_interfaces if populated Russell King (Oracle)
2023-11-24 10:42 ` [PATCH RFC 00/10] net: phylink: improve PHY validation Jie Luo
2023-11-24 12:25 ` [PATCH net-next " Russell King (Oracle)
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).