| Message ID | 20180116232545.22054-1-f.fainelli@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Florian for gmac4.x and gmac3.x series the ACS bit is the Automatic Pad or CRC Stripping, so the core strips the Pad or FCS on frames if the value of the length field is < 1536 bytes. For MAC10-100 there is the Bit 8 (ASTP) of the reg0 that does the same if len is < 46bytes. In your patch I can just suggest to add a new field to strip the PAD/FCS w/o passing the whole netdev struct to the core_init. In the main driver, we could manage the pad-strip feature (also by using dt) or disable it in case of netdev_uses_dsa; then propagating this setting to the core_init or calling a new callback. What do you think? Regards Peppe On 1/17/2018 12:25 AM, Florian Fainelli wrote: > Broadcom tags inserted by Broadcom switches put a 4 byte header after > the MAC SA and before the EtherType, which may look like some sort of 0 > length LLC/SNAP packet (tcpdump and wireshark do think that way). With > ACS enabled in stmmac the packets were truncated to 8 bytes on > reception, whereas clearing this bit allowed normal reception to occur. > > In order to make that possible, we need to pass a net_device argument to > the different core_init() functions and we are dependent on the Broadcom > tagger padding packets correctly (which it now does). To be as little > invasive as possible, this is only done for gmac1000 when the network > device is DSA-enabled (netdev_uses_dsa() returns true). > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > --- > drivers/net/ethernet/stmicro/stmmac/common.h | 2 +- > drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 3 ++- > drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 12 +++++++++++- > drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 3 ++- > drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 11 ++++++++++- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- > 6 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h > index ce2ea2d491ac..2ffe76c0ff74 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/common.h > +++ b/drivers/net/ethernet/stmicro/stmmac/common.h > @@ -474,7 +474,7 @@ struct mac_device_info; > /* Helpers to program the MAC core */ > struct stmmac_ops { > /* MAC core initialization */ > - void (*core_init)(struct mac_device_info *hw, int mtu); > + void (*core_init)(struct mac_device_info *hw, struct net_device *dev); > /* Enable the MAC RX/TX */ > void (*set_mac)(void __iomem *ioaddr, bool enable); > /* Enable and verify that the IPC module is supported */ > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > index 9eb7f65d8000..a3fa65b1ca8e 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > @@ -483,7 +483,8 @@ static int sun8i_dwmac_init(struct platform_device *pdev, void *priv) > return 0; > } > > -static void sun8i_dwmac_core_init(struct mac_device_info *hw, int mtu) > +static void sun8i_dwmac_core_init(struct mac_device_info *hw, > + struct net_device *dev) > { > void __iomem *ioaddr = hw->pcsr; > u32 v; > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c > index 8a86340ff2d3..540d21786a43 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c > @@ -25,18 +25,28 @@ > #include <linux/crc32.h> > #include <linux/slab.h> > #include <linux/ethtool.h> > +#include <net/dsa.h> > #include <asm/io.h> > #include "stmmac_pcs.h" > #include "dwmac1000.h" > > -static void dwmac1000_core_init(struct mac_device_info *hw, int mtu) > +static void dwmac1000_core_init(struct mac_device_info *hw, > + struct net_device *dev) > { > void __iomem *ioaddr = hw->pcsr; > u32 value = readl(ioaddr + GMAC_CONTROL); > + int mtu = dev->mtu; > > /* Configure GMAC core */ > value |= GMAC_CORE_INIT; > > + /* Clear ACS bit because Ethernet switch tagging formats such as > + * Broadcom tags can look like invalid LLC/SNAP packets and cause the > + * hardware to truncate packets on reception. > + */ > + if (netdev_uses_dsa(dev)) > + value &= ~GMAC_CONTROL_ACS; > + > if (mtu > 1500) > value |= GMAC_CONTROL_2K; > if (mtu > 2000) > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c > index 8ef517356313..c1ee427c42cb 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c > @@ -28,7 +28,8 @@ > #include <asm/io.h> > #include "dwmac100.h" > > -static void dwmac100_core_init(struct mac_device_info *hw, int mtu) > +static void dwmac100_core_init(struct mac_device_info *hw, > + struct net_device *dev) > { > void __iomem *ioaddr = hw->pcsr; > u32 value = readl(ioaddr + MAC_CONTROL); > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c > index f3ed8f7853eb..6af5100d3cb2 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c > @@ -20,13 +20,22 @@ > #include "stmmac_pcs.h" > #include "dwmac4.h" > > -static void dwmac4_core_init(struct mac_device_info *hw, int mtu) > +static void dwmac4_core_init(struct mac_device_info *hw, > + struct net_device *dev) > { > void __iomem *ioaddr = hw->pcsr; > u32 value = readl(ioaddr + GMAC_CONFIG); > + int mtu = dev->mtu; > > value |= GMAC_CORE_INIT; > > + /* Clear ACS bit because Ethernet switch tagging formats such as > + * Broadcom tags can look like invalid LLC/SNAP packets and cause the > + * hardware to truncate packets on reception. > + */ > + if (netdev_uses_dsa(dev)) > + value &= ~GMAC_CONTROL_ACS; > + > if (mtu > 1500) > value |= GMAC_CONFIG_2K; > if (mtu > 2000) > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index f99f14c35063..7ad841434ec8 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -2527,7 +2527,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp) > } > > /* Initialize the MAC Core */ > - priv->hw->mac->core_init(priv->hw, dev->mtu); > + priv->hw->mac->core_init(priv->hw, dev); > > /* Initialize MTL*/ > if (priv->synopsys_id >= DWMAC_CORE_4_00)
Hi Peppe, On 01/16/2018 11:06 PM, Giuseppe CAVALLARO wrote: > Hi Florian > > for gmac4.x and gmac3.x series the ACS bit is the Automatic Pad or CRC > Stripping, so the > core strips the Pad or FCS on frames if the value of the length field is > < 1536 bytes. > For MAC10-100 there is the Bit 8 (ASTP) of the reg0 that does the same > if len is < 46bytes. OK, thanks for the detail on the older core, I will update my v2 to take that into account. > In your patch I can just suggest to add a new field to strip the PAD/FCS > w/o passing the whole > netdev struct to the core_init. In the main driver, we could manage the > pad-strip feature (also > by using dt) or disable it in case of netdev_uses_dsa; then propagating > this setting to the core_init > or calling a new callback. What do you think? I don't think adding a DT property is a good idea, because we can determine whether ACS is necessary or not at runtime when DSA with proprietary Ethernet switch tags is used or not using netdev_uses_dsa(). Having the DT property to configure whether ACS is turned on or off means that the Device Tree now describes a policy, rather than a capability, which is counter to the Device Tree spirit. Besides, this leaves room for making mistake, and this creates a support burden. When interfaced with a switch, the switch is guaranteed to egress 64 bytes (including FCS) packets towards stmmac, and the only thing we need to do is ensure that stmmac does the same thing. This is a small penalty to pay for small packets, and this is done only when DSA is used. Regarding passing a net_device to the core_init functions, this was done because dev->mtu was already accessed, and we might need to access additional net_device members in the future, 2 means we need to find a solution that scales, so passing the net_device around was a natural choice. Function pointers described in common.h are not exactly consistent in what type of arguments they take, so I did not see this as being a big problem. Thank you. > > Regards > Peppe > > On 1/17/2018 12:25 AM, Florian Fainelli wrote: >> Broadcom tags inserted by Broadcom switches put a 4 byte header after >> the MAC SA and before the EtherType, which may look like some sort of 0 >> length LLC/SNAP packet (tcpdump and wireshark do think that way). With >> ACS enabled in stmmac the packets were truncated to 8 bytes on >> reception, whereas clearing this bit allowed normal reception to occur. >> >> In order to make that possible, we need to pass a net_device argument to >> the different core_init() functions and we are dependent on the Broadcom >> tagger padding packets correctly (which it now does). To be as little >> invasive as possible, this is only done for gmac1000 when the network >> device is DSA-enabled (netdev_uses_dsa() returns true). >> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> >> --- >> drivers/net/ethernet/stmicro/stmmac/common.h | 2 +- >> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 3 ++- >> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 12 +++++++++++- >> drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 3 ++- >> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 11 ++++++++++- >> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- >> 6 files changed, 27 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h >> b/drivers/net/ethernet/stmicro/stmmac/common.h >> index ce2ea2d491ac..2ffe76c0ff74 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/common.h >> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h >> @@ -474,7 +474,7 @@ struct mac_device_info; >> /* Helpers to program the MAC core */ >> struct stmmac_ops { >> /* MAC core initialization */ >> - void (*core_init)(struct mac_device_info *hw, int mtu); >> + void (*core_init)(struct mac_device_info *hw, struct net_device >> *dev); >> /* Enable the MAC RX/TX */ >> void (*set_mac)(void __iomem *ioaddr, bool enable); >> /* Enable and verify that the IPC module is supported */ >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> index 9eb7f65d8000..a3fa65b1ca8e 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >> @@ -483,7 +483,8 @@ static int sun8i_dwmac_init(struct platform_device >> *pdev, void *priv) >> return 0; >> } >> -static void sun8i_dwmac_core_init(struct mac_device_info *hw, int mtu) >> +static void sun8i_dwmac_core_init(struct mac_device_info *hw, >> + struct net_device *dev) >> { >> void __iomem *ioaddr = hw->pcsr; >> u32 v; >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c >> b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c >> index 8a86340ff2d3..540d21786a43 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c >> @@ -25,18 +25,28 @@ >> #include <linux/crc32.h> >> #include <linux/slab.h> >> #include <linux/ethtool.h> >> +#include <net/dsa.h> >> #include <asm/io.h> >> #include "stmmac_pcs.h" >> #include "dwmac1000.h" >> -static void dwmac1000_core_init(struct mac_device_info *hw, int mtu) >> +static void dwmac1000_core_init(struct mac_device_info *hw, >> + struct net_device *dev) >> { >> void __iomem *ioaddr = hw->pcsr; >> u32 value = readl(ioaddr + GMAC_CONTROL); >> + int mtu = dev->mtu; >> /* Configure GMAC core */ >> value |= GMAC_CORE_INIT; >> + /* Clear ACS bit because Ethernet switch tagging formats such as >> + * Broadcom tags can look like invalid LLC/SNAP packets and cause >> the >> + * hardware to truncate packets on reception. >> + */ >> + if (netdev_uses_dsa(dev)) >> + value &= ~GMAC_CONTROL_ACS; >> + >> if (mtu > 1500) >> value |= GMAC_CONTROL_2K; >> if (mtu > 2000) >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c >> b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c >> index 8ef517356313..c1ee427c42cb 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c >> @@ -28,7 +28,8 @@ >> #include <asm/io.h> >> #include "dwmac100.h" >> -static void dwmac100_core_init(struct mac_device_info *hw, int mtu) >> +static void dwmac100_core_init(struct mac_device_info *hw, >> + struct net_device *dev) >> { >> void __iomem *ioaddr = hw->pcsr; >> u32 value = readl(ioaddr + MAC_CONTROL); >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c >> b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c >> index f3ed8f7853eb..6af5100d3cb2 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c >> @@ -20,13 +20,22 @@ >> #include "stmmac_pcs.h" >> #include "dwmac4.h" >> -static void dwmac4_core_init(struct mac_device_info *hw, int mtu) >> +static void dwmac4_core_init(struct mac_device_info *hw, >> + struct net_device *dev) >> { >> void __iomem *ioaddr = hw->pcsr; >> u32 value = readl(ioaddr + GMAC_CONFIG); >> + int mtu = dev->mtu; >> value |= GMAC_CORE_INIT; >> + /* Clear ACS bit because Ethernet switch tagging formats such as >> + * Broadcom tags can look like invalid LLC/SNAP packets and cause >> the >> + * hardware to truncate packets on reception. >> + */ >> + if (netdev_uses_dsa(dev)) >> + value &= ~GMAC_CONTROL_ACS; >> + >> if (mtu > 1500) >> value |= GMAC_CONFIG_2K; >> if (mtu > 2000) >> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >> index f99f14c35063..7ad841434ec8 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >> @@ -2527,7 +2527,7 @@ static int stmmac_hw_setup(struct net_device >> *dev, bool init_ptp) >> } >> /* Initialize the MAC Core */ >> - priv->hw->mac->core_init(priv->hw, dev->mtu); >> + priv->hw->mac->core_init(priv->hw, dev); >> /* Initialize MTL*/ >> if (priv->synopsys_id >= DWMAC_CORE_4_00) > >
Hi Florian,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Florian-Fainelli/net-stmmac-Fix-reception-of-Broadcom-switches-tags/20180120-044006
config: x86_64-randconfig-x002-201802 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/asm-generic/bug.h:5:0,
from arch/x86/include/asm/bug.h:82,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c:17:
drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c: In function 'dwmac4_core_init':
drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c:36:6: error: implicit declaration of function 'netdev_uses_dsa'; did you mean 'netdev_reset_tc'? [-Werror=implicit-function-declaration]
if (netdev_uses_dsa(dev))
^
include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c:36:2: note: in expansion of macro 'if'
if (netdev_uses_dsa(dev))
^~
drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c:37:13: error: 'GMAC_CONTROL_ACS' undeclared (first use in this function); did you mean 'GMAC_CONFIG_ACS'?
value &= ~GMAC_CONTROL_ACS;
^~~~~~~~~~~~~~~~
GMAC_CONFIG_ACS
drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c:37:13: note: each undeclared identifier is reported only once for each function it appears in
In file included from include/asm-generic/bug.h:5:0,
from arch/x86/include/asm/bug.h:82,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c:17:
drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c: At top level:
include/linux/compiler.h:64:4: warning: '______f' is static but declared in inline function 'strcpy' which is not static
______f = { \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:425:2: note: in expansion of macro 'if'
if (p_size == (size_t)-1 && q_size == (size_t)-1)
^~
include/linux/compiler.h:64:4: warning: '______f' is static but declared in inline function 'kmemdup' which is not static
______f = { \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:415:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:64:4: warning: '______f' is static but declared in inline function 'kmemdup' which is not static
______f = { \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:413:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:64:4: warning: '______f' is static but declared in inline function 'memchr_inv' which is not static
______f = { \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:404:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:64:4: warning: '______f' is static but declared in inline function 'memchr_inv' which is not static
______f = { \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:402:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:64:4: warning: '______f' is static but declared in inline function 'memchr' which is not static
______f = { \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:393:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:64:4: warning: '______f' is static but declared in inline function 'memchr' which is not static
______f = { \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:391:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:64:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:383:2: note: in expansion of macro 'if'
if (p_size < size || q_size < size)
^~
include/linux/compiler.h:64:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:380:3: note: in expansion of macro 'if'
if (q_size < size)
^~
include/linux/compiler.h:64:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:56:23: note: in expansion of macro '__trace_if'
vim +/if +36 drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c
> 17 #include <linux/slab.h>
18 #include <linux/ethtool.h>
19 #include <linux/io.h>
20 #include "stmmac_pcs.h"
21 #include "dwmac4.h"
22
23 static void dwmac4_core_init(struct mac_device_info *hw,
24 struct net_device *dev)
25 {
26 void __iomem *ioaddr = hw->pcsr;
27 u32 value = readl(ioaddr + GMAC_CONFIG);
28 int mtu = dev->mtu;
29
30 value |= GMAC_CORE_INIT;
31
32 /* Clear ACS bit because Ethernet switch tagging formats such as
33 * Broadcom tags can look like invalid LLC/SNAP packets and cause the
34 * hardware to truncate packets on reception.
35 */
> 36 if (netdev_uses_dsa(dev))
37 value &= ~GMAC_CONTROL_ACS;
38
39 if (mtu > 1500)
40 value |= GMAC_CONFIG_2K;
41 if (mtu > 2000)
42 value |= GMAC_CONFIG_JE;
43
44 if (hw->ps) {
45 value |= GMAC_CONFIG_TE;
46
47 value &= hw->link.speed_mask;
48 switch (hw->ps) {
49 case SPEED_1000:
50 value |= hw->link.speed1000;
51 break;
52 case SPEED_100:
53 value |= hw->link.speed100;
54 break;
55 case SPEED_10:
56 value |= hw->link.speed10;
57 break;
58 }
59 }
60
61 writel(value, ioaddr + GMAC_CONFIG);
62
63 /* Mask GMAC interrupts */
64 value = GMAC_INT_DEFAULT_MASK;
65 if (hw->pmt)
66 value |= GMAC_INT_PMT_EN;
67 if (hw->pcs)
68 value |= GMAC_PCS_IRQ_DEFAULT;
69
70 writel(value, ioaddr + GMAC_INT_EN);
71 }
72
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 01/19/2018 02:06 PM, kbuild test robot wrote: > Hi Florian, > > I love your patch! Perhaps something to improve: > > [auto build test WARNING on net-next/master] > > url: https://github.com/0day-ci/linux/commits/Florian-Fainelli/net-stmmac-Fix-reception-of-Broadcom-switches-tags/20180120-044006 > config: x86_64-randconfig-x002-201802 (attached as .config) > compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025 > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > All warnings (new ones prefixed by >>): > > In file included from include/asm-generic/bug.h:5:0, > from arch/x86/include/asm/bug.h:82, > from include/linux/bug.h:5, > from include/linux/mmdebug.h:5, > from include/linux/gfp.h:5, > from include/linux/slab.h:15, > from drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c:17: > drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c: In function 'dwmac4_core_init': > drivers/net//ethernet/stmicro/stmmac/dwmac4_core.c:36:6: error: implicit declaration of function 'netdev_uses_dsa'; did you mean 'netdev_reset_tc'? [-Werror=implicit-function-declaration] > if (netdev_uses_dsa(dev)) This was fixed in v2 that David applied, thanks!
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index ce2ea2d491ac..2ffe76c0ff74 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -474,7 +474,7 @@ struct mac_device_info; /* Helpers to program the MAC core */ struct stmmac_ops { /* MAC core initialization */ - void (*core_init)(struct mac_device_info *hw, int mtu); + void (*core_init)(struct mac_device_info *hw, struct net_device *dev); /* Enable the MAC RX/TX */ void (*set_mac)(void __iomem *ioaddr, bool enable); /* Enable and verify that the IPC module is supported */ diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index 9eb7f65d8000..a3fa65b1ca8e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -483,7 +483,8 @@ static int sun8i_dwmac_init(struct platform_device *pdev, void *priv) return 0; } -static void sun8i_dwmac_core_init(struct mac_device_info *hw, int mtu) +static void sun8i_dwmac_core_init(struct mac_device_info *hw, + struct net_device *dev) { void __iomem *ioaddr = hw->pcsr; u32 v; diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index 8a86340ff2d3..540d21786a43 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -25,18 +25,28 @@ #include <linux/crc32.h> #include <linux/slab.h> #include <linux/ethtool.h> +#include <net/dsa.h> #include <asm/io.h> #include "stmmac_pcs.h" #include "dwmac1000.h" -static void dwmac1000_core_init(struct mac_device_info *hw, int mtu) +static void dwmac1000_core_init(struct mac_device_info *hw, + struct net_device *dev) { void __iomem *ioaddr = hw->pcsr; u32 value = readl(ioaddr + GMAC_CONTROL); + int mtu = dev->mtu; /* Configure GMAC core */ value |= GMAC_CORE_INIT; + /* Clear ACS bit because Ethernet switch tagging formats such as + * Broadcom tags can look like invalid LLC/SNAP packets and cause the + * hardware to truncate packets on reception. + */ + if (netdev_uses_dsa(dev)) + value &= ~GMAC_CONTROL_ACS; + if (mtu > 1500) value |= GMAC_CONTROL_2K; if (mtu > 2000) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c index 8ef517356313..c1ee427c42cb 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c @@ -28,7 +28,8 @@ #include <asm/io.h> #include "dwmac100.h" -static void dwmac100_core_init(struct mac_device_info *hw, int mtu) +static void dwmac100_core_init(struct mac_device_info *hw, + struct net_device *dev) { void __iomem *ioaddr = hw->pcsr; u32 value = readl(ioaddr + MAC_CONTROL); diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c index f3ed8f7853eb..6af5100d3cb2 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c @@ -20,13 +20,22 @@ #include "stmmac_pcs.h" #include "dwmac4.h" -static void dwmac4_core_init(struct mac_device_info *hw, int mtu) +static void dwmac4_core_init(struct mac_device_info *hw, + struct net_device *dev) { void __iomem *ioaddr = hw->pcsr; u32 value = readl(ioaddr + GMAC_CONFIG); + int mtu = dev->mtu; value |= GMAC_CORE_INIT; + /* Clear ACS bit because Ethernet switch tagging formats such as + * Broadcom tags can look like invalid LLC/SNAP packets and cause the + * hardware to truncate packets on reception. + */ + if (netdev_uses_dsa(dev)) + value &= ~GMAC_CONTROL_ACS; + if (mtu > 1500) value |= GMAC_CONFIG_2K; if (mtu > 2000) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index f99f14c35063..7ad841434ec8 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -2527,7 +2527,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp) } /* Initialize the MAC Core */ - priv->hw->mac->core_init(priv->hw, dev->mtu); + priv->hw->mac->core_init(priv->hw, dev); /* Initialize MTL*/ if (priv->synopsys_id >= DWMAC_CORE_4_00)
Broadcom tags inserted by Broadcom switches put a 4 byte header after the MAC SA and before the EtherType, which may look like some sort of 0 length LLC/SNAP packet (tcpdump and wireshark do think that way). With ACS enabled in stmmac the packets were truncated to 8 bytes on reception, whereas clearing this bit allowed normal reception to occur. In order to make that possible, we need to pass a net_device argument to the different core_init() functions and we are dependent on the Broadcom tagger padding packets correctly (which it now does). To be as little invasive as possible, this is only done for gmac1000 when the network device is DSA-enabled (netdev_uses_dsa() returns true). Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- drivers/net/ethernet/stmicro/stmmac/common.h | 2 +- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 3 ++- drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 12 +++++++++++- drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 3 ++- drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 11 ++++++++++- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- 6 files changed, 27 insertions(+), 6 deletions(-)