2010-01-10 22:11:53

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 2/6] b43: N-PHY: add RSSI functions: poll and set 2055 vcm


Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/net/wireless/b43/phy_n.c | 96 ++++++++++++++++++++++++++++++++++++++
1 files changed, 96 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/b43/phy_n.c b/drivers/net/wireless/b43/phy_n.c
index a44fca1..d7e408b 100644
--- a/drivers/net/wireless/b43/phy_n.c
+++ b/drivers/net/wireless/b43/phy_n.c
@@ -711,6 +711,102 @@ static void b43_nphy_rssi_select(struct b43_wldev *dev, u8 code, u8 type)
}
}

+/* http://bcm-v4.sipsolutions.net/802.11/PHY/N/SetRssi2055Vcm */
+static void b43_nphy_set_rssi_2055_vcm(struct b43_wldev *dev, u8 type, u8 *buf)
+{
+ int i;
+ for (i = 0; i < 2; i++) {
+ if (type == 2) {
+ if (i == 0) {
+ b43_radio_maskset(dev, B2055_C1_B0NB_RSSIVCM,
+ 0xFC, buf[0]);
+ b43_radio_maskset(dev, B2055_C1_RX_BB_RSSICTL5,
+ 0xFC, buf[1]);
+ } else {
+ b43_radio_maskset(dev, B2055_C2_B0NB_RSSIVCM,
+ 0xFC, buf[2 * i]);
+ b43_radio_maskset(dev, B2055_C2_RX_BB_RSSICTL5,
+ 0xFC, buf[2 * i + 1]);
+ }
+ } else {
+ if (i == 0)
+ b43_radio_maskset(dev, B2055_C1_RX_BB_RSSICTL5,
+ 0xF3, buf[0] << 2);
+ else
+ b43_radio_maskset(dev, B2055_C2_RX_BB_RSSICTL5,
+ 0xF3, buf[2 * i + 1] << 2);
+ }
+ }
+}
+
+/* http://bcm-v4.sipsolutions.net/802.11/PHY/N/PollRssi */
+static int b43_nphy_poll_rssi(struct b43_wldev *dev, u8 type, s32 *buf,
+ u8 nsamp)
+{
+ int i;
+ int out;
+ u16 save_regs_phy[9];
+ u16 s[2];
+
+ if (dev->phy.rev >= 3) {
+ save_regs_phy[0] = b43_phy_read(dev,
+ B43_NPHY_RFCTL_LUT_TRSW_UP1);
+ save_regs_phy[1] = b43_phy_read(dev,
+ B43_NPHY_RFCTL_LUT_TRSW_UP2);
+ save_regs_phy[2] = b43_phy_read(dev, B43_NPHY_AFECTL_C1);
+ save_regs_phy[3] = b43_phy_read(dev, B43_NPHY_AFECTL_C2);
+ save_regs_phy[4] = b43_phy_read(dev, B43_NPHY_AFECTL_OVER1);
+ save_regs_phy[5] = b43_phy_read(dev, B43_NPHY_AFECTL_OVER);
+ save_regs_phy[6] = b43_phy_read(dev, B43_NPHY_TXF_40CO_B1S0);
+ save_regs_phy[7] = b43_phy_read(dev, B43_NPHY_TXF_40CO_B32S1);
+ }
+
+ b43_nphy_rssi_select(dev, 5, type);
+
+ if (dev->phy.rev < 2) {
+ save_regs_phy[8] = b43_phy_read(dev, B43_NPHY_GPIO_SEL);
+ b43_phy_write(dev, B43_NPHY_GPIO_SEL, 5);
+ }
+
+ for (i = 0; i < 4; i++)
+ buf[i] = 0;
+
+ for (i = 0; i < nsamp; i++) {
+ if (dev->phy.rev < 2) {
+ s[0] = b43_phy_read(dev, B43_NPHY_GPIO_LOOUT);
+ s[1] = b43_phy_read(dev, B43_NPHY_GPIO_HIOUT);
+ } else {
+ s[0] = b43_phy_read(dev, B43_NPHY_RSSI1);
+ s[1] = b43_phy_read(dev, B43_NPHY_RSSI2);
+ }
+
+ buf[0] += (s8)(((s[0] & 0x3F) << 2) >> 2);
+ buf[1] += (s8)((((s[0] >> 8) & 0x3F) << 2) >> 2);
+ buf[2] += (s8)(((s[1] & 0x3F) << 2) >> 2);
+ buf[3] += (s8)((((s[1] >> 8) & 0x3F) << 2) >> 2);
+ }
+ out = (buf[0] & 0xFF) << 24 | (buf[1] & 0xFF) << 16 |
+ (buf[2] & 0xFF) << 8 | (buf[3] & 0xFF);
+
+ if (dev->phy.rev < 2)
+ b43_phy_write(dev, B43_NPHY_GPIO_SEL, save_regs_phy[8]);
+
+ if (dev->phy.rev >= 3) {
+ b43_phy_write(dev, B43_NPHY_RFCTL_LUT_TRSW_UP1,
+ save_regs_phy[0]);
+ b43_phy_write(dev, B43_NPHY_RFCTL_LUT_TRSW_UP2,
+ save_regs_phy[1]);
+ b43_phy_write(dev, B43_NPHY_AFECTL_C1, save_regs_phy[2]);
+ b43_phy_write(dev, B43_NPHY_AFECTL_C2, save_regs_phy[3]);
+ b43_phy_write(dev, B43_NPHY_AFECTL_OVER1, save_regs_phy[4]);
+ b43_phy_write(dev, B43_NPHY_AFECTL_OVER, save_regs_phy[5]);
+ b43_phy_write(dev, B43_NPHY_TXF_40CO_B1S0, save_regs_phy[6]);
+ b43_phy_write(dev, B43_NPHY_TXF_40CO_B32S1, save_regs_phy[7]);
+ }
+
+ return out;
+}
+
/* http://bcm-v4.sipsolutions.net/802.11/PHY/N/RSSICal */
static void b43_nphy_rev2_rssi_cal(struct b43_wldev *dev, u8 type)
{
--
1.6.4.2



2010-01-11 00:53:21

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 2/6] b43: N-PHY: add RSSI functions: poll and set 2055 vcm

On Monday 11 January 2010 01:27:48 Larry Finger wrote:
> On 01/10/2010 04:32 PM, Michael Buesch wrote:
> > On Sunday 10 January 2010 23:13:20 Rafał Miłecki wrote:
> >> + buf[0] += (s8)(((s[0] & 0x3F) << 2) >> 2);
> >> + buf[1] += (s8)((((s[0] >> 8) & 0x3F) << 2) >> 2);
> >> + buf[2] += (s8)(((s[1] & 0x3F) << 2) >> 2);
> >> + buf[3] += (s8)((((s[1] >> 8) & 0x3F) << 2) >> 2);
> >
> > I suggest buf[3] += (s8)((((s[1] >> 8) & 0x3F) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2)
> > ;)
> > No, seriously, why shift left and then shift right? Is this a translation error?
> > I _guess_ it's some mistranslation of the sign extension going on.
> > Or alternatively a compiler going insane on sign extension.
> >
> > The question is: Do we want these integers to be signextended or not?
> >
> > What we currently do is this:
> > buf[3] += (s8)((s[1] >> 8) & 0x3F);
> >
> > which will always result in a positive 8bit integer, as far as I can see.
> > Which smells fishy.
>
> We do want sign extension of the signed 6-bit quantities into an 8-bit word. The
> translation is correct. Do you know of a better way to extend the signs?

Well, I think you got the parenthesis and cast wrong then.
I think what we want is the following:

+ buf[3] += (s8)(((s[1] >> 8) & 0x3F) << 2) >> 2;

The code is different and the specs, too.

--
Greetings, Michael.

2010-01-11 02:39:01

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 2/6] b43: N-PHY: add RSSI functions: poll and set 2055 vcm

On 01/10/2010 04:32 PM, Michael Buesch wrote:
> On Sunday 10 January 2010 23:13:20 Rafał Miłecki wrote:
>> + buf[0] += (s8)(((s[0] & 0x3F) << 2) >> 2);
>> + buf[1] += (s8)((((s[0] >> 8) & 0x3F) << 2) >> 2);
>> + buf[2] += (s8)(((s[1] & 0x3F) << 2) >> 2);
>> + buf[3] += (s8)((((s[1] >> 8) & 0x3F) << 2) >> 2);
>
> I suggest buf[3] += (s8)((((s[1] >> 8) & 0x3F) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2)
> ;)
> No, seriously, why shift left and then shift right? Is this a translation error?
> I _guess_ it's some mistranslation of the sign extension going on.
> Or alternatively a compiler going insane on sign extension.
>
> The question is: Do we want these integers to be signextended or not?
>
> What we currently do is this:
> buf[3] += (s8)((s[1] >> 8) & 0x3F);
>
> which will always result in a positive 8bit integer, as far as I can see.
> Which smells fishy.

Yes, my fault. The specs are now corrected so that these statements are

((s8)((s[1] >> 8) & 0x3F) << 2) >> 2

I think that is right.

Larry

2010-01-10 22:32:28

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 2/6] b43: N-PHY: add RSSI functions: poll and set 2055 vcm

On Sunday 10 January 2010 23:13:20 Rafał Miłecki wrote:
> + buf[0] += (s8)(((s[0] & 0x3F) << 2) >> 2);
> + buf[1] += (s8)((((s[0] >> 8) & 0x3F) << 2) >> 2);
> + buf[2] += (s8)(((s[1] & 0x3F) << 2) >> 2);
> + buf[3] += (s8)((((s[1] >> 8) & 0x3F) << 2) >> 2);

I suggest buf[3] += (s8)((((s[1] >> 8) & 0x3F) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2)
;)
No, seriously, why shift left and then shift right? Is this a translation error?
I _guess_ it's some mistranslation of the sign extension going on.
Or alternatively a compiler going insane on sign extension.

The question is: Do we want these integers to be signextended or not?

What we currently do is this:
buf[3] += (s8)((s[1] >> 8) & 0x3F);

which will always result in a positive 8bit integer, as far as I can see.
Which smells fishy.

--
Greetings, Michael.

2010-01-11 10:09:07

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 2/6] b43: N-PHY: add RSSI functions: poll and set 2055 vcm

On Monday 11 January 2010 03:38:53 Larry Finger wrote:
> Yes, my fault. The specs are now corrected so that these statements are
>
> ((s8)((s[1] >> 8) & 0x3F) << 2) >> 2
>
> I think that is right.

No it is not.

You need to do this:
(s8)(((s[1] >> 8) & 0x3F) << 2) >> 2

Alternatively add another pair of parenthesis to make it clearer:

((s8)(((s[1] >> 8) & 0x3F) << 2)) >> 2

This basically shifts left unsigned and then shifts right _arithmetically_.
In your example, the compiler will optimize both shifts away (it may actually
also optimize the shifts in my case, but the sign extension will persist.

Just try it and you'll see:

mb@homer:~$ cat t.c
#include <stdio.h>
#include <stdint.h>

int main()
{
int8_t s0;
int8_t s1;
uint8_t u;

u = 0x3D;

s0 = ((int8_t)(u & 0x3F) << 2) >> 2;
s1 = ((int8_t)((u & 0x3F) << 2)) >> 2;

printf("%d %d\n", s0, s1);
}
mb@homer:~$ gcc -o t t.c
mb@homer:~$ ./t
61 -3


--
Greetings, Michael.

2010-01-11 00:27:51

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 2/6] b43: N-PHY: add RSSI functions: poll and set 2055 vcm

On 01/10/2010 04:32 PM, Michael Buesch wrote:
> On Sunday 10 January 2010 23:13:20 Rafał Miłecki wrote:
>> + buf[0] += (s8)(((s[0] & 0x3F) << 2) >> 2);
>> + buf[1] += (s8)((((s[0] >> 8) & 0x3F) << 2) >> 2);
>> + buf[2] += (s8)(((s[1] & 0x3F) << 2) >> 2);
>> + buf[3] += (s8)((((s[1] >> 8) & 0x3F) << 2) >> 2);
>
> I suggest buf[3] += (s8)((((s[1] >> 8) & 0x3F) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2) << 2) >> 2)
> ;)
> No, seriously, why shift left and then shift right? Is this a translation error?
> I _guess_ it's some mistranslation of the sign extension going on.
> Or alternatively a compiler going insane on sign extension.
>
> The question is: Do we want these integers to be signextended or not?
>
> What we currently do is this:
> buf[3] += (s8)((s[1] >> 8) & 0x3F);
>
> which will always result in a positive 8bit integer, as far as I can see.
> Which smells fishy.

We do want sign extension of the signed 6-bit quantities into an 8-bit word. The
translation is correct. Do you know of a better way to extend the signs?

Larry