On Thursday 02 November 2006 17:44, Danny van Dyk wrote:
> Hi guys,
> 
> as I'm a bit out of work on this, I'd like to post this patch as RFC first. It
> brings the power control routine on par with
> 
>   http://bcm-specs.sipsolutions.net/InitPowerControl
> 
> but it contains a huge FIXME() where specs aren't as clear as I'd like them.
> Johannes, Joseph: Can you both have a look at
> 
>   http://bcm-specs.sipsolutions.net/GPHY_DC_Lookup_Table
> 
> please? Following your specs, the code needed to loop of a RF Attenuation
> list of 7 elements, as we obviously do support HW Power Control at this point.
> Now, with 7 elements if RF and 9 in baseband attenuation, this makes 63 LO
> pairs. However, we only have 56 = 14 * 4 pairs available. (If i'm not 
> mistaken,
> that is :-)

Please look at the new LO code in my development tree.
It was rewritten and semantics changed a lot.
It's also possible that I already implemented bits of your
patch below in my tree. But I did not compare it.

But please don't try to merge this patch upstream through
John. It will result in horrible rejects in my tree, as I'm
rewriting PHY stuff there. Please prepare a patch which applies
to my tree (which still should be easy. But it will get harder
the more I change).

> Danny
> ---
> diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_phy.c 
> b/drivers/net/wireless/bcm43xx/bcm43xx_phy.c
> index 52ce2a9..e49acfe 100644
> --- a/drivers/net/wireless/bcm43xx/bcm43xx_phy.c
> +++ b/drivers/net/wireless/bcm43xx/bcm43xx_phy.c
> @@ -190,7 +190,70 @@ out:
>       return 0;
>  }
>  
> -/* intialize B PHY power control
> +static inline
> +void bcm43xx_gphy_init_plt(struct bcm43xx_private *bcm)
> +{
> +     int i;
> +     u16 val;
> +
> +     for (i = 0; i < 32; i++)
> +             bcm43xx_ilt_write(bcm, 0x32C0 + i, bcm43xx_tssi2dbm_g_table[i]);
> +
> +     for (i = 0; i < 32; i++)
> +             bcm43xx_ilt_write(bcm, 0x3C00 + i, bcm43xx_tssi2dbm_g_table[i + 
> 32]);
> +
> +     for (i = 0; i < 64; i++) {
> +             val = (u8)bcm43xx_tssi2dbm_b_table[i];
> +             val <<= 8;
> +             val |= (u8)bcm43xx_tssi2dbm_g_table[i];

Ehm, what is this code supposed to do? It assgnes "val"
several times and throws the result away.
And is it right to mix b_table and g_table?

> +     }
> +}
> +
> +static inline
> +struct bcm43xx_lopair * bcm43xx_current_lopair(struct bcm43xx_private *bcm);
> +
> +static inline
> +void bcm43xx_gphy_init_glt(struct bcm43xx_private *bcm)
> +{
> +     struct bcm43xx_radioinfo *radio = bcm43xx_current_radio(bcm);
> +     static const u8 rf_att_table[14] = {
> +             2,  4,  6,  8, 10, 12, 14, // 2050 radio, rev 8
> +             0,  2,  4,  6,  8,  9,  9  // otherwise
> +     };

I think this table already is implemented in a slightly different
way in my code.

> +
> +     unsigned count = 0, i, j, rf_offset;
> +     u16 val, set = (radio->rev == 8) ? 0x50 : 0x40;
> +
> +     if (bcm43xx_current_lopair(bcm)->used) {
> +             //XXX: ^^^ Correct way to check if LO has been measured?

No, I think we have a flag for that in struct bcm43xx_txpower_lo_control
(look at my tree). This flag has been moved from the old bcm43xx_phyinfo.

> +             rf_offset = ((radio->version == 0x2050) && (radio->rev == 8)) ? 
> 7 : 0;
> +             for (i = 0; i < 7; i++) {
> +                     for (j = 0; j < 9; j++) {
> +                             if (count >= 64)
> +                                     goto out;
> +                             val = j << 8;
> +                             val |= set;
> +                             val |= rf_att_table[i + rf_offset];
> +                             bcm43xx_phy_write(bcm, 0x3C0 + count, val);
> +                             count++;
> +                     }
> +             }
> +     } else {
> +             bcm43xx_phy_write(bcm, 0x03FF, 0x0000);
> +     }
> +out:
> +     return;
> +}

Besides that, I think I already implemented that function.

> +static inline void bcm43xx_gphy_init_dclt(struct bcm43xx_private *bcm)
> +{
> +     FIXME();
> +     // http://bcm-specs.sipsolutions.net/GPHY_DC_Lookup_Table
> +     //FIXME: The specs aren't clear here. How many loops? For HW PCTL, we'd 
> need
> +     //7 * 9 == 63 LO pairs. But we only have 14 * 4 == 56 pairs available.
> +}

I already implemented that.

> +/* intialize B/G PHY power control
>   * as described in http://bcm-specs.sipsolutions.net/InitPowerControl
>   */
>  static void bcm43xx_phy_init_pctl(struct bcm43xx_private *bcm)
> @@ -198,48 +261,86 @@ static void bcm43xx_phy_init_pctl(struct
>       struct bcm43xx_phyinfo *phy = bcm43xx_current_phy(bcm);
>       struct bcm43xx_radioinfo *radio = bcm43xx_current_radio(bcm);
>       u16 saved_batt = 0, saved_ratt = 0, saved_txctl1 = 0;
> -     int must_reset_txpower = 0;
> +     int has_hw_pctl = 0;
>  
>       assert(phy->type != BCM43xx_PHYTYPE_A);
>       if ((bcm->board_vendor == PCI_VENDOR_ID_BROADCOM) &&
>           (bcm->board_type == 0x0416))
>               return;
>  
> -     bcm43xx_write16(bcm, 0x03E6, bcm43xx_read16(bcm, 0x03E6) & 0xFFDF);
>       bcm43xx_phy_write(bcm, 0x0028, 0x8018);
> +     bcm43xx_write16(bcm, 0x03E6, bcm43xx_read16(bcm, 0x03E6) & 0xFFDF);
>  
> -     if (phy->type == BCM43xx_PHYTYPE_G) {
> -             if (!phy->connected)
> -                     return;
> -             bcm43xx_phy_write(bcm, 0x047A, 0xC111);
> -     }
> -     if (phy->savedpctlreg != 0xFFFF)
> +     if ((phy->type == BCM43xx_PHYTYPE_G) && (!phy->connected))
>               return;
>  
> -     if (phy->type == BCM43xx_PHYTYPE_B &&
> -         phy->rev >= 2 &&
> -         radio->version == 0x2050) {
> -             bcm43xx_radio_write16(bcm, 0x0076,
> -                                   bcm43xx_radio_read16(bcm, 0x0076) | 
> 0x0084);
> +     if (((phy->type == BCM43xx_PHYTYPE_G) && (phy->rev >= 6)) ||
> +         ((phy->type == BCM43xx_PHYTYPE_A) && (phy->rev >= 5))) {
> +             has_hw_pctl = 1;
> +
> +             bcm43xx_phy_write(bcm, 0x0036, bcm43xx_phy_read(bcm, 0x0036) & 
> 0xFEFF);
> +             bcm43xx_phy_write(bcm, 0x002F, 0x0202);
> +             bcm43xx_phy_write(bcm, 0x047C, bcm43xx_phy_read(bcm, 0x047C) | 
> 0x0002);
> +             bcm43xx_phy_write(bcm, 0x047A, bcm43xx_phy_read(bcm, 0x047A) | 
> 0xF000);
> +
> +             if ((radio->version == 0x2050) && (radio->revision == 8)) {
> +                     bcm43xx_phy_write(bcm, 0x047A, (bcm43xx_phy_read(bcm, 
> 0x047A) & 0xFF0F) | 0x0010);
> +                     bcm43xx_phy_write(bcm, 0x005D, bcm43xx_phy_read(bcm, 
> 0x005D) | 0x8000);
> +                     bcm43xx_phy_write(bcm, 0x004E, (bcm43xx_phy_read(bcm, 
> 0x004E) & 0xFFC0) | 0x0010);
> +                     bcm43xx_phy_write(bcm, 0x002E, 0xC07F);
> +                     bcm43xx_phy_write(bcm, 0x0036, bcm43xx_phy_read(bcm, 
> 0x0036) | 0x0400);
> +             } else {
> +                     bcm43xx_phy_write(bcm, 0x0036, bcm43xx_phy_read(bcm, 
> 0x0036) | 0x0200);
> +                     bcm43xx_phy_write(bcm, 0x0036, bcm43xx_phy_read(bcm, 
> 0x0036) | 0x0400);
> +                     bcm43xx_phy_write(bcm, 0x005D, bcm43xx_phy_read(bcm, 
> 0x005D) & 0x7FFF);
> +                     bcm43xx_phy_write(bcm, 0x004F, bcm43xx_phy_read(bcm, 
> 0x004F) & 0xFFFE);
> +                     bcm43xx_phy_write(bcm, 0x004E, (bcm43xx_phy_read(bcm, 
> 0x004E) & 0xFFC0) | 0x0010);
> +                     bcm43xx_phy_write(bcm, 0x002E, 0xC07F);
> +                     bcm43xx_phy_write(bcm, 0x047A, (bcm43xx_phy_read(bcm, 
> 0x047A) & 0xFF0F) | 0x0010);
> +             }
>       } else {
> -             saved_batt = radio->baseband_atten;
> -             saved_ratt = radio->radio_atten;
> -             saved_txctl1 = radio->txctl1;
> -             if ((radio->revision >= 6) && (radio->revision <= 8)
> -                 && /*FIXME: incomplete specs for 5 < revision < 9 */ 0)
> -                     bcm43xx_radio_set_txpower_bg(bcm, 0xB, 0x1F, 0);
> -             else
> -                     bcm43xx_radio_set_txpower_bg(bcm, 0xB, 9, 0);
> -             must_reset_txpower = 1;
> +             bcm43xx_phy_write(bcm, 0x047A, 0xC111);
>       }
> -     bcm43xx_dummy_transmission(bcm);
>  
> -     phy->savedpctlreg = bcm43xx_phy_read(bcm, BCM43xx_PHY_G_PCTL);
> +     if (phy->savedpctlreg == 0xFFFF) {
> +             if ((radio->version == 0x2050) && (phy->rev == 0)) {
> +                     bcm43xx_radio_write16(bcm, 0x0076, 
> (bcm43xx_radio_read16(bcm, 0x0076) & 0x00F7) | 0x84);
> +             } else {
> +                     saved_batt = radio->baseband_atten;
> +                     saved_ratt = radio->radio_atten;
> +                     saved_txctl1 = radio->txctl1;
> +     
> +                     if (radio->revision == 8)
> +                             bcm43xx_radio_set_txpower_bg(bcm, 0xB, 0x1F, 0);
> +                     else
> +                             bcm43xx_radio_set_txpower_bg(bcm, 0xB, 0x09, 0);
> +             }
> +
> +             bcm43xx_dummy_transmission(bcm);
> +             phy->savedpctlreg = bcm43xx_phy_read(bcm, BCM43xx_PHY_G_PCTL);
> +
> +             if ((radio->version == 0x2050) && (phy->rev == 0))
> +                     bcm43xx_radio_write16(bcm, 0x0076, 
> bcm43xx_radio_read16(bcm, 0x0076) & 0xFF7B);
> +             else {
> +                     bcm43xx_radio_set_txpower_bg(bcm, saved_batt, 
> saved_ratt, saved_txctl1);
> +             }
> +     }
> +
> +     if (has_hw_pctl) {
> +             bcm43xx_phy_write(bcm, 0x0036,
> +                               (bcm43xx_phy_read(bcm, 0x0036) & 0xFFC0) | 
> (u8)(phy->idle_tssi - phy->savedpctlreg));
> +             bcm43xx_phy_write(bcm, 0x0478,
> +                               (bcm43xx_phy_read(bcm, 0x0478) & 0xFF00) | 
> (u8)(phy->idle_tssi - phy->savedpctlreg));
> +             bcm43xx_gphy_init_plt(bcm);
> +             bcm43xx_gphy_init_glt(bcm);
> +             bcm43xx_phy_write(bcm, 0x0060, bcm43xx_phy_read(bcm, 0x0060) & 
> 0xFFBF);
> +             bcm43xx_phy_write(bcm, 0x0014, 0x0000);
> +             bcm43xx_phy_write(bcm, 0x0478, bcm43xx_phy_read(bcm, 0x0478) | 
> 0x0800);
> +             bcm43xx_phy_write(bcm, 0x0478, bcm43xx_phy_read(bcm, 0x0478) & 
> 0xFEFF);
> +             bcm43xx_phy_write(bcm, 0x0801, bcm43xx_phy_read(bcm, 0x0801) & 
> 0xFFBF);
> +             bcm43xx_gphy_init_dclt(bcm);
> +     }
>  
> -     if (must_reset_txpower)
> -             bcm43xx_radio_set_txpower_bg(bcm, saved_batt, saved_ratt, 
> saved_txctl1);
> -     else
> -             bcm43xx_radio_write16(bcm, 0x0076, bcm43xx_radio_read16(bcm, 
> 0x0076) & 0xFF7B);
>       bcm43xx_radio_clear_tssi(bcm);
>  }

Please check, if I did not already implement that. (I think so, but I did
not compare every bit)
I also added a convenient wrapper macro for "has_hw_pctl".

-- 
Greetings Michael.
_______________________________________________
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev

Reply via email to