| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20211214233450.1488736-1-sean.anderson@seco.com>
Date: Tue, 14 Dec 2021 18:34:50 -0500
From: Sean Anderson <sean.anderson@...o.com>
To: Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>
Cc: Marcin Wojtas <mw@...ihalf.com>, UNGLinuxDriver@...rochip.com,
"David S . Miller" <davem@...emloft.net>,
Claudiu Beznea <claudiu.beznea@...rochip.com>,
Jakub Kicinski <kuba@...nel.org>,
Horatiu Vultur <horatiu.vultur@...rochip.com>,
Jose Abreu <Jose.Abreu@...opsys.com>,
Russell King <rmk+kernel@...linux.org.uk>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Ioana Ciornei <ioana.ciornei@....com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, Russell King <linux@...linux.org.uk>,
Sean Anderson <sean.anderson@...o.com>
Subject: [PATCH] net: phylink: Pass state to pcs_config
Although most PCSs only need the interface and advertising to configure
themselves, there is an oddly named "permit_pause_to_mac" parameter
included as well, and only used by mvpp2. This parameter indicates
whether pause settings should be autonegotiated or not. mvpp2 needs this
because it cannot both set the pause mode manually and and advertise
pause support. That is, if you want to set the pause mode, you have to
advertise that you don't support flow control. We can't just
autonegotiate the pause mode and then set it manually, because if
the link goes down we will start advertising the wrong thing. So
instead, we have to set it up front during pcs_config. However, we can't
determine whether we are autonegotiating flow control based on our
advertisement (since we advertise flow control even when it is set
manually).
So we have had this strange additional argument tagging along which is
used by one driver (though soon to be one more since mvneta has the same
problem). We could stick MLO_PAUSE_AN in the "mode" parameter, since
that contains other autonegotiation configuration. However, there are a
lot of places in the codebase which do a direct comparison (e.g. mode ==
MLO_AN_FIXED), so it would be difficult to add an extra bit without
breaking things. But this whole time, mac_config has been getting the
whole state, and it has not suffered unduly. So just pass state and
eliminate these other parameters.
Signed-off-by: Sean Anderson <sean.anderson@...o.com>
---
drivers/net/ethernet/cadence/macb_main.c | 8 ++----
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 28 +++++++++----------
.../microchip/lan966x/lan966x_phylink.c | 10 +++----
.../microchip/sparx5/sparx5_phylink.c | 16 +++++------
drivers/net/pcs/pcs-lynx.c | 15 +++++-----
drivers/net/pcs/pcs-xpcs.c | 6 ++--
drivers/net/phy/phylink.c | 9 ++----
include/linux/phylink.h | 14 +++-------
8 files changed, 42 insertions(+), 64 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index d4da9adf6777..2ae717f262e8 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -540,9 +540,7 @@ static void macb_usx_pcs_get_state(struct phylink_pcs *pcs,
static int macb_usx_pcs_config(struct phylink_pcs *pcs,
unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising,
- bool permit_pause_to_mac)
+ const struct phylink_link_state *state)
{
struct macb *bp = container_of(pcs, struct macb, phylink_pcs);
@@ -565,9 +563,7 @@ static void macb_pcs_an_restart(struct phylink_pcs *pcs)
static int macb_pcs_config(struct phylink_pcs *pcs,
unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising,
- bool permit_pause_to_mac)
+ const struct phylink_link_state *state)
{
return 0;
}
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index b1cce4425296..62de173536bf 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -6146,9 +6146,7 @@ static void mvpp2_xlg_pcs_get_state(struct phylink_pcs *pcs,
static int mvpp2_xlg_pcs_config(struct phylink_pcs *pcs,
unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising,
- bool permit_pause_to_mac)
+ const struct phylink_link_state *state)
{
return 0;
}
@@ -6194,9 +6192,7 @@ static void mvpp2_gmac_pcs_get_state(struct phylink_pcs *pcs,
}
static int mvpp2_gmac_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising,
- bool permit_pause_to_mac)
+ const struct phylink_link_state *state)
{
struct mvpp2_port *port = mvpp2_pcs_to_port(pcs);
u32 mask, val, an, old_an, changed;
@@ -6213,7 +6209,7 @@ static int mvpp2_gmac_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
MVPP2_GMAC_CONFIG_FULL_DUPLEX;
val = MVPP2_GMAC_IN_BAND_AUTONEG;
- if (interface == PHY_INTERFACE_MODE_SGMII) {
+ if (state->interface == PHY_INTERFACE_MODE_SGMII) {
/* SGMII mode receives the speed and duplex from PHY */
val |= MVPP2_GMAC_AN_SPEED_EN |
MVPP2_GMAC_AN_DUPLEX_EN;
@@ -6222,18 +6218,21 @@ static int mvpp2_gmac_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
val |= MVPP2_GMAC_CONFIG_GMII_SPEED |
MVPP2_GMAC_CONFIG_FULL_DUPLEX;
- /* The FLOW_CTRL_AUTONEG bit selects either the hardware
- * automatically or the bits in MVPP22_GMAC_CTRL_4_REG
- * manually controls the GMAC pause modes.
+ /* The FLOW_CTRL_AUTONEG bit selects whether flow
+ * control should come from autonegotiation or whether
+ * it should be manually configured (by
+ * MVPP22_GMAC_CTRL_4_REG). If it is cleared (to select
+ * manual configuration) then flow control
+ * advertisement will be ignored.
*/
- if (permit_pause_to_mac)
+ if (state->pause & MLO_PAUSE_AN)
val |= MVPP2_GMAC_FLOW_CTRL_AUTONEG;
/* Configure advertisement bits */
mask |= MVPP2_GMAC_FC_ADV_EN | MVPP2_GMAC_FC_ADV_ASM_EN;
- if (phylink_test(advertising, Pause))
+ if (phylink_test(state->advertising, Pause))
val |= MVPP2_GMAC_FC_ADV_EN;
- if (phylink_test(advertising, Asym_Pause))
+ if (phylink_test(state->advertising, Asym_Pause))
val |= MVPP2_GMAC_FC_ADV_ASM_EN;
}
} else {
@@ -6632,8 +6631,7 @@ static void mvpp2_acpi_start(struct mvpp2_port *port)
port->phy_interface);
mvpp2_mac_config(&port->phylink_config, MLO_AN_INBAND, &state);
port->phylink_pcs.ops->pcs_config(&port->phylink_pcs, MLO_AN_INBAND,
- port->phy_interface,
- state.advertising, false);
+ &state);
mvpp2_mac_finish(&port->phylink_config, MLO_AN_INBAND,
port->phy_interface);
mvpp2_mac_link_up(&port->phylink_config, NULL,
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c b/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c
index b66a9aa00ea4..dc3dda2fad14 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_phylink.c
@@ -86,19 +86,17 @@ static void lan966x_pcs_get_state(struct phylink_pcs *pcs,
static int lan966x_pcs_config(struct phylink_pcs *pcs,
unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising,
- bool permit_pause_to_mac)
+ const struct phylink_link_state *state)
{
struct lan966x_port *port = lan966x_pcs_to_port(pcs);
struct lan966x_port_config config;
int ret;
config = port->config;
- config.portmode = interface;
+ config.portmode = state->interface;
config.inband = phylink_autoneg_inband(mode);
- config.autoneg = phylink_test(advertising, Autoneg);
- config.advertising = advertising;
+ config.autoneg = phylink_test(state->advertising, Autoneg);
+ config.advertising = state->advertising;
ret = lan966x_port_pcs_set(port, &config);
if (ret)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
index 8ba33bc1a001..07500502d5be 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_phylink.c
@@ -84,9 +84,7 @@ static void sparx5_pcs_get_state(struct phylink_pcs *pcs,
static int sparx5_pcs_config(struct phylink_pcs *pcs,
unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising,
- bool permit_pause_to_mac)
+ const struct phylink_link_state *state)
{
struct sparx5_port *port = sparx5_pcs_to_port(pcs);
struct sparx5_port_config conf;
@@ -94,16 +92,16 @@ static int sparx5_pcs_config(struct phylink_pcs *pcs,
conf = port->conf;
conf.power_down = false;
- conf.portmode = interface;
+ conf.portmode = state->interface;
conf.inband = phylink_autoneg_inband(mode);
- conf.autoneg = phylink_test(advertising, Autoneg);
+ conf.autoneg = phylink_test(state->advertising, Autoneg);
conf.pause_adv = 0;
- if (phylink_test(advertising, Pause))
+ if (phylink_test(state->advertising, Pause))
conf.pause_adv |= ADVERTISE_1000XPAUSE;
- if (phylink_test(advertising, Asym_Pause))
+ if (phylink_test(state->advertising, Asym_Pause))
conf.pause_adv |= ADVERTISE_1000XPSE_ASYM;
- if (sparx5_is_baser(interface)) {
- if (phylink_test(advertising, FIBRE))
+ if (sparx5_is_baser(state->interface)) {
+ if (phylink_test(state->advertising, FIBRE))
conf.media = PHY_MEDIA_SR;
else
conf.media = PHY_MEDIA_DAC;
diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
index af36cd647bf5..7b5351885b4d 100644
--- a/drivers/net/pcs/pcs-lynx.c
+++ b/drivers/net/pcs/pcs-lynx.c
@@ -182,18 +182,18 @@ static int lynx_pcs_config_usxgmii(struct mdio_device *pcs, unsigned int mode,
}
static int lynx_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
- phy_interface_t ifmode,
- const unsigned long *advertising,
- bool permit)
+ const struct phylink_link_state *state)
{
struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
- switch (ifmode) {
+ switch (state->interface) {
case PHY_INTERFACE_MODE_1000BASEX:
- return lynx_pcs_config_1000basex(lynx->mdio, mode, advertising);
+ return lynx_pcs_config_1000basex(lynx->mdio, mode,
+ state->advertising);
case PHY_INTERFACE_MODE_SGMII:
case PHY_INTERFACE_MODE_QSGMII:
- return lynx_pcs_config_sgmii(lynx->mdio, mode, advertising);
+ return lynx_pcs_config_sgmii(lynx->mdio, mode,
+ state->advertising);
case PHY_INTERFACE_MODE_2500BASEX:
if (phylink_autoneg_inband(mode)) {
dev_err(&lynx->mdio->dev,
@@ -202,7 +202,8 @@ static int lynx_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
}
break;
case PHY_INTERFACE_MODE_USXGMII:
- return lynx_pcs_config_usxgmii(lynx->mdio, mode, advertising);
+ return lynx_pcs_config_usxgmii(lynx->mdio, mode,
+ state->advertising);
case PHY_INTERFACE_MODE_10GBASER:
/* Nothing to do here for 10GBASER */
break;
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index cd6742e6ba8b..5eceb84573cb 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -831,13 +831,11 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
EXPORT_SYMBOL_GPL(xpcs_do_config);
static int xpcs_config(struct phylink_pcs *pcs, unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising,
- bool permit_pause_to_mac)
+ const struct phylink_link_state *state)
{
struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
- return xpcs_do_config(xpcs, interface, mode);
+ return xpcs_do_config(xpcs, state->interface, mode);
}
static int xpcs_get_state_c73(struct dw_xpcs *xpcs,
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 20df8af3e201..d47570365b93 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -768,10 +768,7 @@ static void phylink_major_config(struct phylink *pl, bool restart,
if (pl->pcs_ops) {
err = pl->pcs_ops->pcs_config(pl->pcs, pl->cur_link_an_mode,
- state->interface,
- state->advertising,
- !!(pl->link_config.pause &
- MLO_PAUSE_AN));
+ state);
if (err < 0)
phylink_err(pl, "pcs_config failed: %pe\n",
ERR_PTR(err));
@@ -821,9 +818,7 @@ static int phylink_change_inband_advert(struct phylink *pl)
* the programmed advertisement has changed.
*/
ret = pl->pcs_ops->pcs_config(pl->pcs, pl->cur_link_an_mode,
- pl->link_config.interface,
- pl->link_config.advertising,
- !!(pl->link_config.pause & MLO_PAUSE_AN));
+ &pl->link_config);
if (ret < 0)
return ret;
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index a2f266cc3442..d3d09c064596 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -408,9 +408,7 @@ struct phylink_pcs_ops {
void (*pcs_get_state)(struct phylink_pcs *pcs,
struct phylink_link_state *state);
int (*pcs_config)(struct phylink_pcs *pcs, unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising,
- bool permit_pause_to_mac);
+ const struct phylink_link_state *state);
void (*pcs_an_restart)(struct phylink_pcs *pcs);
void (*pcs_link_up)(struct phylink_pcs *pcs, unsigned int mode,
phy_interface_t interface, int speed, int duplex);
@@ -439,13 +437,10 @@ void pcs_get_state(struct phylink_pcs *pcs,
* pcs_config() - Configure the PCS mode and advertisement
* @pcs: a pointer to a &struct phylink_pcs.
* @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
- * @interface: interface mode to be used
- * @advertising: adertisement ethtool link mode mask
- * @permit_pause_to_mac: permit forwarding pause resolution to MAC
+ * @state: the state to configure, containing the interface and advertisement
*
* Configure the PCS for the operating mode, the interface mode, and set
- * the advertisement mask. @permit_pause_to_mac indicates whether the
- * hardware may forward the pause mode resolution to the MAC.
+ * the advertisement mask.
*
* When operating in %MLO_AN_INBAND, inband should always be enabled,
* otherwise inband should be disabled.
@@ -458,8 +453,7 @@ void pcs_get_state(struct phylink_pcs *pcs,
* For most 10GBASE-R, there is no advertisement.
*/
int pcs_config(struct phylink_pcs *pcs, unsigned int mode,
- phy_interface_t interface, const unsigned long *advertising,
- bool permit_pause_to_mac);
+ const struct phylink_link_state *state);
/**
* pcs_an_restart() - restart 802.3z BaseX autonegotiation
--
2.25.1
Powered by blists - more mailing lists