Mistake in commit 0d961b3b52f566f823070ce2366511a7f64b928c breaks cpsw non dual_emac mode.

Mistake in commit 0d961b3b52f566f823070ce2366511a7f64b928c breaks cpsw non dual_emac mode.

[Lennart Sorensen]

I believe commit 0d961b3b52f566f823070ce2366511a7f64b928c made a mistake
while correcting a bug.

It was correct to fix (which applies only in dual_emac mode):

@@ -554,7 +554,7 @@ static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
                 * common for both the interface as the interface shares
                 * the same hardware resource.
                 */
-               for (i = 0; i <= priv->data.slaves; i++)
+               for (i = 0; i < priv->data.slaves; i++)
                        if (priv->slaves[i].ndev->flags & IFF_PROMISC)
                                flag = true;

since there i is used as an index into priv->slaves which is a 0 based array.

However the other two changes (which are only in non dual_emac mode):

@@ -578,7 +578,7 @@ static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
                        unsigned long timeout = jiffies + HZ;
 
                        /* Disable Learn for all ports */
-                       for (i = 0; i <= priv->data.slaves; i++) {
+                       for (i = 0; i < priv->data.slaves; i++) {
                                cpsw_ale_control_set(ale, i,
                                                     ALE_PORT_NOLEARN, 1);
                                cpsw_ale_control_set(ale, i,
@@ -606,7 +606,7 @@ static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
                        cpsw_ale_control_set(ale, 0, ALE_P0_UNI_FLOOD, 0);
 
                        /* Enable Learn for all ports */
-                       for (i = 0; i <= priv->data.slaves; i++) {
+                       for (i = 0; i < priv->data.slaves; i++) {
                                cpsw_ale_control_set(ale, i,
                                                     ALE_PORT_NOLEARN, 0);
                                cpsw_ale_control_set(ale, i,

are wrong since there i is actually the ALE port number, and port 0 is
the host port, while port 1 and up are the slave ports.

This should correct it back to working in non dual_emac mode.

Also make the comment point this out clearly to avoid future confusion
and fix a comment that was missing a "Don't".

Signed-off-by: lsorense@csclub.uwaterloo.ca

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 952e1e4..4683196 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -591,8 +591,8 @@ static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
 		if (enable) {
 			unsigned long timeout = jiffies + HZ;
 
-			/* Disable Learn for all ports */
-			for (i = 0; i < priv->data.slaves; i++) {
+			/* Disable Learn for all ports (host is port 0 and slaves are port 1 and up */
+			for (i = 0; i <= priv->data.slaves; i++) {
 				cpsw_ale_control_set(ale, i,
 						     ALE_PORT_NOLEARN, 1);
 				cpsw_ale_control_set(ale, i,
@@ -616,11 +616,11 @@ static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
 			cpsw_ale_control_set(ale, 0, ALE_P0_UNI_FLOOD, 1);
 			dev_dbg(&ndev->dev, "promiscuity enabled\n");
 		} else {
-			/* Flood All Unicast Packets to Host port */
+			/* Don't Flood All Unicast Packets to Host port */
 			cpsw_ale_control_set(ale, 0, ALE_P0_UNI_FLOOD, 0);
 
-			/* Enable Learn for all ports */
-			for (i = 0; i < priv->data.slaves; i++) {
+			/* Enable Learn for all ports (host is port 0 and slaves are port 1 and up */
+			for (i = 0; i <= priv->data.slaves; i++) {
 				cpsw_ale_control_set(ale, i,
 						     ALE_PORT_NOLEARN, 0);
 				cpsw_ale_control_set(ale, i,
-- 
Len Sorensen

Re: Mistake in commit 0d961b3b52f566f823070ce2366511a7f64b928c breaks cpsw non dual_emac mode.

[David Miller]

From: "Lennart Sorensen" <lsorense@csclub.uwaterloo.ca>
Date: Tue, 28 Oct 2014 13:02:42 -0400

> I believe commit 0d961b3b52f566f823070ce2366511a7f64b928c made a mistake
> while correcting a bug.

This patch submission is not properly formed.

You subject line should be of the form:

subsystem: Description.

"subsystem" here would be "cpsw: " or something like that.

Secondly, you should not refer to a commit ID in the patch
Subject line, instead just describe exactly what is being
fixed in the most succinct yet complete manner that is
possible.

Thirdly, when you do refer to commit ID's in your commit
message body you must do so in the following format:

${SHA1_ID} ("Commit message header line text.")

The commit message body is also not a place to have a general
discussion.  Please avoid saying things like "I think", for example.
State facts, and be exact about what the problem is and exactly
how you are fixing it.

Because this commit message will be read by others looking at your
change days, weeks, years from now.

Thanks.

Re: Mistake in commit 0d961b3b52f566f823070ce2366511a7f64b928c breaks cpsw non dual_emac mode.

[Heiko Schocher]

Hello David, Lennart,

Am 30.10.2014 20:43, schrieb David Miller:
> From: "Lennart Sorensen"<lsorense@csclub.uwaterloo.ca>
> Date: Tue, 28 Oct 2014 13:02:42 -0400
>
>> I believe commit 0d961b3b52f566f823070ce2366511a7f64b928c made a mistake
>> while correcting a bug.

Seems I missed your original patch ... looked in it here:

https://lkml.org/lkml/2014/10/28/837

and I think you are correct, thanks for this fix. You can add my
Acked-by: Heiko Schocher <hs@denx.de>
if you post a corrected v2, as David suggested.

bye,
Heiko

> This patch submission is not properly formed.
>
> You subject line should be of the form:
>
> subsystem: Description.
>
> "subsystem" here would be "cpsw: " or something like that.
>
> Secondly, you should not refer to a commit ID in the patch
> Subject line, instead just describe exactly what is being
> fixed in the most succinct yet complete manner that is
> possible.
>
> Thirdly, when you do refer to commit ID's in your commit
> message body you must do so in the following format:
>
> ${SHA1_ID} ("Commit message header line text.")
>
> The commit message body is also not a place to have a general
> discussion.  Please avoid saying things like "I think", for example.
> State facts, and be exact about what the problem is and exactly
> how you are fixing it.
>
> Because this commit message will be read by others looking at your
> change days, weeks, years from now.
>
> Thanks.
>

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Re: Mistake in commit 0d961b3b52f566f823070ce2366511a7f64b928c breaks cpsw non dual_emac mode.

[Lennart Sorensen]

On Fri, Oct 31, 2014 at 07:49:14AM +0100, Heiko Schocher wrote:
> Seems I missed your original patch ... looked in it here:
> 
> https://lkml.org/lkml/2014/10/28/837
> 
> and I think you are correct, thanks for this fix. You can add my
> Acked-by: Heiko Schocher <hs@denx.de>
> if you post a corrected v2, as David suggested.

Will do.  I noticed a wrong word in one of the messages too so good
thing it gets fixed before getting commited.

-- 
Len Sorensen