* [PATCH] b43: ensure ext PA lines are enabled for BCM4331 @ 2012-05-31 13:49 Seth Forshee 2012-05-31 14:16 ` Hauke Mehrtens 0 siblings, 1 reply; 7+ messages in thread From: Seth Forshee @ 2012-05-31 13:49 UTC (permalink / raw) To: Stefano Brivio, Rafał Miłecki Cc: linux-wireless, b43-dev, John W. Linville, Arend van Spriel Some MacBook Pro models with BCM4331 wireless will stop transmitting after resuming from S3 without external power attached. This is fixed by ensuring that the ext PA lines are enabled in BCMA_CC_CHIPCTL. Export the function in bcma which does this for use by b43 and enable the ext PA lines during wireless core initialization. BugLink: http://bugs.launchpad.net/bugs/925577 Cc: Arend van Spriel <arend@broadcom.com> Cc: stable@vger.kernel.org Signed-off-by: Seth Forshee <seth.forshee@canonical.com> --- drivers/bcma/driver_chipcommon_pmu.c | 1 + drivers/net/wireless/b43/main.c | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/drivers/bcma/driver_chipcommon_pmu.c b/drivers/bcma/driver_chipcommon_pmu.c index a058842..332d7fb 100644 --- a/drivers/bcma/driver_chipcommon_pmu.c +++ b/drivers/bcma/driver_chipcommon_pmu.c @@ -129,6 +129,7 @@ void bcma_chipco_bcm4331_ext_pa_lines_ctl(struct bcma_drv_cc *cc, bool enable) } bcma_cc_write32(cc, BCMA_CC_CHIPCTL, val); } +EXPORT_SYMBOL_GPL(bcma_chipco_bcm4331_ext_pa_lines_ctl); void bcma_pmu_workarounds(struct bcma_drv_cc *cc) { diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c index 5a39b22..6bac5cf 100644 --- a/drivers/net/wireless/b43/main.c +++ b/drivers/net/wireless/b43/main.c @@ -4713,6 +4713,13 @@ static int b43_wireless_core_init(struct b43_wldev *dev) b43_upload_card_macaddress(dev); b43_security_init(dev); +#ifdef CONFIG_B43_BCMA + /* Required for tx to work on BCM4331 */ + if (dev->dev->bus_type == B43_BUS_BCMA && dev->dev->chip_id == 0x4331) + bcma_chipco_bcm4331_ext_pa_lines_ctl(&dev->dev->bdev->bus->drv_cc, + true); +#endif + ieee80211_wake_queues(dev->wl->hw); b43_set_status(dev, B43_STAT_INITIALIZED); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] b43: ensure ext PA lines are enabled for BCM4331 2012-05-31 13:49 [PATCH] b43: ensure ext PA lines are enabled for BCM4331 Seth Forshee @ 2012-05-31 14:16 ` Hauke Mehrtens 2012-05-31 14:26 ` Seth Forshee 0 siblings, 1 reply; 7+ messages in thread From: Hauke Mehrtens @ 2012-05-31 14:16 UTC (permalink / raw) To: Seth Forshee Cc: Stefano Brivio, Rafał Miłecki, Arend van Spriel, linux-wireless, b43-dev On 05/31/2012 03:49 PM, Seth Forshee wrote: > Some MacBook Pro models with BCM4331 wireless will stop transmitting > after resuming from S3 without external power attached. This is fixed by > ensuring that the ext PA lines are enabled in BCMA_CC_CHIPCTL. Export > the function in bcma which does this for use by b43 and enable the ext > PA lines during wireless core initialization. > > BugLink: http://bugs.launchpad.net/bugs/925577 > Cc: Arend van Spriel <arend@broadcom.com> > Cc: stable@vger.kernel.org > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > --- > drivers/bcma/driver_chipcommon_pmu.c | 1 + > drivers/net/wireless/b43/main.c | 7 +++++++ > 2 files changed, 8 insertions(+) > > diff --git a/drivers/bcma/driver_chipcommon_pmu.c b/drivers/bcma/driver_chipcommon_pmu.c > index a058842..332d7fb 100644 > --- a/drivers/bcma/driver_chipcommon_pmu.c > +++ b/drivers/bcma/driver_chipcommon_pmu.c > @@ -129,6 +129,7 @@ void bcma_chipco_bcm4331_ext_pa_lines_ctl(struct bcma_drv_cc *cc, bool enable) > } > bcma_cc_write32(cc, BCMA_CC_CHIPCTL, val); > } > +EXPORT_SYMBOL_GPL(bcma_chipco_bcm4331_ext_pa_lines_ctl); > > void bcma_pmu_workarounds(struct bcma_drv_cc *cc) > { > diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c > index 5a39b22..6bac5cf 100644 > --- a/drivers/net/wireless/b43/main.c > +++ b/drivers/net/wireless/b43/main.c > @@ -4713,6 +4713,13 @@ static int b43_wireless_core_init(struct b43_wldev *dev) > b43_upload_card_macaddress(dev); > b43_security_init(dev); > > +#ifdef CONFIG_B43_BCMA > + /* Required for tx to work on BCM4331 */ > + if (dev->dev->bus_type == B43_BUS_BCMA && dev->dev->chip_id == 0x4331) > + bcma_chipco_bcm4331_ext_pa_lines_ctl(&dev->dev->bdev->bus->drv_cc, > + true); > +#endif > + > ieee80211_wake_queues(dev->wl->hw); > > b43_set_status(dev, B43_STAT_INITIALIZED); Hi Seth, why don't you call this from bcma_pmu_workarounds() in drivers/bcma/driver_chipcommon_pmu.c instead of calling this from b43? I think it looks better to call some workarounds on chip common from bcma and not from b43. According to some Broadcom code this should also be called for chip_id 43431 when turning it on and in the sprom code. Hauke ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] b43: ensure ext PA lines are enabled for BCM4331 2012-05-31 14:16 ` Hauke Mehrtens @ 2012-05-31 14:26 ` Seth Forshee 2012-05-31 16:23 ` Hauke Mehrtens 2012-05-31 17:43 ` Arend van Spriel 0 siblings, 2 replies; 7+ messages in thread From: Seth Forshee @ 2012-05-31 14:26 UTC (permalink / raw) To: Hauke Mehrtens, Arend van Spriel Cc: Stefano Brivio, Rafał Miłecki, linux-wireless, b43-dev On Thu, May 31, 2012 at 04:16:05PM +0200, Hauke Mehrtens wrote: > Hi Seth, > > why don't you call this from bcma_pmu_workarounds() in > drivers/bcma/driver_chipcommon_pmu.c instead of calling this from b43? I > think it looks better to call some workarounds on chip common from bcma > and not from b43. Arend recommended calling it from within b43's start op, but I'm not sure of the reason. Arend? Agreed though that if there's no need to run it every time the interface is started then bcma_pmu_workarounds() would be a nicer place for it. > According to some Broadcom code this should also be called for chip_id > 43431 when turning it on and in the sprom code. I'm having trouble parsing this, specifically the "and in the sprom code" part. Can you clarify? Thanks, Seth ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] b43: ensure ext PA lines are enabled for BCM4331 2012-05-31 14:26 ` Seth Forshee @ 2012-05-31 16:23 ` Hauke Mehrtens 2012-05-31 22:06 ` Seth Forshee 2012-05-31 17:43 ` Arend van Spriel 1 sibling, 1 reply; 7+ messages in thread From: Hauke Mehrtens @ 2012-05-31 16:23 UTC (permalink / raw) To: Seth Forshee Cc: Arend van Spriel, Stefano Brivio, Rafał Miłecki, linux-wireless, b43-dev On 05/31/2012 04:26 PM, Seth Forshee wrote: > On Thu, May 31, 2012 at 04:16:05PM +0200, Hauke Mehrtens wrote: >> Hi Seth, >> >> why don't you call this from bcma_pmu_workarounds() in >> drivers/bcma/driver_chipcommon_pmu.c instead of calling this from b43? I >> think it looks better to call some workarounds on chip common from bcma >> and not from b43. > > Arend recommended calling it from within b43's start op, but I'm not > sure of the reason. Arend? > > Agreed though that if there's no need to run it every time the interface > is started then bcma_pmu_workarounds() would be a nicer place for it. brcmsmac calls the chip specific the workarounds in ai_doattach(), but the ones for BCM4313 and BCM43224 are also in bcma_pmu_workarounds(), this should be cleaned up to have them just at one place. In some old version of brcmsmac the workaround for the BCM4331 was also called from ai_doattach() function, but later removed likely because this devices is not supported by brcmsmac. So if there is not better reason as, the proprietary Broadcom driver does so, I would like to see this call in bcma_pmu_workarounds(). >> According to some Broadcom code this should also be called for chip_id >> 43431 when turning it on and in the sprom code. > > I'm having trouble parsing this, specifically the "and in the sprom > code" part. Can you clarify? In the Broadcom SDK code si_chipcontrl_epa4331(), the same function as bcma_chipco_bcm4331_ext_pa_lines_ctl() in bcma, gets called for devices with a chip id of 0x4331 and 43431, both seam to be BCM4331 devices. We should also call our workarounds for both chip ids. Hauke ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] b43: ensure ext PA lines are enabled for BCM4331 2012-05-31 16:23 ` Hauke Mehrtens @ 2012-05-31 22:06 ` Seth Forshee 2012-05-31 22:58 ` Hauke Mehrtens 0 siblings, 1 reply; 7+ messages in thread From: Seth Forshee @ 2012-05-31 22:06 UTC (permalink / raw) To: Hauke Mehrtens Cc: Arend van Spriel, Stefano Brivio, Rafał Miłecki, linux-wireless, b43-dev On Thu, May 31, 2012 at 06:23:25PM +0200, Hauke Mehrtens wrote: > On 05/31/2012 04:26 PM, Seth Forshee wrote: > > On Thu, May 31, 2012 at 04:16:05PM +0200, Hauke Mehrtens wrote: > >> Hi Seth, > >> > >> why don't you call this from bcma_pmu_workarounds() in > >> drivers/bcma/driver_chipcommon_pmu.c instead of calling this from b43? I > >> think it looks better to call some workarounds on chip common from bcma > >> and not from b43. > > > > Arend recommended calling it from within b43's start op, but I'm not > > sure of the reason. Arend? > > > > Agreed though that if there's no need to run it every time the interface > > is started then bcma_pmu_workarounds() would be a nicer place for it. > > brcmsmac calls the chip specific the workarounds in ai_doattach(), but > the ones for BCM4313 and BCM43224 are also in bcma_pmu_workarounds(), > this should be cleaned up to have them just at one place. In some old > version of brcmsmac the workaround for the BCM4331 was also called from > ai_doattach() function, but later removed likely because this devices is > not supported by brcmsmac. > So if there is not better reason as, the proprietary Broadcom driver > does so, I would like to see this call in bcma_pmu_workarounds(). I found the SDK code from a router source package, and oddly that code is clearing those bits in si_doattach(). In fact si_chipcontrl_epa4331() is never called to enable the ext PA lines, only to disable them. The sprom code reads the value before disabling them and restores it after reading from the sprom. Enabling the ext PA pins is what fixes things for me. Predictably, duplicating this in bcma makes wireless broken all the time. At any rate, it sounds like we're agreed to add the workaround in bcma_pmu_workarounds(), so I'll redo the patch. > >> According to some Broadcom code this should also be called for chip_id > >> 43431 when turning it on and in the sprom code. > > > > I'm having trouble parsing this, specifically the "and in the sprom > > code" part. Can you clarify? > > In the Broadcom SDK code si_chipcontrl_epa4331(), the same function as > bcma_chipco_bcm4331_ext_pa_lines_ctl() in bcma, gets called for devices > with a chip id of 0x4331 and 43431, both seam to be BCM4331 devices. We > should also call our workarounds for both chip ids. I'll fix this too. Thanks, Seth ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] b43: ensure ext PA lines are enabled for BCM4331 2012-05-31 22:06 ` Seth Forshee @ 2012-05-31 22:58 ` Hauke Mehrtens 0 siblings, 0 replies; 7+ messages in thread From: Hauke Mehrtens @ 2012-05-31 22:58 UTC (permalink / raw) To: Seth Forshee Cc: Arend van Spriel, Stefano Brivio, Rafał Miłecki, linux-wireless, b43-dev On 06/01/2012 12:06 AM, Seth Forshee wrote: > On Thu, May 31, 2012 at 06:23:25PM +0200, Hauke Mehrtens wrote: >> On 05/31/2012 04:26 PM, Seth Forshee wrote: >>> On Thu, May 31, 2012 at 04:16:05PM +0200, Hauke Mehrtens wrote: >>>> Hi Seth, >>>> >>>> why don't you call this from bcma_pmu_workarounds() in >>>> drivers/bcma/driver_chipcommon_pmu.c instead of calling this from b43? I >>>> think it looks better to call some workarounds on chip common from bcma >>>> and not from b43. >>> >>> Arend recommended calling it from within b43's start op, but I'm not >>> sure of the reason. Arend? >>> >>> Agreed though that if there's no need to run it every time the interface >>> is started then bcma_pmu_workarounds() would be a nicer place for it. >> >> brcmsmac calls the chip specific the workarounds in ai_doattach(), but >> the ones for BCM4313 and BCM43224 are also in bcma_pmu_workarounds(), >> this should be cleaned up to have them just at one place. In some old >> version of brcmsmac the workaround for the BCM4331 was also called from >> ai_doattach() function, but later removed likely because this devices is >> not supported by brcmsmac. >> So if there is not better reason as, the proprietary Broadcom driver >> does so, I would like to see this call in bcma_pmu_workarounds(). > > I found the SDK code from a router source package, and oddly that code > is clearing those bits in si_doattach(). In fact si_chipcontrl_epa4331() > is never called to enable the ext PA lines, only to disable them. The > sprom code reads the value before disabling them and restores it after > reading from the sprom. Enabling the ext PA pins is what fixes things > for me. Predictably, duplicating this in bcma makes wireless broken all > the time. The open source SDK just contains some of the code, the wireless part is closed source and I haven't seen any call which enables this in the open part too. The initial commit of brcm80211 to the kernel staging area contains a call to this function with true given. It is commit a9533e7ea3c410fed2f4cd8b3e1e213e48529b75 if you are interested. > At any rate, it sounds like we're agreed to add the workaround in > bcma_pmu_workarounds(), so I'll redo the patch. > >>>> According to some Broadcom code this should also be called for chip_id >>>> 43431 when turning it on and in the sprom code. >>> >>> I'm having trouble parsing this, specifically the "and in the sprom >>> code" part. Can you clarify? >> >> In the Broadcom SDK code si_chipcontrl_epa4331(), the same function as >> bcma_chipco_bcm4331_ext_pa_lines_ctl() in bcma, gets called for devices >> with a chip id of 0x4331 and 43431, both seam to be BCM4331 devices. We >> should also call our workarounds for both chip ids. > > I'll fix this too. > > Thanks, > Seth > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] b43: ensure ext PA lines are enabled for BCM4331 2012-05-31 14:26 ` Seth Forshee 2012-05-31 16:23 ` Hauke Mehrtens @ 2012-05-31 17:43 ` Arend van Spriel 1 sibling, 0 replies; 7+ messages in thread From: Arend van Spriel @ 2012-05-31 17:43 UTC (permalink / raw) To: Seth Forshee Cc: Hauke Mehrtens, Stefano Brivio, Rafał Miłecki, linux-wireless, b43-dev On 05/31/2012 04:26 PM, Seth Forshee wrote: > On Thu, May 31, 2012 at 04:16:05PM +0200, Hauke Mehrtens wrote: >> > Hi Seth, >> > >> > why don't you call this from bcma_pmu_workarounds() in >> > drivers/bcma/driver_chipcommon_pmu.c instead of calling this from b43? I >> > think it looks better to call some workarounds on chip common from bcma >> > and not from b43. > Arend recommended calling it from within b43's start op, but I'm not > sure of the reason. Arend? Well, my comment on start op was regarding your observation of CHIPCTL being zero. As the start op does reinitialize the hardware you should check CHIPCTL register at the end of it. This is just to confirm the register contains valid bits. > Agreed though that if there's no need to run it every time the interface > is started then bcma_pmu_workarounds() would be a nicer place for it. > So I agree with Hauke and have this code in bcma. Just not sure whether bcma_pmu_workarounds() is the place to put it. This is not a pmu related register, but as the function was already there I will not stress to move it to driver_chipcommon.c ;-) Gr. AvS ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-05-31 22:58 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-31 13:49 [PATCH] b43: ensure ext PA lines are enabled for BCM4331 Seth Forshee 2012-05-31 14:16 ` Hauke Mehrtens 2012-05-31 14:26 ` Seth Forshee 2012-05-31 16:23 ` Hauke Mehrtens 2012-05-31 22:06 ` Seth Forshee 2012-05-31 22:58 ` Hauke Mehrtens 2012-05-31 17:43 ` Arend van Spriel
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).