* [PATCH 0/2] net: ethernet: bgmac: bug fixes @ 2017-02-01 22:39 Jon Mason 2017-02-01 22:39 ` [PATCH 1/2] net: ethernet: bgmac: init sequence bug Jon Mason 2017-02-01 22:39 ` [PATCH 2/2] net: ethernet: bgmac: mac address change bug Jon Mason 0 siblings, 2 replies; 12+ messages in thread From: Jon Mason @ 2017-02-01 22:39 UTC (permalink / raw) To: David Miller; +Cc: rafal, bcm-kernel-feedback-list, netdev, linux-kernel Bug fixes for bgmac driver Hari Vyas (1): net: ethernet: bgmac: mac address change bug Zac Schroff (1): net: ethernet: bgmac: init sequence bug drivers/net/ethernet/broadcom/bgmac-platform.c | 10 +++++++--- drivers/net/ethernet/broadcom/bgmac.c | 6 +++++- include/linux/bcma/bcma_regs.h | 1 + 3 files changed, 13 insertions(+), 4 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] net: ethernet: bgmac: init sequence bug 2017-02-01 22:39 [PATCH 0/2] net: ethernet: bgmac: bug fixes Jon Mason @ 2017-02-01 22:39 ` Jon Mason 2017-02-01 23:06 ` Rafał Miłecki 2017-02-02 9:47 ` Sergei Shtylyov 2017-02-01 22:39 ` [PATCH 2/2] net: ethernet: bgmac: mac address change bug Jon Mason 1 sibling, 2 replies; 12+ messages in thread From: Jon Mason @ 2017-02-01 22:39 UTC (permalink / raw) To: David Miller Cc: rafal, bcm-kernel-feedback-list, netdev, linux-kernel, Zac Schroff From: Zac Schroff <zschroff@broadcom.com> Fix a bug in the 'bgmac' driver init sequence that blind writes for init sequence where it should preserve most bits other than the ones it is deliberately manipulating. Signed-off-by: Zac Schroff <zschroff@broadcom.com> Signed-off-by: Jon Mason <jon.mason@broadcom.com> Fixes: f6a95a24957 ("net: ethernet: bgmac: Add platform device support") --- drivers/net/ethernet/broadcom/bgmac-platform.c | 10 +++++++--- include/linux/bcma/bcma_regs.h | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c index 6f736c1..9bbe05c 100644 --- a/drivers/net/ethernet/broadcom/bgmac-platform.c +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c @@ -61,15 +61,19 @@ static bool platform_bgmac_clk_enabled(struct bgmac *bgmac) static void platform_bgmac_clk_enable(struct bgmac *bgmac, u32 flags) { - bgmac_idm_write(bgmac, BCMA_IOCTL, - (BCMA_IOCTL_CLK | BCMA_IOCTL_FGC | flags)); + u32 regval; + + /* Some bits of BCMA_IOCTL set by HW/ATF & should not change */ + regval = bgmac_idm_read(bgmac, BCMA_IOCTL) & BCMA_IOCTL_DO_NOT_MODIFY; + regval |= ((flags & (~BCMA_IOCTL_DO_NOT_MODIFY)) | BCMA_IOCTL_CLK); + bgmac_idm_write(bgmac, BCMA_IOCTL, regval | BCMA_IOCTL_FGC); bgmac_idm_read(bgmac, BCMA_IOCTL); bgmac_idm_write(bgmac, BCMA_RESET_CTL, 0); bgmac_idm_read(bgmac, BCMA_RESET_CTL); udelay(1); - bgmac_idm_write(bgmac, BCMA_IOCTL, (BCMA_IOCTL_CLK | flags)); + bgmac_idm_write(bgmac, BCMA_IOCTL, regval); bgmac_idm_read(bgmac, BCMA_IOCTL); udelay(1); } diff --git a/include/linux/bcma/bcma_regs.h b/include/linux/bcma/bcma_regs.h index 9986f82..41d7404 100644 --- a/include/linux/bcma/bcma_regs.h +++ b/include/linux/bcma/bcma_regs.h @@ -31,6 +31,7 @@ #define BCMA_IOCTL_CORE_BITS 0x3FFC #define BCMA_IOCTL_PME_EN 0x4000 #define BCMA_IOCTL_BIST_EN 0x8000 +#define BCMA_IOCTL_DO_NOT_MODIFY 0x7FFFFF80 #define BCMA_IOST 0x0500 /* IO status */ #define BCMA_IOST_CORE_BITS 0x0FFF #define BCMA_IOST_DMA64 0x1000 -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] net: ethernet: bgmac: init sequence bug 2017-02-01 22:39 ` [PATCH 1/2] net: ethernet: bgmac: init sequence bug Jon Mason @ 2017-02-01 23:06 ` Rafał Miłecki 2017-02-02 0:31 ` Zac Schroff 2017-02-02 18:54 ` Jon Mason 2017-02-02 9:47 ` Sergei Shtylyov 1 sibling, 2 replies; 12+ messages in thread From: Rafał Miłecki @ 2017-02-01 23:06 UTC (permalink / raw) To: Jon Mason, David Miller Cc: bcm-kernel-feedback-list, netdev, linux-kernel, Zac Schroff On 02/01/2017 11:39 PM, Jon Mason wrote: > From: Zac Schroff <zschroff@broadcom.com> > > Fix a bug in the 'bgmac' driver init sequence that blind writes for init > sequence where it should preserve most bits other than the ones it is > deliberately manipulating. > > Signed-off-by: Zac Schroff <zschroff@broadcom.com> > Signed-off-by: Jon Mason <jon.mason@broadcom.com> > Fixes: f6a95a24957 ("net: ethernet: bgmac: Add platform device support") > --- > drivers/net/ethernet/broadcom/bgmac-platform.c | 10 +++++++--- > include/linux/bcma/bcma_regs.h | 1 + > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c > index 6f736c1..9bbe05c 100644 > --- a/drivers/net/ethernet/broadcom/bgmac-platform.c > +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c > @@ -61,15 +61,19 @@ static bool platform_bgmac_clk_enabled(struct bgmac *bgmac) > > static void platform_bgmac_clk_enable(struct bgmac *bgmac, u32 flags) > { > - bgmac_idm_write(bgmac, BCMA_IOCTL, > - (BCMA_IOCTL_CLK | BCMA_IOCTL_FGC | flags)); > + u32 regval; > + > + /* Some bits of BCMA_IOCTL set by HW/ATF & should not change */ > + regval = bgmac_idm_read(bgmac, BCMA_IOCTL) & BCMA_IOCTL_DO_NOT_MODIFY; > + regval |= ((flags & (~BCMA_IOCTL_DO_NOT_MODIFY)) | BCMA_IOCTL_CLK); You don't need these braces around whole calculation. This should work the same: (flags & (~BCMA_IOCTL_DO_NOT_MODIFY)) | BCMA_IOCTL_CLK > + bgmac_idm_write(bgmac, BCMA_IOCTL, regval | BCMA_IOCTL_FGC); > bgmac_idm_read(bgmac, BCMA_IOCTL); > > bgmac_idm_write(bgmac, BCMA_RESET_CTL, 0); > bgmac_idm_read(bgmac, BCMA_RESET_CTL); > udelay(1); > > - bgmac_idm_write(bgmac, BCMA_IOCTL, (BCMA_IOCTL_CLK | flags)); > + bgmac_idm_write(bgmac, BCMA_IOCTL, regval); > bgmac_idm_read(bgmac, BCMA_IOCTL); > udelay(1); > } > diff --git a/include/linux/bcma/bcma_regs.h b/include/linux/bcma/bcma_regs.h > index 9986f82..41d7404 100644 > --- a/include/linux/bcma/bcma_regs.h > +++ b/include/linux/bcma/bcma_regs.h > @@ -31,6 +31,7 @@ > #define BCMA_IOCTL_CORE_BITS 0x3FFC > #define BCMA_IOCTL_PME_EN 0x4000 > #define BCMA_IOCTL_BIST_EN 0x8000 > +#define BCMA_IOCTL_DO_NOT_MODIFY 0x7FFFFF80 This sounds like a pretty bad name. Take a look at brcmsmac and SICF_*: http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/brcm80211/brcmsmac/d11.h?v=4.9#L1737 Or b43 and B43_BCMA_IOCTL_*: http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/b43/b43.h?v=4.9#L494 Both drives modify bits you marked as DO_NOT_MODIFY and they are OK. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] net: ethernet: bgmac: init sequence bug 2017-02-01 23:06 ` Rafał Miłecki @ 2017-02-02 0:31 ` Zac Schroff 2017-02-02 20:15 ` Rafał Miłecki 2017-02-02 18:54 ` Jon Mason 1 sibling, 1 reply; 12+ messages in thread From: Zac Schroff @ 2017-02-02 0:31 UTC (permalink / raw) To: Rafał Miłecki Cc: Jon Mason, David Miller, bcm-kernel-feedback-list, netdev, linux-kernel, Zac Schroff How about BCMA_IOCTL_PRESERVE_ACROSS_INIT? On Wed, Feb 1, 2017 at 6:06 PM, Rafał Miłecki <rafal@milecki.pl> wrote: > On 02/01/2017 11:39 PM, Jon Mason wrote: >> >> From: Zac Schroff <zschroff@broadcom.com> >> >> Fix a bug in the 'bgmac' driver init sequence that blind writes for init >> sequence where it should preserve most bits other than the ones it is >> deliberately manipulating. >> >> Signed-off-by: Zac Schroff <zschroff@broadcom.com> >> Signed-off-by: Jon Mason <jon.mason@broadcom.com> >> Fixes: f6a95a24957 ("net: ethernet: bgmac: Add platform device support") >> --- >> drivers/net/ethernet/broadcom/bgmac-platform.c | 10 +++++++--- >> include/linux/bcma/bcma_regs.h | 1 + >> 2 files changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c >> b/drivers/net/ethernet/broadcom/bgmac-platform.c >> index 6f736c1..9bbe05c 100644 >> --- a/drivers/net/ethernet/broadcom/bgmac-platform.c >> +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c >> @@ -61,15 +61,19 @@ static bool platform_bgmac_clk_enabled(struct bgmac >> *bgmac) >> >> static void platform_bgmac_clk_enable(struct bgmac *bgmac, u32 flags) >> { >> - bgmac_idm_write(bgmac, BCMA_IOCTL, >> - (BCMA_IOCTL_CLK | BCMA_IOCTL_FGC | flags)); >> + u32 regval; >> + >> + /* Some bits of BCMA_IOCTL set by HW/ATF & should not change */ >> + regval = bgmac_idm_read(bgmac, BCMA_IOCTL) & >> BCMA_IOCTL_DO_NOT_MODIFY; >> + regval |= ((flags & (~BCMA_IOCTL_DO_NOT_MODIFY)) | >> BCMA_IOCTL_CLK); > > > You don't need these braces around whole calculation. > This should work the same: > (flags & (~BCMA_IOCTL_DO_NOT_MODIFY)) | BCMA_IOCTL_CLK > > >> + bgmac_idm_write(bgmac, BCMA_IOCTL, regval | BCMA_IOCTL_FGC); >> bgmac_idm_read(bgmac, BCMA_IOCTL); >> >> bgmac_idm_write(bgmac, BCMA_RESET_CTL, 0); >> bgmac_idm_read(bgmac, BCMA_RESET_CTL); >> udelay(1); >> >> - bgmac_idm_write(bgmac, BCMA_IOCTL, (BCMA_IOCTL_CLK | flags)); >> + bgmac_idm_write(bgmac, BCMA_IOCTL, regval); >> bgmac_idm_read(bgmac, BCMA_IOCTL); >> udelay(1); >> } >> diff --git a/include/linux/bcma/bcma_regs.h >> b/include/linux/bcma/bcma_regs.h >> index 9986f82..41d7404 100644 >> --- a/include/linux/bcma/bcma_regs.h >> +++ b/include/linux/bcma/bcma_regs.h >> @@ -31,6 +31,7 @@ >> #define BCMA_IOCTL_CORE_BITS 0x3FFC >> #define BCMA_IOCTL_PME_EN 0x4000 >> #define BCMA_IOCTL_BIST_EN 0x8000 >> +#define BCMA_IOCTL_DO_NOT_MODIFY 0x7FFFFF80 > > > This sounds like a pretty bad name. > > Take a look at brcmsmac and SICF_*: > http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/brcm80211/brcmsmac/d11.h?v=4.9#L1737 > > Or b43 and B43_BCMA_IOCTL_*: > http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/b43/b43.h?v=4.9#L494 > > Both drives modify bits you marked as DO_NOT_MODIFY and they are OK. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] net: ethernet: bgmac: init sequence bug 2017-02-02 0:31 ` Zac Schroff @ 2017-02-02 20:15 ` Rafał Miłecki 2017-02-02 20:20 ` Jon Mason 0 siblings, 1 reply; 12+ messages in thread From: Rafał Miłecki @ 2017-02-02 20:15 UTC (permalink / raw) To: Zac Schroff Cc: Jon Mason, David Miller, bcm-kernel-feedback-list, netdev, linux-kernel, Zac Schroff On 2017-02-02 01:31, Zac Schroff wrote: > How about BCMA_IOCTL_PRESERVE_ACROSS_INIT? I think wireless drivers may still set some these bits during init. I've a simpler idea: make it bgmac specific. Call it sth like BGMAC_BCMA_IOCTL_PRESERVE BGMAC_BCMA_IOCTL_RESERVED BGMAC_BCMA_IOCTL_DONT_TOUCH ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] net: ethernet: bgmac: init sequence bug 2017-02-02 20:15 ` Rafał Miłecki @ 2017-02-02 20:20 ` Jon Mason 0 siblings, 0 replies; 12+ messages in thread From: Jon Mason @ 2017-02-02 20:20 UTC (permalink / raw) To: Rafał Miłecki Cc: Zac Schroff, David Miller, BCM Kernel Feedback, Network Development, open list, Zac Schroff On Thu, Feb 2, 2017 at 3:15 PM, Rafał Miłecki <rafal@milecki.pl> wrote: > On 2017-02-02 01:31, Zac Schroff wrote: >> >> How about BCMA_IOCTL_PRESERVE_ACROSS_INIT? > > > I think wireless drivers may still set some these bits during init. > > I've a simpler idea: make it bgmac specific. Call it sth like > BGMAC_BCMA_IOCTL_PRESERVE > BGMAC_BCMA_IOCTL_RESERVED > BGMAC_BCMA_IOCTL_DONT_TOUCH Yes, I am listing out all of the fields in that register. We can be intelligent about what we mask off :) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] net: ethernet: bgmac: init sequence bug 2017-02-01 23:06 ` Rafał Miłecki 2017-02-02 0:31 ` Zac Schroff @ 2017-02-02 18:54 ` Jon Mason 1 sibling, 0 replies; 12+ messages in thread From: Jon Mason @ 2017-02-02 18:54 UTC (permalink / raw) To: Rafał Miłecki Cc: David Miller, BCM Kernel Feedback, Network Development, open list, Zac Schroff On Wed, Feb 1, 2017 at 6:06 PM, Rafał Miłecki <rafal@milecki.pl> wrote: > On 02/01/2017 11:39 PM, Jon Mason wrote: >> >> From: Zac Schroff <zschroff@broadcom.com> >> >> Fix a bug in the 'bgmac' driver init sequence that blind writes for init >> sequence where it should preserve most bits other than the ones it is >> deliberately manipulating. >> >> Signed-off-by: Zac Schroff <zschroff@broadcom.com> >> Signed-off-by: Jon Mason <jon.mason@broadcom.com> >> Fixes: f6a95a24957 ("net: ethernet: bgmac: Add platform device support") >> --- >> drivers/net/ethernet/broadcom/bgmac-platform.c | 10 +++++++--- >> include/linux/bcma/bcma_regs.h | 1 + >> 2 files changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c >> b/drivers/net/ethernet/broadcom/bgmac-platform.c >> index 6f736c1..9bbe05c 100644 >> --- a/drivers/net/ethernet/broadcom/bgmac-platform.c >> +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c >> @@ -61,15 +61,19 @@ static bool platform_bgmac_clk_enabled(struct bgmac >> *bgmac) >> >> static void platform_bgmac_clk_enable(struct bgmac *bgmac, u32 flags) >> { >> - bgmac_idm_write(bgmac, BCMA_IOCTL, >> - (BCMA_IOCTL_CLK | BCMA_IOCTL_FGC | flags)); >> + u32 regval; >> + >> + /* Some bits of BCMA_IOCTL set by HW/ATF & should not change */ >> + regval = bgmac_idm_read(bgmac, BCMA_IOCTL) & >> BCMA_IOCTL_DO_NOT_MODIFY; >> + regval |= ((flags & (~BCMA_IOCTL_DO_NOT_MODIFY)) | >> BCMA_IOCTL_CLK); > > > You don't need these braces around whole calculation. > This should work the same: > (flags & (~BCMA_IOCTL_DO_NOT_MODIFY)) | BCMA_IOCTL_CLK Fair enough > > >> + bgmac_idm_write(bgmac, BCMA_IOCTL, regval | BCMA_IOCTL_FGC); >> bgmac_idm_read(bgmac, BCMA_IOCTL); >> >> bgmac_idm_write(bgmac, BCMA_RESET_CTL, 0); >> bgmac_idm_read(bgmac, BCMA_RESET_CTL); >> udelay(1); >> >> - bgmac_idm_write(bgmac, BCMA_IOCTL, (BCMA_IOCTL_CLK | flags)); >> + bgmac_idm_write(bgmac, BCMA_IOCTL, regval); >> bgmac_idm_read(bgmac, BCMA_IOCTL); >> udelay(1); >> } >> diff --git a/include/linux/bcma/bcma_regs.h >> b/include/linux/bcma/bcma_regs.h >> index 9986f82..41d7404 100644 >> --- a/include/linux/bcma/bcma_regs.h >> +++ b/include/linux/bcma/bcma_regs.h >> @@ -31,6 +31,7 @@ >> #define BCMA_IOCTL_CORE_BITS 0x3FFC >> #define BCMA_IOCTL_PME_EN 0x4000 >> #define BCMA_IOCTL_BIST_EN 0x8000 >> +#define BCMA_IOCTL_DO_NOT_MODIFY 0x7FFFFF80 > > > This sounds like a pretty bad name. Name change coming > Take a look at brcmsmac and SICF_*: > http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/brcm80211/brcmsmac/d11.h?v=4.9#L1737 > > Or b43 and B43_BCMA_IOCTL_*: > http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/b43/b43.h?v=4.9#L494 > > Both drives modify bits you marked as DO_NOT_MODIFY and they are OK. I think the point Zac was trying to make is that this is changing bits that aren't meaning to be modified. We should only be flipping the bits necessary to enable the clocks, etc. Bootloaders, etc might be setting bits (and in our case they are) which are being removed forcing it to a predefined value. Thanks, Jon ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] net: ethernet: bgmac: init sequence bug 2017-02-01 22:39 ` [PATCH 1/2] net: ethernet: bgmac: init sequence bug Jon Mason 2017-02-01 23:06 ` Rafał Miłecki @ 2017-02-02 9:47 ` Sergei Shtylyov 1 sibling, 0 replies; 12+ messages in thread From: Sergei Shtylyov @ 2017-02-02 9:47 UTC (permalink / raw) To: Jon Mason, David Miller Cc: rafal, bcm-kernel-feedback-list, netdev, linux-kernel, Zac Schroff Hello! On 2/2/2017 1:39 AM, Jon Mason wrote: > From: Zac Schroff <zschroff@broadcom.com> > > Fix a bug in the 'bgmac' driver init sequence that blind writes for init > sequence where it should preserve most bits other than the ones it is > deliberately manipulating. > > Signed-off-by: Zac Schroff <zschroff@broadcom.com> > Signed-off-by: Jon Mason <jon.mason@broadcom.com> > Fixes: f6a95a24957 ("net: ethernet: bgmac: Add platform device support") > --- > drivers/net/ethernet/broadcom/bgmac-platform.c | 10 +++++++--- > include/linux/bcma/bcma_regs.h | 1 + > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c > index 6f736c1..9bbe05c 100644 > --- a/drivers/net/ethernet/broadcom/bgmac-platform.c > +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c > @@ -61,15 +61,19 @@ static bool platform_bgmac_clk_enabled(struct bgmac *bgmac) > > static void platform_bgmac_clk_enable(struct bgmac *bgmac, u32 flags) > { > - bgmac_idm_write(bgmac, BCMA_IOCTL, > - (BCMA_IOCTL_CLK | BCMA_IOCTL_FGC | flags)); > + u32 regval; > + > + /* Some bits of BCMA_IOCTL set by HW/ATF & should not change */ > + regval = bgmac_idm_read(bgmac, BCMA_IOCTL) & BCMA_IOCTL_DO_NOT_MODIFY; > + regval |= ((flags & (~BCMA_IOCTL_DO_NOT_MODIFY)) | BCMA_IOCTL_CLK); The innermost parens aren't necessary. And the outermost as well. [...] MBR, Sergei ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] net: ethernet: bgmac: mac address change bug 2017-02-01 22:39 [PATCH 0/2] net: ethernet: bgmac: bug fixes Jon Mason 2017-02-01 22:39 ` [PATCH 1/2] net: ethernet: bgmac: init sequence bug Jon Mason @ 2017-02-01 22:39 ` Jon Mason 2017-02-01 23:12 ` Rafał Miłecki 2017-02-02 9:49 ` Sergei Shtylyov 1 sibling, 2 replies; 12+ messages in thread From: Jon Mason @ 2017-02-01 22:39 UTC (permalink / raw) To: David Miller Cc: rafal, bcm-kernel-feedback-list, netdev, linux-kernel, Hari Vyas 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 0e066dc6..ea24072 100644 --- a/drivers/net/ethernet/broadcom/bgmac.c +++ b/drivers/net/ethernet/broadcom/bgmac.c @@ -1222,11 +1222,15 @@ static int bgmac_set_mac_address(struct net_device *net_dev, void *addr) { struct bgmac *bgmac = netdev_priv(net_dev); int ret; + struct sockaddr *sa = addr; 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] net: ethernet: bgmac: mac address change bug 2017-02-01 22:39 ` [PATCH 2/2] net: ethernet: bgmac: mac address change bug Jon Mason @ 2017-02-01 23:12 ` Rafał Miłecki 2017-02-02 18:13 ` Jon Mason 2017-02-02 9:49 ` Sergei Shtylyov 1 sibling, 1 reply; 12+ messages in thread From: Rafał Miłecki @ 2017-02-01 23:12 UTC (permalink / raw) To: Jon Mason, David Miller Cc: bcm-kernel-feedback-list, netdev, linux-kernel, Hari Vyas On 02/01/2017 11:39 PM, Jon Mason wrote: > 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") Sounds OK, would it make sense to Cc stable? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] net: ethernet: bgmac: mac address change bug 2017-02-01 23:12 ` Rafał Miłecki @ 2017-02-02 18:13 ` Jon Mason 0 siblings, 0 replies; 12+ messages in thread From: Jon Mason @ 2017-02-02 18:13 UTC (permalink / raw) To: Rafał Miłecki Cc: David Miller, BCM Kernel Feedback, Network Development, open list, Hari Vyas On Wed, Feb 1, 2017 at 6:12 PM, Rafał Miłecki <rafal@milecki.pl> wrote: > On 02/01/2017 11:39 PM, Jon Mason wrote: >> >> 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") > > > Sounds OK, would it make sense to Cc stable? Sure, I'll add Sergei's mods and do a v2 with stable on the Cc list ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] net: ethernet: bgmac: mac address change bug 2017-02-01 22:39 ` [PATCH 2/2] net: ethernet: bgmac: mac address change bug Jon Mason 2017-02-01 23:12 ` Rafał Miłecki @ 2017-02-02 9:49 ` Sergei Shtylyov 1 sibling, 0 replies; 12+ messages in thread From: Sergei Shtylyov @ 2017-02-02 9:49 UTC (permalink / raw) To: Jon Mason, David Miller Cc: rafal, bcm-kernel-feedback-list, netdev, linux-kernel, Hari Vyas On 2/2/2017 1:39 AM, Jon Mason wrote: > 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 0e066dc6..ea24072 100644 > --- a/drivers/net/ethernet/broadcom/bgmac.c > +++ b/drivers/net/ethernet/broadcom/bgmac.c > @@ -1222,11 +1222,15 @@ static int bgmac_set_mac_address(struct net_device *net_dev, void *addr) > { > struct bgmac *bgmac = netdev_priv(net_dev); > int ret; > + struct sockaddr *sa = addr; DaveM prefers the declarations to be arranged from longest to shortest. [...] MBR, Sergei ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-02-03 2:13 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-01 22:39 [PATCH 0/2] net: ethernet: bgmac: bug fixes Jon Mason 2017-02-01 22:39 ` [PATCH 1/2] net: ethernet: bgmac: init sequence bug Jon Mason 2017-02-01 23:06 ` Rafał Miłecki 2017-02-02 0:31 ` Zac Schroff 2017-02-02 20:15 ` Rafał Miłecki 2017-02-02 20:20 ` Jon Mason 2017-02-02 18:54 ` Jon Mason 2017-02-02 9:47 ` Sergei Shtylyov 2017-02-01 22:39 ` [PATCH 2/2] net: ethernet: bgmac: mac address change bug Jon Mason 2017-02-01 23:12 ` Rafał Miłecki 2017-02-02 18:13 ` Jon Mason 2017-02-02 9:49 ` Sergei Shtylyov
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).