Message-ID: <4FD0FBB5.6040008@broadcom.com> (sfid-20120607_210649_866758_E04303CF)
Date: Thu, 7 Jun 2012 21:06:29 +0200
From: "Arend van Spriel" <arend@broadcom.com>
MIME-Version: 1.0
To: "Hauke Mehrtens" <hauke@hauke-m.de>
cc: linville@tuxdriver.com, brudley@broadcom.com,
	linux-wireless@vger.kernel.org
Subject: Re: [PATCH 16/18] brcmsmac: do not call brcms_c_chipmatch() for
 non PCI
References: <1338937641-8519-1-git-send-email-hauke@hauke-m.de>
 <1338937641-8519-17-git-send-email-hauke@hauke-m.de>
 <4FCF3DA7.1080508@broadcom.com> <4FD0AA77.4000108@hauke-m.de>
In-Reply-To: <4FD0AA77.4000108@hauke-m.de>
Content-Type: text/plain;
 charset=iso-8859-1
Sender: linux-wireless-owner@vger.kernel.org

On 06/07/2012 03:19 PM, Hauke Mehrtens wrote:
> Hi Arend,
> 
> On 06/06/2012 01:23 PM, Arend van Spriel wrote:
>> On 06/06/2012 01:07 AM, Hauke Mehrtens wrote:
>>> brcms_c_chipmatch() just works for PCIe devices and returns false for
>>> non PCIe devices. This stops brcms_ops_stop() from calling it when the
>>> devices is not a PCIe device.
>>
>> Although this is true you may want to consider what this function
>> provides. The intent is to have a more accurate filter to determine
>> support the device by the driver, ie. more accurate than what is in the
>> driver device table.
>>
>> So in brcms_c_chipmatch() we may want a host-type independent filtering
>> and validate bcma_device_id, bcma_chipinfo, and possibly bcma_boardinfo.
> I do not like this function at all. ;-)
> 
> Are these restrictions in this function really needed? I would more like
> to restrict it to something like core revs, which is done in the device
> table and the phy types and versions. A restriction by chipid would also
> be reasonable as some chips are needing some special workarounds, these
> checks could also be done for SoCs.

I look where the function came from and it is called in several places
in our proprietary driver, which has a pretty coarse filter in the
device table.

The BCMA device table in brcmsmac is more finegrained and currently has
entries for the different 80211 core revisions. However, the brcmsmac
may have additional revision requirement for chipcommon and other cores
inside the chip so this function may have it merits for checking that.

Gr. AvS