Date:	Tue, 24 Oct 2006 10:09:59 -0400
From:	"Holden Karau" <holden@...scanfly.ca>
To:	"Daniel Drake" <dsd@...too.org>
Cc:	zd1211-devs@...ts.sourceforge.net, linville@...driver.com,
	netdev <netdev@...r.kernel.org>, linux-kernel@...r.kernel.org,
	holdenk@...dros.com, "Ulrich Kunitz" <kune@...ne-taler.de>
Subject: Re: [PATCH] wireless-2.6 zd1211rw check against regulatory domain rather than hardcoded value of 11

I've changed the patch based on your suggestions :-)
Hopefully my mailer doesn't eat the tabs this time, if it does I've
put the patch up at
http://www.holdenkarau.com/~holden/projects/zd1211rw/zd1211rw-use-geo-for-channels-r2.patch

On 10/23/06, Daniel Drake <dsd@...too.org> wrote:
> Holden Karau wrote:
> > From: Holden Karau <holden@...scanfly.ca> http://www.holdenkarau.com
> >
> > I have made a small patch for the zd1211rw driver which uses the
> > boundry channels of the regulatory domain, rather than the hard coded
> > values of 1 & 11.
> > Signed-off-by: Holden Karau <holden@...scanfly.ca>
> > http://www.holdenkarau.com
>
> Thanks for the patch! Please always look up the MAINTAINERS entry for
> the code you are modifying and CC the developers on patches.
>
I looked at the mainteners file, but the zd1211rw website (
http://zd1211.ath.cx/wiki/DriverRewrite ) says not to e-mail you guys
directly, rather use the mailing list.
> Comments below, all minor points.
>
> > I'm not entirely sure how useful this patch is, but it seems like a
> > good idea. If its totally misguided, let me know :-) In case the patch
> > gets mangled I've put it up at
> > http://www.holdenkarau.com/~holden/projects/zd1211rw/zd1211rw-use-geo-for-channels.patch
>
> Your mailer ate tabs and wrapped long lines. You're going to need to fix
> that.
>
whoops. Sorry about that.
> > --- a/drivers/net/wireless/zd1211rw/zd_chip.c    2006-10-23
> > 10:07:39.000000000 -0400
> > +++ b/drivers/net/wireless/zd1211rw/zd_chip.c    2006-10-23
> > 10:41:51.000000000 -0400
> > @@ -38,6 +38,8 @@ void zd_chip_init(struct zd_chip *chip,
> >     mutex_init(&chip->mutex);
> >     zd_usb_init(&chip->usb, netdev, intf);
> >     zd_rf_init(&chip->rf);
> > +    /* The chip needs to know which geo it is in */
> > +    chip->geo =
> > ieee80211_get_geo(zd_mac_to_ieee80211(zd_netdev_mac(netdev)));
>
> There is no need to store a geo reference here. You can use
> zd_chip_to_mac() to go from chip to mac, then mac-to-ieee80211 is easy.
>
That is much cleaner. Thanks :-)
> > }
> >
> > void zd_chip_clear(struct zd_chip *chip)
> > @@ -606,14 +608,17 @@ static int patch_6m_band_edge(struct zd_
> >         { CR128, 0x14 }, { CR129, 0x12 }, { CR130, 0x10 },
> >         { CR47,  0x1e },
> >     };
> > +    struct ieee80211_geo *geo = chip->geo;
> >
> >     if (!chip->patch_6m_band_edge || !chip->rf.patch_6m_band_edge)
> >         return 0;
> >
> > -    /* FIXME: Channel 11 is not the edge for all regulatory domains. */
> > -    if (channel == 1 || channel == 11)
> > +    /* Checks the channel boundry of the region */
> > +    dev_dbg_f("checking boundry == %d || %d\n" , 1 , geo->bg_channels);
> > +    if (channel == 1 || channel == geo->bg_channels)
>
> Typo, you mean boundary. Also, I think the debug message can go once
> you're confident it's working correctly.
>
> >         ioreqs[0].value = 0x12;
> >
> > +
>
> This added line could go as well.
>
> >     dev_dbg_f(zd_chip_dev(chip), "patching for channel %d\n", channel);
> >     return zd_iowrite16a_locked(chip, ioreqs, ARRAY_SIZE(ioreqs));
> > }
>
> I think that after the above changes, your modifications to zd_chip.h
> can be removed.
Yup. Much cleaner :-)
>
> > --- a/drivers/net/wireless/zd1211rw/zd_chip.h    2006-10-23
> > 10:07:39.000000000 -0400
> > +++ b/drivers/net/wireless/zd1211rw/zd_chip.h    2006-10-23
> > 10:39:08.000000000 -0400
> > @@ -21,6 +21,8 @@
> > #include "zd_types.h"
> > #include "zd_rf.h"
> > #include "zd_usb.h"
> > +#include "zd_ieee80211.h"
> > +#include <linux/wireless.h>
> >
> > /* Header for the Media Access Controller (MAC) and the Baseband Processor
> >  * (BBP). It appears that the ZD1211 wraps the old ZD1205 with USB glue and
> > @@ -669,6 +671,7 @@ struct zd_chip {
> >     /* SetPointOFDM in the vendor driver */
> >     u8 ofdm_cal_values[3][E2P_CHANNEL_COUNT];
> >     u16 link_led;
> > +      struct ieee80211_geo* geo;
> >     unsigned int pa_type:4,
> >         patch_cck_gain:1, patch_cr157:1, patch_6m_band_edge:1,
> >         new_phy_layout:1,
> > -
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
>
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
The new patch :
--- a/wireless-2.6/drivers/net/wireless/zd1211rw/zd_chip.c	2006-10-23
10:07:39.000000000 -0400
+++ b/wireless-2.6/drivers/net/wireless/zd1211rw/zd_chip.c	2006-10-24
09:39:10.000000000 -0400
@@ -606,12 +606,13 @@ static int patch_6m_band_edge(struct zd_
 		{ CR128, 0x14 }, { CR129, 0x12 }, { CR130, 0x10 },
 		{ CR47,  0x1e },
 	};
+	struct ieee80211_geo *geo =
ieee80211_get_geo(zd_mac_to_ieee80211(zd_chip_to_mac(chip)));

 	if (!chip->patch_6m_band_edge || !chip->rf.patch_6m_band_edge)
 		return 0;

-	/* FIXME: Channel 11 is not the edge for all regulatory domains. */
-	if (channel == 1 || channel == 11)
+	/* Checks the channel boundary of the region */
+	 if (channel == 1 || channel == geo->bg_channels)
 		ioreqs[0].value = 0x12;

 	dev_dbg_f(zd_chip_dev(chip), "patching for channel %d\n", channel);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists