[PATCH] net: fec: Fix multicast list setup in fec_restart().

[PATCH] net: fec: Fix multicast list setup in fec_restart().

[Christoph Müllner]

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(-)

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);
-- 
1.7.4.1

Re: [PATCH] net: fec: Fix multicast list setup in fec_restart().

[Fabio Estevam]

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);

Re: [PATCH] net: fec: Fix multicast list setup in fec_restart().

[Christoph Müllner]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

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);


[-- Attachment #2: 0001-net-fec-Fix-multicast-list-setup-in-fec_restart.patch --]
[-- Type: application/octet-stream, Size: 1506 bytes --]

From efc0a82aa810cf3c37505cf3c753ad6f1cf33572 Mon Sep 17 00:00:00 2001
From: Christoph Muellner <christoph.muellner@theobroma-systems.com>
Date: Fri, 21 Jun 2013 17:59:52 +0200
Subject: [PATCH] net: fec: Fix multicast list setup in fec_restart().

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 or when resuming from
suspend.

Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 drivers/net/ethernet/freescale/fec_main.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d48099f..4c32709 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
@@ -471,9 +473,8 @@ fec_restart(struct net_device *ndev, int duplex)
 	/* Clear any outstanding interrupt. */
 	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);
+	/* Setup multicast filter. */
+	set_multicast_list(ndev);
 #ifndef CONFIG_M5272
 	writel(0, fep->hwp + FEC_HASH_TABLE_HIGH);
 	writel(0, fep->hwp + FEC_HASH_TABLE_LOW);
-- 
1.7.4.1

Re: [PATCH] net: fec: Fix multicast list setup in fec_restart().

[Fabio Estevam]

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

Re: [PATCH] net: fec: Fix multicast list setup in fec_restart().

[Joe Perches]

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) ?

[PATCH v2] net: fec: Fix multicast list setup in fec_restart().

[Christoph Muellner]

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 or when resuming from
suspend.

Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
Changes since v1:
- Adapt the comment to the code change

 drivers/net/ethernet/freescale/fec_main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d48099f..4c32709 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
@@ -471,9 +473,8 @@ fec_restart(struct net_device *ndev, int duplex)
 	/* Clear any outstanding interrupt. */
 	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);
+	/* Setup multicast filter. */
+	set_multicast_list(ndev);
 #ifndef CONFIG_M5272
 	writel(0, fep->hwp + FEC_HASH_TABLE_HIGH);
 	writel(0, fep->hwp + FEC_HASH_TABLE_LOW);
-- 
1.8.3.1

Re: [PATCH v2] net: fec: Fix multicast list setup in fec_restart().

[David Miller]

From: Christoph Muellner <christoph.muellner@theobroma-systems.com>
Date: Thu, 27 Jun 2013 21:18:23 +0200

> 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 or when resuming from
> suspend.
> 
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>

Applied.