[PATCH net-next v2 0/4] Marvell mvpp2 improvements

[PATCH net-next v2 0/4] Marvell mvpp2 improvements

[Russell King - ARM Linux admin]

Hi,

This series primarily cleans up mvpp2, but also fixes a left-over from
91a208f2185a ("net: phylink: propagate resolved link config via
mac_link_up()").

Patch 1 introduces some port helpers:
  mvpp2_port_supports_xlg() - does the port support the XLG MAC
  mvpp2_port_supports_rgmii() - does the port support RGMII modes

Patch 2 introduces mvpp2_phylink_to_port(), rather than having repeated
  open coding of container_of().

Patch 3 introduces mvpp2_modify(), which reads-modifies-writes a
  register - I've converted the phylink specific code to use this
  helper.

Patch 4 moves the hardware control of the pause modes from
  mvpp2_xlg_config() (which is called via the phylink_config method)
  to mvpp2_mac_link_up() - a change that was missed in the above
  referenced commit.

v2: remove "inline" in patch 2.

 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 163 +++++++++++++-----------
 1 file changed, 88 insertions(+), 75 deletions(-)

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

[PATCH net-next 1/4] net: mvpp2: add port support helpers

[Russell King]

The mvpp2 code has tests scattered amongst the code to determine
whether the port supports the XLG, and whether the port supports
RGMII mode.

Rather than having these tests scattered, provide a couple of helper
functions, so that future additions can ensure that they get these
tests correct.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 43 ++++++++++++-------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 24f4d8e0da98..7653277d03b7 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1114,6 +1114,17 @@ mvpp2_shared_interrupt_mask_unmask(struct mvpp2_port *port, bool mask)
 	}
 }
 
+/* Only GOP port 0 has an XLG MAC */
+static bool mvpp2_port_supports_xlg(struct mvpp2_port *port)
+{
+	return port->gop_id == 0;
+}
+
+static bool mvpp2_port_supports_rgmii(struct mvpp2_port *port)
+{
+	return !(port->priv->hw_version == MVPP22 && port->gop_id == 0);
+}
+
 /* Port configuration routines */
 static bool mvpp2_is_xlg(phy_interface_t interface)
 {
@@ -1194,7 +1205,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		if (port->gop_id == 0)
+		if (!mvpp2_port_supports_rgmii(port))
 			goto invalid_conf;
 		mvpp22_gop_init_rgmii(port);
 		break;
@@ -1204,7 +1215,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port)
 		mvpp22_gop_init_sgmii(port);
 		break;
 	case PHY_INTERFACE_MODE_10GBASER:
-		if (port->gop_id != 0)
+		if (!mvpp2_port_supports_xlg(port))
 			goto invalid_conf;
 		mvpp22_gop_init_10gkr(port);
 		break;
@@ -1246,7 +1257,7 @@ static void mvpp22_gop_unmask_irq(struct mvpp2_port *port)
 		writel(val, port->base + MVPP22_GMAC_INT_SUM_MASK);
 	}
 
-	if (port->gop_id == 0) {
+	if (mvpp2_port_supports_xlg(port)) {
 		/* Enable the XLG/GIG irqs for this port */
 		val = readl(port->base + MVPP22_XLG_EXT_INT_MASK);
 		if (mvpp2_is_xlg(port->phy_interface))
@@ -1261,7 +1272,7 @@ static void mvpp22_gop_mask_irq(struct mvpp2_port *port)
 {
 	u32 val;
 
-	if (port->gop_id == 0) {
+	if (mvpp2_port_supports_xlg(port)) {
 		val = readl(port->base + MVPP22_XLG_EXT_INT_MASK);
 		val &= ~(MVPP22_XLG_EXT_INT_MASK_XLG |
 			 MVPP22_XLG_EXT_INT_MASK_GIG);
@@ -1290,7 +1301,7 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
 		writel(val, port->base + MVPP22_GMAC_INT_MASK);
 	}
 
-	if (port->gop_id == 0) {
+	if (mvpp2_port_supports_xlg(port)) {
 		val = readl(port->base + MVPP22_XLG_INT_MASK);
 		val |= MVPP22_XLG_INT_MASK_LINK;
 		writel(val, port->base + MVPP22_XLG_INT_MASK);
@@ -1328,8 +1339,8 @@ static void mvpp2_port_enable(struct mvpp2_port *port)
 {
 	u32 val;
 
-	/* Only GOP port 0 has an XLG MAC */
-	if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface)) {
+	if (mvpp2_port_supports_xlg(port) &&
+	    mvpp2_is_xlg(port->phy_interface)) {
 		val = readl(port->base + MVPP22_XLG_CTRL0_REG);
 		val |= MVPP22_XLG_CTRL0_PORT_EN;
 		val &= ~MVPP22_XLG_CTRL0_MIB_CNT_DIS;
@@ -1346,8 +1357,8 @@ static void mvpp2_port_disable(struct mvpp2_port *port)
 {
 	u32 val;
 
-	/* Only GOP port 0 has an XLG MAC */
-	if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface)) {
+	if (mvpp2_port_supports_xlg(port) &&
+	    mvpp2_is_xlg(port->phy_interface)) {
 		val = readl(port->base + MVPP22_XLG_CTRL0_REG);
 		val &= ~MVPP22_XLG_CTRL0_PORT_EN;
 		writel(val, port->base + MVPP22_XLG_CTRL0_REG);
@@ -2740,7 +2751,8 @@ static irqreturn_t mvpp2_link_status_isr(int irq, void *dev_id)
 
 	mvpp22_gop_mask_irq(port);
 
-	if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface)) {
+	if (mvpp2_port_supports_xlg(port) &&
+	    mvpp2_is_xlg(port->phy_interface)) {
 		val = readl(port->base + MVPP22_XLG_INT_STAT);
 		if (val & MVPP22_XLG_INT_STAT_LINK) {
 			event = true;
@@ -3430,8 +3442,7 @@ static void mvpp22_mode_reconfigure(struct mvpp2_port *port)
 
 	mvpp22_pcs_reset_deassert(port);
 
-	/* Only GOP port 0 has an XLG MAC */
-	if (port->gop_id == 0) {
+	if (mvpp2_port_supports_xlg(port)) {
 		ctrl3 = readl(port->base + MVPP22_XLG_CTRL3_REG);
 		ctrl3 &= ~MVPP22_XLG_CTRL3_MACMODESELECT_MASK;
 
@@ -3443,7 +3454,7 @@ static void mvpp22_mode_reconfigure(struct mvpp2_port *port)
 		writel(ctrl3, port->base + MVPP22_XLG_CTRL3_REG);
 	}
 
-	if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface))
+	if (mvpp2_port_supports_xlg(port) && mvpp2_is_xlg(port->phy_interface))
 		mvpp2_xlg_max_rx_size_set(port);
 	else
 		mvpp2_gmac_max_rx_size_set(port);
@@ -4768,14 +4779,14 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
 	switch (state->interface) {
 	case PHY_INTERFACE_MODE_10GBASER:
 	case PHY_INTERFACE_MODE_XAUI:
-		if (port->gop_id != 0)
+		if (!mvpp2_port_supports_xlg(port))
 			goto empty_set;
 		break;
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		if (port->priv->hw_version == MVPP22 && port->gop_id == 0)
+		if (!mvpp2_port_supports_rgmii(port))
 			goto empty_set;
 		break;
 	default:
@@ -4791,7 +4802,7 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
 	case PHY_INTERFACE_MODE_10GBASER:
 	case PHY_INTERFACE_MODE_XAUI:
 	case PHY_INTERFACE_MODE_NA:
-		if (port->gop_id == 0) {
+		if (mvpp2_port_supports_xlg(port)) {
 			phylink_set(mask, 10000baseT_Full);
 			phylink_set(mask, 10000baseCR_Full);
 			phylink_set(mask, 10000baseSR_Full);
-- 
2.20.1

[PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Russell King]

Add a helper to convert the struct phylink_config pointer passed in
from phylink to the drivers internal struct mvpp2_port.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 7653277d03b7..313f5a60a605 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
 	eth_hw_addr_random(dev);
 }
 
+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+	return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
 				   unsigned long *supported,
 				   struct phylink_link_state *state)
 {
-	struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-					       phylink_config);
+	struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
 	/* Invalid combinations */
@@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
 static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 					    struct phylink_link_state *state)
 {
-	struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-					       phylink_config);
+	struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 
 	if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
 		u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
@@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
 
 static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-	struct mvpp2_port *port = container_of(config, struct mvpp2_port,
-					       phylink_config);
+	struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 	u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
 	writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
@@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
 			     const struct phylink_link_state *state)
 {
-	struct net_device *dev = to_net_dev(config->dev);
-	struct mvpp2_port *port = netdev_priv(dev);
+	struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 	bool change_interface = port->phy_interface != state->interface;
 
 	/* Check for invalid configuration */
 	if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-		netdev_err(dev, "Invalid mode on %s\n", dev->name);
+		netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
 		return;
 	}
 
@@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 			      int speed, int duplex,
 			      bool tx_pause, bool rx_pause)
 {
-	struct net_device *dev = to_net_dev(config->dev);
-	struct mvpp2_port *port = netdev_priv(dev);
+	struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 	u32 val;
 
 	if (mvpp2_is_xlg(interface)) {
@@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
 	mvpp2_egress_enable(port);
 	mvpp2_ingress_enable(port);
-	netif_tx_wake_all_queues(dev);
+	netif_tx_wake_all_queues(port->dev);
 }
 
 static void mvpp2_mac_link_down(struct phylink_config *config,
 				unsigned int mode, phy_interface_t interface)
 {
-	struct net_device *dev = to_net_dev(config->dev);
-	struct mvpp2_port *port = netdev_priv(dev);
+	struct mvpp2_port *port = mvpp2_phylink_to_port(config);
 	u32 val;
 
 	if (!phylink_autoneg_inband(mode)) {
@@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
 		}
 	}
 
-	netif_tx_stop_all_queues(dev);
+	netif_tx_stop_all_queues(port->dev);
 	mvpp2_egress_disable(port);
 	mvpp2_ingress_disable(port);
 
-- 
2.20.1

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Marcin Wojtas]

Hi,

sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
>
> Add a helper to convert the struct phylink_config pointer passed in
> from phylink to the drivers internal struct mvpp2_port.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
>  1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 7653277d03b7..313f5a60a605 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
>         eth_hw_addr_random(dev);
>  }
>
> +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> +{
> +       return container_of(config, struct mvpp2_port, phylink_config);
> +}
> +
>  static void mvpp2_phylink_validate(struct phylink_config *config,
>                                    unsigned long *supported,
>                                    struct phylink_link_state *state)
>  {
> -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> -                                              phylink_config);
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>         __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>
>         /* Invalid combinations */
> @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
>  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
>                                             struct phylink_link_state *state)
>  {
> -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> -                                              phylink_config);
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>
>         if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
>                 u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
>
>  static void mvpp2_mac_an_restart(struct phylink_config *config)
>  {
> -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> -                                              phylink_config);
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>         u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
>
>         writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
>  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
>                              const struct phylink_link_state *state)
>  {
> -       struct net_device *dev = to_net_dev(config->dev);
> -       struct mvpp2_port *port = netdev_priv(dev);
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>         bool change_interface = port->phy_interface != state->interface;
>
>         /* Check for invalid configuration */
>         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
>                 return;
>         }
>
> @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
>                               int speed, int duplex,
>                               bool tx_pause, bool rx_pause)
>  {
> -       struct net_device *dev = to_net_dev(config->dev);
> -       struct mvpp2_port *port = netdev_priv(dev);
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>         u32 val;
>
>         if (mvpp2_is_xlg(interface)) {
> @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
>
>         mvpp2_egress_enable(port);
>         mvpp2_ingress_enable(port);
> -       netif_tx_wake_all_queues(dev);
> +       netif_tx_wake_all_queues(port->dev);
>  }
>
>  static void mvpp2_mac_link_down(struct phylink_config *config,
>                                 unsigned int mode, phy_interface_t interface)
>  {
> -       struct net_device *dev = to_net_dev(config->dev);
> -       struct mvpp2_port *port = netdev_priv(dev);
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>         u32 val;
>
>         if (!phylink_autoneg_inband(mode)) {
> @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
>                 }
>         }
>
> -       netif_tx_stop_all_queues(dev);
> +       netif_tx_stop_all_queues(port->dev);
>         mvpp2_egress_disable(port);
>         mvpp2_ingress_disable(port);
>
> --
> 2.20.1
>

This patch fixes a regression that was introduced in v5.3:
Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")

Above results in a NULL pointer dereference when booting the
Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
problematic especially for the distros using LTSv5.4 and above (the
issue was reported on Fedora 32).

Please help with backporting to the stable v5.3+ branches (it applies
smoothly on v5.4/v5.6/v5.8).

Best regards,
Marcin

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Marcin Wojtas]

Hi Greg and Sasha,

pt., 9 paź 2020 o 05:43 Marcin Wojtas <mw@semihalf.com> napisał(a):
>
> Hi,
>
> sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
> >
> > Add a helper to convert the struct phylink_config pointer passed in
> > from phylink to the drivers internal struct mvpp2_port.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
> >  1 file changed, 14 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > index 7653277d03b7..313f5a60a605 100644
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> >         eth_hw_addr_random(dev);
> >  }
> >
> > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> > +{
> > +       return container_of(config, struct mvpp2_port, phylink_config);
> > +}
> > +
> >  static void mvpp2_phylink_validate(struct phylink_config *config,
> >                                    unsigned long *supported,
> >                                    struct phylink_link_state *state)
> >  {
> > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > -                                              phylink_config);
> > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >         __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> >
> >         /* Invalid combinations */
> > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
> >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> >                                             struct phylink_link_state *state)
> >  {
> > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > -                                              phylink_config);
> > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >
> >         if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
> >                 u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> > @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> >
> >  static void mvpp2_mac_an_restart(struct phylink_config *config)
> >  {
> > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > -                                              phylink_config);
> > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >         u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> >
> >         writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> > @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
> >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> >                              const struct phylink_link_state *state)
> >  {
> > -       struct net_device *dev = to_net_dev(config->dev);
> > -       struct mvpp2_port *port = netdev_priv(dev);
> > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >         bool change_interface = port->phy_interface != state->interface;
> >
> >         /* Check for invalid configuration */
> >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> >                 return;
> >         }
> >
> > @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> >                               int speed, int duplex,
> >                               bool tx_pause, bool rx_pause)
> >  {
> > -       struct net_device *dev = to_net_dev(config->dev);
> > -       struct mvpp2_port *port = netdev_priv(dev);
> > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >         u32 val;
> >
> >         if (mvpp2_is_xlg(interface)) {
> > @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> >
> >         mvpp2_egress_enable(port);
> >         mvpp2_ingress_enable(port);
> > -       netif_tx_wake_all_queues(dev);
> > +       netif_tx_wake_all_queues(port->dev);
> >  }
> >
> >  static void mvpp2_mac_link_down(struct phylink_config *config,
> >                                 unsigned int mode, phy_interface_t interface)
> >  {
> > -       struct net_device *dev = to_net_dev(config->dev);
> > -       struct mvpp2_port *port = netdev_priv(dev);
> > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >         u32 val;
> >
> >         if (!phylink_autoneg_inband(mode)) {
> > @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
> >                 }
> >         }
> >
> > -       netif_tx_stop_all_queues(dev);
> > +       netif_tx_stop_all_queues(port->dev);
> >         mvpp2_egress_disable(port);
> >         mvpp2_ingress_disable(port);
> >
> > --
> > 2.20.1
> >
>
> This patch fixes a regression that was introduced in v5.3:
> Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")
>
> Above results in a NULL pointer dereference when booting the
> Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
> problematic especially for the distros using LTSv5.4 and above (the
> issue was reported on Fedora 32).
>
> Please help with backporting to the stable v5.3+ branches (it applies
> smoothly on v5.4/v5.6/v5.8).
>

Any chances to backport this patch to relevant v5.3+ stable branches?

Best regards,
Marcin

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Greg Kroah-Hartman]

On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
> Hi Greg and Sasha,
> 
> pt., 9 paź 2020 o 05:43 Marcin Wojtas <mw@semihalf.com> napisał(a):
> >
> > Hi,
> >
> > sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
> > >
> > > Add a helper to convert the struct phylink_config pointer passed in
> > > from phylink to the drivers internal struct mvpp2_port.
> > >
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > ---
> > >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
> > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > index 7653277d03b7..313f5a60a605 100644
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> > >         eth_hw_addr_random(dev);
> > >  }
> > >
> > > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> > > +{
> > > +       return container_of(config, struct mvpp2_port, phylink_config);
> > > +}
> > > +
> > >  static void mvpp2_phylink_validate(struct phylink_config *config,
> > >                                    unsigned long *supported,
> > >                                    struct phylink_link_state *state)
> > >  {
> > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > -                                              phylink_config);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >         __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > >
> > >         /* Invalid combinations */
> > > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
> > >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > >                                             struct phylink_link_state *state)
> > >  {
> > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > -                                              phylink_config);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >
> > >         if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
> > >                 u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> > > @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > >
> > >  static void mvpp2_mac_an_restart(struct phylink_config *config)
> > >  {
> > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > -                                              phylink_config);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >         u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > >
> > >         writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> > > @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
> > >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> > >                              const struct phylink_link_state *state)
> > >  {
> > > -       struct net_device *dev = to_net_dev(config->dev);
> > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >         bool change_interface = port->phy_interface != state->interface;
> > >
> > >         /* Check for invalid configuration */
> > >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> > > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> > > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> > >                 return;
> > >         }
> > >
> > > @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > >                               int speed, int duplex,
> > >                               bool tx_pause, bool rx_pause)
> > >  {
> > > -       struct net_device *dev = to_net_dev(config->dev);
> > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >         u32 val;
> > >
> > >         if (mvpp2_is_xlg(interface)) {
> > > @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > >
> > >         mvpp2_egress_enable(port);
> > >         mvpp2_ingress_enable(port);
> > > -       netif_tx_wake_all_queues(dev);
> > > +       netif_tx_wake_all_queues(port->dev);
> > >  }
> > >
> > >  static void mvpp2_mac_link_down(struct phylink_config *config,
> > >                                 unsigned int mode, phy_interface_t interface)
> > >  {
> > > -       struct net_device *dev = to_net_dev(config->dev);
> > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >         u32 val;
> > >
> > >         if (!phylink_autoneg_inband(mode)) {
> > > @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
> > >                 }
> > >         }
> > >
> > > -       netif_tx_stop_all_queues(dev);
> > > +       netif_tx_stop_all_queues(port->dev);
> > >         mvpp2_egress_disable(port);
> > >         mvpp2_ingress_disable(port);
> > >
> > > --
> > > 2.20.1
> > >
> >
> > This patch fixes a regression that was introduced in v5.3:
> > Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")
> >
> > Above results in a NULL pointer dereference when booting the
> > Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
> > problematic especially for the distros using LTSv5.4 and above (the
> > issue was reported on Fedora 32).
> >
> > Please help with backporting to the stable v5.3+ branches (it applies
> > smoothly on v5.4/v5.6/v5.8).
> >
> 
> Any chances to backport this patch to relevant v5.3+ stable branches?

What patch?  What git commit id needs to be backported?

confused,

greg k-h

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Marcin Wojtas]

Hi Greg,

Apologies for delayed response:.


pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
<gregkh@linuxfoundation.org> napisał(a):
>
> On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
> > Hi Greg and Sasha,
> >
> > pt., 9 paź 2020 o 05:43 Marcin Wojtas <mw@semihalf.com> napisał(a):
> > >
> > > Hi,
> > >
> > > sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
> > > >
> > > > Add a helper to convert the struct phylink_config pointer passed in
> > > > from phylink to the drivers internal struct mvpp2_port.
> > > >
> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > > ---
> > > >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
> > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > index 7653277d03b7..313f5a60a605 100644
> > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> > > >         eth_hw_addr_random(dev);
> > > >  }
> > > >
> > > > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> > > > +{
> > > > +       return container_of(config, struct mvpp2_port, phylink_config);
> > > > +}
> > > > +
> > > >  static void mvpp2_phylink_validate(struct phylink_config *config,
> > > >                                    unsigned long *supported,
> > > >                                    struct phylink_link_state *state)
> > > >  {
> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > > -                                              phylink_config);
> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >         __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > > >
> > > >         /* Invalid combinations */
> > > > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
> > > >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > > >                                             struct phylink_link_state *state)
> > > >  {
> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > > -                                              phylink_config);
> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >
> > > >         if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
> > > >                 u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> > > > @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > > >
> > > >  static void mvpp2_mac_an_restart(struct phylink_config *config)
> > > >  {
> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > > -                                              phylink_config);
> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >         u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > > >
> > > >         writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> > > > @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
> > > >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> > > >                              const struct phylink_link_state *state)
> > > >  {
> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >         bool change_interface = port->phy_interface != state->interface;
> > > >
> > > >         /* Check for invalid configuration */
> > > >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> > > > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> > > > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> > > >                 return;
> > > >         }
> > > >
> > > > @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > > >                               int speed, int duplex,
> > > >                               bool tx_pause, bool rx_pause)
> > > >  {
> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >         u32 val;
> > > >
> > > >         if (mvpp2_is_xlg(interface)) {
> > > > @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > > >
> > > >         mvpp2_egress_enable(port);
> > > >         mvpp2_ingress_enable(port);
> > > > -       netif_tx_wake_all_queues(dev);
> > > > +       netif_tx_wake_all_queues(port->dev);
> > > >  }
> > > >
> > > >  static void mvpp2_mac_link_down(struct phylink_config *config,
> > > >                                 unsigned int mode, phy_interface_t interface)
> > > >  {
> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >         u32 val;
> > > >
> > > >         if (!phylink_autoneg_inband(mode)) {
> > > > @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
> > > >                 }
> > > >         }
> > > >
> > > > -       netif_tx_stop_all_queues(dev);
> > > > +       netif_tx_stop_all_queues(port->dev);
> > > >         mvpp2_egress_disable(port);
> > > >         mvpp2_ingress_disable(port);
> > > >
> > > > --
> > > > 2.20.1
> > > >
> > >
> > > This patch fixes a regression that was introduced in v5.3:
> > > Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")
> > >
> > > Above results in a NULL pointer dereference when booting the
> > > Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
> > > problematic especially for the distros using LTSv5.4 and above (the
> > > issue was reported on Fedora 32).
> > >
> > > Please help with backporting to the stable v5.3+ branches (it applies
> > > smoothly on v5.4/v5.6/v5.8).
> > >
> >
> > Any chances to backport this patch to relevant v5.3+ stable branches?
>
> What patch?  What git commit id needs to be backported?
>

The actual patch is:
Commit 6c2b49eb9671  ("net: mvpp2: add mvpp2_phylink_to_port() helper").

URL for reference:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172

Do you think it would be possible to get it merged to v5.3+ stable branches?

Best regards,
Marcin

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Sasha Levin]

On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
>Hi Greg,
>
>Apologies for delayed response:.
>
>
>pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
><gregkh@linuxfoundation.org> napisał(a):
>>
>> On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
>> > Hi Greg and Sasha,
>> >
>> > pt., 9 paź 2020 o 05:43 Marcin Wojtas <mw@semihalf.com> napisał(a):
>> > >
>> > > Hi,
>> > >
>> > > sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
>> > > >
>> > > > Add a helper to convert the struct phylink_config pointer passed in
>> > > > from phylink to the drivers internal struct mvpp2_port.
>> > > >
>> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> > > > ---
>> > > >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
>> > > >  1 file changed, 14 insertions(+), 15 deletions(-)
>> > > >
>> > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
>> > > > index 7653277d03b7..313f5a60a605 100644
>> > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
>> > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
>> > > > @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
>> > > >         eth_hw_addr_random(dev);
>> > > >  }
>> > > >
>> > > > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
>> > > > +{
>> > > > +       return container_of(config, struct mvpp2_port, phylink_config);
>> > > > +}
>> > > > +
>> > > >  static void mvpp2_phylink_validate(struct phylink_config *config,
>> > > >                                    unsigned long *supported,
>> > > >                                    struct phylink_link_state *state)
>> > > >  {
>> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
>> > > > -                                              phylink_config);
>> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>> > > >         __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>> > > >
>> > > >         /* Invalid combinations */
>> > > > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
>> > > >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
>> > > >                                             struct phylink_link_state *state)
>> > > >  {
>> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
>> > > > -                                              phylink_config);
>> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>> > > >
>> > > >         if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
>> > > >                 u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
>> > > > @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
>> > > >
>> > > >  static void mvpp2_mac_an_restart(struct phylink_config *config)
>> > > >  {
>> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
>> > > > -                                              phylink_config);
>> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>> > > >         u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
>> > > >
>> > > >         writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
>> > > > @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
>> > > >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
>> > > >                              const struct phylink_link_state *state)
>> > > >  {
>> > > > -       struct net_device *dev = to_net_dev(config->dev);
>> > > > -       struct mvpp2_port *port = netdev_priv(dev);
>> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>> > > >         bool change_interface = port->phy_interface != state->interface;
>> > > >
>> > > >         /* Check for invalid configuration */
>> > > >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
>> > > > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
>> > > > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
>> > > >                 return;
>> > > >         }
>> > > >
>> > > > @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
>> > > >                               int speed, int duplex,
>> > > >                               bool tx_pause, bool rx_pause)
>> > > >  {
>> > > > -       struct net_device *dev = to_net_dev(config->dev);
>> > > > -       struct mvpp2_port *port = netdev_priv(dev);
>> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>> > > >         u32 val;
>> > > >
>> > > >         if (mvpp2_is_xlg(interface)) {
>> > > > @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
>> > > >
>> > > >         mvpp2_egress_enable(port);
>> > > >         mvpp2_ingress_enable(port);
>> > > > -       netif_tx_wake_all_queues(dev);
>> > > > +       netif_tx_wake_all_queues(port->dev);
>> > > >  }
>> > > >
>> > > >  static void mvpp2_mac_link_down(struct phylink_config *config,
>> > > >                                 unsigned int mode, phy_interface_t interface)
>> > > >  {
>> > > > -       struct net_device *dev = to_net_dev(config->dev);
>> > > > -       struct mvpp2_port *port = netdev_priv(dev);
>> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>> > > >         u32 val;
>> > > >
>> > > >         if (!phylink_autoneg_inband(mode)) {
>> > > > @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
>> > > >                 }
>> > > >         }
>> > > >
>> > > > -       netif_tx_stop_all_queues(dev);
>> > > > +       netif_tx_stop_all_queues(port->dev);
>> > > >         mvpp2_egress_disable(port);
>> > > >         mvpp2_ingress_disable(port);
>> > > >
>> > > > --
>> > > > 2.20.1
>> > > >
>> > >
>> > > This patch fixes a regression that was introduced in v5.3:
>> > > Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")
>> > >
>> > > Above results in a NULL pointer dereference when booting the
>> > > Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
>> > > problematic especially for the distros using LTSv5.4 and above (the
>> > > issue was reported on Fedora 32).
>> > >
>> > > Please help with backporting to the stable v5.3+ branches (it applies
>> > > smoothly on v5.4/v5.6/v5.8).
>> > >
>> >
>> > Any chances to backport this patch to relevant v5.3+ stable branches?
>>
>> What patch?  What git commit id needs to be backported?
>>
>
>The actual patch is:
>Commit 6c2b49eb9671  ("net: mvpp2: add mvpp2_phylink_to_port() helper").
>
>URL for reference:
>https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172
>
>Do you think it would be possible to get it merged to v5.3+ stable branches?

Could you explain how that patch fixes anything? It reads like a
cleanup.

-- 
Thanks,
Sasha

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Marcin Wojtas]

Hi Sasha,

wt., 8 gru 2020 o 14:35 Sasha Levin <sashal@kernel.org> napisał(a):
>
> On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
> >Hi Greg,
> >
> >Apologies for delayed response:.
> >
> >
> >pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
> ><gregkh@linuxfoundation.org> napisał(a):
> >>
> >> On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
> >> > Hi Greg and Sasha,
> >> >
> >> > pt., 9 paź 2020 o 05:43 Marcin Wojtas <mw@semihalf.com> napisał(a):
> >> > >
> >> > > Hi,
> >> > >
> >> > > sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
> >> > > >
> >> > > > Add a helper to convert the struct phylink_config pointer passed in
> >> > > > from phylink to the drivers internal struct mvpp2_port.
> >> > > >
> >> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >> > > > ---
> >> > > >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
> >> > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> >> > > >
> >> > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> >> > > > index 7653277d03b7..313f5a60a605 100644
> >> > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> >> > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> >> > > > @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> >> > > >         eth_hw_addr_random(dev);
> >> > > >  }
> >> > > >
> >> > > > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> >> > > > +{
> >> > > > +       return container_of(config, struct mvpp2_port, phylink_config);
> >> > > > +}
> >> > > > +
> >> > > >  static void mvpp2_phylink_validate(struct phylink_config *config,
> >> > > >                                    unsigned long *supported,
> >> > > >                                    struct phylink_link_state *state)
> >> > > >  {
> >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> >> > > > -                                              phylink_config);
> >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >> > > >         __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> >> > > >
> >> > > >         /* Invalid combinations */
> >> > > > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
> >> > > >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> >> > > >                                             struct phylink_link_state *state)
> >> > > >  {
> >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> >> > > > -                                              phylink_config);
> >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >> > > >
> >> > > >         if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
> >> > > >                 u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> >> > > > @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> >> > > >
> >> > > >  static void mvpp2_mac_an_restart(struct phylink_config *config)
> >> > > >  {
> >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> >> > > > -                                              phylink_config);
> >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >> > > >         u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> >> > > >
> >> > > >         writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> >> > > > @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
> >> > > >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> >> > > >                              const struct phylink_link_state *state)
> >> > > >  {
> >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >> > > >         bool change_interface = port->phy_interface != state->interface;
> >> > > >
> >> > > >         /* Check for invalid configuration */
> >> > > >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> >> > > > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> >> > > > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> >> > > >                 return;
> >> > > >         }
> >> > > >
> >> > > > @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> >> > > >                               int speed, int duplex,
> >> > > >                               bool tx_pause, bool rx_pause)
> >> > > >  {
> >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >> > > >         u32 val;
> >> > > >
> >> > > >         if (mvpp2_is_xlg(interface)) {
> >> > > > @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> >> > > >
> >> > > >         mvpp2_egress_enable(port);
> >> > > >         mvpp2_ingress_enable(port);
> >> > > > -       netif_tx_wake_all_queues(dev);
> >> > > > +       netif_tx_wake_all_queues(port->dev);
> >> > > >  }
> >> > > >
> >> > > >  static void mvpp2_mac_link_down(struct phylink_config *config,
> >> > > >                                 unsigned int mode, phy_interface_t interface)
> >> > > >  {
> >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >> > > >         u32 val;
> >> > > >
> >> > > >         if (!phylink_autoneg_inband(mode)) {
> >> > > > @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
> >> > > >                 }
> >> > > >         }
> >> > > >
> >> > > > -       netif_tx_stop_all_queues(dev);
> >> > > > +       netif_tx_stop_all_queues(port->dev);
> >> > > >         mvpp2_egress_disable(port);
> >> > > >         mvpp2_ingress_disable(port);
> >> > > >
> >> > > > --
> >> > > > 2.20.1
> >> > > >
> >> > >
> >> > > This patch fixes a regression that was introduced in v5.3:
> >> > > Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")
> >> > >
> >> > > Above results in a NULL pointer dereference when booting the
> >> > > Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
> >> > > problematic especially for the distros using LTSv5.4 and above (the
> >> > > issue was reported on Fedora 32).
> >> > >
> >> > > Please help with backporting to the stable v5.3+ branches (it applies
> >> > > smoothly on v5.4/v5.6/v5.8).
> >> > >
> >> >
> >> > Any chances to backport this patch to relevant v5.3+ stable branches?
> >>
> >> What patch?  What git commit id needs to be backported?
> >>
> >
> >The actual patch is:
> >Commit 6c2b49eb9671  ("net: mvpp2: add mvpp2_phylink_to_port() helper").
> >
> >URL for reference:
> >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172
> >
> >Do you think it would be possible to get it merged to v5.3+ stable branches?
>
> Could you explain how that patch fixes anything? It reads like a
> cleanup.
>

Indeed, I am aware of it, but I'm not sure about the best way to fix
it. In fact the mentioned patch is an unintentional fix. Commit
44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK
API") reworked an argument list of mvpp2_mac_config() routine in a way
that resulted in a NULL pointer dereference when booting the
Armada7k8k/CN913x with ACPI between 5.3 and 5.8. Part of Russell's
patch resolves this issue.

What is the best way to handle that?

Thanks,
Marcin

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Greg Kroah-Hartman]

On Tue, Dec 08, 2020 at 04:02:50PM +0100, Marcin Wojtas wrote:
> Hi Sasha,
> 
> wt., 8 gru 2020 o 14:35 Sasha Levin <sashal@kernel.org> napisał(a):
> >
> > On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
> > >Hi Greg,
> > >
> > >Apologies for delayed response:.
> > >
> > >
> > >pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
> > ><gregkh@linuxfoundation.org> napisał(a):
> > >>
> > >> On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
> > >> > Hi Greg and Sasha,
> > >> >
> > >> > pt., 9 paź 2020 o 05:43 Marcin Wojtas <mw@semihalf.com> napisał(a):
> > >> > >
> > >> > > Hi,
> > >> > >
> > >> > > sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
> > >> > > >
> > >> > > > Add a helper to convert the struct phylink_config pointer passed in
> > >> > > > from phylink to the drivers internal struct mvpp2_port.
> > >> > > >
> > >> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > >> > > > ---
> > >> > > >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
> > >> > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > >> > > >
> > >> > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > >> > > > index 7653277d03b7..313f5a60a605 100644
> > >> > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > >> > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > >> > > > @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> > >> > > >         eth_hw_addr_random(dev);
> > >> > > >  }
> > >> > > >
> > >> > > > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> > >> > > > +{
> > >> > > > +       return container_of(config, struct mvpp2_port, phylink_config);
> > >> > > > +}
> > >> > > > +
> > >> > > >  static void mvpp2_phylink_validate(struct phylink_config *config,
> > >> > > >                                    unsigned long *supported,
> > >> > > >                                    struct phylink_link_state *state)
> > >> > > >  {
> > >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > >> > > > -                                              phylink_config);
> > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >> > > >         __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > >> > > >
> > >> > > >         /* Invalid combinations */
> > >> > > > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
> > >> > > >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > >> > > >                                             struct phylink_link_state *state)
> > >> > > >  {
> > >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > >> > > > -                                              phylink_config);
> > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >> > > >
> > >> > > >         if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
> > >> > > >                 u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> > >> > > > @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > >> > > >
> > >> > > >  static void mvpp2_mac_an_restart(struct phylink_config *config)
> > >> > > >  {
> > >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > >> > > > -                                              phylink_config);
> > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >> > > >         u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > >> > > >
> > >> > > >         writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> > >> > > > @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
> > >> > > >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> > >> > > >                              const struct phylink_link_state *state)
> > >> > > >  {
> > >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >> > > >         bool change_interface = port->phy_interface != state->interface;
> > >> > > >
> > >> > > >         /* Check for invalid configuration */
> > >> > > >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> > >> > > > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> > >> > > > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> > >> > > >                 return;
> > >> > > >         }
> > >> > > >
> > >> > > > @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > >> > > >                               int speed, int duplex,
> > >> > > >                               bool tx_pause, bool rx_pause)
> > >> > > >  {
> > >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >> > > >         u32 val;
> > >> > > >
> > >> > > >         if (mvpp2_is_xlg(interface)) {
> > >> > > > @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > >> > > >
> > >> > > >         mvpp2_egress_enable(port);
> > >> > > >         mvpp2_ingress_enable(port);
> > >> > > > -       netif_tx_wake_all_queues(dev);
> > >> > > > +       netif_tx_wake_all_queues(port->dev);
> > >> > > >  }
> > >> > > >
> > >> > > >  static void mvpp2_mac_link_down(struct phylink_config *config,
> > >> > > >                                 unsigned int mode, phy_interface_t interface)
> > >> > > >  {
> > >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >> > > >         u32 val;
> > >> > > >
> > >> > > >         if (!phylink_autoneg_inband(mode)) {
> > >> > > > @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
> > >> > > >                 }
> > >> > > >         }
> > >> > > >
> > >> > > > -       netif_tx_stop_all_queues(dev);
> > >> > > > +       netif_tx_stop_all_queues(port->dev);
> > >> > > >         mvpp2_egress_disable(port);
> > >> > > >         mvpp2_ingress_disable(port);
> > >> > > >
> > >> > > > --
> > >> > > > 2.20.1
> > >> > > >
> > >> > >
> > >> > > This patch fixes a regression that was introduced in v5.3:
> > >> > > Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")
> > >> > >
> > >> > > Above results in a NULL pointer dereference when booting the
> > >> > > Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
> > >> > > problematic especially for the distros using LTSv5.4 and above (the
> > >> > > issue was reported on Fedora 32).
> > >> > >
> > >> > > Please help with backporting to the stable v5.3+ branches (it applies
> > >> > > smoothly on v5.4/v5.6/v5.8).
> > >> > >
> > >> >
> > >> > Any chances to backport this patch to relevant v5.3+ stable branches?
> > >>
> > >> What patch?  What git commit id needs to be backported?
> > >>
> > >
> > >The actual patch is:
> > >Commit 6c2b49eb9671  ("net: mvpp2: add mvpp2_phylink_to_port() helper").
> > >
> > >URL for reference:
> > >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172
> > >
> > >Do you think it would be possible to get it merged to v5.3+ stable branches?
> >
> > Could you explain how that patch fixes anything? It reads like a
> > cleanup.
> >
> 
> Indeed, I am aware of it, but I'm not sure about the best way to fix
> it. In fact the mentioned patch is an unintentional fix. Commit
> 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK
> API") reworked an argument list of mvpp2_mac_config() routine in a way
> that resulted in a NULL pointer dereference when booting the
> Armada7k8k/CN913x with ACPI between 5.3 and 5.8. Part of Russell's
> patch resolves this issue.

What part fixes the issue?  I can't see it...

thanks,

greg k-h

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Marcin Wojtas]

Hi Greg,

śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman
<gregkh@linuxfoundation.org> napisał(a):
>
> On Tue, Dec 08, 2020 at 04:02:50PM +0100, Marcin Wojtas wrote:
> > Hi Sasha,
> >
> > wt., 8 gru 2020 o 14:35 Sasha Levin <sashal@kernel.org> napisał(a):
> > >
> > > On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
> > > >Hi Greg,
> > > >
> > > >Apologies for delayed response:.
> > > >
> > > >
> > > >pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
> > > ><gregkh@linuxfoundation.org> napisał(a):
> > > >>
> > > >> On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
> > > >> > Hi Greg and Sasha,
> > > >> >
> > > >> > pt., 9 paź 2020 o 05:43 Marcin Wojtas <mw@semihalf.com> napisał(a):
> > > >> > >
> > > >> > > Hi,
> > > >> > >
> > > >> > > sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
> > > >> > > >
> > > >> > > > Add a helper to convert the struct phylink_config pointer passed in
> > > >> > > > from phylink to the drivers internal struct mvpp2_port.
> > > >> > > >
> > > >> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > >> > > > ---
> > > >> > > >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
> > > >> > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > > >> > > >
> > > >> > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > >> > > > index 7653277d03b7..313f5a60a605 100644
> > > >> > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > >> > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > >> > > > @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> > > >> > > >         eth_hw_addr_random(dev);
> > > >> > > >  }
> > > >> > > >
> > > >> > > > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> > > >> > > > +{
> > > >> > > > +       return container_of(config, struct mvpp2_port, phylink_config);
> > > >> > > > +}
> > > >> > > > +
> > > >> > > >  static void mvpp2_phylink_validate(struct phylink_config *config,
> > > >> > > >                                    unsigned long *supported,
> > > >> > > >                                    struct phylink_link_state *state)
> > > >> > > >  {
> > > >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > >> > > > -                                              phylink_config);
> > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >> > > >         __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > > >> > > >
> > > >> > > >         /* Invalid combinations */
> > > >> > > > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
> > > >> > > >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > > >> > > >                                             struct phylink_link_state *state)
> > > >> > > >  {
> > > >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > >> > > > -                                              phylink_config);
> > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >> > > >
> > > >> > > >         if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
> > > >> > > >                 u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> > > >> > > > @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > > >> > > >
> > > >> > > >  static void mvpp2_mac_an_restart(struct phylink_config *config)
> > > >> > > >  {
> > > >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > >> > > > -                                              phylink_config);
> > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >> > > >         u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > > >> > > >
> > > >> > > >         writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> > > >> > > > @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
> > > >> > > >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> > > >> > > >                              const struct phylink_link_state *state)
> > > >> > > >  {
> > > >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > > >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >> > > >         bool change_interface = port->phy_interface != state->interface;
> > > >> > > >
> > > >> > > >         /* Check for invalid configuration */
> > > >> > > >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> > > >> > > > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> > > >> > > > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> > > >> > > >                 return;
> > > >> > > >         }
> > > >> > > >
> > > >> > > > @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > > >> > > >                               int speed, int duplex,
> > > >> > > >                               bool tx_pause, bool rx_pause)
> > > >> > > >  {
> > > >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > > >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >> > > >         u32 val;
> > > >> > > >
> > > >> > > >         if (mvpp2_is_xlg(interface)) {
> > > >> > > > @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > > >> > > >
> > > >> > > >         mvpp2_egress_enable(port);
> > > >> > > >         mvpp2_ingress_enable(port);
> > > >> > > > -       netif_tx_wake_all_queues(dev);
> > > >> > > > +       netif_tx_wake_all_queues(port->dev);
> > > >> > > >  }
> > > >> > > >
> > > >> > > >  static void mvpp2_mac_link_down(struct phylink_config *config,
> > > >> > > >                                 unsigned int mode, phy_interface_t interface)
> > > >> > > >  {
> > > >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > > >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > >> > > >         u32 val;
> > > >> > > >
> > > >> > > >         if (!phylink_autoneg_inband(mode)) {
> > > >> > > > @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
> > > >> > > >                 }
> > > >> > > >         }
> > > >> > > >
> > > >> > > > -       netif_tx_stop_all_queues(dev);
> > > >> > > > +       netif_tx_stop_all_queues(port->dev);
> > > >> > > >         mvpp2_egress_disable(port);
> > > >> > > >         mvpp2_ingress_disable(port);
> > > >> > > >
> > > >> > > > --
> > > >> > > > 2.20.1
> > > >> > > >
> > > >> > >
> > > >> > > This patch fixes a regression that was introduced in v5.3:
> > > >> > > Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")
> > > >> > >
> > > >> > > Above results in a NULL pointer dereference when booting the
> > > >> > > Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
> > > >> > > problematic especially for the distros using LTSv5.4 and above (the
> > > >> > > issue was reported on Fedora 32).
> > > >> > >
> > > >> > > Please help with backporting to the stable v5.3+ branches (it applies
> > > >> > > smoothly on v5.4/v5.6/v5.8).
> > > >> > >
> > > >> >
> > > >> > Any chances to backport this patch to relevant v5.3+ stable branches?
> > > >>
> > > >> What patch?  What git commit id needs to be backported?
> > > >>
> > > >
> > > >The actual patch is:
> > > >Commit 6c2b49eb9671  ("net: mvpp2: add mvpp2_phylink_to_port() helper").
> > > >
> > > >URL for reference:
> > > >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172
> > > >
> > > >Do you think it would be possible to get it merged to v5.3+ stable branches?
> > >
> > > Could you explain how that patch fixes anything? It reads like a
> > > cleanup.
> > >
> >
> > Indeed, I am aware of it, but I'm not sure about the best way to fix
> > it. In fact the mentioned patch is an unintentional fix. Commit
> > 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK
> > API") reworked an argument list of mvpp2_mac_config() routine in a way
> > that resulted in a NULL pointer dereference when booting the
> > Armada7k8k/CN913x with ACPI between 5.3 and 5.8. Part of Russell's
> > patch resolves this issue.
>
> What part fixes the issue?  I can't see it...
>

I re-checked in my setup and here's the smallest part of the original
patch, that fixes previously described issue:
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index e98be8372780..9d71a4fe1750 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4767,6 +4767,11 @@ static void mvpp2_port_copy_mac_addr(struct
net_device *dev, struct mvpp2 *priv,
        eth_hw_addr_random(dev);
 }

+static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
+{
+       return container_of(config, struct mvpp2_port, phylink_config);
+}
+
 static void mvpp2_phylink_validate(struct phylink_config *config,
                                   unsigned long *supported,
                                   struct phylink_link_state *state)
@@ -5105,13 +5110,12 @@ static void mvpp2_gmac_config(struct
mvpp2_port *port, unsigned int mode,
 static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
                             const struct phylink_link_state *state)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        bool change_interface = port->phy_interface != state->interface;

        /* Check for invalid configuration */
        if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
-               netdev_err(dev, "Invalid mode on %s\n", dev->name);
+               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
                return;
        }

@@ -5151,8 +5155,7 @@ static void mvpp2_mac_link_up(struct
phylink_config *config,
                              int speed, int duplex,
                              bool tx_pause, bool rx_pause)
 {
-       struct net_device *dev = to_net_dev(config->dev);
-       struct mvpp2_port *port = netdev_priv(dev);
+       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
        u32 val;

        if (mvpp2_is_xlg(interface)) {
@@ -5199,7 +5202,7 @@ static void mvpp2_mac_link_up(struct
phylink_config *config,

        mvpp2_egress_enable(port);
        mvpp2_ingress_enable(port);
-       netif_tx_wake_all_queues(dev);
+       netif_tx_wake_all_queues(port->dev);
 }

 static void mvpp2_mac_link_down(struct phylink_config *config,
-- 

Do you think there is a point of slicing the original patch or rather
cherry-pick as-is?

Best regards,
Marcin

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Russell King - ARM Linux admin]

On Thu, Dec 10, 2020 at 03:35:29PM +0100, Marcin Wojtas wrote:
> Hi Greg,
> 
> śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> napisał(a):
> > What part fixes the issue?  I can't see it...
> 
> I re-checked in my setup and here's the smallest part of the original
> patch, that fixes previously described issue:
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index e98be8372780..9d71a4fe1750 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -4767,6 +4767,11 @@ static void mvpp2_port_copy_mac_addr(struct
> net_device *dev, struct mvpp2 *priv,
>         eth_hw_addr_random(dev);
>  }
> 
> +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> +{
> +       return container_of(config, struct mvpp2_port, phylink_config);
> +}
> +
>  static void mvpp2_phylink_validate(struct phylink_config *config,
>                                    unsigned long *supported,
>                                    struct phylink_link_state *state)
> @@ -5105,13 +5110,12 @@ static void mvpp2_gmac_config(struct
> mvpp2_port *port, unsigned int mode,
>  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
>                              const struct phylink_link_state *state)
>  {
> -       struct net_device *dev = to_net_dev(config->dev);
> -       struct mvpp2_port *port = netdev_priv(dev);
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>         bool change_interface = port->phy_interface != state->interface;
> 
>         /* Check for invalid configuration */
>         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
>                 return;
>         }
> 
> @@ -5151,8 +5155,7 @@ static void mvpp2_mac_link_up(struct
> phylink_config *config,
>                               int speed, int duplex,
>                               bool tx_pause, bool rx_pause)
>  {
> -       struct net_device *dev = to_net_dev(config->dev);
> -       struct mvpp2_port *port = netdev_priv(dev);
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>         u32 val;
> 
>         if (mvpp2_is_xlg(interface)) {
> @@ -5199,7 +5202,7 @@ static void mvpp2_mac_link_up(struct
> phylink_config *config,
> 
>         mvpp2_egress_enable(port);
>         mvpp2_ingress_enable(port);
> -       netif_tx_wake_all_queues(dev);
> +       netif_tx_wake_all_queues(port->dev);
>  }
> 
>  static void mvpp2_mac_link_down(struct phylink_config *config,

The problem is caused by this hack:

                /* Phylink isn't used as of now for ACPI, so the MAC has to be
                 * configured manually when the interface is started. This will
                 * be removed as soon as the phylink ACPI support lands in.
                 */
                struct phylink_link_state state = {
                        .interface = port->phy_interface,
                };
                mvpp2_mac_config(&port->phylink_config, MLO_AN_INBAND, &state);
                mvpp2_mac_link_up(&port->phylink_config, MLO_AN_INBAND,
                                  port->phy_interface, NULL);

which passes an un-initialised (zeroed) port->phylink_config, as
phylink is not used in ACPI setups.

The problem occurs because port->phylink_config.dev (which is a
NULL pointer in this instance) is passed to to_net_dev():

#define to_net_dev(d) container_of(d, struct net_device, dev)

Which then means netdev_priv(dev) attempts to dereference a not-quite
NULL pointer, leading to an oops.

The problem here is that the bug was not noticed; it seems hardly
anyone bothers to run mainline kernels with ACPI on Marvell platforms,
or if they do, they don't bother reporting to mainline communities
when they have problems. Likely, there's posts on some random web-based
bulletin board or mailing list that kernel developers don't read
somewhere complaining that there's an oops.

Like...

https://lists.einval.com/pipermail/macchiato/2020-January/000309.html
https://gist.github.com/AdrianKoshka/ff9862da2183a2d8e26d47baf8dc04b9

This kind of segmentation is very disappointing; it means potentially
lots of bugs go by unnoticed by kernel developers, and bugs only get
fixed by chance.  Had it been reported to somewhere known earlier
this year, it is likely that a proper fix patch would have been
created.

How this gets handled is ultimately up to the stable developers to
decide what they wish to accept. Do they wish to accept a back-ported
full version of my commit 6c2b49eb9671 ("net: mvpp2: add
mvpp2_phylink_to_port() helper") that unintentionally fixed this
unknown issue, or do they want a more minimal fix such as the cut-down
version of that commit that Marcin has supplied.

Until something changes in the way bugs get reported, I suspect this
won't be the only instance of bug-fixing-by-accident happening.

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

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Marcin Wojtas]

Hi Russell,

czw., 10 gru 2020 o 16:46 Russell King - ARM Linux admin
<linux@armlinux.org.uk> napisał(a):
>
> On Thu, Dec 10, 2020 at 03:35:29PM +0100, Marcin Wojtas wrote:
> > Hi Greg,
> >
> > śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> napisał(a):
> > > What part fixes the issue?  I can't see it...
> >
> > I re-checked in my setup and here's the smallest part of the original
> > patch, that fixes previously described issue:
> > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > index e98be8372780..9d71a4fe1750 100644
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -4767,6 +4767,11 @@ static void mvpp2_port_copy_mac_addr(struct
> > net_device *dev, struct mvpp2 *priv,
> >         eth_hw_addr_random(dev);
> >  }
> >
> > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> > +{
> > +       return container_of(config, struct mvpp2_port, phylink_config);
> > +}
> > +
> >  static void mvpp2_phylink_validate(struct phylink_config *config,
> >                                    unsigned long *supported,
> >                                    struct phylink_link_state *state)
> > @@ -5105,13 +5110,12 @@ static void mvpp2_gmac_config(struct
> > mvpp2_port *port, unsigned int mode,
> >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> >                              const struct phylink_link_state *state)
> >  {
> > -       struct net_device *dev = to_net_dev(config->dev);
> > -       struct mvpp2_port *port = netdev_priv(dev);
> > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >         bool change_interface = port->phy_interface != state->interface;
> >
> >         /* Check for invalid configuration */
> >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> >                 return;
> >         }
> >
> > @@ -5151,8 +5155,7 @@ static void mvpp2_mac_link_up(struct
> > phylink_config *config,
> >                               int speed, int duplex,
> >                               bool tx_pause, bool rx_pause)
> >  {
> > -       struct net_device *dev = to_net_dev(config->dev);
> > -       struct mvpp2_port *port = netdev_priv(dev);
> > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >         u32 val;
> >
> >         if (mvpp2_is_xlg(interface)) {
> > @@ -5199,7 +5202,7 @@ static void mvpp2_mac_link_up(struct
> > phylink_config *config,
> >
> >         mvpp2_egress_enable(port);
> >         mvpp2_ingress_enable(port);
> > -       netif_tx_wake_all_queues(dev);
> > +       netif_tx_wake_all_queues(port->dev);
> >  }
> >
> >  static void mvpp2_mac_link_down(struct phylink_config *config,
>
> The problem is caused by this hack:
>
>                 /* Phylink isn't used as of now for ACPI, so the MAC has to be
>                  * configured manually when the interface is started. This will
>                  * be removed as soon as the phylink ACPI support lands in.
>                  */
>                 struct phylink_link_state state = {
>                         .interface = port->phy_interface,
>                 };
>                 mvpp2_mac_config(&port->phylink_config, MLO_AN_INBAND, &state);
>                 mvpp2_mac_link_up(&port->phylink_config, MLO_AN_INBAND,
>                                   port->phy_interface, NULL);
>
> which passes an un-initialised (zeroed) port->phylink_config, as
> phylink is not used in ACPI setups.
>
> The problem occurs because port->phylink_config.dev (which is a
> NULL pointer in this instance) is passed to to_net_dev():
>
> #define to_net_dev(d) container_of(d, struct net_device, dev)
>
> Which then means netdev_priv(dev) attempts to dereference a not-quite
> NULL pointer, leading to an oops.

Correct. Hopefully the MDIO+ACPI patchset will land and I'll be able
to get rid of this hack and do not maintain dual handling paths any
longer.

>
> The problem here is that the bug was not noticed; it seems hardly
> anyone bothers to run mainline kernels with ACPI on Marvell platforms,
> or if they do, they don't bother reporting to mainline communities
> when they have problems. Likely, there's posts on some random web-based
> bulletin board or mailing list that kernel developers don't read
> somewhere complaining that there's an oops.
>

I must admit that due to other duties I did not follow the mainline
mvpp2 for a couple revisions (and I am not maintainer of it). However
recently I got reached-out directly by different developers - the
trigger was different distros upgrading the kernel above v5.4+ and for
some reasons the DT path is not chosen there (and the ACPI will be
chosen more and more in the SystemReady world).

> Like...
>
> https://lists.einval.com/pipermail/macchiato/2020-January/000309.html
> https://gist.github.com/AdrianKoshka/ff9862da2183a2d8e26d47baf8dc04b9
>
> This kind of segmentation is very disappointing; it means potentially
> lots of bugs go by unnoticed by kernel developers, and bugs only get
> fixed by chance.  Had it been reported to somewhere known earlier
> this year, it is likely that a proper fix patch would have been
> created.

Totally agree. As soon as I got noticed directly it took me no time to
find the regression root cause.

>
> How this gets handled is ultimately up to the stable developers to
> decide what they wish to accept. Do they wish to accept a back-ported
> full version of my commit 6c2b49eb9671 ("net: mvpp2: add
> mvpp2_phylink_to_port() helper") that unintentionally fixed this
> unknown issue, or do they want a more minimal fix such as the cut-down
> version of that commit that Marcin has supplied.
>
> Until something changes in the way bugs get reported, I suspect this
> won't be the only instance of bug-fixing-by-accident happening.
>

Thank you Russell for your input.

Best regards,
Marcin

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Russell King - ARM Linux admin]

On Thu, Dec 10, 2020 at 06:43:50PM +0100, Marcin Wojtas wrote:
> I must admit that due to other duties I did not follow the mainline
> mvpp2 for a couple revisions (and I am not maintainer of it). However
> recently I got reached-out directly by different developers - the
> trigger was different distros upgrading the kernel above v5.4+ and for
> some reasons the DT path is not chosen there (and the ACPI will be
> chosen more and more in the SystemReady world).

Please note that there is no active maintainer for mvpp2.

It will be good to get rid of the ACPI hack here, as that means we'll
be using the same code paths for both ACPI and DT, meaning hopefully
less bugs like this go unnoticed.

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

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Marcin Wojtas]

czw., 10 gru 2020 o 18:56 Russell King - ARM Linux admin
<linux@armlinux.org.uk> napisał(a):
>
> On Thu, Dec 10, 2020 at 06:43:50PM +0100, Marcin Wojtas wrote:
> > I must admit that due to other duties I did not follow the mainline
> > mvpp2 for a couple revisions (and I am not maintainer of it). However
> > recently I got reached-out directly by different developers - the
> > trigger was different distros upgrading the kernel above v5.4+ and for
> > some reasons the DT path is not chosen there (and the ACPI will be
> > chosen more and more in the SystemReady world).
>
> Please note that there is no active maintainer for mvpp2.
>

Right. I think I can volunteer to be one (my name is in the driver
code anyway :) ) and spend some time on reviewing/testing the patches,
unless there are no objections. In such case, are the drivers/net
Mainteners who decide/accept it?

> It will be good to get rid of the ACPI hack here, as that means we'll
> be using the same code paths for both ACPI and DT, meaning hopefully
> less bugs like this go unnoticed.
>

+1. As soon as the MDIO+ACPI lands, I plan to do the rework.

Best regards,
Marcin

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Andrew Lunn]

> +1. As soon as the MDIO+ACPI lands, I plan to do the rework.

Don't hold you breath. It has gone very quiet about ACPI in net
devices.

	Andrew

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Marcin Wojtas]

czw., 10 gru 2020 o 21:26 Andrew Lunn <andrew@lunn.ch> napisał(a):
>
> > +1. As soon as the MDIO+ACPI lands, I plan to do the rework.
>
> Don't hold you breath. It has gone very quiet about ACPI in net
> devices.

I saw the results of the upcoming next revision from NXP, so I'm
rather optimistic.

Best regards,
Marcin

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Jon Nettleton]

On Thu, Dec 10, 2020 at 9:27 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +1. As soon as the MDIO+ACPI lands, I plan to do the rework.
>
> Don't hold you breath. It has gone very quiet about ACPI in net
> devices.

NXP resources were re-allocated for their next internal BSP release.
I have been working with Calvin over the past week and a half and the new
patchset will be submitted early next week most likely.

-Jon

>
>         Andrew

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Andrew Lunn]

On Fri, Dec 11, 2020 at 06:03:57AM +0100, Jon Nettleton wrote:
> On Thu, Dec 10, 2020 at 9:27 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > +1. As soon as the MDIO+ACPI lands, I plan to do the rework.
> >
> > Don't hold you breath. It has gone very quiet about ACPI in net
> > devices.
> 
> NXP resources were re-allocated for their next internal BSP release.
> I have been working with Calvin over the past week and a half and the new
> patchset will be submitted early next week most likely.

Hi Jon

Cool, i just hope you get buy in from the ACPI maintainers this time.

      Andrew

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Marcin Wojtas]

Hi Jakub,

czw., 10 gru 2020 o 15:35 Marcin Wojtas <mw@semihalf.com> napisał(a):
>
> Hi Greg,
>
> śr., 9 gru 2020 o 11:59 Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> napisał(a):
> >
> > On Tue, Dec 08, 2020 at 04:02:50PM +0100, Marcin Wojtas wrote:
> > > Hi Sasha,
> > >
> > > wt., 8 gru 2020 o 14:35 Sasha Levin <sashal@kernel.org> napisał(a):
> > > >
> > > > On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
> > > > >Hi Greg,
> > > > >
> > > > >Apologies for delayed response:.
> > > > >
> > > > >
> > > > >pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
> > > > ><gregkh@linuxfoundation.org> napisał(a):
> > > > >>
> > > > >> On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
> > > > >> > Hi Greg and Sasha,
> > > > >> >
> > > > >> > pt., 9 paź 2020 o 05:43 Marcin Wojtas <mw@semihalf.com> napisał(a):
> > > > >> > >
> > > > >> > > Hi,
> > > > >> > >
> > > > >> > > sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
> > > > >> > > >
> > > > >> > > > Add a helper to convert the struct phylink_config pointer passed in
> > > > >> > > > from phylink to the drivers internal struct mvpp2_port.
> > > > >> > > >
> > > > >> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > > >> > > > ---
> > > > >> > > >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
> > > > >> > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > > > >> > > >
> > > > >> > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > >> > > > index 7653277d03b7..313f5a60a605 100644
> > > > >> > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > >> > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > > >> > > > @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> > > > >> > > >         eth_hw_addr_random(dev);
> > > > >> > > >  }
> > > > >> > > >
> > > > >> > > > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> > > > >> > > > +{
> > > > >> > > > +       return container_of(config, struct mvpp2_port, phylink_config);
> > > > >> > > > +}
> > > > >> > > > +
> > > > >> > > >  static void mvpp2_phylink_validate(struct phylink_config *config,
> > > > >> > > >                                    unsigned long *supported,
> > > > >> > > >                                    struct phylink_link_state *state)
> > > > >> > > >  {
> > > > >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > > >> > > > -                                              phylink_config);
> > > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > > >> > > >         __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > > > >> > > >
> > > > >> > > >         /* Invalid combinations */
> > > > >> > > > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
> > > > >> > > >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > > > >> > > >                                             struct phylink_link_state *state)
> > > > >> > > >  {
> > > > >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > > >> > > > -                                              phylink_config);
> > > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > > >> > > >
> > > > >> > > >         if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
> > > > >> > > >                 u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> > > > >> > > > @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > > > >> > > >
> > > > >> > > >  static void mvpp2_mac_an_restart(struct phylink_config *config)
> > > > >> > > >  {
> > > > >> > > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > > >> > > > -                                              phylink_config);
> > > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > > >> > > >         u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > > > >> > > >
> > > > >> > > >         writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> > > > >> > > > @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
> > > > >> > > >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> > > > >> > > >                              const struct phylink_link_state *state)
> > > > >> > > >  {
> > > > >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > > > >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > > >> > > >         bool change_interface = port->phy_interface != state->interface;
> > > > >> > > >
> > > > >> > > >         /* Check for invalid configuration */
> > > > >> > > >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> > > > >> > > > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> > > > >> > > > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> > > > >> > > >                 return;
> > > > >> > > >         }
> > > > >> > > >
> > > > >> > > > @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > > > >> > > >                               int speed, int duplex,
> > > > >> > > >                               bool tx_pause, bool rx_pause)
> > > > >> > > >  {
> > > > >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > > > >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > > >> > > >         u32 val;
> > > > >> > > >
> > > > >> > > >         if (mvpp2_is_xlg(interface)) {
> > > > >> > > > @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > > > >> > > >
> > > > >> > > >         mvpp2_egress_enable(port);
> > > > >> > > >         mvpp2_ingress_enable(port);
> > > > >> > > > -       netif_tx_wake_all_queues(dev);
> > > > >> > > > +       netif_tx_wake_all_queues(port->dev);
> > > > >> > > >  }
> > > > >> > > >
> > > > >> > > >  static void mvpp2_mac_link_down(struct phylink_config *config,
> > > > >> > > >                                 unsigned int mode, phy_interface_t interface)
> > > > >> > > >  {
> > > > >> > > > -       struct net_device *dev = to_net_dev(config->dev);
> > > > >> > > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > > >> > > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > > >> > > >         u32 val;
> > > > >> > > >
> > > > >> > > >         if (!phylink_autoneg_inband(mode)) {
> > > > >> > > > @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
> > > > >> > > >                 }
> > > > >> > > >         }
> > > > >> > > >
> > > > >> > > > -       netif_tx_stop_all_queues(dev);
> > > > >> > > > +       netif_tx_stop_all_queues(port->dev);
> > > > >> > > >         mvpp2_egress_disable(port);
> > > > >> > > >         mvpp2_ingress_disable(port);
> > > > >> > > >
> > > > >> > > > --
> > > > >> > > > 2.20.1
> > > > >> > > >
> > > > >> > >
> > > > >> > > This patch fixes a regression that was introduced in v5.3:
> > > > >> > > Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")
> > > > >> > >
> > > > >> > > Above results in a NULL pointer dereference when booting the
> > > > >> > > Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
> > > > >> > > problematic especially for the distros using LTSv5.4 and above (the
> > > > >> > > issue was reported on Fedora 32).
> > > > >> > >
> > > > >> > > Please help with backporting to the stable v5.3+ branches (it applies
> > > > >> > > smoothly on v5.4/v5.6/v5.8).
> > > > >> > >
> > > > >> >
> > > > >> > Any chances to backport this patch to relevant v5.3+ stable branches?
> > > > >>
> > > > >> What patch?  What git commit id needs to be backported?
> > > > >>
> > > > >
> > > > >The actual patch is:
> > > > >Commit 6c2b49eb9671  ("net: mvpp2: add mvpp2_phylink_to_port() helper").
> > > > >
> > > > >URL for reference:
> > > > >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172
> > > > >
> > > > >Do you think it would be possible to get it merged to v5.3+ stable branches?
> > > >
> > > > Could you explain how that patch fixes anything? It reads like a
> > > > cleanup.
> > > >
> > >
> > > Indeed, I am aware of it, but I'm not sure about the best way to fix
> > > it. In fact the mentioned patch is an unintentional fix. Commit
> > > 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK
> > > API") reworked an argument list of mvpp2_mac_config() routine in a way
> > > that resulted in a NULL pointer dereference when booting the
> > > Armada7k8k/CN913x with ACPI between 5.3 and 5.8. Part of Russell's
> > > patch resolves this issue.
> >
> > What part fixes the issue?  I can't see it...
> >
>
> I re-checked in my setup and here's the smallest part of the original
> patch, that fixes previously described issue:
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index e98be8372780..9d71a4fe1750 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -4767,6 +4767,11 @@ static void mvpp2_port_copy_mac_addr(struct
> net_device *dev, struct mvpp2 *priv,
>         eth_hw_addr_random(dev);
>  }
>
> +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> +{
> +       return container_of(config, struct mvpp2_port, phylink_config);
> +}
> +
>  static void mvpp2_phylink_validate(struct phylink_config *config,
>                                    unsigned long *supported,
>                                    struct phylink_link_state *state)
> @@ -5105,13 +5110,12 @@ static void mvpp2_gmac_config(struct
> mvpp2_port *port, unsigned int mode,
>  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
>                              const struct phylink_link_state *state)
>  {
> -       struct net_device *dev = to_net_dev(config->dev);
> -       struct mvpp2_port *port = netdev_priv(dev);
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>         bool change_interface = port->phy_interface != state->interface;
>
>         /* Check for invalid configuration */
>         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
>                 return;
>         }
>
> @@ -5151,8 +5155,7 @@ static void mvpp2_mac_link_up(struct
> phylink_config *config,
>                               int speed, int duplex,
>                               bool tx_pause, bool rx_pause)
>  {
> -       struct net_device *dev = to_net_dev(config->dev);
> -       struct mvpp2_port *port = netdev_priv(dev);
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
>         u32 val;
>
>         if (mvpp2_is_xlg(interface)) {
> @@ -5199,7 +5202,7 @@ static void mvpp2_mac_link_up(struct
> phylink_config *config,
>
>         mvpp2_egress_enable(port);
>         mvpp2_ingress_enable(port);
> -       netif_tx_wake_all_queues(dev);
> +       netif_tx_wake_all_queues(port->dev);
>  }
>
>  static void mvpp2_mac_link_down(struct phylink_config *config,
> --
>
> Do you think there is a point of slicing the original patch or rather
> cherry-pick as-is?
>

Do you think there is a chance to get the above fix merged to stable (v5.3+)?

Best regards,
Marcin

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Jakub Kicinski]

On Sun, 20 Dec 2020 18:08:19 +0100 Marcin Wojtas wrote:
> > > > > >> > > This patch fixes a regression that was introduced in v5.3:
> > > > > >> > > Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")
> > > > > >> > >
> > > > > >> > > Above results in a NULL pointer dereference when booting the
> > > > > >> > > Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
> > > > > >> > > problematic especially for the distros using LTSv5.4 and above (the
> > > > > >> > > issue was reported on Fedora 32).
> > > > > >> > >
> > > > > >> > > Please help with backporting to the stable v5.3+ branches (it applies
> > > > > >> > > smoothly on v5.4/v5.6/v5.8).
> > > > > >> > >  
> > > > > >> >
> > > > > >> > Any chances to backport this patch to relevant v5.3+ stable branches?  
> > > > > >>
> > > > > >> What patch?  What git commit id needs to be backported?
> > > > > >>  
> > > > > >
> > > > > >The actual patch is:
> > > > > >Commit 6c2b49eb9671  ("net: mvpp2: add mvpp2_phylink_to_port() helper").
> > > > > >
> > > > > >URL for reference:
> > > > > >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/marvell/mvpp2?h=v5.10-rc7&id=6c2b49eb96716e91f202756bfbd3f5fea3b2b172
> > > > > >
> > > > > >Do you think it would be possible to get it merged to v5.3+ stable branches?  
> > > > >
> > > > > Could you explain how that patch fixes anything? It reads like a
> > > > > cleanup.
> > > > >  
> > > >
> > > > Indeed, I am aware of it, but I'm not sure about the best way to fix
> > > > it. In fact the mentioned patch is an unintentional fix. Commit
> > > > 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK
> > > > API") reworked an argument list of mvpp2_mac_config() routine in a way
> > > > that resulted in a NULL pointer dereference when booting the
> > > > Armada7k8k/CN913x with ACPI between 5.3 and 5.8. Part of Russell's
> > > > patch resolves this issue.  
> > >
> > > What part fixes the issue?  I can't see it...
> > >  
> >
> > I re-checked in my setup and here's the smallest part of the original
> > patch, that fixes previously described issue:
> > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > index e98be8372780..9d71a4fe1750 100644
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -4767,6 +4767,11 @@ static void mvpp2_port_copy_mac_addr(struct
> > net_device *dev, struct mvpp2 *priv,
> >         eth_hw_addr_random(dev);
> >  }
> >
> > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> > +{
> > +       return container_of(config, struct mvpp2_port, phylink_config);
> > +}
> > +
> >  static void mvpp2_phylink_validate(struct phylink_config *config,
> >                                    unsigned long *supported,
> >                                    struct phylink_link_state *state)
> > @@ -5105,13 +5110,12 @@ static void mvpp2_gmac_config(struct
> > mvpp2_port *port, unsigned int mode,
> >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> >                              const struct phylink_link_state *state)
> >  {
> > -       struct net_device *dev = to_net_dev(config->dev);
> > -       struct mvpp2_port *port = netdev_priv(dev);
> > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >         bool change_interface = port->phy_interface != state->interface;
> >
> >         /* Check for invalid configuration */
> >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> >                 return;
> >         }
> >
> > @@ -5151,8 +5155,7 @@ static void mvpp2_mac_link_up(struct
> > phylink_config *config,
> >                               int speed, int duplex,
> >                               bool tx_pause, bool rx_pause)
> >  {
> > -       struct net_device *dev = to_net_dev(config->dev);
> > -       struct mvpp2_port *port = netdev_priv(dev);
> > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> >         u32 val;
> >
> >         if (mvpp2_is_xlg(interface)) {
> > @@ -5199,7 +5202,7 @@ static void mvpp2_mac_link_up(struct
> > phylink_config *config,
> >
> >         mvpp2_egress_enable(port);
> >         mvpp2_ingress_enable(port);
> > -       netif_tx_wake_all_queues(dev);
> > +       netif_tx_wake_all_queues(port->dev);
> >  }
> >
> >  static void mvpp2_mac_link_down(struct phylink_config *config,
> > --
> >
> > Do you think there is a point of slicing the original patch or rather
> > cherry-pick as-is?
> >  
> 
> Do you think there is a chance to get the above fix merged to stable (v5.3+)?

We need to work with stable maintainers on this, let's see..

Greg asked for a clear description of what happens, from your 
previous response it sounds like a null-deref in mvpp2_mac_config(). 
Is the netdev -> config -> netdev linking not ready by the time
mvpp2_mac_config() is called?

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Russell King - ARM Linux admin]

On Mon, Dec 21, 2020 at 10:25:39AM -0800, Jakub Kicinski wrote:
> We need to work with stable maintainers on this, let's see..
> 
> Greg asked for a clear description of what happens, from your 
> previous response it sounds like a null-deref in mvpp2_mac_config(). 
> Is the netdev -> config -> netdev linking not ready by the time
> mvpp2_mac_config() is called?

We are going round in circles, so nothing is going to happen.

I stated in detail in one of my emails on the 10th December why the
problem occurs. So, Greg has the description already. There is no
need to repeat it.

Can we please move forward with this?

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

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Jakub Kicinski]

On Mon, 21 Dec 2020 18:30:32 +0000 Russell King - ARM Linux admin wrote:
> On Mon, Dec 21, 2020 at 10:25:39AM -0800, Jakub Kicinski wrote:
> > We need to work with stable maintainers on this, let's see..
> > 
> > Greg asked for a clear description of what happens, from your 
> > previous response it sounds like a null-deref in mvpp2_mac_config(). 
> > Is the netdev -> config -> netdev linking not ready by the time
> > mvpp2_mac_config() is called?  
> 
> We are going round in circles, so nothing is going to happen.
> 
> I stated in detail in one of my emails on the 10th December why the
> problem occurs. So, Greg has the description already. There is no
> need to repeat it.
> 
> Can we please move forward with this?

Well, the fact it wasn't quoted in Marcin's reply and that I didn't
spot it when scanning the 30 email thread should be a clear enough
indication whether pinging threads is a good strategy..

A clear, fresh backport request would had been much more successful 
and easier for Greg to process. If you still don't see a reply in
2 weeks, please just do that.

In case Greg is in fact reading this:


Greg, can we backport: 

6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper")

to 5.4? Quoting Russell:

The problem is that mvpp2_acpi_start() passes an un-initialised
(zeroed) port->phylink_config, as phylink is not used in ACPI setups.

Crash occurs because port->phylink_config.dev (which is a NULL pointer
in this instance) is passed to to_net_dev():

#define to_net_dev(d) container_of(d, struct net_device, dev)

Which then means netdev_priv(dev) attempts to dereference a not-quite
NULL pointer, leading to an oops.


Folks here are willing to provide a more cut down fix if necessary.

Thanks!

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Sasha Levin]

On Mon, Dec 21, 2020 at 10:47:57AM -0800, Jakub Kicinski wrote:
>On Mon, 21 Dec 2020 18:30:32 +0000 Russell King - ARM Linux admin wrote:
>> On Mon, Dec 21, 2020 at 10:25:39AM -0800, Jakub Kicinski wrote:
>> > We need to work with stable maintainers on this, let's see..
>> >
>> > Greg asked for a clear description of what happens, from your
>> > previous response it sounds like a null-deref in mvpp2_mac_config().
>> > Is the netdev -> config -> netdev linking not ready by the time
>> > mvpp2_mac_config() is called?
>>
>> We are going round in circles, so nothing is going to happen.
>>
>> I stated in detail in one of my emails on the 10th December why the
>> problem occurs. So, Greg has the description already. There is no
>> need to repeat it.
>>
>> Can we please move forward with this?
>
>Well, the fact it wasn't quoted in Marcin's reply and that I didn't
>spot it when scanning the 30 email thread should be a clear enough
>indication whether pinging threads is a good strategy..
>
>A clear, fresh backport request would had been much more successful
>and easier for Greg to process. If you still don't see a reply in
>2 weeks, please just do that.
>
>In case Greg is in fact reading this:
>
>
>Greg, can we backport:
>
>6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper")

I've queued it for 5.4, thanks!

-- 
Thanks,
Sasha

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Marcin Wojtas]

pon., 21 gru 2020 o 20:07 Sasha Levin <sashal@kernel.org> napisał(a):
>
> On Mon, Dec 21, 2020 at 10:47:57AM -0800, Jakub Kicinski wrote:
> >On Mon, 21 Dec 2020 18:30:32 +0000 Russell King - ARM Linux admin wrote:
> >> On Mon, Dec 21, 2020 at 10:25:39AM -0800, Jakub Kicinski wrote:
> >> > We need to work with stable maintainers on this, let's see..
> >> >
> >> > Greg asked for a clear description of what happens, from your
> >> > previous response it sounds like a null-deref in mvpp2_mac_config().
> >> > Is the netdev -> config -> netdev linking not ready by the time
> >> > mvpp2_mac_config() is called?
> >>
> >> We are going round in circles, so nothing is going to happen.
> >>
> >> I stated in detail in one of my emails on the 10th December why the
> >> problem occurs. So, Greg has the description already. There is no
> >> need to repeat it.
> >>
> >> Can we please move forward with this?
> >
> >Well, the fact it wasn't quoted in Marcin's reply and that I didn't
> >spot it when scanning the 30 email thread should be a clear enough
> >indication whether pinging threads is a good strategy..
> >
> >A clear, fresh backport request would had been much more successful
> >and easier for Greg to process. If you still don't see a reply in
> >2 weeks, please just do that.
> >
> >In case Greg is in fact reading this:
> >
> >
> >Greg, can we backport:
> >
> >6c2b49eb9671 ("net: mvpp2: add mvpp2_phylink_to_port() helper")
>
> I've queued it for 5.4, thanks!
>

Thank you!

Best regards,
Marcin

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Russell King - ARM Linux admin]

On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
> Hi Greg and Sasha,
> 
> pt., 9 paź 2020 o 05:43 Marcin Wojtas <mw@semihalf.com> napisał(a):
> >
> > Hi,
> >
> > sob., 20 cze 2020 o 11:21 Russell King <rmk+kernel@armlinux.org.uk> napisał(a):
> > >
> > > Add a helper to convert the struct phylink_config pointer passed in
> > > from phylink to the drivers internal struct mvpp2_port.
> > >
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > ---
> > >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +++++++++----------
> > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > index 7653277d03b7..313f5a60a605 100644
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> > >         eth_hw_addr_random(dev);
> > >  }
> > >
> > > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config *config)
> > > +{
> > > +       return container_of(config, struct mvpp2_port, phylink_config);
> > > +}
> > > +
> > >  static void mvpp2_phylink_validate(struct phylink_config *config,
> > >                                    unsigned long *supported,
> > >                                    struct phylink_link_state *state)
> > >  {
> > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > -                                              phylink_config);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >         __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > >
> > >         /* Invalid combinations */
> > > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct mvpp2_port *port,
> > >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > >                                             struct phylink_link_state *state)
> > >  {
> > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > -                                              phylink_config);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >
> > >         if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
> > >                 u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> > > @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > >
> > >  static void mvpp2_mac_an_restart(struct phylink_config *config)
> > >  {
> > > -       struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > -                                              phylink_config);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >         u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > >
> > >         writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> > > @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
> > >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> > >                              const struct phylink_link_state *state)
> > >  {
> > > -       struct net_device *dev = to_net_dev(config->dev);
> > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >         bool change_interface = port->phy_interface != state->interface;
> > >
> > >         /* Check for invalid configuration */
> > >         if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> > > -               netdev_err(dev, "Invalid mode on %s\n", dev->name);
> > > +               netdev_err(port->dev, "Invalid mode on %s\n", port->dev->name);
> > >                 return;
> > >         }
> > >
> > > @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > >                               int speed, int duplex,
> > >                               bool tx_pause, bool rx_pause)
> > >  {
> > > -       struct net_device *dev = to_net_dev(config->dev);
> > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >         u32 val;
> > >
> > >         if (mvpp2_is_xlg(interface)) {
> > > @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
> > >
> > >         mvpp2_egress_enable(port);
> > >         mvpp2_ingress_enable(port);
> > > -       netif_tx_wake_all_queues(dev);
> > > +       netif_tx_wake_all_queues(port->dev);
> > >  }
> > >
> > >  static void mvpp2_mac_link_down(struct phylink_config *config,
> > >                                 unsigned int mode, phy_interface_t interface)
> > >  {
> > > -       struct net_device *dev = to_net_dev(config->dev);
> > > -       struct mvpp2_port *port = netdev_priv(dev);
> > > +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >         u32 val;
> > >
> > >         if (!phylink_autoneg_inband(mode)) {
> > > @@ -5223,7 +5222,7 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
> > >                 }
> > >         }
> > >
> > > -       netif_tx_stop_all_queues(dev);
> > > +       netif_tx_stop_all_queues(port->dev);
> > >         mvpp2_egress_disable(port);
> > >         mvpp2_ingress_disable(port);
> > >
> > > --
> > > 2.20.1
> > >
> >
> > This patch fixes a regression that was introduced in v5.3:
> > Commit 44cc27e43fa3 ("net: phylink: Add struct phylink_config to PHYLINK API")
> >
> > Above results in a NULL pointer dereference when booting the
> > Armada7k8k/CN913x with ACPI between 5.3 and 5.8, which will be
> > problematic especially for the distros using LTSv5.4 and above (the
> > issue was reported on Fedora 32).
> >
> > Please help with backporting to the stable v5.3+ branches (it applies
> > smoothly on v5.4/v5.6/v5.8).
> 
> Any chances to backport this patch to relevant v5.3+ stable branches?

Who are you asking to do the backport? I guess you're asking me to
backport it, but I don't generally follow the stable kernels, and
I don't use ACPI on ARM systems, so I wouldn't be in a position to
test that a backported version fixes the problem you are reporting.

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

[PATCH net-next 3/4] net: mvpp2: add register modification helper

[Russell King]

Add a helper to read-modify-write a register, and use it in the phylink
helpers.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 88 ++++++++++---------
 1 file changed, 46 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 313f5a60a605..375e3c657162 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1132,6 +1132,17 @@ static bool mvpp2_is_xlg(phy_interface_t interface)
 	       interface == PHY_INTERFACE_MODE_XAUI;
 }
 
+static void mvpp2_modify(void __iomem *ptr, u32 mask, u32 set)
+{
+	u32 old, val;
+
+	old = val = readl(ptr);
+	val &= ~mask;
+	val |= set;
+	if (old != val)
+		writel(val, ptr);
+}
+
 static void mvpp22_gop_init_rgmii(struct mvpp2_port *port)
 {
 	struct mvpp2 *priv = port->priv;
@@ -4946,38 +4957,29 @@ static void mvpp2_mac_an_restart(struct phylink_config *config)
 static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
 			     const struct phylink_link_state *state)
 {
-	u32 old_ctrl0, ctrl0;
-	u32 old_ctrl4, ctrl4;
-
-	old_ctrl0 = ctrl0 = readl(port->base + MVPP22_XLG_CTRL0_REG);
-	old_ctrl4 = ctrl4 = readl(port->base + MVPP22_XLG_CTRL4_REG);
-
-	ctrl0 |= MVPP22_XLG_CTRL0_MAC_RESET_DIS;
+	u32 val;
 
+	val = MVPP22_XLG_CTRL0_MAC_RESET_DIS;
 	if (state->pause & MLO_PAUSE_TX)
-		ctrl0 |= MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
-	else
-		ctrl0 &= ~MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
+		val |= MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
 
 	if (state->pause & MLO_PAUSE_RX)
-		ctrl0 |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
-	else
-		ctrl0 &= ~MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
-
-	ctrl4 &= ~(MVPP22_XLG_CTRL4_MACMODSELECT_GMAC |
-		   MVPP22_XLG_CTRL4_EN_IDLE_CHECK);
-	ctrl4 |= MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC;
-
-	if (old_ctrl0 != ctrl0)
-		writel(ctrl0, port->base + MVPP22_XLG_CTRL0_REG);
-	if (old_ctrl4 != ctrl4)
-		writel(ctrl4, port->base + MVPP22_XLG_CTRL4_REG);
-
-	if (!(old_ctrl0 & MVPP22_XLG_CTRL0_MAC_RESET_DIS)) {
-		while (!(readl(port->base + MVPP22_XLG_CTRL0_REG) &
-			 MVPP22_XLG_CTRL0_MAC_RESET_DIS))
-			continue;
-	}
+		val |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
+
+	mvpp2_modify(port->base + MVPP22_XLG_CTRL0_REG,
+		     MVPP22_XLG_CTRL0_MAC_RESET_DIS |
+		     MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN |
+		     MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN, val);
+	mvpp2_modify(port->base + MVPP22_XLG_CTRL4_REG,
+		     MVPP22_XLG_CTRL4_MACMODSELECT_GMAC |
+		     MVPP22_XLG_CTRL4_EN_IDLE_CHECK |
+		     MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC,
+		     MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC);
+
+	/* Wait for reset to deassert */
+	do {
+		val = readl(port->base + MVPP22_XLG_CTRL0_REG);
+	} while (!(val & MVPP22_XLG_CTRL0_MAC_RESET_DIS));
 }
 
 static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
@@ -5157,19 +5159,14 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
 	if (mvpp2_is_xlg(interface)) {
 		if (!phylink_autoneg_inband(mode)) {
-			val = readl(port->base + MVPP22_XLG_CTRL0_REG);
-			val &= ~MVPP22_XLG_CTRL0_FORCE_LINK_DOWN;
-			val |= MVPP22_XLG_CTRL0_FORCE_LINK_PASS;
-			writel(val, port->base + MVPP22_XLG_CTRL0_REG);
+			mvpp2_modify(port->base + MVPP22_XLG_CTRL0_REG,
+				     MVPP22_XLG_CTRL0_FORCE_LINK_DOWN |
+				     MVPP22_XLG_CTRL0_FORCE_LINK_PASS,
+				     MVPP22_XLG_CTRL0_FORCE_LINK_PASS);
 		}
 	} else {
 		if (!phylink_autoneg_inband(mode)) {
-			val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
-			val &= ~(MVPP2_GMAC_FORCE_LINK_DOWN |
-				 MVPP2_GMAC_CONFIG_MII_SPEED |
-				 MVPP2_GMAC_CONFIG_GMII_SPEED |
-				 MVPP2_GMAC_CONFIG_FULL_DUPLEX);
-			val |= MVPP2_GMAC_FORCE_LINK_PASS;
+			val = MVPP2_GMAC_FORCE_LINK_PASS;
 
 			if (speed == SPEED_1000 || speed == SPEED_2500)
 				val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
@@ -5179,20 +5176,27 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 			if (duplex == DUPLEX_FULL)
 				val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
 
-			writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+			mvpp2_modify(port->base + MVPP2_GMAC_AUTONEG_CONFIG,
+				     MVPP2_GMAC_FORCE_LINK_DOWN |
+				     MVPP2_GMAC_FORCE_LINK_PASS |
+				     MVPP2_GMAC_CONFIG_MII_SPEED |
+				     MVPP2_GMAC_CONFIG_GMII_SPEED |
+				     MVPP2_GMAC_CONFIG_FULL_DUPLEX, val);
 		}
 
 		/* We can always update the flow control enable bits;
 		 * these will only be effective if flow control AN
 		 * (MVPP2_GMAC_FLOW_CTRL_AUTONEG) is disabled.
 		 */
-		val = readl(port->base + MVPP22_GMAC_CTRL_4_REG);
-		val &= ~(MVPP22_CTRL4_RX_FC_EN | MVPP22_CTRL4_TX_FC_EN);
+		val = 0;
 		if (tx_pause)
 			val |= MVPP22_CTRL4_TX_FC_EN;
 		if (rx_pause)
 			val |= MVPP22_CTRL4_RX_FC_EN;
-		writel(val, port->base + MVPP22_GMAC_CTRL_4_REG);
+
+		mvpp2_modify(port->base + MVPP22_GMAC_CTRL_4_REG,
+			     MVPP22_CTRL4_RX_FC_EN | MVPP22_CTRL4_TX_FC_EN,
+			     val);
 	}
 
 	mvpp2_port_enable(port);
-- 
2.20.1

[PATCH net-next 4/4] net: mvpp2: set xlg flow control in mvpp2_mac_link_up()

[Russell King]

Set the flow control settings in mvpp2_mac_link_up() for 10G links
just as we do for 1G and slower links. This is now the preferred
location.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 23 +++++++++----------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 375e3c657162..22891f588c8a 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4959,17 +4959,9 @@ static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
 {
 	u32 val;
 
-	val = MVPP22_XLG_CTRL0_MAC_RESET_DIS;
-	if (state->pause & MLO_PAUSE_TX)
-		val |= MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
-
-	if (state->pause & MLO_PAUSE_RX)
-		val |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
-
 	mvpp2_modify(port->base + MVPP22_XLG_CTRL0_REG,
-		     MVPP22_XLG_CTRL0_MAC_RESET_DIS |
-		     MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN |
-		     MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN, val);
+		     MVPP22_XLG_CTRL0_MAC_RESET_DIS,
+		     MVPP22_XLG_CTRL0_MAC_RESET_DIS);
 	mvpp2_modify(port->base + MVPP22_XLG_CTRL4_REG,
 		     MVPP22_XLG_CTRL4_MACMODSELECT_GMAC |
 		     MVPP22_XLG_CTRL4_EN_IDLE_CHECK |
@@ -5159,10 +5151,17 @@ static void mvpp2_mac_link_up(struct phylink_config *config,
 
 	if (mvpp2_is_xlg(interface)) {
 		if (!phylink_autoneg_inband(mode)) {
+			val = MVPP22_XLG_CTRL0_FORCE_LINK_PASS;
+			if (tx_pause)
+				val |= MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
+			if (rx_pause)
+				val |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
+
 			mvpp2_modify(port->base + MVPP22_XLG_CTRL0_REG,
 				     MVPP22_XLG_CTRL0_FORCE_LINK_DOWN |
-				     MVPP22_XLG_CTRL0_FORCE_LINK_PASS,
-				     MVPP22_XLG_CTRL0_FORCE_LINK_PASS);
+				     MVPP22_XLG_CTRL0_FORCE_LINK_PASS |
+				     MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN |
+				     MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN, val);
 		}
 	} else {
 		if (!phylink_autoneg_inband(mode)) {
-- 
2.20.1

Re: [PATCH net-next v2 0/4] Marvell mvpp2 improvements

[David Miller]

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Sat, 20 Jun 2020 10:20:47 +0100

> This series primarily cleans up mvpp2, but also fixes a left-over from
> 91a208f2185a ("net: phylink: propagate resolved link config via
> mac_link_up()").
> 
> Patch 1 introduces some port helpers:
>   mvpp2_port_supports_xlg() - does the port support the XLG MAC
>   mvpp2_port_supports_rgmii() - does the port support RGMII modes
> 
> Patch 2 introduces mvpp2_phylink_to_port(), rather than having repeated
>   open coding of container_of().
> 
> Patch 3 introduces mvpp2_modify(), which reads-modifies-writes a
>   register - I've converted the phylink specific code to use this
>   helper.
> 
> Patch 4 moves the hardware control of the pause modes from
>   mvpp2_xlg_config() (which is called via the phylink_config method)
>   to mvpp2_mac_link_up() - a change that was missed in the above
>   referenced commit.
> 
> v2: remove "inline" in patch 2.

Series applied, thanks Russell.

[PATCH net-next 0/4] Marvell mvpp2 improvements

[Russell King - ARM Linux admin]

Hi,

This series primarily cleans up mvpp2, but also fixes a left-over from
91a208f2185a ("net: phylink: propagate resolved link config via
mac_link_up()").

Patch 1 introduces some port helpers:
  mvpp2_port_supports_xlg() - does the port support the XLG MAC
  mvpp2_port_supports_rgmii() - does the port support RGMII modes

Patch 2 introduces mvpp2_phylink_to_port(), rather than having repeated
  open coding of container_of().

Patch 3 introduces mvpp2_modify(), which reads-modifies-writes a
  register - I've converted the phylink specific code to use this
  helper.

Patch 4 moves the hardware control of the pause modes from
  mvpp2_xlg_config() (which is called via the phylink_config method)
  to mvpp2_mac_link_up() - a change that was missed in the above
  referenced commit.

 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 164 +++++++++++++-----------
 1 file changed, 89 insertions(+), 75 deletions(-)

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

[PATCH net-next 1/4] net: mvpp2: add port support helpers

[Russell King]

The mvpp2 code has tests scattered amongst the code to determine
whether the port supports the XLG, and whether the port supports
RGMII mode.

Rather than having these tests scattered, provide a couple of helper
functions, so that future additions can ensure that they get these
tests correct.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 43 ++++++++++++-------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 24f4d8e0da98..7653277d03b7 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1114,6 +1114,17 @@ mvpp2_shared_interrupt_mask_unmask(struct mvpp2_port *port, bool mask)
 	}
 }
 
+/* Only GOP port 0 has an XLG MAC */
+static bool mvpp2_port_supports_xlg(struct mvpp2_port *port)
+{
+	return port->gop_id == 0;
+}
+
+static bool mvpp2_port_supports_rgmii(struct mvpp2_port *port)
+{
+	return !(port->priv->hw_version == MVPP22 && port->gop_id == 0);
+}
+
 /* Port configuration routines */
 static bool mvpp2_is_xlg(phy_interface_t interface)
 {
@@ -1194,7 +1205,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		if (port->gop_id == 0)
+		if (!mvpp2_port_supports_rgmii(port))
 			goto invalid_conf;
 		mvpp22_gop_init_rgmii(port);
 		break;
@@ -1204,7 +1215,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port)
 		mvpp22_gop_init_sgmii(port);
 		break;
 	case PHY_INTERFACE_MODE_10GBASER:
-		if (port->gop_id != 0)
+		if (!mvpp2_port_supports_xlg(port))
 			goto invalid_conf;
 		mvpp22_gop_init_10gkr(port);
 		break;
@@ -1246,7 +1257,7 @@ static void mvpp22_gop_unmask_irq(struct mvpp2_port *port)
 		writel(val, port->base + MVPP22_GMAC_INT_SUM_MASK);
 	}
 
-	if (port->gop_id == 0) {
+	if (mvpp2_port_supports_xlg(port)) {
 		/* Enable the XLG/GIG irqs for this port */
 		val = readl(port->base + MVPP22_XLG_EXT_INT_MASK);
 		if (mvpp2_is_xlg(port->phy_interface))
@@ -1261,7 +1272,7 @@ static void mvpp22_gop_mask_irq(struct mvpp2_port *port)
 {
 	u32 val;
 
-	if (port->gop_id == 0) {
+	if (mvpp2_port_supports_xlg(port)) {
 		val = readl(port->base + MVPP22_XLG_EXT_INT_MASK);
 		val &= ~(MVPP22_XLG_EXT_INT_MASK_XLG |
 			 MVPP22_XLG_EXT_INT_MASK_GIG);
@@ -1290,7 +1301,7 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
 		writel(val, port->base + MVPP22_GMAC_INT_MASK);
 	}
 
-	if (port->gop_id == 0) {
+	if (mvpp2_port_supports_xlg(port)) {
 		val = readl(port->base + MVPP22_XLG_INT_MASK);
 		val |= MVPP22_XLG_INT_MASK_LINK;
 		writel(val, port->base + MVPP22_XLG_INT_MASK);
@@ -1328,8 +1339,8 @@ static void mvpp2_port_enable(struct mvpp2_port *port)
 {
 	u32 val;
 
-	/* Only GOP port 0 has an XLG MAC */
-	if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface)) {
+	if (mvpp2_port_supports_xlg(port) &&
+	    mvpp2_is_xlg(port->phy_interface)) {
 		val = readl(port->base + MVPP22_XLG_CTRL0_REG);
 		val |= MVPP22_XLG_CTRL0_PORT_EN;
 		val &= ~MVPP22_XLG_CTRL0_MIB_CNT_DIS;
@@ -1346,8 +1357,8 @@ static void mvpp2_port_disable(struct mvpp2_port *port)
 {
 	u32 val;
 
-	/* Only GOP port 0 has an XLG MAC */
-	if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface)) {
+	if (mvpp2_port_supports_xlg(port) &&
+	    mvpp2_is_xlg(port->phy_interface)) {
 		val = readl(port->base + MVPP22_XLG_CTRL0_REG);
 		val &= ~MVPP22_XLG_CTRL0_PORT_EN;
 		writel(val, port->base + MVPP22_XLG_CTRL0_REG);
@@ -2740,7 +2751,8 @@ static irqreturn_t mvpp2_link_status_isr(int irq, void *dev_id)
 
 	mvpp22_gop_mask_irq(port);
 
-	if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface)) {
+	if (mvpp2_port_supports_xlg(port) &&
+	    mvpp2_is_xlg(port->phy_interface)) {
 		val = readl(port->base + MVPP22_XLG_INT_STAT);
 		if (val & MVPP22_XLG_INT_STAT_LINK) {
 			event = true;
@@ -3430,8 +3442,7 @@ static void mvpp22_mode_reconfigure(struct mvpp2_port *port)
 
 	mvpp22_pcs_reset_deassert(port);
 
-	/* Only GOP port 0 has an XLG MAC */
-	if (port->gop_id == 0) {
+	if (mvpp2_port_supports_xlg(port)) {
 		ctrl3 = readl(port->base + MVPP22_XLG_CTRL3_REG);
 		ctrl3 &= ~MVPP22_XLG_CTRL3_MACMODESELECT_MASK;
 
@@ -3443,7 +3454,7 @@ static void mvpp22_mode_reconfigure(struct mvpp2_port *port)
 		writel(ctrl3, port->base + MVPP22_XLG_CTRL3_REG);
 	}
 
-	if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface))
+	if (mvpp2_port_supports_xlg(port) && mvpp2_is_xlg(port->phy_interface))
 		mvpp2_xlg_max_rx_size_set(port);
 	else
 		mvpp2_gmac_max_rx_size_set(port);
@@ -4768,14 +4779,14 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
 	switch (state->interface) {
 	case PHY_INTERFACE_MODE_10GBASER:
 	case PHY_INTERFACE_MODE_XAUI:
-		if (port->gop_id != 0)
+		if (!mvpp2_port_supports_xlg(port))
 			goto empty_set;
 		break;
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		if (port->priv->hw_version == MVPP22 && port->gop_id == 0)
+		if (!mvpp2_port_supports_rgmii(port))
 			goto empty_set;
 		break;
 	default:
@@ -4791,7 +4802,7 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
 	case PHY_INTERFACE_MODE_10GBASER:
 	case PHY_INTERFACE_MODE_XAUI:
 	case PHY_INTERFACE_MODE_NA:
-		if (port->gop_id == 0) {
+		if (mvpp2_port_supports_xlg(port)) {
 			phylink_set(mask, 10000baseT_Full);
 			phylink_set(mask, 10000baseCR_Full);
 			phylink_set(mask, 10000baseSR_Full);
-- 
2.20.1