From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from server19320154104.serverpool.info ([193.201.54.104]:49696 "EHLO hauke-m.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756095Ab2EaW6p (ORCPT ); Thu, 31 May 2012 18:58:45 -0400 Message-ID: <4FC7F78B.6080103@hauke-m.de> (sfid-20120601_005849_745973_EAF121F3) Date: Fri, 01 Jun 2012 00:58:19 +0200 From: Hauke Mehrtens MIME-Version: 1.0 To: Seth Forshee CC: Arend van Spriel , Stefano Brivio , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , linux-wireless@vger.kernel.org, b43-dev@lists.infradead.org Subject: Re: [PATCH] b43: ensure ext PA lines are enabled for BCM4331 References: <1338472163-3810-1-git-send-email-seth.forshee@canonical.com> <4FC77D25.4060100@hauke-m.de> <20120531142652.GA32179@thinkpad-t410> <4FC79AFD.6030803@hauke-m.de> <20120531220635.GE32179@thinkpad-t410> In-Reply-To: <20120531220635.GE32179@thinkpad-t410> Content-Type: text/plain; charset=UTF-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: 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 >