Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1589698 > unrolled thread

[PATCH net v4 0/2] net: ethernet: bgmac: bug fixes

Back to article view | Back to linux.kernel

Contents

  [PATCH net v4 0/2] net: ethernet: bgmac: bug fixes Jon Mason <jon.mason@broadcom.com> - 2017-02-28 19:50 +0100
    [PATCH net v4 2/2] net: ethernet: bgmac: mac address change bug Jon Mason <jon.mason@broadcom.com> - 2017-02-28 20:00 +0100
    Re: [PATCH net v4 0/2] net: ethernet: bgmac: bug fixes David Miller <davem@davemloft.net> - 2017-03-02 22:20 +0100
      Re: [PATCH net v4 0/2] net: ethernet: bgmac: bug fixes Jon Mason <jon.mason@broadcom.com> - 2017-03-03 03:50 +0100
    Re: [PATCH net v4 0/2] net: ethernet: bgmac: bug fixes David Miller <davem@davemloft.net> - 2017-03-02 22:50 +0100

#1589698 — [PATCH net v4 0/2] net: ethernet: bgmac: bug fixes

Changes in v4:
* Added the udelays from the previous code (per David Miller)

Changes in v3:
* Reworked the init sequence patch to only remove the device reset if
  the device is actually in reset.  Given that this code doesn't bear
  much resemblance to the original code, I'm changing the author of the
  patch.  This was tested on NS2 SVK.

Changes in v2:
* Reworked the first match to make it more obvious what portions of the
  register were being preserved (Per Rafal Mileki)
* Style change to reorder the function variables in patch 2 (per Sergei
  Shtylyov)


Bug fixes for bgmac driver


Hari Vyas (1):
  net: ethernet: bgmac: mac address change bug

Jon Mason (1):
  net: ethernet: bgmac: init sequence bug

 drivers/net/ethernet/broadcom/bgmac-platform.c | 27 +++++++++++++++++---------
 drivers/net/ethernet/broadcom/bgmac.c          |  6 +++++-
 drivers/net/ethernet/broadcom/bgmac.h          | 16 +++++++++++++++
 3 files changed, 39 insertions(+), 10 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1589705 — [PATCH net v4 2/2] net: ethernet: bgmac: mac address change bug

From: Hari Vyas <hariv@broadcom.com>

ndo_set_mac_address() passes struct sockaddr * as 2nd parameter to
bgmac_set_mac_address() but code assumed u8 *.  This caused two bytes
chopping and the wrong mac address was configured.

Signed-off-by: Hari Vyas <hariv@broadcom.com>
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
Fixes: 4e209001b86 ("bgmac: write mac address to hardware in ndo_set_mac_address")
---
 drivers/net/ethernet/broadcom/bgmac.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 4150467..6b7782f 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1223,12 +1223,16 @@ static netdev_tx_t bgmac_start_xmit(struct sk_buff *skb,
 static int bgmac_set_mac_address(struct net_device *net_dev, void *addr)
 {
 	struct bgmac *bgmac = netdev_priv(net_dev);
+	struct sockaddr *sa = addr;
 	int ret;
 
 	ret = eth_prepare_mac_addr_change(net_dev, addr);
 	if (ret < 0)
 		return ret;
-	bgmac_write_mac_address(bgmac, (u8 *)addr);
+
+	ether_addr_copy(bgmac->mac_addr, sa->sa_data);
+	bgmac_write_mac_address(bgmac, bgmac->mac_addr);
+
 	eth_commit_mac_addr_change(net_dev, addr);
 	return 0;
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1591468

From: David Miller <davem@davemloft.net>
Date: Thu, 02 Mar 2017 12:50:15 -0800 (PST)

> From: Jon Mason <jon.mason@broadcom.com>
> Date: Tue, 28 Feb 2017 13:41:49 -0500
> 
>> Changes in v4:
>> * Added the udelays from the previous code (per David Miller)
>> 
>> Changes in v3:
>> * Reworked the init sequence patch to only remove the device reset if
>>   the device is actually in reset.  Given that this code doesn't bear
>>   much resemblance to the original code, I'm changing the author of the
>>   patch.  This was tested on NS2 SVK.
>> 
>> Changes in v2:
>> * Reworked the first match to make it more obvious what portions of the
>>   register were being preserved (Per Rafal Mileki)
>> * Style change to reorder the function variables in patch 2 (per Sergei
>>   Shtylyov)
>> 
>> Bug fixes for bgmac driver
> 
> Series applied.

Actually, this doesn't even compile.  Reverted...

[davem@kkuri net]$ make -s -j4
drivers/net/ethernet/broadcom/bgmac.c: In function ‘bgmac_set_mac_address’:
drivers/net/ethernet/broadcom/bgmac.c:1233:23: error: ‘struct bgmac’ has no member named ‘mac_addr’; did you mean ‘phyaddr’?
  ether_addr_copy(bgmac->mac_addr, sa->sa_data);
                       ^~
drivers/net/ethernet/broadcom/bgmac.c:1234:38: error: ‘struct bgmac’ has no member named ‘mac_addr’; did you mean ‘phyaddr’?
  bgmac_write_mac_address(bgmac, bgmac->mac_addr);
                                      ^~

[toc] | [prev] | [next] | [standalone]

#1591636

On Thu, Mar 02, 2017 at 12:56:05PM -0800, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Thu, 02 Mar 2017 12:50:15 -0800 (PST)
> 
> > From: Jon Mason <jon.mason@broadcom.com>
> > Date: Tue, 28 Feb 2017 13:41:49 -0500
> > 
> >> Changes in v4:
> >> * Added the udelays from the previous code (per David Miller)
> >> 
> >> Changes in v3:
> >> * Reworked the init sequence patch to only remove the device reset if
> >>   the device is actually in reset.  Given that this code doesn't bear
> >>   much resemblance to the original code, I'm changing the author of the
> >>   patch.  This was tested on NS2 SVK.
> >> 
> >> Changes in v2:
> >> * Reworked the first match to make it more obvious what portions of the
> >>   register were being preserved (Per Rafal Mileki)
> >> * Style change to reorder the function variables in patch 2 (per Sergei
> >>   Shtylyov)
> >> 
> >> Bug fixes for bgmac driver
> > 
> > Series applied.
> 
> Actually, this doesn't even compile.  Reverted...
> 
> [davem@kkuri net]$ make -s -j4
> drivers/net/ethernet/broadcom/bgmac.c: In function ‘bgmac_set_mac_address’:
> drivers/net/ethernet/broadcom/bgmac.c:1233:23: error: ‘struct bgmac’ has no member named ‘mac_addr’; did you mean ‘phyaddr’?
>   ether_addr_copy(bgmac->mac_addr, sa->sa_data);
>                        ^~
> drivers/net/ethernet/broadcom/bgmac.c:1234:38: error: ‘struct bgmac’ has no member named ‘mac_addr’; did you mean ‘phyaddr’?
>   bgmac_write_mac_address(bgmac, bgmac->mac_addr);
>                                       ^~

Well this is embarrassing.  I didn't rebase, even though I acked the
patch which changed it out from under me.  Sorry, I should've known
better.

Rebased, compiled, and tested patch coming shortly.  I appreciate your
patience.

Thanks,
Jon

[toc] | [prev] | [next] | [standalone]

#1591491

From: Jon Mason <jon.mason@broadcom.com>
Date: Tue, 28 Feb 2017 13:41:49 -0500

> Changes in v4:
> * Added the udelays from the previous code (per David Miller)
> 
> Changes in v3:
> * Reworked the init sequence patch to only remove the device reset if
>   the device is actually in reset.  Given that this code doesn't bear
>   much resemblance to the original code, I'm changing the author of the
>   patch.  This was tested on NS2 SVK.
> 
> Changes in v2:
> * Reworked the first match to make it more obvious what portions of the
>   register were being preserved (Per Rafal Mileki)
> * Style change to reorder the function variables in patch 2 (per Sergei
>   Shtylyov)
> 
> Bug fixes for bgmac driver

Series applied.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web