On Tue, Jan 31, 2012 at 9:14 AM, Stanislaw Gruszka <sgruszka@xxxxxxxxxx> wrote:
> On Mon, Jan 30, 2012 at 11:06:49PM +0100, Gertjan van Wingerde wrote:
>> On 01/30/12 16:17, Stanislaw Gruszka wrote:
>> > + rt2800_rfcsr_read(rt2x00dev, 1, &rfcsr);
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_RX0_PD, 0);
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_TX0_PD, 0);
>> > + if (rt2x00_rt(rt2x00dev, RT3390)) {
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_RX1_PD,
>> > + rt2x00dev->default_ant.rx_chain_num == 1);
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_TX1_PD,
>> > + rt2x00dev->default_ant.tx_chain_num == 1);
>> > + } else {
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_RX1_PD, 0);
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_TX1_PD, 0);
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_RX2_PD, 0);
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_TX2_PD, 0);
>> > +
>> > + switch (rt2x00dev->default_ant.tx_chain_num) {
>> > + case 1:
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_TX1_PD, 1);
>> > + /* fall through */
>> > + case 2:
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_TX2_PD, 1);
>> > + break;
>> > + }
>> > +
>> > + switch (rt2x00dev->default_ant.rx_chain_num) {
>> > + case 1:
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_RX1_PD, 1);
>> > + /* fall through */
>> > + case 2:
>> > + rt2x00_set_field8(&rfcsr, RFCSR1_RX2_PD, 1);
>> > + break;
>> > + }
>> > + }
>> > + rt2800_rfcsr_write(rt2x00dev, 1, rfcsr);
>> > +
>> > rt2800_rfcsr_read(rt2x00dev, 23, &rfcsr);
>> > rt2x00_set_field8(&rfcsr, RFCSR23_FREQ_OFFSET, rt2x00dev->freq_offset);
>> > rt2800_rfcsr_write(rt2x00dev, 23, rfcsr);
>>
>> To be honest, I think that this can be simplied to a single case for
>> both RT30xx and RT33xx. Just take the RT30xx branch of the added
>> if-statement and it should just work fine on both chipset families.
>>
>> Yes, I am aware the Ralink driver has slightly different code here, but
>> that just seems to be because they work with knowledge of the
>> limitations of RT33xx, which ensures that tx_chain_num and rx_chain_num
>> can never be 2 on that chipset, but still handling it doesn't harm. It
>> would merely result in better readable code.
>
> Not only the code is different, but RF_R1 register value we program
> is different for 30xx and 33xx when chain_num == 1 (changed by
> RFCSR1_{RX2,TX2)_PD bit).
>
> I'm not against merging these two cases and program different values
> into register than Ralink driver do, but maybe in the next linux
> release (counting from release of that change), so any breakage
> eventually caused by that merge could be easily detected.
OK. Indeed let's do this in a different commit, at least. I don't know
if it has to be a
different kernel release, as long as we are able to bisect it.
Acked-by: Gertjan van Wingerde <gwingerde@xxxxxxxxx>
for this patch as well.
---
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
