2010-10-06 05:50:04

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 0/3] b43: channel switching little cleaning

It defines B43_SHM_SH_CHAN_40MHZ and simplifies channel switching for N-PHY.

Rafał Miłecki (3):
b43: N-PHY: simplify channel switching
b43: define B43_SHM_SH_CHAN_40MHZ
b43: N-PHY: don't duplicate setting channel in shared memory

drivers/net/wireless/b43/b43.h | 3 ++-
drivers/net/wireless/b43/phy_common.c | 6 ++++--
drivers/net/wireless/b43/phy_n.c | 32 ++++++++++----------------------
3 files changed, 16 insertions(+), 25 deletions(-)



2010-10-06 15:35:39

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 3/3] b43: N-PHY: don't duplicate setting channel in shared memory

On Wed, 2010-10-06 at 07:50 +0200, Rafał Miłecki wrote:
> It's already set in PHY common code.
>
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> drivers/net/wireless/b43/phy_n.c | 13 +++----------
> 1 files changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/b43/phy_n.c b/drivers/net/wireless/b43/phy_n.c
> index 47c58cd..bb8cb36 100644
> --- a/drivers/net/wireless/b43/phy_n.c
> +++ b/drivers/net/wireless/b43/phy_n.c
> @@ -73,8 +73,6 @@ static void b43_nphy_rf_control_override(struct b43_wldev *dev, u16 field,
> u16 value, u8 core, bool off);
> static void b43_nphy_rf_control_intc_override(struct b43_wldev *dev, u8 field,
> u16 value, u8 core);
> -static int b43_nphy_op_switch_channel(struct b43_wldev *dev,
> - unsigned int new_channel);
>
> static inline bool b43_empty_chanspec(struct b43_chanspec *chanspec)
> {
> @@ -224,7 +222,7 @@ static void b43_radio_init2055_post(struct b43_wldev *dev)
> if (i)
> b43err(dev->wl, "radio post init timeout\n");
> b43_radio_mask(dev, B2055_CAL_LPOCTL, 0xFF7F);
> - b43_nphy_op_switch_channel(dev, dev->phy.channel);
> + b43_switch_channel(dev, dev->phy.channel);
> b43_radio_write(dev, B2055_C1_RX_BB_LPF, 0x9);
> b43_radio_write(dev, B2055_C2_RX_BB_LPF, 0x9);
> b43_radio_write(dev, B2055_C1_RX_BB_MIDACHP, 0x83);
> @@ -3352,12 +3350,7 @@ static void b43_nphy_chanspec_setup(struct b43_wldev *dev,
>
> b43_chantab_phy_upload(dev, e);
>
> - tmp = chanspec.channel;
> - if (chanspec.b_freq == 1)
> - tmp |= 0x0100;
> - if (chanspec.b_width == 3)
> - tmp |= 0x0200;
> - b43_shm_write16(dev, B43_SHM_SHARED, 0xA0, tmp);
> + /* Don't follow specs, B43_SHM_SH_CHAN is set by b43_switch_channel */

Looks great. I dunno if a comment is really needed and whether it is
correct, though. We _do_ follow the specs. We just do it elsewhere.


> if (nphy->radio_chanspec.channel == 14) {
> b43_nphy_classifier(dev, 2, 0);
> @@ -3559,7 +3552,7 @@ static void b43_nphy_op_software_rfkill(struct b43_wldev *dev,
> } else {
> if (dev->phy.rev >= 3) {
> b43_radio_init2056(dev);
> - b43_nphy_op_switch_channel(dev, dev->phy.channel);
> + b43_switch_channel(dev, dev->phy.channel);
> } else {
> b43_radio_init2055(dev);
> }


--
Greetings Michael.


2010-10-06 20:30:10

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 3/3] b43: N-PHY: don't duplicate setting channel in shared memory

On Wed, Oct 06, 2010 at 06:07:44PM +0200, Rafał Miłecki wrote:
> W dniu 6 października 2010 17:30 użytkownik Michael Büsch
> <[email protected]> napisał:
> > On Wed, 2010-10-06 at 07:50 +0200, Rafał Miłecki wrote:

> >> +     /* Don't follow specs, B43_SHM_SH_CHAN is set by b43_switch_channel */
> >
> > Looks great. I dunno if a comment is really needed and whether it is
> > correct, though. We _do_ follow the specs. We just do it elsewhere.
>
> I got "Don't strictly follow specs here" earlier, but made it shorter
> to match 80 chars ;) Maybe it wasn't too good idea. Should left it
> longer or don't put at all.
>
> John: could you drop this comment line when applying? Or would you
> like me to resend this fixed?

Got it...

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-10-06 05:50:10

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 3/3] b43: N-PHY: don't duplicate setting channel in shared memory

It's already set in PHY common code.

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

diff --git a/drivers/net/wireless/b43/phy_n.c b/drivers/net/wireless/b43/phy_n.c
index 47c58cd..bb8cb36 100644
--- a/drivers/net/wireless/b43/phy_n.c
+++ b/drivers/net/wireless/b43/phy_n.c
@@ -73,8 +73,6 @@ static void b43_nphy_rf_control_override(struct b43_wldev *dev, u16 field,
u16 value, u8 core, bool off);
static void b43_nphy_rf_control_intc_override(struct b43_wldev *dev, u8 field,
u16 value, u8 core);
-static int b43_nphy_op_switch_channel(struct b43_wldev *dev,
- unsigned int new_channel);

static inline bool b43_empty_chanspec(struct b43_chanspec *chanspec)
{
@@ -224,7 +222,7 @@ static void b43_radio_init2055_post(struct b43_wldev *dev)
if (i)
b43err(dev->wl, "radio post init timeout\n");
b43_radio_mask(dev, B2055_CAL_LPOCTL, 0xFF7F);
- b43_nphy_op_switch_channel(dev, dev->phy.channel);
+ b43_switch_channel(dev, dev->phy.channel);
b43_radio_write(dev, B2055_C1_RX_BB_LPF, 0x9);
b43_radio_write(dev, B2055_C2_RX_BB_LPF, 0x9);
b43_radio_write(dev, B2055_C1_RX_BB_MIDACHP, 0x83);
@@ -3352,12 +3350,7 @@ static void b43_nphy_chanspec_setup(struct b43_wldev *dev,

b43_chantab_phy_upload(dev, e);

- tmp = chanspec.channel;
- if (chanspec.b_freq == 1)
- tmp |= 0x0100;
- if (chanspec.b_width == 3)
- tmp |= 0x0200;
- b43_shm_write16(dev, B43_SHM_SHARED, 0xA0, tmp);
+ /* Don't follow specs, B43_SHM_SH_CHAN is set by b43_switch_channel */

if (nphy->radio_chanspec.channel == 14) {
b43_nphy_classifier(dev, 2, 0);
@@ -3559,7 +3552,7 @@ static void b43_nphy_op_software_rfkill(struct b43_wldev *dev,
} else {
if (dev->phy.rev >= 3) {
b43_radio_init2056(dev);
- b43_nphy_op_switch_channel(dev, dev->phy.channel);
+ b43_switch_channel(dev, dev->phy.channel);
} else {
b43_radio_init2055(dev);
}
--
1.7.1


2010-10-06 05:50:08

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 2/3] b43: define B43_SHM_SH_CHAN_40MHZ

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/net/wireless/b43/b43.h | 3 ++-
drivers/net/wireless/b43/phy_common.c | 6 ++++--
2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/b43/b43.h b/drivers/net/wireless/b43/b43.h
index 73376ff..29302ea 100644
--- a/drivers/net/wireless/b43/b43.h
+++ b/drivers/net/wireless/b43/b43.h
@@ -187,7 +187,8 @@ enum {
#define B43_SHM_SH_PHYTXNOI 0x006E /* PHY noise directly after TX (lower 8bit only) */
#define B43_SHM_SH_RFRXSP1 0x0072 /* RF RX SP Register 1 */
#define B43_SHM_SH_CHAN 0x00A0 /* Current channel (low 8bit only) */
-#define B43_SHM_SH_CHAN_5GHZ 0x0100 /* Bit set, if 5Ghz channel */
+#define B43_SHM_SH_CHAN_5GHZ 0x0100 /* Bit set, if 5 Ghz channel */
+#define B43_SHM_SH_CHAN_40MHZ 0x0200 /* Bit set, if 40 Mhz channel width */
#define B43_SHM_SH_BCMCFIFOID 0x0108 /* Last posted cookie to the bcast/mcast FIFO */
/* TSSI information */
#define B43_SHM_SH_TSSI_CCK 0x0058 /* TSSI for last 4 CCK frames (32bit) */
diff --git a/drivers/net/wireless/b43/phy_common.c b/drivers/net/wireless/b43/phy_common.c
index 10b9e6f..44f0064 100644
--- a/drivers/net/wireless/b43/phy_common.c
+++ b/drivers/net/wireless/b43/phy_common.c
@@ -294,8 +294,10 @@ int b43_switch_channel(struct b43_wldev *dev, unsigned int new_channel)
*/
channelcookie = new_channel;
if (b43_current_band(dev->wl) == IEEE80211_BAND_5GHZ)
- channelcookie |= 0x100;
- //FIXME set 40Mhz flag if required
+ channelcookie |= B43_SHM_SH_CHAN_5GHZ;
+ /* FIXME: set 40Mhz flag if required */
+ if (0)
+ channelcookie |= B43_SHM_SH_CHAN_40MHZ;
savedcookie = b43_shm_read16(dev, B43_SHM_SHARED, B43_SHM_SH_CHAN);
b43_shm_write16(dev, B43_SHM_SHARED, B43_SHM_SH_CHAN, channelcookie);

--
1.7.1


2010-10-06 16:07:45

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 3/3] b43: N-PHY: don't duplicate setting channel in shared memory

W dniu 6 października 2010 17:30 użytkownik Michael Büsch
<[email protected]> napisał:
> On Wed, 2010-10-06 at 07:50 +0200, Rafał Miłecki wrote:
>> It's already set in PHY common code.
>>
>> Signed-off-by: Rafał Miłecki <[email protected]>
>> ---
>>  drivers/net/wireless/b43/phy_n.c |   13 +++----------
>>  1 files changed, 3 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/wireless/b43/phy_n.c b/drivers/net/wireless/b43/phy_n.c
>> index 47c58cd..bb8cb36 100644
>> --- a/drivers/net/wireless/b43/phy_n.c
>> +++ b/drivers/net/wireless/b43/phy_n.c
>> @@ -73,8 +73,6 @@ static void b43_nphy_rf_control_override(struct b43_wldev *dev, u16 field,
>>                                               u16 value, u8 core, bool off);
>>  static void b43_nphy_rf_control_intc_override(struct b43_wldev *dev, u8 field,
>>                                               u16 value, u8 core);
>> -static int b43_nphy_op_switch_channel(struct b43_wldev *dev,
>> -                                   unsigned int new_channel);
>>
>>  static inline bool b43_empty_chanspec(struct b43_chanspec *chanspec)
>>  {
>> @@ -224,7 +222,7 @@ static void b43_radio_init2055_post(struct b43_wldev *dev)
>>       if (i)
>>               b43err(dev->wl, "radio post init timeout\n");
>>       b43_radio_mask(dev, B2055_CAL_LPOCTL, 0xFF7F);
>> -     b43_nphy_op_switch_channel(dev, dev->phy.channel);
>> +     b43_switch_channel(dev, dev->phy.channel);
>>       b43_radio_write(dev, B2055_C1_RX_BB_LPF, 0x9);
>>       b43_radio_write(dev, B2055_C2_RX_BB_LPF, 0x9);
>>       b43_radio_write(dev, B2055_C1_RX_BB_MIDACHP, 0x83);
>> @@ -3352,12 +3350,7 @@ static void b43_nphy_chanspec_setup(struct b43_wldev *dev,
>>
>>       b43_chantab_phy_upload(dev, e);
>>
>> -     tmp = chanspec.channel;
>> -     if (chanspec.b_freq == 1)
>> -             tmp |= 0x0100;
>> -     if (chanspec.b_width == 3)
>> -             tmp |= 0x0200;
>> -     b43_shm_write16(dev, B43_SHM_SHARED, 0xA0, tmp);
>> +     /* Don't follow specs, B43_SHM_SH_CHAN is set by b43_switch_channel */
>
> Looks great. I dunno if a comment is really needed and whether it is
> correct, though. We _do_ follow the specs. We just do it elsewhere.

I got "Don't strictly follow specs here" earlier, but made it shorter
to match 80 chars ;) Maybe it wasn't too good idea. Should left it
longer or don't put at all.

John: could you drop this comment line when applying? Or would you
like me to resend this fixed?

--
Rafał

2010-10-06 05:50:06

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 1/3] b43: N-PHY: simplify channel switching

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

diff --git a/drivers/net/wireless/b43/phy_n.c b/drivers/net/wireless/b43/phy_n.c
index ac217da..47c58cd 100644
--- a/drivers/net/wireless/b43/phy_n.c
+++ b/drivers/net/wireless/b43/phy_n.c
@@ -73,7 +73,8 @@ static void b43_nphy_rf_control_override(struct b43_wldev *dev, u16 field,
u16 value, u8 core, bool off);
static void b43_nphy_rf_control_intc_override(struct b43_wldev *dev, u8 field,
u16 value, u8 core);
-static int nphy_channel_switch(struct b43_wldev *dev, unsigned int channel);
+static int b43_nphy_op_switch_channel(struct b43_wldev *dev,
+ unsigned int new_channel);

static inline bool b43_empty_chanspec(struct b43_chanspec *chanspec)
{
@@ -223,7 +224,7 @@ static void b43_radio_init2055_post(struct b43_wldev *dev)
if (i)
b43err(dev->wl, "radio post init timeout\n");
b43_radio_mask(dev, B2055_CAL_LPOCTL, 0xFF7F);
- nphy_channel_switch(dev, dev->phy.channel);
+ b43_nphy_op_switch_channel(dev, dev->phy.channel);
b43_radio_write(dev, B2055_C1_RX_BB_LPF, 0x9);
b43_radio_write(dev, B2055_C2_RX_BB_LPF, 0x9);
b43_radio_write(dev, B2055_C1_RX_BB_MIDACHP, 0x83);
@@ -3438,18 +3439,6 @@ static int b43_nphy_set_chanspec(struct b43_wldev *dev,
return 0;
}

-/* Tune the hardware to a new channel */
-static int nphy_channel_switch(struct b43_wldev *dev, unsigned int channel)
-{
- struct b43_phy_n *nphy = dev->phy.n;
-
- struct b43_chanspec chanspec;
- chanspec = nphy->radio_chanspec;
- chanspec.channel = channel;
-
- return b43_nphy_set_chanspec(dev, chanspec);
-}
-
static int b43_nphy_op_allocate(struct b43_wldev *dev)
{
struct b43_phy_n *nphy;
@@ -3570,7 +3559,7 @@ static void b43_nphy_op_software_rfkill(struct b43_wldev *dev,
} else {
if (dev->phy.rev >= 3) {
b43_radio_init2056(dev);
- b43_nphy_set_chanspec(dev, nphy->radio_chanspec);
+ b43_nphy_op_switch_channel(dev, dev->phy.channel);
} else {
b43_radio_init2055(dev);
}
@@ -3586,6 +3575,9 @@ static void b43_nphy_op_switch_analog(struct b43_wldev *dev, bool on)
static int b43_nphy_op_switch_channel(struct b43_wldev *dev,
unsigned int new_channel)
{
+ struct b43_phy_n *nphy = dev->phy.n;
+ struct b43_chanspec chanspec;
+
if (b43_current_band(dev->wl) == IEEE80211_BAND_2GHZ) {
if ((new_channel < 1) || (new_channel > 14))
return -EINVAL;
@@ -3594,7 +3586,10 @@ static int b43_nphy_op_switch_channel(struct b43_wldev *dev,
return -EINVAL;
}

- return nphy_channel_switch(dev, new_channel);
+ chanspec = nphy->radio_chanspec;
+ chanspec.channel = new_channel;
+
+ return b43_nphy_set_chanspec(dev, chanspec);
}

static unsigned int b43_nphy_op_get_default_chan(struct b43_wldev *dev)
--
1.7.1