[net-next 1/3] stmmac: remove the STMMAC_DUAL_MAC option

[net-next 1/3] stmmac: remove the STMMAC_DUAL_MAC option

[Giuseppe CAVALLARO]

The STMMAC_DUAL_MAC is now removed from the driver's Kconfig.
It will be available from a specific STM boards Kconfig.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/Kconfig |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/net/stmmac/Kconfig b/drivers/net/stmmac/Kconfig
index eb63d44..2513555 100644
--- a/drivers/net/stmmac/Kconfig
+++ b/drivers/net/stmmac/Kconfig
@@ -20,15 +20,6 @@ config STMMAC_DA
 	  By default, the DMA arbitration scheme is based on Round-robin
 	  (rx:tx priority is 1:1).
 
-config STMMAC_DUAL_MAC
-	bool "STMMAC: dual mac support (EXPERIMENTAL)"
-	default n
-        depends on EXPERIMENTAL && STMMAC_ETH && !STMMAC_TIMER
-	help
-	  Some ST SoCs (for example the stx7141 and stx7200c2) have two
-	  Ethernet Controllers. This option turns on the second Ethernet
-	  device on this kind of platforms.
-
 config STMMAC_TIMER
 	bool "STMMAC Timer optimisation"
 	default n
-- 
1.5.5.6

[net-next 2/3] stmmac: fix timer setup when use dual mac Kconfig

[Giuseppe CAVALLARO]

The driver erroneously sets the tmrate to zero when the
TMU initialisation fails. This actually generates problems
while using the dual GMAC configuration.

With this patch, enabling both the dual gmac and the timer
optimisation, the first interface opened will use the tmu
channel 2, the second one won't be able to use the timer but
will continue to work without mitigating the interrupts by
using the external timer (i.e. TMU channel 2).

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/stmmac_main.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 0bdd332..1083334 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -829,7 +829,6 @@ static int stmmac_open(struct net_device *dev)
 	 * In case of failure continue without timer. */
 	if (unlikely((stmmac_open_ext_timer(dev, priv->tm)) < 0)) {
 		pr_warning("stmmaceth: cannot attach the external timer.\n");
-		tmrate = 0;
 		priv->tm->freq = 0;
 		priv->tm->timer_start = stmmac_no_timer_started;
 		priv->tm->timer_stop = stmmac_no_timer_stopped;
-- 
1.5.5.6

[net-next 3/3] stmmac: fix automatic PAD/FCS stripping

[Giuseppe CAVALLARO]

For Simple Ethernet frames (802.2 and 802.3) the GMAC Core
never strips pad and fcs. This means the ACS has no effect
on IPv4/6 frames.
The FL bits, in the RDES0, include the FCS so the driver
has to remove it in SW.
For 802.3 frame format with LLC or LLC-SNAP, when set the ACS
bit, the HW strips both PAD and FCS.
The FL bits, in the RDES0, actually represents the frame length
already stripped.
This patch fixes this logic within the device driver that
erroneously removed 4byte from 802.3 frames already stripped
corrupting the payload.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/common.h      |    1 +
 drivers/net/stmmac/dwmac1000.h   |    2 +-
 drivers/net/stmmac/enh_desc.c    |    2 +-
 drivers/net/stmmac/stmmac_main.c |    8 ++++++--
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/stmmac/common.h b/drivers/net/stmmac/common.h
index 144f76f..66b9da0 100644
--- a/drivers/net/stmmac/common.h
+++ b/drivers/net/stmmac/common.h
@@ -108,6 +108,7 @@ enum rx_frame_status { /* IPC status */
 	good_frame = 0,
 	discard_frame = 1,
 	csum_none = 2,
+	llc_snap = 4,
 };
 
 enum tx_dma_irq_status {
diff --git a/drivers/net/stmmac/dwmac1000.h b/drivers/net/stmmac/dwmac1000.h
index d8d0f35..8b20b19 100644
--- a/drivers/net/stmmac/dwmac1000.h
+++ b/drivers/net/stmmac/dwmac1000.h
@@ -93,7 +93,7 @@ enum inter_frame_gap {
 #define GMAC_CONTROL_IPC	0x00000400 /* Checksum Offload */
 #define GMAC_CONTROL_DR		0x00000200 /* Disable Retry */
 #define GMAC_CONTROL_LUD	0x00000100 /* Link up/down */
-#define GMAC_CONTROL_ACS	0x00000080 /* Automatic Pad Stripping */
+#define GMAC_CONTROL_ACS	0x00000080 /* Automatic Pad/FCS Stripping */
 #define GMAC_CONTROL_DC		0x00000010 /* Deferral Check */
 #define GMAC_CONTROL_TE		0x00000008 /* Transmitter Enable */
 #define GMAC_CONTROL_RE		0x00000004 /* Receiver Enable */
diff --git a/drivers/net/stmmac/enh_desc.c b/drivers/net/stmmac/enh_desc.c
index 3c18ebe..f612f98 100644
--- a/drivers/net/stmmac/enh_desc.c
+++ b/drivers/net/stmmac/enh_desc.c
@@ -123,7 +123,7 @@ static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err)
 	 */
 	if (status == 0x0) {
 		CHIP_DBG(KERN_INFO "RX Des0 status: IEEE 802.3 Type frame.\n");
-		ret = good_frame;
+		ret = llc_snap;
 	} else if (status == 0x4) {
 		CHIP_DBG(KERN_INFO "RX Des0 status: IPv4/6 No CSUM errorS.\n");
 		ret = good_frame;
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 1083334..bbb7951 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -1216,9 +1216,13 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit)
 			priv->dev->stats.rx_errors++;
 		else {
 			struct sk_buff *skb;
-			/* Length should omit the CRC */
-			int frame_len = priv->hw->desc->get_rx_frame_len(p) - 4;
+			int frame_len;
 
+			frame_len = priv->hw->desc->get_rx_frame_len(p);
+			/* ACS is set; GMAC core strips PAD/FCS for IEEE 802.3
+			 * Type frames (LLC/LLC-SNAP) */
+			if (unlikely(status != llc_snap))
+				frame_len -= ETH_FCS_LEN;
 #ifdef STMMAC_RX_DEBUG
 			if (frame_len > ETH_FRAME_LEN)
 				pr_debug("\tRX frame size %d, COE status: %d\n",
-- 
1.5.5.6

Re: [net-next 2/3] stmmac: fix timer setup when use dual mac Kconfig

[David Miller]

From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Tue, 27 Jul 2010 12:09:46 +0200

> The driver erroneously sets the tmrate to zero when the
> TMU initialisation fails. This actually generates problems
> while using the dual GMAC configuration.
> 
> With this patch, enabling both the dual gmac and the timer
> optimisation, the first interface opened will use the tmu
> channel 2, the second one won't be able to use the timer but
> will continue to work without mitigating the interrupts by
> using the external timer (i.e. TMU channel 2).
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

This is not how we do things.

All of the options that influence the driver should be right next
to the main driver option.

What the platform SOC Kconfig's can do is 'select' those option.

But even better is to get rid of all of these feature Kconfig options,
and communicate the capability in the platform_device probe
information or similar.

Re: [net-next 2/3] stmmac: fix timer setup when use dual mac Kconfig

[David Miller]

From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Tue, 27 Jul 2010 12:09:46 +0200

> The driver erroneously sets the tmrate to zero when the
> TMU initialisation fails. This actually generates problems
> while using the dual GMAC configuration.
> 
> With this patch, enabling both the dual gmac and the timer
> optimisation, the first interface opened will use the tmu
> channel 2, the second one won't be able to use the timer but
> will continue to work without mitigating the interrupts by
> using the external timer (i.e. TMU channel 2).
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

Applied.

Re: [net-next 3/3] stmmac: fix automatic PAD/FCS stripping

[David Miller]

From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Tue, 27 Jul 2010 12:09:47 +0200

> For Simple Ethernet frames (802.2 and 802.3) the GMAC Core
> never strips pad and fcs. This means the ACS has no effect
> on IPv4/6 frames.
> The FL bits, in the RDES0, include the FCS so the driver
> has to remove it in SW.
> For 802.3 frame format with LLC or LLC-SNAP, when set the ACS
> bit, the HW strips both PAD and FCS.
> The FL bits, in the RDES0, actually represents the frame length
> already stripped.
> This patch fixes this logic within the device driver that
> erroneously removed 4byte from 802.3 frames already stripped
> corrupting the payload.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

Applied.

Re: [net-next 2/3] stmmac: fix timer setup when use dual mac Kconfig

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 27 Jul 2010 20:45:28 -0700 (PDT)

Sorry, I meant to say this in reply to patch #1 not #2 :)

> From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
> Date: Tue, 27 Jul 2010 12:09:46 +0200
> 
>> The driver erroneously sets the tmrate to zero when the
>> TMU initialisation fails. This actually generates problems
>> while using the dual GMAC configuration.
>> 
>> With this patch, enabling both the dual gmac and the timer
>> optimisation, the first interface opened will use the tmu
>> channel 2, the second one won't be able to use the timer but
>> will continue to work without mitigating the interrupts by
>> using the external timer (i.e. TMU channel 2).
>> 
>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> 
> This is not how we do things.
> 
> All of the options that influence the driver should be right next
> to the main driver option.
> 
> What the platform SOC Kconfig's can do is 'select' those option.
> 
> But even better is to get rid of all of these feature Kconfig options,
> and communicate the capability in the platform_device probe
> information or similar.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [net-next 1/3] stmmac: remove the STMMAC_DUAL_MAC option

[Peppe CAVALLARO]

> -----Original Message-----
> From: Giuseppe CAVALLARO [mailto:peppe.cavallaro@st.com]
> Sent: Tuesday, July 27, 2010 12:10 PM
> To: netdev@vger.kernel.org
> Cc: Peppe CAVALLARO
> Subject: [net-next 1/3] stmmac: remove the STMMAC_DUAL_MAC option
> 
> The STMMAC_DUAL_MAC is now removed from the driver's Kconfig.
> It will be available from a specific STM boards Kconfig.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>


> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, July 28, 2010 5:45 AM
> To: Peppe CAVALLARO
> Cc: netdev@vger.kernel.org
> Subject: Re: [net-next 2/3] stmmac: fix timer setup when use dual mac
> Kconfig

[snip]
 
> This is not how we do things.
> 
> All of the options that influence the driver should be right next
> to the main driver option.
> 
> What the platform SOC Kconfig's can do is 'select' those option.
> 
> But even better is to get rid of all of these feature Kconfig options,
> and communicate the capability in the platform_device probe
> information or similar.

Hi David,

Indeed, this option doesn't impact the driver itself. Some STM SoCs
(7105,7106, 7108 ...) have two GMAC cores integrated in the same SoC.
So the meaning of this option was to turn-on the second device, only.
This option is also used within our board's setup files.
In the future, it could also generate some misunderstanding on other
platforms (ARM based) where there is no second GMAC device.
For this reason, I've removed it from the driver's Kconfig.

Welcome advice.

Thanks for the feedback.

Regards,
Peppe

> ---
>  drivers/net/stmmac/Kconfig |    9 ---------
>  1 files changed, 0 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/stmmac/Kconfig b/drivers/net/stmmac/Kconfig
> index eb63d44..2513555 100644
> --- a/drivers/net/stmmac/Kconfig
> +++ b/drivers/net/stmmac/Kconfig
> @@ -20,15 +20,6 @@ config STMMAC_DA
>  	  By default, the DMA arbitration scheme is based on Round-robin
>  	  (rx:tx priority is 1:1).
> 
> -config STMMAC_DUAL_MAC
> -	bool "STMMAC: dual mac support (EXPERIMENTAL)"
> -	default n
> -        depends on EXPERIMENTAL && STMMAC_ETH && !STMMAC_TIMER
> -	help
> -	  Some ST SoCs (for example the stx7141 and stx7200c2) have two
> -	  Ethernet Controllers. This option turns on the second Ethernet
> -	  device on this kind of platforms.
> -
>  config STMMAC_TIMER
>  	bool "STMMAC Timer optimisation"
>  	default n
> --
> 1.5.5.6