[rt2x00-users] [PATCH] rt2x00: Add support for RT3572/RT3592/RT3592+Bluetooth combo card

[Helmut Schaa]

Hi,

Am Dienstag, 12. April 2011 schrieb RA-Shiang Tu:

[...]

> > > +1474,55 @@ void rt2800_config_ant(struct rt2x00_dev
> > *rt2x00dev, struct antenna_setup *ant)
> > >     u8 r1;
> > >     u8 r3;
> > >     u16 eeprom;
> > > +   u8 is_3592cb_card;
> >
> > Do we expect more bluetooth combo cards that need this special casing?
> > If yes this should be named "is_bt_combo" or similar.
> >
> This special configuration now only used for RT3592+BT combo card to adjust the antenna related setting.
> So I naming it as "is_3592cb_card". For rest of wifi+bt combo cards (e.g., rt3090+bt, rt5390+bt, etc.), they don't need these
> settings.

Ok, fine with me then. To keep rt2800_config_ant a bit shorter you could move
the 3592cb specific code into its own function (just the hunk below including
the eeprom check for BT coexistence)?
 
> > >     rt2800_bbp_read(rt2x00dev, 1, &r1);
> > >     rt2800_bbp_read(rt2x00dev, 3, &r3);
> > >
> > > +   if (rt2x00_rt(rt2x00dev, RT3572)) {
> > > +           rt2x00_eeprom_read(rt2x00dev,
> > > +                           EEPROM_NIC_CONF1, &eeprom);
> > > +           is_3592cb_card = rt2x00_get_field16(eeprom,
> > > +                           EEPROM_NIC_CONF1_BT_COEXIST) ? 1 : 0;
> > > +   } else
> > > +           is_3592cb_card = 0;
> > > +
> > > +   if (is_3592cb_card) {
> > > +           u32 reg;
> > > +           u8 led_ctrl, led_g_mode, led_r_mode;
> > > +
> > > +           rt2800_register_read(rt2x00dev, GPIO_SWITCH, &reg);
> > > +           if (rt2x00dev->curr_band == IEEE80211_BAND_5GHZ) {
> > > +                   rt2x00_set_field32(&reg, GPIO_SWITCH_0, 1);
> > > +                   rt2x00_set_field32(&reg, GPIO_SWITCH_1, 1);
> > > +           } else {
> > > +                   rt2x00_set_field32(&reg, GPIO_SWITCH_0, 0);
> > > +                   rt2x00_set_field32(&reg, GPIO_SWITCH_1, 0);
> > > +           }
> > > +           rt2800_register_write(rt2x00dev, GPIO_SWITCH, reg);
> > > +
> > > +           rt2800_register_read(rt2x00dev, LED_CFG, &reg);
> > > +           led_g_mode = rt2x00_get_field32(reg,
> > LED_CFG_LED_POLAR) ? 3 : 0;
> > > +           led_r_mode = rt2x00_get_field32(reg,
> > LED_CFG_LED_POLAR) ? 0 : 3;
> > > +           if (led_g_mode != rt2x00_get_field32(reg,
> > LED_CFG_G_LED_MODE) ||
> > > +               led_r_mode != rt2x00_get_field32(reg,
> > LED_CFG_R_LED_MODE)) {
> > > +                   rt2x00_eeprom_read(rt2x00dev,
> > EEPROM_FREQ, &eeprom);
> > > +                   led_ctrl = rt2x00_get_field16(eeprom,
> > > +                                           EEPROM_FREQ_LED_MODE);
> > > +                   if (led_ctrl == 0 || led_ctrl > 0x40) {
> > > +                           rt2x00_set_field32(&reg,
> > LED_CFG_G_LED_MODE,
> > > +                                                   led_g_mode);
> > > +                           rt2x00_set_field32(&reg,
> > LED_CFG_R_LED_MODE,
> > > +                                                   led_r_mode);
> > > +
> > rt2800_register_write(rt2x00dev, LED_CFG, reg);
> > > +                   } else {
> > > +                           rt2800_mcu_request(rt2x00dev,
> > MCU_BAND_SELECT,
> > > +                                   0xff, (led_g_mode << 2)
> > | led_r_mode,
> > > +                                   1);
> > > +                   }
> > > +           }
> > > +   }
> > > +
> >
> > Could you please add some comments to this code. As far as I
> > understood this seems to enable BT coexistance via a GPIO line, right?
> 
> This's related to some antenna switch setting. Ok, I will add some comments here.

Thanks.

[...]

> > @@ -1806,12 +2039,17 @@
> > > static void rt2800_config_channel(struct rt2x00_dev *rt2x00dev,
> > >     rt2x00_set_field32(&reg, TX_BAND_CFG_BG, rf->channel <= 14);
> > >     rt2800_register_write(rt2x00dev, TX_BAND_CFG, reg);
> > >
> > > +   if (rt2x00_rt(rt2x00dev, RT3572))
> > > +           rt2800_rfcsr_write(rt2x00dev, 8, 0);
> > > +
> > >     tx_pin = 0;
> > >
> > >     /* Turn on unused PA or LNA when not using 1T or 1R */
> > >     if (rt2x00dev->default_ant.tx_chain_num == 2) {
> > > -           rt2x00_set_field32(&tx_pin, TX_PIN_CFG_PA_PE_A1_EN, 1);
> > > -           rt2x00_set_field32(&tx_pin, TX_PIN_CFG_PA_PE_G1_EN, 1);
> > > +           rt2x00_set_field32(&tx_pin, TX_PIN_CFG_PA_PE_A1_EN,
> > > +                           rf->channel > 14);
> > > +           rt2x00_set_field32(&tx_pin, TX_PIN_CFG_PA_PE_G1_EN,
> > > +                           rf->channel <= 14);
> >
> > This is not RT3572 specific. Mind to explain this change?
> 
> Actually, the PA_PE enable bit may only enable when work exactly in A or G band and don't need to enable them both.
> It's truly not related RT3572, I'll remove this in this patch.

Ok, please send this as separate patch then, with a proper changelog of course :)

Thanks,
Helmut