b43: ensure ext PA lines are enabled for BCM4331
PATCH 1/1 b43: ensure ext PA lines are enabled for BCM4331
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.
--- drivers/bcma/driver_chipcommon_pmu.c | 1 + drivers/net/wireless/b43/main.c | 7 +++++++ 2 files changed, 8 insertions(+)
drivers/bcma/driver_chipcommon_pmu.c +1 -0
@@ -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) {
drivers/net/wireless/b43/main.c +7 -0
@@ -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 );
Signature · 1 line
1.7.9.5
PATCH 1/1 b43: ensure ext PA lines are enabled for BCM4331
Quoted 35 lines · Seth Forshee
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.Cc Arend van Spriel <[email protected]>Cc stable at vger.kernel.orgSigned-off-by Seth Forshee <[email protected]>--- drivers/bcma/driver_chipcommon_pmu.c | 1 + drivers/net/wireless/b43/main.c | 7 +++++++ 2 files changed, 8 insertions(+)drivers/bcma/driver_chipcommon_pmu.c +1
-0@@ -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) {drivers/net/wireless/b43/main.c +0
-0@@ -4713,6 +4713,13 @@ static int b43_wireless_core_init(struct b43_wldev *dev)b43_upload_card_macaddress (dev);b43_security_init (dev);
drivers/net/wireless/b43/main.c static int b43_wireless_core_init(struct b43_wldev *dev) · line 4725
Quoted 10 lines
drivers/net/wireless/b43/main.c +7
-0+#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
PATCH 1/1 b43: ensure ext PA lines are enabled for BCM4331
Quoted 6 lines · Hauke Mehrtens
Hi Seth,
why don't you call this from
bcma_pmu_workarounds()indrivers/bcma/driver_chipcommon_pmu.cinstead 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.
Quoted 2 lines
According to some Broadcom code this should also be called for
chip_id43431 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
PATCH 1/1 b43: ensure ext PA lines are enabled for BCM4331
Quoted 8 lines · Seth Forshee
Quoted 6 lines · Hauke Mehrtens
Hi Seth,
why don't you call this from
bcma_pmu_workarounds()indrivers/bcma/driver_chipcommon_pmu.cinstead of calling this from b43? I think it looks better to call some workarounds on chip common from bcma and not from b43.
Quoted 5 lines
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().
Quoted 5 lines
Quoted 2 lines
According to some Broadcom code this should also be called for
chip_id43431 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
PATCH 1/1 b43: ensure ext PA lines are enabled for BCM4331
Quoted 15 lines · Hauke Mehrtens
Quoted 8 lines · Seth Forshee
Quoted 6 lines · Hauke Mehrtens
Hi Seth,
why don't you call this from
bcma_pmu_workarounds()indrivers/bcma/driver_chipcommon_pmu.cinstead of calling this from b43? I think it looks better to call some workarounds on chip common from bcma and not from b43.Quoted 5 lines
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.
Quoted 8 lines
brcmsmac calls the chip specific the workarounds in
ai_doattach(), but the ones for BCM4313 and BCM43224 are also inbcma_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 fromai_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 inbcma_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.
Quoted 10 lines
Quoted 5 lines
Quoted 2 lines
According to some Broadcom code this should also be called for
chip_id43431 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 asbcma_chipco_bcm4331_ext_pa_lines_ctl()in bcma, gets called for devices with a chip id of0x4331and 43431, both seam to be BCM4331 devices. We should also call our workarounds for both chip ids.
I'll fix this too.
Thanks,
Seth
PATCH 1/1 b43: ensure ext PA lines are enabled for BCM4331
Quoted 25 lines · Seth Forshee
Quoted 15 lines · Hauke Mehrtens
Quoted 8 lines · Seth Forshee
Quoted 6 lines · Hauke Mehrtens
Hi Seth,
why don't you call this from
bcma_pmu_workarounds()indrivers/bcma/driver_chipcommon_pmu.cinstead of calling this from b43? I think it looks better to call some workarounds on chip common from bcma and not from b43.Quoted 5 lines
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.Quoted 8 lines
brcmsmac calls the chip specific the workarounds in
ai_doattach(), but the ones for BCM4313 and BCM43224 are also inbcma_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 fromai_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 inbcma_pmu_workarounds().
Quoted 7 lines
I found the SDK code from a router source package, and oddly that code is clearing those bits in
si_doattach(). In factsi_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.
Quoted 19 lines
At any rate, it sounds like we're agreed to add the workaround in
bcma_pmu_workarounds(), so I'll redo the patch.Quoted 10 lines
Quoted 5 lines
Quoted 2 lines
According to some Broadcom code this should also be called for
chip_id43431 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 asbcma_chipco_bcm4331_ext_pa_lines_ctl()in bcma, gets called for devices with a chip id of0x4331and 43431, both seam to be BCM4331 devices. We should also call our workarounds for both chip ids.I'll fix this too.
Thanks,
Seth
PATCH 1/1 b43: ensure ext PA lines are enabled for BCM4331
In reply to Seth ForsheeQuoted 9 lines · Seth Forshee
Quoted 6 lines · Hauke Mehrtens
Quoted 6 lines
Hi Seth,
why don't you call this from
bcma_pmu_workarounds()indrivers/bcma/driver_chipcommon_pmu.cinstead 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.
Quoted 3 lines
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