* [PATCH net 0/2] net: systemport: misc fixes
@ 2014-06-20 20:01 Florian Fainelli
2014-06-20 20:01 ` [PATCH net 1/2] net: systemport: do not clear IFF_MULTICAST flag Florian Fainelli
2014-06-20 20:01 ` [PATCH net 2/2] net: systemport: fix UniMAC reset logic Florian Fainelli
0 siblings, 2 replies; 6+ messages in thread
From: Florian Fainelli @ 2014-06-20 20:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
Hi David,
Here are two small fixes for SYSTEMPORT targeted at the 'net' tree, thanks!
Florian Fainelli (2):
net: systemport: do not clear IFF_MULTICAST flag
net: systemport: fix UniMAC reset logic
drivers/net/ethernet/broadcom/bcmsysport.c | 39 +++++++-----------------------
1 file changed, 9 insertions(+), 30 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 1/2] net: systemport: do not clear IFF_MULTICAST flag
2014-06-20 20:01 [PATCH net 0/2] net: systemport: misc fixes Florian Fainelli
@ 2014-06-20 20:01 ` Florian Fainelli
2014-06-20 20:01 ` [PATCH net 2/2] net: systemport: fix UniMAC reset logic Florian Fainelli
1 sibling, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2014-06-20 20:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
The SYSTEMPORT Ethernet MAC supports multicast just fine, it just lacks
any sort of Unicast/Broadcast/Multicasting filtering at the Ethernet MAC
level since that is handled by the front end Ethernet switch, but that
is properly handled by bcm_sysport_set_rx_mode().
Some user-space applications might be relying on the presence of this
flag to prevent using multicast sockets, this also prevents that
interface from joining the IPv6 all-router mcast group.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 141160ef249a..f6bccd847ee6 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1589,12 +1589,6 @@ static int bcm_sysport_probe(struct platform_device *pdev)
BUILD_BUG_ON(sizeof(struct bcm_tsb) != 8);
dev->needed_headroom += sizeof(struct bcm_tsb);
- /* We are interfaced to a switch which handles the multicast
- * filtering for us, so we do not support programming any
- * multicast hash table in this Ethernet MAC.
- */
- dev->flags &= ~IFF_MULTICAST;
-
/* libphy will adjust the link state accordingly */
netif_carrier_off(dev);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/2] net: systemport: fix UniMAC reset logic
2014-06-20 20:01 [PATCH net 0/2] net: systemport: misc fixes Florian Fainelli
2014-06-20 20:01 ` [PATCH net 1/2] net: systemport: do not clear IFF_MULTICAST flag Florian Fainelli
@ 2014-06-20 20:01 ` Florian Fainelli
2014-06-20 20:14 ` Sergei Shtylyov
2014-06-24 23:12 ` David Miller
1 sibling, 2 replies; 6+ messages in thread
From: Florian Fainelli @ 2014-06-20 20:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
The UniMAC CMD_SW_RESET bit is not a self-clearing bit, so we need to
assert it, wait a bit and clear it manually. As a result, umac_reset()
is updated not to return any value.
By setting CMD_SW_RESET and only that bit, we were also clearing the
other 31 bits, overriding the hardware reset defaults which are
correctly set on purpose.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 33 ++++++++----------------------
1 file changed, 9 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index f6bccd847ee6..d31f7d239064 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1254,28 +1254,17 @@ static inline void umac_enable_set(struct bcm_sysport_priv *priv,
usleep_range(1000, 2000);
}
-static inline int umac_reset(struct bcm_sysport_priv *priv)
+static inline void umac_reset(struct bcm_sysport_priv *priv)
{
- unsigned int timeout = 0;
u32 reg;
- int ret = 0;
-
- umac_writel(priv, 0, UMAC_CMD);
- while (timeout++ < 1000) {
- reg = umac_readl(priv, UMAC_CMD);
- if (!(reg & CMD_SW_RESET))
- break;
-
- udelay(1);
- }
-
- if (timeout == 1000) {
- dev_err(&priv->pdev->dev,
- "timeout waiting for MAC to come out of reset\n");
- ret = -ETIMEDOUT;
- }
- return ret;
+ reg = umac_readl(priv, UMAC_CMD);
+ reg |= CMD_SW_RESET;
+ umac_writel(priv, reg, UMAC_CMD);
+ udelay(10);
+ reg = umac_readl(priv, UMAC_CMD);
+ reg &= ~CMD_SW_RESET;
+ umac_writel(priv, reg, UMAC_CMD);
}
static void umac_set_hw_addr(struct bcm_sysport_priv *priv,
@@ -1303,11 +1292,7 @@ static int bcm_sysport_open(struct net_device *dev)
int ret;
/* Reset UniMAC */
- ret = umac_reset(priv);
- if (ret) {
- netdev_err(dev, "UniMAC reset failed\n");
- return ret;
- }
+ umac_reset(priv);
/* Flush TX and RX FIFOs at TOPCTRL level */
topctrl_flush(priv);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net: systemport: fix UniMAC reset logic
2014-06-20 20:01 ` [PATCH net 2/2] net: systemport: fix UniMAC reset logic Florian Fainelli
@ 2014-06-20 20:14 ` Sergei Shtylyov
2014-06-24 23:12 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2014-06-20 20:14 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem
Hello.
On 06/21/2014 12:01 AM, Florian Fainelli wrote:
> The UniMAC CMD_SW_RESET bit is not a self-clearing bit, so we need to
> assert it, wait a bit and clear it manually. As a result, umac_reset()
> is updated not to return any value.
> By setting CMD_SW_RESET and only that bit, we were also clearing the
I wonder where you were setting this bit before -- the only umac_writel()
there was wrote 0 to UMAC_CMD...
> other 31 bits, overriding the hardware reset defaults which are
> correctly set on purpose.
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/broadcom/bcmsysport.c | 33 ++++++++----------------------
> 1 file changed, 9 insertions(+), 24 deletions(-)
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index f6bccd847ee6..d31f7d239064 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -1254,28 +1254,17 @@ static inline void umac_enable_set(struct bcm_sysport_priv *priv,
> usleep_range(1000, 2000);
> }
>
> -static inline int umac_reset(struct bcm_sysport_priv *priv)
> +static inline void umac_reset(struct bcm_sysport_priv *priv)
> {
> - unsigned int timeout = 0;
> u32 reg;
> - int ret = 0;
> -
> - umac_writel(priv, 0, UMAC_CMD);
> - while (timeout++ < 1000) {
> - reg = umac_readl(priv, UMAC_CMD);
> - if (!(reg & CMD_SW_RESET))
> - break;
> -
> - udelay(1);
> - }
> -
> - if (timeout == 1000) {
> - dev_err(&priv->pdev->dev,
> - "timeout waiting for MAC to come out of reset\n");
> - ret = -ETIMEDOUT;
> - }
>
> - return ret;
> + reg = umac_readl(priv, UMAC_CMD);
> + reg |= CMD_SW_RESET;
> + umac_writel(priv, reg, UMAC_CMD);
> + udelay(10);
> + reg = umac_readl(priv, UMAC_CMD);
> + reg &= ~CMD_SW_RESET;
> + umac_writel(priv, reg, UMAC_CMD);
> }
>
> static void umac_set_hw_addr(struct bcm_sysport_priv *priv,
[...]
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net: systemport: fix UniMAC reset logic
2014-06-20 20:01 ` [PATCH net 2/2] net: systemport: fix UniMAC reset logic Florian Fainelli
2014-06-20 20:14 ` Sergei Shtylyov
@ 2014-06-24 23:12 ` David Miller
2014-06-24 23:16 ` Florian Fainelli
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2014-06-24 23:12 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 20 Jun 2014 13:01:40 -0700
> The UniMAC CMD_SW_RESET bit is not a self-clearing bit, so we need to
> assert it, wait a bit and clear it manually. As a result, umac_reset()
> is updated not to return any value.
>
> By setting CMD_SW_RESET and only that bit, we were also clearing the
> other 31 bits, overriding the hardware reset defaults which are
> correctly set on purpose.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
I agree with Sergei, this is really suspicious that the existing code
never wrote the sw reset bit into the register at all, it just writes
a zero.
And you don't mention this at all in this commit message.
If I were to guess, I would fathom that the sequence was partially
copied from the other broadcom driver, the genet one. There it
issues CMD_SW_RESET by first clearing the entire regiser to zero,
then writing CMD_SW_RESET, and then polling for that bit to clear.
I'm assuming that is being done for a good reason, and I'd like
you to make sure that same reason does not apply here.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net: systemport: fix UniMAC reset logic
2014-06-24 23:12 ` David Miller
@ 2014-06-24 23:16 ` Florian Fainelli
0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2014-06-24 23:16 UTC (permalink / raw)
To: David Miller; +Cc: netdev
2014-06-24 16:12 GMT-07:00 David Miller <davem@davemloft.net>:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Fri, 20 Jun 2014 13:01:40 -0700
>
>> The UniMAC CMD_SW_RESET bit is not a self-clearing bit, so we need to
>> assert it, wait a bit and clear it manually. As a result, umac_reset()
>> is updated not to return any value.
>>
>> By setting CMD_SW_RESET and only that bit, we were also clearing the
>> other 31 bits, overriding the hardware reset defaults which are
>> correctly set on purpose.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> I agree with Sergei, this is really suspicious that the existing code
> never wrote the sw reset bit into the register at all, it just writes
> a zero.
>
> And you don't mention this at all in this commit message.
>
> If I were to guess, I would fathom that the sequence was partially
> copied from the other broadcom driver, the genet one. There it
> issues CMD_SW_RESET by first clearing the entire regiser to zero,
> then writing CMD_SW_RESET, and then polling for that bit to clear.
>
> I'm assuming that is being done for a good reason, and I'd like
> you to make sure that same reason does not apply here.
I'll resubmit with a proper explanation, thanks.
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-06-24 23:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-20 20:01 [PATCH net 0/2] net: systemport: misc fixes Florian Fainelli
2014-06-20 20:01 ` [PATCH net 1/2] net: systemport: do not clear IFF_MULTICAST flag Florian Fainelli
2014-06-20 20:01 ` [PATCH net 2/2] net: systemport: fix UniMAC reset logic Florian Fainelli
2014-06-20 20:14 ` Sergei Shtylyov
2014-06-24 23:12 ` David Miller
2014-06-24 23:16 ` Florian Fainelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).