Patchwork net: fec: Fix multicast list setup in fec_restart().

login
register
mail settings
Submitter Christoph Müllner
Date June 21, 2013, 4:40 p.m.
Message ID <61455.62.178.124.14.1371832855.squirrel@mail.theobroma-systems.com>
Download mbox | patch
Permalink /patch/253290/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Christoph Müllner - June 21, 2013, 4:40 p.m.
Setup the multicast list of the net_device instead of
clearing it blindly. This restores the multicast groups
in case of a link down/up event.

Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 drivers/net/ethernet/freescale/fec_main.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
Fabio Estevam - June 21, 2013, 5:15 p.m.
On Fri, Jun 21, 2013 at 1:40 PM, Christoph Müllner
<christoph.muellner@theobroma-systems.com> wrote:

> @@ -472,8 +474,7 @@ fec_restart(struct net_device *ndev, int duplex)
>         writel(0xffc00000, fep->hwp + FEC_IEVENT);
>
>         /* Reset all multicast. */

Should this comment be removed/updated now?

> -       writel(0, fep->hwp + FEC_GRP_HASH_TABLE_HIGH);
> -       writel(0, fep->hwp + FEC_GRP_HASH_TABLE_LOW);
> +       set_multicast_list(ndev);
--
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
Joe Perches - June 21, 2013, 5:25 p.m.
On Fri, 2013-06-21 at 18:40 +0200, Christoph Müllner wrote:
> Setup the multicast list of the net_device instead of
> clearing it blindly. This restores the multicast groups
> in case of a link down/up event.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>

Theobroma?
food of the gods or does your company just like chocolate?

> diff --git a/drivers/net/ethernet/freescale/fec_main.c
[]
> +static void set_multicast_list(struct net_device *ndev);
[]
> @@ -472,8 +474,7 @@ fec_restart(struct net_device *ndev, int duplex)
>         writel(0xffc00000, fep->hwp + FEC_IEVENT);
> 
>         /* Reset all multicast. */
> -       writel(0, fep->hwp + FEC_GRP_HASH_TABLE_HIGH);
> -       writel(0, fep->hwp + FEC_GRP_HASH_TABLE_LOW);
> +       set_multicast_list(ndev);

maybe dev_set_rx_mode(ndev) ?


--
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
Christoph Müllner - June 27, 2013, 5:14 p.m.
On Jun 21, 2013, at 7:15 PM, Fabio Estevam <festevam@gmail.com> wrote:

> On Fri, Jun 21, 2013 at 1:40 PM, Christoph Müllner
> <christoph.muellner@theobroma-systems.com> wrote:
> 
>> @@ -472,8 +474,7 @@ fec_restart(struct net_device *ndev, int duplex)
>>        writel(0xffc00000, fep->hwp + FEC_IEVENT);
>> 
>>        /* Reset all multicast. */
> 
> Should this comment be removed/updated now?

Is updated in the attached patch.
Thank you for the feedback!

> 
>> -       writel(0, fep->hwp + FEC_GRP_HASH_TABLE_HIGH);
>> -       writel(0, fep->hwp + FEC_GRP_HASH_TABLE_LOW);
>> +       set_multicast_list(ndev);
Fabio Estevam - June 27, 2013, 5:21 p.m.
On Thu, Jun 27, 2013 at 2:14 PM, Christoph Müllner
<christoph.muellner@theobroma-systems.com> wrote:

> Is updated in the attached patch.
> Thank you for the feedback!

Looks good.  Some suggestions:

- Mark the patch as the second version:
Subject: [PATCH v2] net: fec: Fix multicast list setup in fec_restart().

- Add a comment below the --- line explaining what you changed in v2.

Something like:

---
Changes since v1:
- Adapt the comment to the code change

- Then re-send it via git send-email to the list instead of sending it
via attachment.

Thanks,

Fabio Estevam
--
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

Patch

diff --git a/drivers/net/ethernet/freescale/fec_main.c
b/drivers/net/ethernet/freescale/fec_main.c
index a667015..8e8e743 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -60,6 +60,8 @@ 

 #include "fec.h"

+static void set_multicast_list(struct net_device *ndev);
+
 #if defined(CONFIG_ARM)
 #define FEC_ALIGNMENT  0xf
 #else
@@ -472,8 +474,7 @@  fec_restart(struct net_device *ndev, int duplex)
        writel(0xffc00000, fep->hwp + FEC_IEVENT);

        /* Reset all multicast. */
-       writel(0, fep->hwp + FEC_GRP_HASH_TABLE_HIGH);
-       writel(0, fep->hwp + FEC_GRP_HASH_TABLE_LOW);
+       set_multicast_list(ndev);
 #ifndef CONFIG_M5272
        writel(0, fep->hwp + FEC_HASH_TABLE_HIGH);
        writel(0, fep->hwp + FEC_HASH_TABLE_LOW);