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. In my opinion this functions results in preventing users from running this driver with some device which is working with this driver but which is not in this list of supported devices, because some board vendor just changed some id and no developer got such a card to test it and change the detection code. It is like with the PCIe card I found connected to my BCM4718, this card is working expect for some problems with the PCIe host controller on the BCM4716, but brcms_c_chipmatch() returned false for it. By the way, why is a call to this function in brcms_ops_stop() in addition to the call in the init code? In this case the device was already started. >> Signed-off-by: Hauke Mehrtens <hauke@xxxxxxxxxx> >> --- >> .../net/wireless/brcm80211/brcmsmac/mac80211_if.c | 22 +++++++++++++------- >> 1 file changed, 14 insertions(+), 8 deletions(-) > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
