[PATCH 0/2] net: mvneta: fix usage as a module

[PATCH 0/2] net: mvneta: fix usage as a module

[Thomas Petazzoni]

David,

The following set of two patches fix the usage of the mvneta driver
when built as a module, and used in RGMII configurations. It is
somewhat similar to a previous fix that was made by Arnaud Patard, but
which was limited to SGMII configurations.

Thanks,

Thomas

Thomas Petazzoni (2):
  net: mvneta: rename MVNETA_GMAC2_PSC_ENABLE to MVNETA_GMAC2_PCS_ENABLE
  net: mvneta: fix usage as a module on RGMII configurations

 drivers/net/ethernet/marvell/mvneta.c | 43 ++++++++---------------------------
 1 file changed, 9 insertions(+), 34 deletions(-)

-- 
1.8.3.2

[PATCH 1/2] net: mvneta: rename MVNETA_GMAC2_PSC_ENABLE to MVNETA_GMAC2_PCS_ENABLE

[Thomas Petazzoni]

Bit 3 of the MVNETA_GMAC_CTRL_2 is actually used to enable the PCS,
not the PSC: there was a typo in the name of the define, which this
commit fixes.

Cc: stable at vger.kernel.org
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
This patch is needed for a followup commit that fixes using the mvneta
driver as a module in RGMII configurations.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index f418f4f..d6b04d0 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -161,7 +161,7 @@
 #define      MVNETA_GMAC_MAX_RX_SIZE_MASK        0x7ffc
 #define      MVNETA_GMAC0_PORT_ENABLE            BIT(0)
 #define MVNETA_GMAC_CTRL_2                       0x2c08
-#define      MVNETA_GMAC2_PSC_ENABLE             BIT(3)
+#define      MVNETA_GMAC2_PCS_ENABLE             BIT(3)
 #define      MVNETA_GMAC2_PORT_RGMII             BIT(4)
 #define      MVNETA_GMAC2_PORT_RESET             BIT(6)
 #define MVNETA_GMAC_STATUS                       0x2c10
@@ -733,7 +733,7 @@ static void mvneta_port_sgmii_config(struct mvneta_port *pp)
 	u32 val;
 
 	val = mvreg_read(pp, MVNETA_GMAC_CTRL_2);
-	val |= MVNETA_GMAC2_PSC_ENABLE;
+	val |= MVNETA_GMAC2_PCS_ENABLE;
 	mvreg_write(pp, MVNETA_GMAC_CTRL_2, val);
 
 	mvreg_write(pp, MVNETA_SGMII_SERDES_CFG, MVNETA_SGMII_SERDES_PROTO);
-- 
1.8.3.2

[PATCH 2/2] net: mvneta: fix usage as a module on RGMII configurations

[Thomas Petazzoni]

Commit 5445eaf309ff ('mvneta: Try to fix mvneta when compiled as
module') fixed the mvneta driver to make it work properly when loaded
as a module in SGMII configuration, which was tested successful by the
author on the Armada XP OpenBlocks AX3, which uses SGMII.

However, it turns out that the Armada XP GP, which uses RGMII, is
affected by a similar problem: its SERDES configuration is lost when
mvneta is loaded as a module, because this configuration is set by the
bootloader, and then lost because the clock is gated by the clock
framework until the mvneta driver is loaded again and the clock is
re-enabled.

However, it turns out that for the RGMII case, setting the SERDES
configuration is not sufficient: the PCS enable bit in the
MVNETA_GMAC_CTRL_2 register must also be set, like in the SGMII
configuration.

Therefore, this commit reworks the SGMII/RGMII initialization: the
only difference between the two now is a different SERDES
configuration, all the rest is identical.

In detail, to achieve this, the commit:

 * Renames MVNETA_SGMII_SERDES_CFG to MVNETA_SERDES_CFG because it is
   not specific to SGMII, but also used on RGMII configurations.

 * Adds a MVNETA_RGMII_SERDES_PROTO definition, that must be used as
   the MVNETA_SERDES_CFG value in RGMII configurations.

 * Removes the mvneta_gmac_rgmii_set() and mvneta_port_sgmii_config()
   functions, and instead directly do the SGMII/RGMII configuration in
   mvneta_port_up(), from where those functions where called. It is
   worth mentioning that mvneta_gmac_rgmii_set() had an 'enable'
   parameter that was always passed as '1', so it was pretty useless.

 * Reworks the mvneta_port_up() function to set the MVNETA_SERDES_CFG
   register to the appropriate value depending on the RGMII vs. SGMII
   configuration. It also unconditionally set the PCS_ENABLE bit (was
   already done for SGMII, but is now also needed for RGMII), and sets
   the PORT_RGMII bit (which was already done for both SGMII and
   RGMII).

This commit was successfully tested with mvneta compiled as a module,
on both the OpenBlocks AX3 (SGMII configuration) and the Armada XP GP
(RGMII configuration).

Reported-by: Steve McIntyre <steve@einval.com>
Cc: stable at vger.kernel.org # 3.11.x: 5445eaf309ff mvneta: Try to fix mvneta when compiled as module
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 41 +++++++----------------------------
 1 file changed, 8 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index d6b04d0..c9c2faa 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -88,8 +88,9 @@
 #define      MVNETA_TX_IN_PRGRS                  BIT(1)
 #define      MVNETA_TX_FIFO_EMPTY                BIT(8)
 #define MVNETA_RX_MIN_FRAME_SIZE                 0x247c
-#define MVNETA_SGMII_SERDES_CFG			 0x24A0
+#define MVNETA_SERDES_CFG			 0x24A0
 #define      MVNETA_SGMII_SERDES_PROTO		 0x0cc7
+#define      MVNETA_RGMII_SERDES_PROTO		 0x0667
 #define MVNETA_TYPE_PRIO                         0x24bc
 #define      MVNETA_FORCE_UNI                    BIT(21)
 #define MVNETA_TXQ_CMD_1                         0x24e4
@@ -710,35 +711,6 @@ static void mvneta_rxq_bm_disable(struct mvneta_port *pp,
 	mvreg_write(pp, MVNETA_RXQ_CONFIG_REG(rxq->id), val);
 }
 
-
-
-/* Sets the RGMII Enable bit (RGMIIEn) in port MAC control register */
-static void mvneta_gmac_rgmii_set(struct mvneta_port *pp, int enable)
-{
-	u32  val;
-
-	val = mvreg_read(pp, MVNETA_GMAC_CTRL_2);
-
-	if (enable)
-		val |= MVNETA_GMAC2_PORT_RGMII;
-	else
-		val &= ~MVNETA_GMAC2_PORT_RGMII;
-
-	mvreg_write(pp, MVNETA_GMAC_CTRL_2, val);
-}
-
-/* Config SGMII port */
-static void mvneta_port_sgmii_config(struct mvneta_port *pp)
-{
-	u32 val;
-
-	val = mvreg_read(pp, MVNETA_GMAC_CTRL_2);
-	val |= MVNETA_GMAC2_PCS_ENABLE;
-	mvreg_write(pp, MVNETA_GMAC_CTRL_2, val);
-
-	mvreg_write(pp, MVNETA_SGMII_SERDES_CFG, MVNETA_SGMII_SERDES_PROTO);
-}
-
 /* Start the Ethernet port RX and TX activity */
 static void mvneta_port_up(struct mvneta_port *pp)
 {
@@ -2756,12 +2728,15 @@ static void mvneta_port_power_up(struct mvneta_port *pp, int phy_mode)
 	mvreg_write(pp, MVNETA_UNIT_INTR_CAUSE, 0);
 
 	if (phy_mode == PHY_INTERFACE_MODE_SGMII)
-		mvneta_port_sgmii_config(pp);
+		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_SGMII_SERDES_PROTO);
+	else
+		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_RGMII_SERDES_PROTO);
 
-	mvneta_gmac_rgmii_set(pp, 1);
+	val = mvreg_read(pp, MVNETA_GMAC_CTRL_2);
+
+	val |= MVNETA_GMAC2_PCS_ENABLE | MVNETA_GMAC2_PORT_RGMII;
 
 	/* Cancel Port Reset */
-	val = mvreg_read(pp, MVNETA_GMAC_CTRL_2);
 	val &= ~MVNETA_GMAC2_PORT_RESET;
 	mvreg_write(pp, MVNETA_GMAC_CTRL_2, val);
 
-- 
1.8.3.2

[PATCH 2/2] net: mvneta: fix usage as a module on RGMII configurations

[Gregory CLEMENT]

Hi,

On 26/03/2014 00:25, Thomas Petazzoni wrote:
> Commit 5445eaf309ff ('mvneta: Try to fix mvneta when compiled as
> module') fixed the mvneta driver to make it work properly when loaded
> as a module in SGMII configuration, which was tested successful by the
> author on the Armada XP OpenBlocks AX3, which uses SGMII.
> 
> However, it turns out that the Armada XP GP, which uses RGMII, is
> affected by a similar problem: its SERDES configuration is lost when
> mvneta is loaded as a module, because this configuration is set by the
> bootloader, and then lost because the clock is gated by the clock
> framework until the mvneta driver is loaded again and the clock is
> re-enabled.
> 
> However, it turns out that for the RGMII case, setting the SERDES
> configuration is not sufficient: the PCS enable bit in the
> MVNETA_GMAC_CTRL_2 register must also be set, like in the SGMII
> configuration.
> 
> Therefore, this commit reworks the SGMII/RGMII initialization: the
> only difference between the two now is a different SERDES
> configuration, all the rest is identical.
> 
> In detail, to achieve this, the commit:
> 
>  * Renames MVNETA_SGMII_SERDES_CFG to MVNETA_SERDES_CFG because it is
>    not specific to SGMII, but also used on RGMII configurations.
> 
>  * Adds a MVNETA_RGMII_SERDES_PROTO definition, that must be used as
>    the MVNETA_SERDES_CFG value in RGMII configurations.
> 
>  * Removes the mvneta_gmac_rgmii_set() and mvneta_port_sgmii_config()
>    functions, and instead directly do the SGMII/RGMII configuration in
>    mvneta_port_up(), from where those functions where called. It is
>    worth mentioning that mvneta_gmac_rgmii_set() had an 'enable'
>    parameter that was always passed as '1', so it was pretty useless.
> 
>  * Reworks the mvneta_port_up() function to set the MVNETA_SERDES_CFG
>    register to the appropriate value depending on the RGMII vs. SGMII
>    configuration. It also unconditionally set the PCS_ENABLE bit (was
>    already done for SGMII, but is now also needed for RGMII), and sets
>    the PORT_RGMII bit (which was already done for both SGMII and
>    RGMII).
> 
> This commit was successfully tested with mvneta compiled as a module,
> on both the OpenBlocks AX3 (SGMII configuration) and the Armada XP GP
> (RGMII configuration).

Unfortunately with this patch, mvneta doesn't work anymore on the
Mirabox (Armada 370 based board) on 3.14.I didn't managed to do a
simple ping.

Once I removed this commit then the driver worked again.

Gregory


> 
> Reported-by: Steve McIntyre <steve@einval.com>
> Cc: stable at vger.kernel.org # 3.11.x: 5445eaf309ff mvneta: Try to fix mvneta when compiled as module
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 41 +++++++----------------------------
>  1 file changed, 8 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index d6b04d0..c9c2faa 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -88,8 +88,9 @@
>  #define      MVNETA_TX_IN_PRGRS                  BIT(1)
>  #define      MVNETA_TX_FIFO_EMPTY                BIT(8)
>  #define MVNETA_RX_MIN_FRAME_SIZE                 0x247c
> -#define MVNETA_SGMII_SERDES_CFG			 0x24A0
> +#define MVNETA_SERDES_CFG			 0x24A0
>  #define      MVNETA_SGMII_SERDES_PROTO		 0x0cc7
> +#define      MVNETA_RGMII_SERDES_PROTO		 0x0667
>  #define MVNETA_TYPE_PRIO                         0x24bc
>  #define      MVNETA_FORCE_UNI                    BIT(21)
>  #define MVNETA_TXQ_CMD_1                         0x24e4
> @@ -710,35 +711,6 @@ static void mvneta_rxq_bm_disable(struct mvneta_port *pp,
>  	mvreg_write(pp, MVNETA_RXQ_CONFIG_REG(rxq->id), val);
>  }
>  
> -
> -
> -/* Sets the RGMII Enable bit (RGMIIEn) in port MAC control register */
> -static void mvneta_gmac_rgmii_set(struct mvneta_port *pp, int enable)
> -{
> -	u32  val;
> -
> -	val = mvreg_read(pp, MVNETA_GMAC_CTRL_2);
> -
> -	if (enable)
> -		val |= MVNETA_GMAC2_PORT_RGMII;
> -	else
> -		val &= ~MVNETA_GMAC2_PORT_RGMII;
> -
> -	mvreg_write(pp, MVNETA_GMAC_CTRL_2, val);
> -}
> -
> -/* Config SGMII port */
> -static void mvneta_port_sgmii_config(struct mvneta_port *pp)
> -{
> -	u32 val;
> -
> -	val = mvreg_read(pp, MVNETA_GMAC_CTRL_2);
> -	val |= MVNETA_GMAC2_PCS_ENABLE;
> -	mvreg_write(pp, MVNETA_GMAC_CTRL_2, val);
> -
> -	mvreg_write(pp, MVNETA_SGMII_SERDES_CFG, MVNETA_SGMII_SERDES_PROTO);
> -}
> -
>  /* Start the Ethernet port RX and TX activity */
>  static void mvneta_port_up(struct mvneta_port *pp)
>  {
> @@ -2756,12 +2728,15 @@ static void mvneta_port_power_up(struct mvneta_port *pp, int phy_mode)
>  	mvreg_write(pp, MVNETA_UNIT_INTR_CAUSE, 0);
>  
>  	if (phy_mode == PHY_INTERFACE_MODE_SGMII)
> -		mvneta_port_sgmii_config(pp);
> +		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_SGMII_SERDES_PROTO);
> +	else
> +		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_RGMII_SERDES_PROTO);
>  
> -	mvneta_gmac_rgmii_set(pp, 1);
> +	val = mvreg_read(pp, MVNETA_GMAC_CTRL_2);
> +
> +	val |= MVNETA_GMAC2_PCS_ENABLE | MVNETA_GMAC2_PORT_RGMII;
>  
>  	/* Cancel Port Reset */
> -	val = mvreg_read(pp, MVNETA_GMAC_CTRL_2);
>  	val &= ~MVNETA_GMAC2_PORT_RESET;
>  	mvreg_write(pp, MVNETA_GMAC_CTRL_2, val);
>  
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 2/2] net: mvneta: fix usage as a module on RGMII configurations

[Thomas Petazzoni]

Dear Gregory CLEMENT,

On Wed, 02 Apr 2014 16:15:17 +0200, Gregory CLEMENT wrote:

> > This commit was successfully tested with mvneta compiled as a module,
> > on both the OpenBlocks AX3 (SGMII configuration) and the Armada XP GP
> > (RGMII configuration).
> 
> Unfortunately with this patch, mvneta doesn't work anymore on the
> Mirabox (Armada 370 based board) on 3.14.I didn't managed to do a
> simple ping.
> 
> Once I removed this commit then the driver worked again.

Problem reproduced. It turns out that some RGMII platforms need the
PCS_ENABLE bit to be set (e.g: Armada XP GP), while some other
platforms need the PCS_ENABLE bit to be cleared (e.g: Armada 370
Mirabox). I've verified that on both platforms.

I've asked for more details about this bit to understand in which
situation it should be set or cleared. I'll get back to you with an
updated patch once I have enough information to write a fix.

Thanks for the report,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 2/2] net: mvneta: fix usage as a module on RGMII configurations

[Ezequiel Garcia]

Hello Thomas,

On Apr 03, Thomas Petazzoni wrote:
> On Wed, 02 Apr 2014 16:15:17 +0200, Gregory CLEMENT wrote:
> 
> > > This commit was successfully tested with mvneta compiled as a module,
> > > on both the OpenBlocks AX3 (SGMII configuration) and the Armada XP GP
> > > (RGMII configuration).
> > 
> > Unfortunately with this patch, mvneta doesn't work anymore on the
> > Mirabox (Armada 370 based board) on 3.14.I didn't managed to do a
> > simple ping.
> > 
> > Once I removed this commit then the driver worked again.
> 
> Problem reproduced. It turns out that some RGMII platforms need the
> PCS_ENABLE bit to be set (e.g: Armada XP GP), while some other
> platforms need the PCS_ENABLE bit to be cleared (e.g: Armada 370
> Mirabox). I've verified that on both platforms.
> 
> I've asked for more details about this bit to understand in which
> situation it should be set or cleared. I'll get back to you with an
> updated patch once I have enough information to write a fix.
> 

Just found this commit is in v3.12.17 and prevents booting the A370
Mirabox with nfsroot.

Maybe we should revert it in stable for now until we can solve it?

Unless you have a better idea.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 2/2] net: mvneta: fix usage as a module on RGMII configurations

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Wed, 9 Apr 2014 09:22:41 -0300, Ezequiel Garcia wrote:

> Just found this commit is in v3.12.17 and prevents booting the A370
> Mirabox with nfsroot.
> 
> Maybe we should revert it in stable for now until we can solve it?
> 
> Unless you have a better idea.

Yes, this problem is already tracked at
https://bugzilla.kernel.org/show_bug.cgi?id=73401, where I've explained
what's going on.

Unfortunately, I'm still waiting for details from Marvell to understand
in which cases the PCS needs to be set, and in which situations it
needs to be cleared.

Since getting this information is apparently going to take much more
time that I originally hoped, I'm starting to think that the safest and
fastest course of action is indeed to revert this patch.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 0/2] net: mvneta: fix usage as a module

[David Miller]

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Wed, 26 Mar 2014 00:25:40 +0100

> The following set of two patches fix the usage of the mvneta driver
> when built as a module, and used in RGMII configurations. It is
> somewhat similar to a previous fix that was made by Arnaud Patard, but
> which was limited to SGMII configurations.

Series applied, thanks.