2012-02-10 00:22:29

by Stanislav Yakovlev

[permalink] [raw]
Subject: [PATCH] ipw2x00: remove ipw2100_rates_11b[]

It's just a duplicate of ipw2100_bg_rates[].

Signed-off-by: Stanislav Yakovlev <[email protected]>
---
drivers/net/wireless/ipw2x00/ipw2100.c | 11 ++---------
1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
index a0e5c21..63567fb 100644
--- a/drivers/net/wireless/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/ipw2x00/ipw2100.c
@@ -309,13 +309,6 @@ static const long ipw2100_frequencies[] = {

#define FREQ_COUNT ARRAY_SIZE(ipw2100_frequencies)

-static const long ipw2100_rates_11b[] = {
- 1000000,
- 2000000,
- 5500000,
- 11000000
-};
-
static struct ieee80211_rate ipw2100_bg_rates[] = {
{ .bitrate = 10 },
{ .bitrate = 20, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
@@ -323,7 +316,7 @@ static struct ieee80211_rate ipw2100_bg_rates[] = {
{ .bitrate = 110, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
};

-#define RATE_COUNT ARRAY_SIZE(ipw2100_rates_11b)
+#define RATE_COUNT ARRAY_SIZE(ipw2100_bg_rates)

/* Pre-decl until we get the code solid and then we can clean it up */
static void ipw2100_tx_send_commands(struct ipw2100_priv *priv);
@@ -6896,7 +6889,7 @@ static int ipw2100_wx_get_range(struct net_device *dev,
range->num_bitrates = RATE_COUNT;

for (i = 0; i < RATE_COUNT && i < IW_MAX_BITRATES; i++) {
- range->bitrate[i] = ipw2100_rates_11b[i];
+ range->bitrate[i] = ipw2100_bg_rates[i].bitrate * 100 * 1000;
}

range->min_rts = MIN_RTS_THRESHOLD;
--
1.7.2.5



2012-02-10 00:56:48

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH] ipw2x00: remove ipw2100_rates_11b[]

Hi Dan,

On Fri, Feb 10, 2012 at 11:46, Dan Williams <[email protected]> wrote:
> On Fri, 2012-02-10 at 11:26 +1100, Julian Calaby wrote:
>> Hi Stanislav,
>>
>> On Fri, Feb 10, 2012 at 12:23, Stanislav Yakovlev
>> <[email protected]> wrote:
>> > It's just a duplicate of ipw2100_bg_rates[].
>>
>> Looks sensible to me.
>
> Except that the 2100 is a B-only device; it doesn't do G at all. ?So
> wouldn't it make sense to get rid of ipw2100_rates_bg[] instead?

The extra data in ipw2100_bg_rates[] is used when setting up the wiphy
bands, ipw2100_rates_11b[] is just ipw2100_bg_rates[].bitrate * 100 *
1000.

The cards may not support G, but that doesn't mean the structure can't
be named as if they do. Looking at the driver, it seems that whoever
wrote the band handling code just lumped B and G together and only
used B rates.

Thanks,

--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

2012-02-22 21:49:12

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH] ipw2x00: remove ipw2100_rates_11b[]

Hi Stanislav,

On Thu, Feb 23, 2012 at 04:59, Stanislav Yakovlev
<[email protected]> wrote:
> On 10 February 2012 03:46, Dan Williams <[email protected]> wrote:
>> On Fri, 2012-02-10 at 11:26 +1100, Julian Calaby wrote:
>>> Hi Stanislav,
>>>
>>> On Fri, Feb 10, 2012 at 12:23, Stanislav Yakovlev
>>> <[email protected]> wrote:
>>> > It's just a duplicate of ipw2100_bg_rates[].
>>>
>>> Looks sensible to me.
>>
>> Except that the 2100 is a B-only device; it doesn't do G at all. ?So
>> wouldn't it make sense to get rid of ipw2100_rates_bg[] instead?
>
> It looks like we all agree that one of them should be removed. I did
> not see an easy way to remove ipw2100_rates_bg[]. Maybe it makes more
> sense to rename it to ipw2100_rates_b[]?

I'm not sure it makes much of a difference, but it can't hurt.

Thanks,

--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

2012-02-22 17:59:35

by Stanislav Yakovlev

[permalink] [raw]
Subject: Re: [PATCH] ipw2x00: remove ipw2100_rates_11b[]

On 10 February 2012 03:46, Dan Williams <[email protected]> wrote:
> On Fri, 2012-02-10 at 11:26 +1100, Julian Calaby wrote:
>> Hi Stanislav,
>>
>> On Fri, Feb 10, 2012 at 12:23, Stanislav Yakovlev
>> <[email protected]> wrote:
>> > It's just a duplicate of ipw2100_bg_rates[].
>>
>> Looks sensible to me.
>
> Except that the 2100 is a B-only device; it doesn't do G at all. ?So
> wouldn't it make sense to get rid of ipw2100_rates_bg[] instead?

It looks like we all agree that one of them should be removed. I did
not see an easy way to remove ipw2100_rates_bg[]. Maybe it makes more
sense to rename it to ipw2100_rates_b[]?

Stanislav


> Dan
>
>> Reviewed-by: Julian Calaby <[email protected]>
>>
>> > Signed-off-by: Stanislav Yakovlev <[email protected]>
>> > ---
>> > ?drivers/net/wireless/ipw2x00/ipw2100.c | ? 11 ++---------
>> > ?1 files changed, 2 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
>> > index a0e5c21..63567fb 100644
>> > --- a/drivers/net/wireless/ipw2x00/ipw2100.c
>> > +++ b/drivers/net/wireless/ipw2x00/ipw2100.c
>> > @@ -309,13 +309,6 @@ static const long ipw2100_frequencies[] = {
>> >
>> > ?#define FREQ_COUNT ? ? ARRAY_SIZE(ipw2100_frequencies)
>> >
>> > -static const long ipw2100_rates_11b[] = {
>> > - ? ? ? 1000000,
>> > - ? ? ? 2000000,
>> > - ? ? ? 5500000,
>> > - ? ? ? 11000000
>> > -};
>> > -
>> > ?static struct ieee80211_rate ipw2100_bg_rates[] = {
>> > ? ? ? ?{ .bitrate = 10 },
>> > ? ? ? ?{ .bitrate = 20, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
>> > @@ -323,7 +316,7 @@ static struct ieee80211_rate ipw2100_bg_rates[] = {
>> > ? ? ? ?{ .bitrate = 110, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
>> > ?};
>> >
>> > -#define RATE_COUNT ARRAY_SIZE(ipw2100_rates_11b)
>> > +#define RATE_COUNT ARRAY_SIZE(ipw2100_bg_rates)
>> >
>> > ?/* Pre-decl until we get the code solid and then we can clean it up */
>> > ?static void ipw2100_tx_send_commands(struct ipw2100_priv *priv);
>> > @@ -6896,7 +6889,7 @@ static int ipw2100_wx_get_range(struct net_device *dev,
>> > ? ? ? ?range->num_bitrates = RATE_COUNT;
>> >
>> > ? ? ? ?for (i = 0; i < RATE_COUNT && i < IW_MAX_BITRATES; i++) {
>> > - ? ? ? ? ? ? ? range->bitrate[i] = ipw2100_rates_11b[i];
>> > + ? ? ? ? ? ? ? range->bitrate[i] = ipw2100_bg_rates[i].bitrate * 100 * 1000;
>> > ? ? ? ?}
>> >
>> > ? ? ? ?range->min_rts = MIN_RTS_THRESHOLD;
>> > --
>> > 1.7.2.5
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> > the body of a message to [email protected]
>> > More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>
>

2012-02-10 00:45:09

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] ipw2x00: remove ipw2100_rates_11b[]

On Fri, 2012-02-10 at 11:26 +1100, Julian Calaby wrote:
> Hi Stanislav,
>
> On Fri, Feb 10, 2012 at 12:23, Stanislav Yakovlev
> <[email protected]> wrote:
> > It's just a duplicate of ipw2100_bg_rates[].
>
> Looks sensible to me.

Except that the 2100 is a B-only device; it doesn't do G at all. So
wouldn't it make sense to get rid of ipw2100_rates_bg[] instead?

Dan

> Reviewed-by: Julian Calaby <[email protected]>
>
> > Signed-off-by: Stanislav Yakovlev <[email protected]>
> > ---
> > drivers/net/wireless/ipw2x00/ipw2100.c | 11 ++---------
> > 1 files changed, 2 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
> > index a0e5c21..63567fb 100644
> > --- a/drivers/net/wireless/ipw2x00/ipw2100.c
> > +++ b/drivers/net/wireless/ipw2x00/ipw2100.c
> > @@ -309,13 +309,6 @@ static const long ipw2100_frequencies[] = {
> >
> > #define FREQ_COUNT ARRAY_SIZE(ipw2100_frequencies)
> >
> > -static const long ipw2100_rates_11b[] = {
> > - 1000000,
> > - 2000000,
> > - 5500000,
> > - 11000000
> > -};
> > -
> > static struct ieee80211_rate ipw2100_bg_rates[] = {
> > { .bitrate = 10 },
> > { .bitrate = 20, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
> > @@ -323,7 +316,7 @@ static struct ieee80211_rate ipw2100_bg_rates[] = {
> > { .bitrate = 110, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
> > };
> >
> > -#define RATE_COUNT ARRAY_SIZE(ipw2100_rates_11b)
> > +#define RATE_COUNT ARRAY_SIZE(ipw2100_bg_rates)
> >
> > /* Pre-decl until we get the code solid and then we can clean it up */
> > static void ipw2100_tx_send_commands(struct ipw2100_priv *priv);
> > @@ -6896,7 +6889,7 @@ static int ipw2100_wx_get_range(struct net_device *dev,
> > range->num_bitrates = RATE_COUNT;
> >
> > for (i = 0; i < RATE_COUNT && i < IW_MAX_BITRATES; i++) {
> > - range->bitrate[i] = ipw2100_rates_11b[i];
> > + range->bitrate[i] = ipw2100_bg_rates[i].bitrate * 100 * 1000;
> > }
> >
> > range->min_rts = MIN_RTS_THRESHOLD;
> > --
> > 1.7.2.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>



2012-02-10 00:27:06

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH] ipw2x00: remove ipw2100_rates_11b[]

Hi Stanislav,

On Fri, Feb 10, 2012 at 12:23, Stanislav Yakovlev
<[email protected]> wrote:
> It's just a duplicate of ipw2100_bg_rates[].

Looks sensible to me.

Reviewed-by: Julian Calaby <[email protected]>

> Signed-off-by: Stanislav Yakovlev <[email protected]>
> ---
> ?drivers/net/wireless/ipw2x00/ipw2100.c | ? 11 ++---------
> ?1 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
> index a0e5c21..63567fb 100644
> --- a/drivers/net/wireless/ipw2x00/ipw2100.c
> +++ b/drivers/net/wireless/ipw2x00/ipw2100.c
> @@ -309,13 +309,6 @@ static const long ipw2100_frequencies[] = {
>
> ?#define FREQ_COUNT ? ? ARRAY_SIZE(ipw2100_frequencies)
>
> -static const long ipw2100_rates_11b[] = {
> - ? ? ? 1000000,
> - ? ? ? 2000000,
> - ? ? ? 5500000,
> - ? ? ? 11000000
> -};
> -
> ?static struct ieee80211_rate ipw2100_bg_rates[] = {
> ? ? ? ?{ .bitrate = 10 },
> ? ? ? ?{ .bitrate = 20, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
> @@ -323,7 +316,7 @@ static struct ieee80211_rate ipw2100_bg_rates[] = {
> ? ? ? ?{ .bitrate = 110, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
> ?};
>
> -#define RATE_COUNT ARRAY_SIZE(ipw2100_rates_11b)
> +#define RATE_COUNT ARRAY_SIZE(ipw2100_bg_rates)
>
> ?/* Pre-decl until we get the code solid and then we can clean it up */
> ?static void ipw2100_tx_send_commands(struct ipw2100_priv *priv);
> @@ -6896,7 +6889,7 @@ static int ipw2100_wx_get_range(struct net_device *dev,
> ? ? ? ?range->num_bitrates = RATE_COUNT;
>
> ? ? ? ?for (i = 0; i < RATE_COUNT && i < IW_MAX_BITRATES; i++) {
> - ? ? ? ? ? ? ? range->bitrate[i] = ipw2100_rates_11b[i];
> + ? ? ? ? ? ? ? range->bitrate[i] = ipw2100_bg_rates[i].bitrate * 100 * 1000;
> ? ? ? ?}
>
> ? ? ? ?range->min_rts = MIN_RTS_THRESHOLD;
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html



--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/