[PATCH net-next 00/15] net: mvpp2: fixes and improvements

[PATCH net-next 00/15] net: mvpp2: fixes and improvements

[Antoine Tenart]

Hello,

This series aims to improve the Marvell PPv2 driver and to fix various
issues we encountered while testing the ports in many different
configurations. The series is based on top of Russell PPv2 phylink
rework and improvement.

I'm not sending a v2 of the previous fixes series as half the patches
are not the same and lots of development happened in between.

While this series contains fixes, it's sent to net-next as it is based
on top of Russell patches that were merged into net-next. I'm also
aiming at net-next as the series reworks critical paths of the PPv2
driver, such as the reset handling of various blocks, to let more weeks
for users to tests and for possible fixes to be sent before it lands
into a stable kernel version.

The series is divided into three parts:

- Patches 1 to 3 are cosmetic changes, sent alongside the series, as I
  saw these small issues while working on this.

- Patches 5 to 8 are fixing (or improving) individual issues that we
  found while testing PPv2.1 and PPv2.2 ports while using various
  interfaces.

  Notable fixes are we support back RGMII interfaces (on both PPv2.1 and
  PPv2.2), as their support was broken by previous patches. We also
  reworked the RXQ computation as the RXQ assignment was not checking
  the maximum number of RXQ available, and was broken for PPv2.1.

- As discussed in a previous fixes series, patches 9 to 14 rework the
  way blocks are set in reset in the PPv2 engine (plus related changes).

  There are four blocks we want to control the reset status: two MAC
  (GMAC and XLG MAC) and two PCS (MPCS and XPCS). The XLG MAC is used
  for 10G connexions and uses the MPCS or the XPCS depending on the mode
  used (10GKR / XAUI / RXAUI) and the GMAC is used for the other modes.

  The idea is to set all blocks in reset by default, and when not used,
  and to de-assert the reset only when a block is used. There are four
  cases to take in account:

  1. Boot time: all four blocks should be put in reset, as we do not
     know their initial state (configured by the firmware/bootloader).

  2. Link up: only the blocks used by a given mode should be put out of
     reset (eg. 10GKR uses the XLG MAC and the MPCS).

  3. Mode reconfiguration: some ports may support mode reconfiguration,
     and switching between the GMAC and the XLG MAC (or between the two
     PCS). All blocks should be put in reset, and only the one used
     should be put out of reset.

  4. Link down: all four blocks are put in reset.

Thanks!
Antoine

Antoine Tenart (15):
  net: mvpp2: fix a typo in the header
  net: mvpp2: update the port documentation regarding the GoP
  net: mvpp2: fix alignment of MVPP2_GMAC_CONFIG_MII_SPEED definition
  net: mvpp2: a port can be disabled even if we use the link IRQ
  net: mvpp2: reconfiguring the port interface is PPv2.2 specific
  net: mvpp2: fix validate for PPv2.1
  net: mvpp2: fix the computation of the RXQs
  net: mvpp2: some AN fields require the link to be down when updated
  net: mvpp2: always disable both MACs when disabling a port
  net: mvpp2: only update the XLG configuration when needed
  net: mvpp2: force the XLG MAC link up or down when not using in-band
  net: mvpp2: rework the XLG MAC reset handling
  net: mvpp2: reset the MACs when reconfiguring a port
  net: mvpp2: set the XPCS and MPCS in reset when not used
  net: mvpp2: set the GMAC, XLG MAC, XPCS and MPCS in reset when a port
    is down

 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  11 +-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 210 +++++++++++++-----
 2 files changed, 156 insertions(+), 65 deletions(-)

-- 
2.20.1

[PATCH net-next 01/15] net: mvpp2: fix a typo in the header

[Antoine Tenart]

This cosmetic patch fixes a typo made in a comment in the Marvell PPv2
Ethernet driver header.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 96e3f0669032..83fb2b03e789 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -549,7 +549,7 @@
 #define MVPP2_MAX_TSO_SEGS		300
 #define MVPP2_MAX_SKB_DESCS		(MVPP2_MAX_TSO_SEGS * 2 + MAX_SKB_FRAGS)
 
-/* Dfault number of RXQs in use */
+/* Default number of RXQs in use */
 #define MVPP2_DEFAULT_RXQ		1
 
 /* Max number of Rx descriptors */
-- 
2.20.1

[PATCH net-next 02/15] net: mvpp2: update the port documentation regarding the GoP

[Antoine Tenart]

The Marvell PPv2 port structure stores the GoP id of a given port. This
information is specific to PPv2.2, but cannot be used by PPv2.1. Update
its comment to denote this specificity.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 83fb2b03e789..9b05fcbc6953 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -803,7 +803,7 @@ struct mvpp2_port {
 	u8 id;
 
 	/* Index of the port from the "group of ports" complex point
-	 * of view
+	 * of view. This is specific to PPv2.2.
 	 */
 	int gop_id;
 
-- 
2.20.1

[PATCH net-next 03/15] net: mvpp2: fix alignment of MVPP2_GMAC_CONFIG_MII_SPEED definition

[Antoine Tenart]

Cosmetic patch fix the alignment of the MVPP2_GMAC_CONFIG_MII_SPEED
macro definition.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 9b05fcbc6953..17ff330cce5f 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -389,7 +389,7 @@
 #define     MVPP2_GMAC_IN_BAND_AUTONEG		BIT(2)
 #define     MVPP2_GMAC_IN_BAND_AUTONEG_BYPASS	BIT(3)
 #define     MVPP2_GMAC_IN_BAND_RESTART_AN	BIT(4)
-#define     MVPP2_GMAC_CONFIG_MII_SPEED	BIT(5)
+#define     MVPP2_GMAC_CONFIG_MII_SPEED		BIT(5)
 #define     MVPP2_GMAC_CONFIG_GMII_SPEED	BIT(6)
 #define     MVPP2_GMAC_AN_SPEED_EN		BIT(7)
 #define     MVPP2_GMAC_FC_ADV_EN		BIT(9)
-- 
2.20.1

[PATCH net-next 04/15] net: mvpp2: a port can be disabled even if we use the link IRQ

[Antoine Tenart]

We had a check in the mvpp2_mac_link_down() function (called by phylink)
to avoid disabling the port when link interrupts are used. It turned out
the interrupt can still be used with the port disabled. We can thus
remove this check.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 37d3c97c0b9b..71a3fdb76b12 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4739,13 +4739,6 @@ static void mvpp2_mac_link_down(struct net_device *dev, unsigned int mode,
 	mvpp2_egress_disable(port);
 	mvpp2_ingress_disable(port);
 
-	/* When using link interrupts to notify phylink of a MAC state change,
-	 * we do not want the port to be disabled (we want to receive further
-	 * interrupts, to be notified when the port will have a link later).
-	 */
-	if (!port->has_phy)
-		return;
-
 	mvpp2_port_disable(port);
 }
 
-- 
2.20.1

[PATCH net-next 05/15] net: mvpp2: reconfiguring the port interface is PPv2.2 specific

[Antoine Tenart]

This patch adds a check on the PPv2 version in-use not to reconfigure
the port mode when an interface is updated when using PPv2.1 as the
functions called are PPv2.2 specific.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 71a3fdb76b12..204cae75a05b 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4673,16 +4673,14 @@ static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
 
 	/* Make sure the port is disabled when reconfiguring the mode */
 	mvpp2_port_disable(port);
-	if (change_interface) {
+	if (port->priv->hw_version == MVPP22 && change_interface) {
 		mvpp22_gop_mask_irq(port);
 
-		if (port->priv->hw_version == MVPP22) {
-			port->phy_interface = state->interface;
+		port->phy_interface = state->interface;
 
-			/* Reconfigure the serdes lanes */
-			phy_power_off(port->comphy);
-			mvpp22_mode_reconfigure(port);
-		}
+		/* Reconfigure the serdes lanes */
+		phy_power_off(port->comphy);
+		mvpp22_mode_reconfigure(port);
 	}
 
 	/* mac (re)configuration */
@@ -4696,7 +4694,7 @@ static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
 	if (port->priv->hw_version == MVPP21 && port->flags & MVPP2_F_LOOPBACK)
 		mvpp2_port_loopback_set(port, state);
 
-	if (change_interface)
+	if (port->priv->hw_version == MVPP22 && change_interface)
 		mvpp22_gop_unmask_irq(port);
 
 	mvpp2_port_enable(port);
-- 
2.20.1

[PATCH net-next 06/15] net: mvpp2: fix validate for PPv2.1

[Antoine Tenart]

The Phylink validate function is the Marvell PPv2 driver makes a check
on the GoP id. This is valid an has to be done when using PPv2.2 engines
but makes no sense when using PPv2.1. The check done when using an RGMII
interface makes sure the GoP id is not 0, but this breaks PPv2.1. Fixes
it.

Fixes: 0fb628f0f250 ("net: mvpp2: fix phylink handling of invalid PHY modes")
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 204cae75a05b..24cee6cbe309 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4364,7 +4364,7 @@ static void mvpp2_phylink_validate(struct net_device *dev,
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		if (port->gop_id == 0)
+		if (port->priv->hw_version == MVPP22 && port->gop_id == 0)
 			goto empty_set;
 		break;
 	default:
-- 
2.20.1

[PATCH net-next 07/15] net: mvpp2: fix the computation of the RXQs

[Antoine Tenart]

The patch fixes the computation of RXQs being used by the PPv2 driver,
which is set depending on the PPv2 engine version and the queue mode
used. There are three cases:

- PPv2.1: 1 RXQ per CPU.
- PPV2.2 with MVPP2_QDIST_MULTI_MODE: 1 RXQ per CPU.
- PPv2.2 with MVPP2_QDIST_SINGLE_MODE: 1 RXQ is shared between the CPUs.

The PPv2 engine supports a maximum of 32 queues per port. This patch
adds a check so that we do not overstep this maximum.

It appeared the calculation was broken for PPv2.1 engines since
f8c6ba8424b0, as PPv2.1 ports ended up with a single RXQ while they
needed 4. This patch fixes it.

Fixes: f8c6ba8424b0 ("net: mvpp2: use only one rx queue per port per CPU")
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  4 ++--
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 23 ++++++++++++-------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 17ff330cce5f..687e011de5ef 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -549,8 +549,8 @@
 #define MVPP2_MAX_TSO_SEGS		300
 #define MVPP2_MAX_SKB_DESCS		(MVPP2_MAX_TSO_SEGS * 2 + MAX_SKB_FRAGS)
 
-/* Default number of RXQs in use */
-#define MVPP2_DEFAULT_RXQ		1
+/* Max number of RXQs per port */
+#define MVPP2_PORT_MAX_RXQ		32
 
 /* Max number of Rx descriptors */
 #define MVPP2_MAX_RXD_MAX		1024
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 24cee6cbe309..9c6200a59910 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4062,8 +4062,8 @@ static int mvpp2_multi_queue_vectors_init(struct mvpp2_port *port,
 			snprintf(irqname, sizeof(irqname), "hif%d", i);
 
 		if (queue_mode == MVPP2_QDIST_MULTI_MODE) {
-			v->first_rxq = i * MVPP2_DEFAULT_RXQ;
-			v->nrxqs = MVPP2_DEFAULT_RXQ;
+			v->first_rxq = i;
+			v->nrxqs = 1;
 		} else if (queue_mode == MVPP2_QDIST_SINGLE_MODE &&
 			   i == (port->nqvecs - 1)) {
 			v->first_rxq = 0;
@@ -4156,8 +4156,7 @@ static int mvpp2_port_init(struct mvpp2_port *port)
 	    MVPP2_MAX_PORTS * priv->max_port_rxqs)
 		return -EINVAL;
 
-	if (port->nrxqs % MVPP2_DEFAULT_RXQ ||
-	    port->nrxqs > priv->max_port_rxqs || port->ntxqs > MVPP2_MAX_TXQ)
+	if (port->nrxqs > priv->max_port_rxqs || port->ntxqs > MVPP2_MAX_TXQ)
 		return -EINVAL;
 
 	/* Disable port */
@@ -4778,10 +4777,18 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	}
 
 	ntxqs = MVPP2_MAX_TXQ;
-	if (priv->hw_version == MVPP22 && queue_mode == MVPP2_QDIST_MULTI_MODE)
-		nrxqs = MVPP2_DEFAULT_RXQ * num_possible_cpus();
-	else
-		nrxqs = MVPP2_DEFAULT_RXQ;
+	if (priv->hw_version == MVPP22 && queue_mode == MVPP2_QDIST_SINGLE_MODE) {
+		nrxqs = 1;
+	} else {
+		/* According to the PPv2.2 datasheet and our experiments on
+		 * PPv2.1, RX queues have an allocation granularity of 4 (when
+		 * more than a single one on PPv2.2).
+		 * Round up to nearest multiple of 4.
+		 */
+		nrxqs = (num_possible_cpus() + 3) & ~0x3;
+		if (nrxqs > MVPP2_PORT_MAX_RXQ)
+			nrxqs = MVPP2_PORT_MAX_RXQ;
+	}
 
 	dev = alloc_etherdev_mqs(sizeof(*port), ntxqs, nrxqs);
 	if (!dev)
-- 
2.20.1

[PATCH net-next 08/15] net: mvpp2: some AN fields require the link to be down when updated

[Antoine Tenart]

The GMAC configuration helper modifies values in the auto-negotiation
register. Some of its values require the port to be forced down when
modifying their values. This patches fixes the check made on the bit to
be updated in this register, so that the port is forced down when
needed. This fix cases where some of those parameters were updated, but
not taken into account, such as when using RGMII interfaces.

Fixes: d14e078f23cc ("net: marvell: mvpp2: only reprogram what is necessary on mac_config")
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 9c6200a59910..11284918d907 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4627,9 +4627,19 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 		}
 	}
 
+/* Some fields of the auto-negotiation register require the port to be down when
+ * their value is updated.
+ */
+#define MVPP2_GMAC_AN_PORT_DOWN_MASK	\
+		(MVPP2_GMAC_IN_BAND_AUTONEG | \
+		 MVPP2_GMAC_IN_BAND_AUTONEG_BYPASS | \
+		 MVPP2_GMAC_CONFIG_MII_SPEED | MVPP2_GMAC_CONFIG_GMII_SPEED | \
+		 MVPP2_GMAC_AN_SPEED_EN | MVPP2_GMAC_CONFIG_FULL_DUPLEX | \
+		 MVPP2_GMAC_AN_DUPLEX_EN)
+
 	if ((old_ctrl0 ^ ctrl0) & MVPP2_GMAC_PORT_TYPE_MASK ||
 	    (old_ctrl2 ^ ctrl2) & MVPP2_GMAC_INBAND_AN_MASK ||
-	    (old_an ^ an) & MVPP2_GMAC_IN_BAND_AUTONEG) {
+	    (old_an ^ an) & MVPP2_GMAC_AN_PORT_DOWN_MASK) {
 		/* Force link down */
 		old_an &= ~MVPP2_GMAC_FORCE_LINK_PASS;
 		old_an |= MVPP2_GMAC_FORCE_LINK_DOWN;
-- 
2.20.1

[PATCH net-next 09/15] net: mvpp2: always disable both MACs when disabling a port

[Antoine Tenart]

This patch modifies the port_disable() helper to always disable both the
GMAC and the XLG MAC when called. At boot time we do not know of a port
was enabled in the firmware/bootloader, and if so what mode was used
(hence which of the two MACs was used).

This also help in implementing a logic where all blocks are disabled
when not used, and only enabled regarding the current mode used on a
given port.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 11284918d907..b27966355df9 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1214,11 +1214,11 @@ static void mvpp2_port_disable(struct mvpp2_port *port)
 		/* Disable & reset should be done separately */
 		val &= ~MVPP22_XLG_CTRL0_MAC_RESET_DIS;
 		writel(val, port->base + MVPP22_XLG_CTRL0_REG);
-	} else {
-		val = readl(port->base + MVPP2_GMAC_CTRL_0_REG);
-		val &= ~(MVPP2_GMAC_PORT_EN_MASK);
-		writel(val, port->base + MVPP2_GMAC_CTRL_0_REG);
 	}
+
+	val = readl(port->base + MVPP2_GMAC_CTRL_0_REG);
+	val &= ~(MVPP2_GMAC_PORT_EN_MASK);
+	writel(val, port->base + MVPP2_GMAC_CTRL_0_REG);
 }
 
 /* Set IEEE 802.3x Flow Control Xon Packet Transmission Mode */
-- 
2.20.1

[PATCH net-next 10/15] net: mvpp2: only update the XLG configuration when needed

[Antoine Tenart]

This patch improves the XLG configuration function, to only update the
XLG configuration register when a change is needed. This helps not
writing over and over the same XLG configuration each time phylink
request the MAC to be configured. This mimics the GMAC configuration
function.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index b27966355df9..59ee9e7545b0 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4506,10 +4506,11 @@ static void mvpp2_mac_an_restart(struct net_device *dev)
 static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
 			     const struct phylink_link_state *state)
 {
-	u32 ctrl0, ctrl4;
+	u32 old_ctrl0, ctrl0;
+	u32 old_ctrl4, ctrl4;
 
-	ctrl0 = readl(port->base + MVPP22_XLG_CTRL0_REG);
-	ctrl4 = readl(port->base + MVPP22_XLG_CTRL4_REG);
+	old_ctrl0 = ctrl0 = readl(port->base + MVPP22_XLG_CTRL0_REG);
+	old_ctrl4 = ctrl4 = readl(port->base + MVPP22_XLG_CTRL4_REG);
 
 	if (state->pause & MLO_PAUSE_TX)
 		ctrl0 |= MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
@@ -4525,8 +4526,10 @@ static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
 	ctrl4 |= MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC |
 		 MVPP22_XLG_CTRL4_EN_IDLE_CHECK;
 
-	writel(ctrl0, port->base + MVPP22_XLG_CTRL0_REG);
-	writel(ctrl4, port->base + MVPP22_XLG_CTRL4_REG);
+	if (old_ctrl0 != ctrl0)
+		writel(ctrl0, port->base + MVPP22_XLG_CTRL0_REG);
+	if (old_ctrl4 != ctrl4)
+		writel(ctrl4, port->base + MVPP22_XLG_CTRL4_REG);
 }
 
 static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
-- 
2.20.1

[PATCH net-next 11/15] net: mvpp2: force the XLG MAC link up or down when not using in-band

[Antoine Tenart]

This patch force the XLG MAC link state in the phylink link_up() and
link_down() helpers when not using in-band auto-negotiation. This mimics
what's already done for the GMAC and follows what's advised in the
phylink documentation.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  2 ++
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 35 +++++++++++++------
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 687e011de5ef..c9edeac9ec01 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -430,6 +430,8 @@
 #define MVPP22_XLG_CTRL0_REG			0x100
 #define     MVPP22_XLG_CTRL0_PORT_EN		BIT(0)
 #define     MVPP22_XLG_CTRL0_MAC_RESET_DIS	BIT(1)
+#define     MVPP22_XLG_CTRL0_FORCE_LINK_DOWN	BIT(2)
+#define     MVPP22_XLG_CTRL0_FORCE_LINK_PASS	BIT(3)
 #define     MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN	BIT(7)
 #define     MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN	BIT(8)
 #define     MVPP22_XLG_CTRL0_MIB_CNT_DIS	BIT(14)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 59ee9e7545b0..9138baa392d8 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4685,6 +4685,7 @@ static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
 
 	/* Make sure the port is disabled when reconfiguring the mode */
 	mvpp2_port_disable(port);
+
 	if (port->priv->hw_version == MVPP22 && change_interface) {
 		mvpp22_gop_mask_irq(port);
 
@@ -4718,11 +4719,18 @@ static void mvpp2_mac_link_up(struct net_device *dev, unsigned int mode,
 	struct mvpp2_port *port = netdev_priv(dev);
 	u32 val;
 
-	if (!phylink_autoneg_inband(mode) && !mvpp2_is_xlg(interface)) {
-		val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
-		val &= ~MVPP2_GMAC_FORCE_LINK_DOWN;
-		val |= MVPP2_GMAC_FORCE_LINK_PASS;
-		writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+	if (!phylink_autoneg_inband(mode)) {
+		if (mvpp2_is_xlg(interface)) {
+			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);
+		} else {
+			val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+			val &= ~MVPP2_GMAC_FORCE_LINK_DOWN;
+			val |= MVPP2_GMAC_FORCE_LINK_PASS;
+			writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+		}
 	}
 
 	mvpp2_port_enable(port);
@@ -4738,11 +4746,18 @@ static void mvpp2_mac_link_down(struct net_device *dev, unsigned int mode,
 	struct mvpp2_port *port = netdev_priv(dev);
 	u32 val;
 
-	if (!phylink_autoneg_inband(mode) && !mvpp2_is_xlg(interface)) {
-		val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
-		val &= ~MVPP2_GMAC_FORCE_LINK_PASS;
-		val |= MVPP2_GMAC_FORCE_LINK_DOWN;
-		writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+	if (!phylink_autoneg_inband(mode)) {
+		if (mvpp2_is_xlg(interface)) {
+			val = readl(port->base + MVPP22_XLG_CTRL0_REG);
+			val &= ~MVPP22_XLG_CTRL0_FORCE_LINK_PASS;
+			val |= MVPP22_XLG_CTRL0_FORCE_LINK_DOWN;
+			writel(val, port->base + MVPP22_XLG_CTRL0_REG);
+		} else {
+			val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+			val &= ~MVPP2_GMAC_FORCE_LINK_PASS;
+			val |= MVPP2_GMAC_FORCE_LINK_DOWN;
+			writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+		}
 	}
 
 	netif_tx_stop_all_queues(dev);
-- 
2.20.1

[PATCH net-next 12/15] net: mvpp2: rework the XLG MAC reset handling

[Antoine Tenart]

This patch reworks the way the XLG MAC is set in reset: the XLG MAC is
set in reset at probe time and taken out of this state only when used.
The idea is to move forward a situation where only the blocks used are
taken out of reset. This also has the effect to handle the GMAC and the
XLG MAC in a similar way (the GMAC already is set in reset at boot
time).

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 27 ++++++++++++-------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 9138baa392d8..13c4fcbc4269 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1189,8 +1189,7 @@ static void mvpp2_port_enable(struct mvpp2_port *port)
 	/* Only GOP port 0 has an XLG MAC */
 	if (port->gop_id == 0 && mvpp2_is_xlg(port->phy_interface)) {
 		val = readl(port->base + MVPP22_XLG_CTRL0_REG);
-		val |= MVPP22_XLG_CTRL0_PORT_EN |
-		       MVPP22_XLG_CTRL0_MAC_RESET_DIS;
+		val |= MVPP22_XLG_CTRL0_PORT_EN;
 		val &= ~MVPP22_XLG_CTRL0_MIB_CNT_DIS;
 		writel(val, port->base + MVPP22_XLG_CTRL0_REG);
 	} else {
@@ -1210,10 +1209,6 @@ static void mvpp2_port_disable(struct mvpp2_port *port)
 		val = readl(port->base + MVPP22_XLG_CTRL0_REG);
 		val &= ~MVPP22_XLG_CTRL0_PORT_EN;
 		writel(val, port->base + MVPP22_XLG_CTRL0_REG);
-
-		/* Disable & reset should be done separately */
-		val &= ~MVPP22_XLG_CTRL0_MAC_RESET_DIS;
-		writel(val, port->base + MVPP22_XLG_CTRL0_REG);
 	}
 
 	val = readl(port->base + MVPP2_GMAC_CTRL_0_REG);
@@ -1370,10 +1365,10 @@ static int mvpp2_ethtool_get_sset_count(struct net_device *dev, int sset)
 	return -EOPNOTSUPP;
 }
 
-static void mvpp2_port_reset(struct mvpp2_port *port)
+static void mvpp2_mac_reset(struct mvpp2_port *port)
 {
-	u32 val;
 	unsigned int i;
+	u32 val;
 
 	/* Read the GOP statistics to reset the hardware counters */
 	for (i = 0; i < ARRAY_SIZE(mvpp2_ethtool_regs); i++)
@@ -1382,6 +1377,12 @@ static void mvpp2_port_reset(struct mvpp2_port *port)
 	val = readl(port->base + MVPP2_GMAC_CTRL_2_REG) |
 	      MVPP2_GMAC_PORT_RESET_MASK;
 	writel(val, port->base + MVPP2_GMAC_CTRL_2_REG);
+
+	if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
+		val = readl(port->base + MVPP22_XLG_CTRL0_REG) &
+		      ~MVPP22_XLG_CTRL0_MAC_RESET_DIS;
+		writel(val, port->base + MVPP22_XLG_CTRL0_REG);
+	}
 }
 
 /* Change maximum receive size of the port */
@@ -4512,6 +4513,8 @@ static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
 	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;
+
 	if (state->pause & MLO_PAUSE_TX)
 		ctrl0 |= MVPP22_XLG_CTRL0_TX_FLOW_CTRL_EN;
 	else
@@ -4530,6 +4533,12 @@ static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
 		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;
+	}
 }
 
 static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
@@ -4947,7 +4956,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 
 	mvpp2_port_periodic_xon_disable(port);
 
-	mvpp2_port_reset(port);
+	mvpp2_mac_reset(port);
 
 	port->pcpu = alloc_percpu(struct mvpp2_port_pcpu);
 	if (!port->pcpu) {
-- 
2.20.1

[PATCH net-next 13/15] net: mvpp2: reset the MACs when reconfiguring a port

[Antoine Tenart]

This patch makes sure both PPv2 MACs (GMAC + XLG MAC) are set in reset
while a port is reconfigured. This is done so that we make sure a MAC is
in a reset state when not used, as only one of the two will be set out
of reset after the port is configured properly.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 13c4fcbc4269..36e709a4e7d9 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3137,6 +3137,9 @@ static void mvpp22_mode_reconfigure(struct mvpp2_port *port)
 {
 	u32 ctrl3;
 
+	/* Set the GMAC & XLG MAC in reset */
+	mvpp2_mac_reset(port);
+
 	/* comphy reconfiguration */
 	mvpp22_comphy_init(port);
 
-- 
2.20.1

[PATCH net-next 14/15] net: mvpp2: set the XPCS and MPCS in reset when not used

[Antoine Tenart]

This patch sets both the XPCS and MPCS blocks in reset when they aren't
used. This is done both at boot time and when reconfiguring a port mode.
The advantage now is that only the PCS used is set out of reset when the
port is configured (10GKR uses the MCPS while RXAUI uses the XPCS).

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  1 +
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 62 ++++++++++++++++---
 2 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index c9edeac9ec01..ff0f4c503f53 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -483,6 +483,7 @@
 /* XPCS registers. PPv2.2 only */
 #define MVPP22_XPCS_BASE(port)			(0x7400 + (port) * 0x1000)
 #define MVPP22_XPCS_CFG0			0x0
+#define     MVPP22_XPCS_CFG0_RESET_DIS		BIT(0)
 #define     MVPP22_XPCS_CFG0_PCS_MODE(n)	((n) << 3)
 #define     MVPP22_XPCS_CFG0_ACTIVE_LANE(n)	((n) << 5)
 
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 36e709a4e7d9..29d32cb3d52b 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1016,27 +1016,20 @@ static void mvpp22_gop_init_10gkr(struct mvpp2_port *port)
 	void __iomem *xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
 	u32 val;
 
-	/* XPCS */
 	val = readl(xpcs + MVPP22_XPCS_CFG0);
 	val &= ~(MVPP22_XPCS_CFG0_PCS_MODE(0x3) |
 		 MVPP22_XPCS_CFG0_ACTIVE_LANE(0x3));
 	val |= MVPP22_XPCS_CFG0_ACTIVE_LANE(2);
 	writel(val, xpcs + MVPP22_XPCS_CFG0);
 
-	/* MPCS */
 	val = readl(mpcs + MVPP22_MPCS_CTRL);
 	val &= ~MVPP22_MPCS_CTRL_FWD_ERR_CONN;
 	writel(val, mpcs + MVPP22_MPCS_CTRL);
 
 	val = readl(mpcs + MVPP22_MPCS_CLK_RESET);
-	val &= ~(MVPP22_MPCS_CLK_RESET_DIV_RATIO(0x7) | MAC_CLK_RESET_MAC |
-		 MAC_CLK_RESET_SD_RX | MAC_CLK_RESET_SD_TX);
+	val &= ~MVPP22_MPCS_CLK_RESET_DIV_RATIO(0x7);
 	val |= MVPP22_MPCS_CLK_RESET_DIV_RATIO(1);
 	writel(val, mpcs + MVPP22_MPCS_CLK_RESET);
-
-	val &= ~MVPP22_MPCS_CLK_RESET_DIV_SET;
-	val |= MAC_CLK_RESET_MAC | MAC_CLK_RESET_SD_RX | MAC_CLK_RESET_SD_TX;
-	writel(val, mpcs + MVPP22_MPCS_CLK_RESET);
 }
 
 static int mvpp22_gop_init(struct mvpp2_port *port)
@@ -1385,6 +1378,53 @@ static void mvpp2_mac_reset(struct mvpp2_port *port)
 	}
 }
 
+static void mvpp22_pcs_reset(struct mvpp2_port *port)
+{
+	struct mvpp2 *priv = port->priv;
+	void __iomem *mpcs = priv->iface_base + MVPP22_MPCS_BASE(port->gop_id);
+	void __iomem *xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
+	u32 val;
+
+	if (port->priv->hw_version != MVPP22 || port->gop_id != 0)
+		return;
+
+	val = readl(mpcs + MVPP22_MPCS_CLK_RESET);
+	val &= ~(MAC_CLK_RESET_MAC | MAC_CLK_RESET_SD_RX | MAC_CLK_RESET_SD_TX);
+	val |= MVPP22_MPCS_CLK_RESET_DIV_SET;
+	writel(val, mpcs + MVPP22_MPCS_CLK_RESET);
+
+	val = readl(xpcs + MVPP22_XPCS_CFG0);
+	writel(val & ~MVPP22_XPCS_CFG0_RESET_DIS, xpcs + MVPP22_XPCS_CFG0);
+}
+
+static void mvpp22_pcs_unreset(struct mvpp2_port *port)
+{
+	struct mvpp2 *priv = port->priv;
+	void __iomem *mpcs = priv->iface_base + MVPP22_MPCS_BASE(port->gop_id);
+	void __iomem *xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
+	u32 val;
+
+	if (port->priv->hw_version != MVPP22 || port->gop_id != 0)
+		return;
+
+	switch (port->phy_interface) {
+	case PHY_INTERFACE_MODE_10GKR:
+		val = readl(mpcs + MVPP22_MPCS_CLK_RESET);
+		val |= MAC_CLK_RESET_MAC | MAC_CLK_RESET_SD_RX |
+		       MAC_CLK_RESET_SD_TX;
+		val &= ~MVPP22_MPCS_CLK_RESET_DIV_SET;
+		writel(val, mpcs + MVPP22_MPCS_CLK_RESET);
+		break;
+	case PHY_INTERFACE_MODE_XAUI:
+	case PHY_INTERFACE_MODE_RXAUI:
+		val = readl(xpcs + MVPP22_XPCS_CFG0);
+		writel(val | MVPP22_XPCS_CFG0_RESET_DIS, xpcs + MVPP22_XPCS_CFG0);
+		break;
+	default:
+		break;
+	}
+}
+
 /* Change maximum receive size of the port */
 static inline void mvpp2_gmac_max_rx_size_set(struct mvpp2_port *port)
 {
@@ -3140,12 +3180,17 @@ static void mvpp22_mode_reconfigure(struct mvpp2_port *port)
 	/* Set the GMAC & XLG MAC in reset */
 	mvpp2_mac_reset(port);
 
+	/* Set the MPCS and XPCS in reset */
+	mvpp22_pcs_reset(port);
+
 	/* comphy reconfiguration */
 	mvpp22_comphy_init(port);
 
 	/* gop reconfiguration */
 	mvpp22_gop_init(port);
 
+	mvpp22_pcs_unreset(port);
+
 	/* Only GOP port 0 has an XLG MAC */
 	if (port->gop_id == 0) {
 		ctrl3 = readl(port->base + MVPP22_XLG_CTRL3_REG);
@@ -4960,6 +5005,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	mvpp2_port_periodic_xon_disable(port);
 
 	mvpp2_mac_reset(port);
+	mvpp22_pcs_reset(port);
 
 	port->pcpu = alloc_percpu(struct mvpp2_port_pcpu);
 	if (!port->pcpu) {
-- 
2.20.1

[PATCH net-next 15/15] net: mvpp2: set the GMAC, XLG MAC, XPCS and MPCS in reset when a port is down

[Antoine Tenart]

This patch adds calls in the stop() helper to ensure both MACs and
both PCS blocks are set in reset when the user manually sets a port
down. This is done so that we have the exact same block reset states at
boot time and when a port is set down.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 29d32cb3d52b..c10fd894c86f 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3257,6 +3257,7 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
 
 	if (port->phylink)
 		phylink_stop(port->phylink);
+
 	phy_power_off(port->comphy);
 }
 
@@ -3520,6 +3521,9 @@ static int mvpp2_stop(struct net_device *dev)
 	mvpp2_cleanup_rxqs(port);
 	mvpp2_cleanup_txqs(port);
 
+	mvpp2_mac_reset(port);
+	mvpp22_pcs_reset(port);
+
 	cancel_delayed_work_sync(&port->stats_work);
 
 	return 0;
-- 
2.20.1

RE: [EXT] [PATCH net-next 15/15] net: mvpp2: set the GMAC, XLG MAC, XPCS and MPCS in reset when a port is down

[Yan Markman]

Hi

The eth-down/stop may concurrent with pre-scheduled stats_work.
So it seems better to cancel stats_work first and then make resetS.

Yan Markman

-----Original Message-----
From: Antoine Tenart <antoine.tenart@bootlin.com> 
Sent: Thursday, February 28, 2019 3:21 PM
To: davem@davemloft.net; linux@armlinux.org.uk
Cc: Antoine Tenart <antoine.tenart@bootlin.com>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; thomas.petazzoni@bootlin.com; maxime.chevallier@bootlin.com; gregory.clement@bootlin.com; miquel.raynal@bootlin.com; Nadav Haklai <nadavh@marvell.com>; Stefan Chulski <stefanc@marvell.com>; Yan Markman <ymarkman@marvell.com>; mw@semihalf.com
Subject: [EXT] [PATCH net-next 15/15] net: mvpp2: set the GMAC, XLG MAC, XPCS and MPCS in reset when a port is down

External Email

----------------------------------------------------------------------
This patch adds calls in the stop() helper to ensure both MACs and
both PCS blocks are set in reset when the user manually sets a port
down. This is done so that we have the exact same block reset states at
boot time and when a port is set down.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 29d32cb3d52b..c10fd894c86f 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3257,6 +3257,7 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
 
 	if (port->phylink)
 		phylink_stop(port->phylink);
+
 	phy_power_off(port->comphy);
 }
 
@@ -3520,6 +3521,9 @@ static int mvpp2_stop(struct net_device *dev)
 	mvpp2_cleanup_rxqs(port);
 	mvpp2_cleanup_txqs(port);
 
+	mvpp2_mac_reset(port);
+	mvpp22_pcs_reset(port);
+
 	cancel_delayed_work_sync(&port->stats_work);
 
 	return 0;
-- 
2.20.1

Re: [EXT] [PATCH net-next 15/15] net: mvpp2: set the GMAC, XLG MAC, XPCS and MPCS in reset when a port is down

[Antoine Tenart]

Hi Yan,

On Thu, Feb 28, 2019 at 03:00:34PM +0000, Yan Markman wrote:
> 
> The eth-down/stop may concurrent with pre-scheduled stats_work.
> So it seems better to cancel stats_work first and then make resetS.

I'm not sure this would actually result in an issue, but to keep it
logical I'll change that.

Thanks,
Antoine

> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 29d32cb3d52b..c10fd894c86f 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -3257,6 +3257,7 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
>  
>  	if (port->phylink)
>  		phylink_stop(port->phylink);
> +
>  	phy_power_off(port->comphy);
>  }
>  
> @@ -3520,6 +3521,9 @@ static int mvpp2_stop(struct net_device *dev)
>  	mvpp2_cleanup_rxqs(port);
>  	mvpp2_cleanup_txqs(port);
>  
> +	mvpp2_mac_reset(port);
> +	mvpp22_pcs_reset(port);
> +
>  	cancel_delayed_work_sync(&port->stats_work);
>  
>  	return 0;

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [EXT] [PATCH net-next 07/15] net: mvpp2: fix the computation of the RXQs

[Yan Markman]

Some real and "potential" functionalities have been shrunk in September-2018
on transition from single-big file to the current split driver onto several files.

Regarding the MVPP2_DEFAULT_RXQ
Seems the current variant is flexible, permitting easy customize the configuration according to customer's needs.

Regarding the Queue in probe():
Looking into old code there where no2 queue-modes but 3:
enum mv_pp2_queue_distribution_mode {
	MVPP2_QDIST_SINGLE_MODE,
	MVPP2_QDIST_MULTI_MODE,
	MVPP2_SINGLE_RESOURCE_MODE
};
The current   if(MVPP2_QDIST_MULTI_MODE)else is correct also for the MVPP2_SINGLE_RESOURCE_MODE,
but new/patched isn't.

Since this patch doesn't change any functionality (right now) but reduces the flexibility I do not see real reason to apply it.

Regards
Yan Markman

-----Original Message-----
From: Antoine Tenart <antoine.tenart@bootlin.com> 
Sent: Thursday, February 28, 2019 3:21 PM
To: davem@davemloft.net; linux@armlinux.org.uk
Cc: Antoine Tenart <antoine.tenart@bootlin.com>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; thomas.petazzoni@bootlin.com; maxime.chevallier@bootlin.com; gregory.clement@bootlin.com; miquel.raynal@bootlin.com; Nadav Haklai <nadavh@marvell.com>; Stefan Chulski <stefanc@marvell.com>; Yan Markman <ymarkman@marvell.com>; mw@semihalf.com
Subject: [EXT] [PATCH net-next 07/15] net: mvpp2: fix the computation of the RXQs

External Email

----------------------------------------------------------------------
The patch fixes the computation of RXQs being used by the PPv2 driver, which is set depending on the PPv2 engine version and the queue mode used. There are three cases:

- PPv2.1: 1 RXQ per CPU.
- PPV2.2 with MVPP2_QDIST_MULTI_MODE: 1 RXQ per CPU.
- PPv2.2 with MVPP2_QDIST_SINGLE_MODE: 1 RXQ is shared between the CPUs.

The PPv2 engine supports a maximum of 32 queues per port. This patch adds a check so that we do not overstep this maximum.

It appeared the calculation was broken for PPv2.1 engines since f8c6ba8424b0, as PPv2.1 ports ended up with a single RXQ while they needed 4. This patch fixes it.

Fixes: f8c6ba8424b0 ("net: mvpp2: use only one rx queue per port per CPU")
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  4 ++--
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 23 ++++++++++++-------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 17ff330cce5f..687e011de5ef 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -549,8 +549,8 @@
 #define MVPP2_MAX_TSO_SEGS		300
 #define MVPP2_MAX_SKB_DESCS		(MVPP2_MAX_TSO_SEGS * 2 + MAX_SKB_FRAGS)
 
-/* Default number of RXQs in use */
-#define MVPP2_DEFAULT_RXQ		1
+/* Max number of RXQs per port */
+#define MVPP2_PORT_MAX_RXQ		32
 
 /* Max number of Rx descriptors */
 #define MVPP2_MAX_RXD_MAX		1024
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 24cee6cbe309..9c6200a59910 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4062,8 +4062,8 @@ static int mvpp2_multi_queue_vectors_init(struct mvpp2_port *port,
 			snprintf(irqname, sizeof(irqname), "hif%d", i);
 
 		if (queue_mode == MVPP2_QDIST_MULTI_MODE) {
-			v->first_rxq = i * MVPP2_DEFAULT_RXQ;
-			v->nrxqs = MVPP2_DEFAULT_RXQ;
+			v->first_rxq = i;
+			v->nrxqs = 1;
 		} else if (queue_mode == MVPP2_QDIST_SINGLE_MODE &&
 			   i == (port->nqvecs - 1)) {
 			v->first_rxq = 0;
@@ -4156,8 +4156,7 @@ static int mvpp2_port_init(struct mvpp2_port *port)
 	    MVPP2_MAX_PORTS * priv->max_port_rxqs)
 		return -EINVAL;
 
-	if (port->nrxqs % MVPP2_DEFAULT_RXQ ||
-	    port->nrxqs > priv->max_port_rxqs || port->ntxqs > MVPP2_MAX_TXQ)
+	if (port->nrxqs > priv->max_port_rxqs || port->ntxqs > MVPP2_MAX_TXQ)
 		return -EINVAL;
 
 	/* Disable port */
@@ -4778,10 +4777,18 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	}
 
 	ntxqs = MVPP2_MAX_TXQ;
-	if (priv->hw_version == MVPP22 && queue_mode == MVPP2_QDIST_MULTI_MODE)
-		nrxqs = MVPP2_DEFAULT_RXQ * num_possible_cpus();
-	else
-		nrxqs = MVPP2_DEFAULT_RXQ;
+	if (priv->hw_version == MVPP22 && queue_mode == MVPP2_QDIST_SINGLE_MODE) {
+		nrxqs = 1;
+	} else {
+		/* According to the PPv2.2 datasheet and our experiments on
+		 * PPv2.1, RX queues have an allocation granularity of 4 (when
+		 * more than a single one on PPv2.2).
+		 * Round up to nearest multiple of 4.
+		 */
+		nrxqs = (num_possible_cpus() + 3) & ~0x3;
+		if (nrxqs > MVPP2_PORT_MAX_RXQ)
+			nrxqs = MVPP2_PORT_MAX_RXQ;
+	}
 
 	dev = alloc_etherdev_mqs(sizeof(*port), ntxqs, nrxqs);
 	if (!dev)
--
2.20.1

Re: [EXT] [PATCH net-next 07/15] net: mvpp2: fix the computation of the RXQs

[Antoine Tenart]

Yan,

On Thu, Feb 28, 2019 at 03:40:50PM +0000, Yan Markman wrote:
> 
> Regarding the MVPP2_DEFAULT_RXQ
> Seems the current variant is flexible, permitting easy customize the
> configuration according to customer's needs.
> 
> Regarding the Queue in probe():
> Looking into old code there where no2 queue-modes but 3:
> enum mv_pp2_queue_distribution_mode {
> 	MVPP2_QDIST_SINGLE_MODE,
> 	MVPP2_QDIST_MULTI_MODE,
> 	MVPP2_SINGLE_RESOURCE_MODE
> };
> The current   if(MVPP2_QDIST_MULTI_MODE)else is correct also for the
> MVPP2_SINGLE_RESOURCE_MODE, but new/patched isn't.

There are only 2 modes supported in the upstream kernel:
MVPP2_QDIST_SINGLE_MODE and MVPP2_QDIST_MULTI_MODE. The third one you
mentioned is only supported in out-of-tree kernels.

Therefore patches sent to the upstream kernel do not take in into
account, as it is not supported.

> Since this patch doesn't change any functionality (right now) but
> reduces the flexibility I do not see real reason to apply it.

This patch do not break the upstream support of PPv2 and does improve
two thing:

- It limits the total number of RXQs being allocated, to ensure the
  number of RXQs being used do not exceed the number of available RXQ
  (which would make the driver to fail).
- It does fix PPv2.1 support, which was broken.

I do think the patch will benefit the upstream PPv2 support.

Thanks,
Antoine

> The patch fixes the computation of RXQs being used by the PPv2 driver,
> which is set depending on the PPv2 engine version and the
> queue mode used. There are three cases:
> 
> - PPv2.1: 1 RXQ per CPU.
> - PPV2.2 with MVPP2_QDIST_MULTI_MODE: 1 RXQ per CPU.
> - PPv2.2 with MVPP2_QDIST_SINGLE_MODE: 1 RXQ is shared between the CPUs.
> 
> The PPv2 engine supports a maximum of 32 queues per port. This patch
> adds a check so that we do not overstep this maximum.
> 
> It appeared the calculation was broken for PPv2.1 engines since
> f8c6ba8424b0, as PPv2.1 ports ended up with a single RXQ while they
> needed 4. This patch fixes it.
> 
> Fixes: f8c6ba8424b0 ("net: mvpp2: use only one rx queue per port per CPU")
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  4 ++--
>  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 23 ++++++++++++-------
>  2 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> index 17ff330cce5f..687e011de5ef 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> @@ -549,8 +549,8 @@
>  #define MVPP2_MAX_TSO_SEGS		300
>  #define MVPP2_MAX_SKB_DESCS		(MVPP2_MAX_TSO_SEGS * 2 + MAX_SKB_FRAGS)
>  
> -/* Default number of RXQs in use */
> -#define MVPP2_DEFAULT_RXQ		1
> +/* Max number of RXQs per port */
> +#define MVPP2_PORT_MAX_RXQ		32
>  
>  /* Max number of Rx descriptors */
>  #define MVPP2_MAX_RXD_MAX		1024
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 24cee6cbe309..9c6200a59910 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -4062,8 +4062,8 @@ static int mvpp2_multi_queue_vectors_init(struct mvpp2_port *port,
>  			snprintf(irqname, sizeof(irqname), "hif%d", i);
>  
>  		if (queue_mode == MVPP2_QDIST_MULTI_MODE) {
> -			v->first_rxq = i * MVPP2_DEFAULT_RXQ;
> -			v->nrxqs = MVPP2_DEFAULT_RXQ;
> +			v->first_rxq = i;
> +			v->nrxqs = 1;
>  		} else if (queue_mode == MVPP2_QDIST_SINGLE_MODE &&
>  			   i == (port->nqvecs - 1)) {
>  			v->first_rxq = 0;
> @@ -4156,8 +4156,7 @@ static int mvpp2_port_init(struct mvpp2_port *port)
>  	    MVPP2_MAX_PORTS * priv->max_port_rxqs)
>  		return -EINVAL;
>  
> -	if (port->nrxqs % MVPP2_DEFAULT_RXQ ||
> -	    port->nrxqs > priv->max_port_rxqs || port->ntxqs > MVPP2_MAX_TXQ)
> +	if (port->nrxqs > priv->max_port_rxqs || port->ntxqs > MVPP2_MAX_TXQ)
>  		return -EINVAL;
>  
>  	/* Disable port */
> @@ -4778,10 +4777,18 @@ static int mvpp2_port_probe(struct platform_device *pdev,
>  	}
>  
>  	ntxqs = MVPP2_MAX_TXQ;
> -	if (priv->hw_version == MVPP22 && queue_mode == MVPP2_QDIST_MULTI_MODE)
> -		nrxqs = MVPP2_DEFAULT_RXQ * num_possible_cpus();
> -	else
> -		nrxqs = MVPP2_DEFAULT_RXQ;
> +	if (priv->hw_version == MVPP22 && queue_mode == MVPP2_QDIST_SINGLE_MODE) {
> +		nrxqs = 1;
> +	} else {
> +		/* According to the PPv2.2 datasheet and our experiments on
> +		 * PPv2.1, RX queues have an allocation granularity of 4 (when
> +		 * more than a single one on PPv2.2).
> +		 * Round up to nearest multiple of 4.
> +		 */
> +		nrxqs = (num_possible_cpus() + 3) & ~0x3;
> +		if (nrxqs > MVPP2_PORT_MAX_RXQ)
> +			nrxqs = MVPP2_PORT_MAX_RXQ;
> +	}
>  
>  	dev = alloc_etherdev_mqs(sizeof(*port), ntxqs, nrxqs);
>  	if (!dev)
> --
> 2.20.1
> 

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [EXT] [PATCH net-next 07/15] net: mvpp2: fix the computation of the RXQs

[Yan Markman]

OK-OK. The " It does fix PPv2.1 support, which was broken" is great reason!

-----Original Message-----
From: Antoine Tenart <antoine.tenart@bootlin.com> 
Sent: Thursday, February 28, 2019 5:51 PM
To: Yan Markman <ymarkman@marvell.com>
Cc: Antoine Tenart <antoine.tenart@bootlin.com>; davem@davemloft.net; linux@armlinux.org.uk; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; thomas.petazzoni@bootlin.com; maxime.chevallier@bootlin.com; gregory.clement@bootlin.com; miquel.raynal@bootlin.com; Nadav Haklai <nadavh@marvell.com>; Stefan Chulski <stefanc@marvell.com>; mw@semihalf.com
Subject: Re: [EXT] [PATCH net-next 07/15] net: mvpp2: fix the computation of the RXQs

Yan,

On Thu, Feb 28, 2019 at 03:40:50PM +0000, Yan Markman wrote:
> 
> Regarding the MVPP2_DEFAULT_RXQ
> Seems the current variant is flexible, permitting easy customize the 
> configuration according to customer's needs.
> 
> Regarding the Queue in probe():
> Looking into old code there where no2 queue-modes but 3:
> enum mv_pp2_queue_distribution_mode {
> 	MVPP2_QDIST_SINGLE_MODE,
> 	MVPP2_QDIST_MULTI_MODE,
> 	MVPP2_SINGLE_RESOURCE_MODE
> };
> The current   if(MVPP2_QDIST_MULTI_MODE)else is correct also for the
> MVPP2_SINGLE_RESOURCE_MODE, but new/patched isn't.

There are only 2 modes supported in the upstream kernel:
MVPP2_QDIST_SINGLE_MODE and MVPP2_QDIST_MULTI_MODE. The third one you mentioned is only supported in out-of-tree kernels.

Therefore patches sent to the upstream kernel do not take in into account, as it is not supported.

> Since this patch doesn't change any functionality (right now) but 
> reduces the flexibility I do not see real reason to apply it.

This patch do not break the upstream support of PPv2 and does improve two thing:

- It limits the total number of RXQs being allocated, to ensure the
  number of RXQs being used do not exceed the number of available RXQ
  (which would make the driver to fail).
- It does fix PPv2.1 support, which was broken.

I do think the patch will benefit the upstream PPv2 support.

Thanks,
Antoine

> The patch fixes the computation of RXQs being used by the PPv2 driver, 
> which is set depending on the PPv2 engine version and the queue mode 
> used. There are three cases:
> 
> - PPv2.1: 1 RXQ per CPU.
> - PPV2.2 with MVPP2_QDIST_MULTI_MODE: 1 RXQ per CPU.
> - PPv2.2 with MVPP2_QDIST_SINGLE_MODE: 1 RXQ is shared between the CPUs.
> 
> The PPv2 engine supports a maximum of 32 queues per port. This patch 
> adds a check so that we do not overstep this maximum.
> 
> It appeared the calculation was broken for PPv2.1 engines since 
> f8c6ba8424b0, as PPv2.1 ports ended up with a single RXQ while they 
> needed 4. This patch fixes it.
> 
> Fixes: f8c6ba8424b0 ("net: mvpp2: use only one rx queue per port per 
> CPU")
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  4 ++--
>  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 23 ++++++++++++-------
>  2 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h 
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> index 17ff330cce5f..687e011de5ef 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> @@ -549,8 +549,8 @@
>  #define MVPP2_MAX_TSO_SEGS		300
>  #define MVPP2_MAX_SKB_DESCS		(MVPP2_MAX_TSO_SEGS * 2 + MAX_SKB_FRAGS)
>  
> -/* Default number of RXQs in use */
> -#define MVPP2_DEFAULT_RXQ		1
> +/* Max number of RXQs per port */
> +#define MVPP2_PORT_MAX_RXQ		32
>  
>  /* Max number of Rx descriptors */
>  #define MVPP2_MAX_RXD_MAX		1024
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c 
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 24cee6cbe309..9c6200a59910 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -4062,8 +4062,8 @@ static int mvpp2_multi_queue_vectors_init(struct mvpp2_port *port,
>  			snprintf(irqname, sizeof(irqname), "hif%d", i);
>  
>  		if (queue_mode == MVPP2_QDIST_MULTI_MODE) {
> -			v->first_rxq = i * MVPP2_DEFAULT_RXQ;
> -			v->nrxqs = MVPP2_DEFAULT_RXQ;
> +			v->first_rxq = i;
> +			v->nrxqs = 1;
>  		} else if (queue_mode == MVPP2_QDIST_SINGLE_MODE &&
>  			   i == (port->nqvecs - 1)) {
>  			v->first_rxq = 0;
> @@ -4156,8 +4156,7 @@ static int mvpp2_port_init(struct mvpp2_port *port)
>  	    MVPP2_MAX_PORTS * priv->max_port_rxqs)
>  		return -EINVAL;
>  
> -	if (port->nrxqs % MVPP2_DEFAULT_RXQ ||
> -	    port->nrxqs > priv->max_port_rxqs || port->ntxqs > MVPP2_MAX_TXQ)
> +	if (port->nrxqs > priv->max_port_rxqs || port->ntxqs > 
> +MVPP2_MAX_TXQ)
>  		return -EINVAL;
>  
>  	/* Disable port */
> @@ -4778,10 +4777,18 @@ static int mvpp2_port_probe(struct platform_device *pdev,
>  	}
>  
>  	ntxqs = MVPP2_MAX_TXQ;
> -	if (priv->hw_version == MVPP22 && queue_mode == MVPP2_QDIST_MULTI_MODE)
> -		nrxqs = MVPP2_DEFAULT_RXQ * num_possible_cpus();
> -	else
> -		nrxqs = MVPP2_DEFAULT_RXQ;
> +	if (priv->hw_version == MVPP22 && queue_mode == MVPP2_QDIST_SINGLE_MODE) {
> +		nrxqs = 1;
> +	} else {
> +		/* According to the PPv2.2 datasheet and our experiments on
> +		 * PPv2.1, RX queues have an allocation granularity of 4 (when
> +		 * more than a single one on PPv2.2).
> +		 * Round up to nearest multiple of 4.
> +		 */
> +		nrxqs = (num_possible_cpus() + 3) & ~0x3;
> +		if (nrxqs > MVPP2_PORT_MAX_RXQ)
> +			nrxqs = MVPP2_PORT_MAX_RXQ;
> +	}
>  
>  	dev = alloc_etherdev_mqs(sizeof(*port), ntxqs, nrxqs);
>  	if (!dev)
> --
> 2.20.1
> 

--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH net-next 14/15] net: mvpp2: set the XPCS and MPCS in reset when not used

[David Miller]

From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Thu, 28 Feb 2019 14:21:27 +0100

> +static void mvpp22_pcs_reset(struct mvpp2_port *port)
> +{
> +	struct mvpp2 *priv = port->priv;
> +	void __iomem *mpcs = priv->iface_base + MVPP22_MPCS_BASE(port->gop_id);
> +	void __iomem *xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
> +	u32 val;

Reverse christmas tree please.

 ...
> +static void mvpp22_pcs_unreset(struct mvpp2_port *port)
> +{
> +	struct mvpp2 *priv = port->priv;
> +	void __iomem *mpcs = priv->iface_base + MVPP22_MPCS_BASE(port->gop_id);
> +	void __iomem *xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
> +	u32 val;

Likewise.

Re: [PATCH net-next 14/15] net: mvpp2: set the XPCS and MPCS in reset when not used

[Antoine Tenart]

Hi David,

On Thu, Feb 28, 2019 at 10:40:51AM -0800, David Miller wrote:
> From: Antoine Tenart <antoine.tenart@bootlin.com>
> Date: Thu, 28 Feb 2019 14:21:27 +0100
> 
> > +static void mvpp22_pcs_reset(struct mvpp2_port *port)
> > +{
> > +	struct mvpp2 *priv = port->priv;
> > +	void __iomem *mpcs = priv->iface_base + MVPP22_MPCS_BASE(port->gop_id);
> > +	void __iomem *xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
> > +	u32 val;
> 
> Reverse christmas tree please.
> 
>  ...
> > +static void mvpp22_pcs_unreset(struct mvpp2_port *port)
> > +{
> > +	struct mvpp2 *priv = port->priv;
> > +	void __iomem *mpcs = priv->iface_base + MVPP22_MPCS_BASE(port->gop_id);
> > +	void __iomem *xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
> > +	u32 val;
> 
> Likewise.

Right. But here we do need priv to be assigned first in order to assign
mpcs and xpcs. Do you want the pointer definition and assignment to be
in two steps to respect the christmas tree rule or is it acceptable in
such situations?

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH net-next 14/15] net: mvpp2: set the XPCS and MPCS in reset when not used

[David Miller]

From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Thu, 28 Feb 2019 22:23:12 +0100

> Right. But here we do need priv to be assigned first in order to assign
> mpcs and xpcs.

Then put the latter assignments into the body of the function.