Re: [PATCH 5/9] rt2800: prepare for rt2800*_probe_hw_mode() unification

[Gertjan van Wingerde <gwingerde@gmail.com>]

On 11/08/09 18:48, Bartlomiej Zolnierkiewicz wrote:
> On Sunday 08 November 2009 15:35:19 Gertjan van Wingerde wrote:
>> On 11/08/09 14:39, Bartlomiej Zolnierkiewicz wrote:
>>> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>> Subject: [PATCH] rt2800: prepare for rt2800*_probe_hw_mode() unification
>>>
>>> Enclose interface specific code in rt2800[pci,usb]_probe_hw_mode()
>>> with rt2x00_intf_is_[pci,usb]() checks.
>>>
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 
> [ please remove needles parts of mails, thanks! ]
> 
>>> @@ -800,17 +804,18 @@ static int rt2800usb_probe_hw_mode(struc
>>>  	spec->supported_bands = SUPPORT_BAND_2GHZ;
>>>  	spec->supported_rates = SUPPORT_RATE_CCK | SUPPORT_RATE_OFDM;
>>>  
>>> -	if (rt2x00_rf(&rt2x00dev->chip, RF2820) ||
>>> -	    rt2x00_rf(&rt2x00dev->chip, RF2720)) {
>>> +	if (rt2x00_rf(chip, RF2820) ||
>>> +	    rt2x00_rf(chip, RF2720)) {
>>>  		spec->num_channels = 14;
>>>  		spec->channels = rf_vals;
>>> -	} else if (rt2x00_rf(&rt2x00dev->chip, RF2850) ||
>>> -		   rt2x00_rf(&rt2x00dev->chip, RF2750)) {
>>> +	} else if (rt2x00_rf(chip, RF2850) ||
>>> +		   rt2x00_rf(chip, RF2750)) {
>>>  		spec->supported_bands |= SUPPORT_BAND_5GHZ;
>>>  		spec->num_channels = ARRAY_SIZE(rf_vals);
>>>  		spec->channels = rf_vals;
>>> -	} else if (rt2x00_rf(&rt2x00dev->chip, RF3020) ||
>>> -		   rt2x00_rf(&rt2x00dev->chip, RF2020)) {
>>> +	} else if (rt2x00_intf_is_usb(rt2x00dev) &&
>>> +		    (rt2x00_rf(chip, RF3020) ||
>>> +		     rt2x00_rf(chip, RF2020))) {
>>>  		spec->num_channels = ARRAY_SIZE(rf_vals_3070);
>>>  		spec->channels = rf_vals_3070;
>>>  	}
>>>
>>
>> Hmm, another one where we can benefit from decoupling RF chipset code from the actual interface (USB or PCI) used. I do not see the need to check for
>> USB or PCI support, we just need to unify on the RF chipset level.
>>
>> BTW this rf_vals_3070 initialization looks weird. It doesn't resemble any other rf_channel initializations we have. I'd say we can go with the rt2800pci variant of the initialization here.
> 
> IIRC from the vendor driver rf_vals_3070 is needed so by working
> in the incremental way I prefer to leave it as it is before somebody
> verifies this with the vendor driver, also it should be a separate
> patch for better bisectability anyway.
> 

The vendor driver does this in a slightly different way. I was caught off-guard on the complexity of this code.
I'll look at that myself. So, on second thought,

Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>

---
Gertjan.