On 06/01/2012 12:06 AM, Seth Forshee wrote:

On Thu, May 31, 2012 at 06:23:25PM +0200, Hauke Mehrtens wrote:

quoted

On 05/31/2012 04:26 PM, Seth Forshee wrote:

quoted

On Thu, May 31, 2012 at 04:16:05PM +0200, Hauke Mehrtens wrote:

quoted

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.

quoted

quoted

quoted

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