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@xxxxxxxxx>
>>> 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@xxxxxxxxx>
>
> [ 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@xxxxxxxxx>
---
Gertjan.
--
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
