Linux wireless drivers development
 help / color / mirror / Atom feed
* Questions about regulatory domain & passive scanning
From: Holger Schurig @ 2009-09-11 12:27 UTC (permalink / raw)
  To: linux-wireless, Luis R. Rodriguez

Hi !

I'm playing with regulatory domain using wireless-testing, iw, 
crda and ath5 on Debian. Here are some observations:

1) Debian sucks here (while it's otherwise great!). No
   wireless-regdb, wireless-crda packages, not even in
   experimental. Maybe I need to add some Ubuntu repository
   as "deb-src" to my apt sources.list, apt-get source that
   stuff and recompile it manually.

2) I remove CONFIG_WIRELESS_OLD_REGULATORY. Then, after inserting
   the card, I see in "iw list":

         * 2412 MHz [1] (20.0 dBm)
         * 2417 MHz [2] (20.0 dBm)
         * 2422 MHz [3] (20.0 dBm)
         * 2427 MHz [4] (20.0 dBm)
         * 2432 MHz [5] (20.0 dBm)
         * 2437 MHz [6] (20.0 dBm)
         * 2442 MHz [7] (20.0 dBm)
         * 2447 MHz [8] (20.0 dBm)
         * 2452 MHz [9] (20.0 dBm)
         * 2457 MHz [10] (20.0 dBm)
         * 2462 MHz [11] (20.0 dBm)
         * 2467 MHz [12] (20.0 dBm) (passive scanning)
         * 2472 MHz [13] (20.0 dBm) (passive scanning)
         * 2484 MHz [14] (20.0 dBm) (passive scanning)

   I'd like to highlight the "passive scanning". Is this info
   from the card EEPROM?

3) After "iw reg set DE" (for germany), it's now

          2412 MHz [1] (20.0 dBm)
          2417 MHz [2] (20.0 dBm)
          2422 MHz [3] (20.0 dBm)
          2427 MHz [4] (20.0 dBm)
          2432 MHz [5] (20.0 dBm)
          2437 MHz [6] (20.0 dBm)
          2442 MHz [7] (20.0 dBm)
          2447 MHz [8] (20.0 dBm)
          2452 MHz [9] (20.0 dBm)
          2457 MHz [10] (20.0 dBm)
          2462 MHz [11] (20.0 dBm)
          2467 MHz [12] (20.0 dBm) (passive scanning)
          2472 MHz [13] (20.0 dBm) (passive scanning)
          2484 MHz [14] (disabled)

   Great, CRDA worked obviously: channel 14 has been disabled.

   And, as I understand it, CRDA can just limit settings
   further, not widening it. Therefore channels 12 and 13
   are still marked as "passive scanning".

   If I want to get them to "active scanning", would I need to
   modify the EEPROM of the card?  ath_info says "Reg. Domain:
   0x60".


-- 
http://www.holgerschurig.de

^ permalink raw reply

* Re: [PATCH 3/4] ath5k: define ath_common ops
From: Bob Copeland @ 2009-09-11 11:35 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Jiri Slaby, Nick Kossifidis, devel, ath9k-devel, linux-wireless,
	Alan Cox, Linus Torvalds, Jeff Garzik
In-Reply-To: <43e72e890909110023k62a512bejd712a3449cc8328d@mail.gmail.com>

On Fri, Sep 11, 2009 at 3:23 AM, Luis R. Rodriguez
<lrodriguez@atheros.com> wrote:
> ath9k/ath9k/ath9k_htc. But -- is there really is a measurable cost
> penalty?
>
> This is why I asked if someone can test and give measurable
> differences over this. If there really isn't then that's not strong
> point against it.

Honestly, it probably won't matter in the grand scheme of things, but I
think if you are proposing a patch that touches every hotpath in two
drivers, then you need to do the work to say "by the way, this has benefit
X which outweighs the very small (or absent) performance regression Y,
and here are the numbers.

-- 
Bob Copeland %% www.bobcopeland.com

^ permalink raw reply

* [PATCH v2] sdio: pass whitelisted cis funce tuples to sdio drivers
From: Albert Herranz @ 2009-09-11  9:54 UTC (permalink / raw)
  To: akpm, pierre, linux-mmc; +Cc: bcm43xx-dev, linux-wireless, Albert Herranz
In-Reply-To: <1252662852-20548-1-git-send-email-albert_herranz@yahoo.es>

Some manufacturers provide vendor information in non-vendor specific CIS
tuples. For example, Broadcom uses an Extended Function tuple to provide
the MAC address on some of their network cards, as in the case of the
Nintendo Wii WLAN daughter card.

This patch allows passing whitelisted FUNCE tuples unknown to the SDIO core to
a matching SDIO driver instead of rejecting them and failing.

Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
---
v1
- fixed typo in commit message
- CC'd akpm as suggested by mb
- required by commit 4ea602e183ca20a7577ebe253323d0e5d0f9847 in net-next-2.6

v2
- renamed from "sdio: pass unknown cis tuples to sdio drivers"
- added funce tuple whitelist concept
- fixed leak reported by akpm

 drivers/mmc/core/sdio_cis.c |   65 ++++++++++++++++++++++++++++++++----------
 1 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c
index 963f293..4e4fac1 100644
--- a/drivers/mmc/core/sdio_cis.c
+++ b/drivers/mmc/core/sdio_cis.c
@@ -98,6 +98,22 @@ static const unsigned char speed_val[16] =
 static const unsigned int speed_unit[8] =
 	{ 10000, 100000, 1000000, 10000000, 0, 0, 0, 0 };
 
+/* FUNCE tuples with these types get passed to SDIO drivers */
+static const unsigned char funce_type_whitelist[] = {
+	4 /* CISTPL_FUNCE_LAN_NODE_ID used in Broadcom cards */
+};
+
+static int cistpl_funce_whitelisted(unsigned char type)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(funce_type_whitelist); i++) {
+		if (funce_type_whitelist[i] == type)
+			return 1;
+	}
+	return 0;
+}
+
 static int cistpl_funce_common(struct mmc_card *card,
 			       const unsigned char *buf, unsigned size)
 {
@@ -120,6 +136,10 @@ static int cistpl_funce_func(struct sdio_func *func,
 	unsigned vsn;
 	unsigned min_size;
 
+	/* let SDIO drivers take care of whitelisted FUNCE tuples */
+	if (cistpl_funce_whitelisted(buf[0]))
+		return -EILSEQ;
+
 	vsn = func->card->cccr.sdio_vsn;
 	min_size = (vsn == SDIO_SDIO_REV_1_00) ? 28 : 42;
 
@@ -154,13 +174,12 @@ static int cistpl_funce(struct mmc_card *card, struct sdio_func *func,
 	else
 		ret = cistpl_funce_common(card, buf, size);
 
-	if (ret) {
+	if (ret && ret != -EILSEQ) {
 		printk(KERN_ERR "%s: bad CISTPL_FUNCE size %u "
 		       "type %u\n", mmc_hostname(card->host), size, buf[0]);
-		return ret;
 	}
 
-	return 0;
+	return ret;
 }
 
 typedef int (tpl_parse_t)(struct mmc_card *, struct sdio_func *,
@@ -253,21 +272,12 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func)
 		for (i = 0; i < ARRAY_SIZE(cis_tpl_list); i++)
 			if (cis_tpl_list[i].code == tpl_code)
 				break;
-		if (i >= ARRAY_SIZE(cis_tpl_list)) {
-			/* this tuple is unknown to the core */
-			this->next = NULL;
-			this->code = tpl_code;
-			this->size = tpl_link;
-			*prev = this;
-			prev = &this->next;
-			printk(KERN_DEBUG
-			       "%s: queuing CIS tuple 0x%02x length %u\n",
-			       mmc_hostname(card->host), tpl_code, tpl_link);
-		} else {
+		if (i < ARRAY_SIZE(cis_tpl_list)) {
 			const struct cis_tpl *tpl = cis_tpl_list + i;
 			if (tpl_link < tpl->min_size) {
 				printk(KERN_ERR
-				       "%s: bad CIS tuple 0x%02x (length = %u, expected >= %u)\n",
+				       "%s: bad CIS tuple 0x%02x"
+				       " (length = %u, expected >= %u)\n",
 				       mmc_hostname(card->host),
 				       tpl_code, tpl_link, tpl->min_size);
 				ret = -EINVAL;
@@ -275,7 +285,30 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func)
 				ret = tpl->parse(card, func,
 						 this->data, tpl_link);
 			}
-			kfree(this);
+			/*
+			 * We don't need the tuple anymore if it was
+			 * successfully parsed by the SDIO core or if it is
+			 * not going to be parsed by SDIO drivers.
+			 */
+			if (!ret || ret != -EILSEQ)
+				kfree(this);
+		} else {
+			/* unknown tuple */
+			ret = -EILSEQ;
+		}
+
+		if (ret == -EILSEQ) {
+			/* this tuple is unknown to the core or whitelisted */
+			this->next = NULL;
+			this->code = tpl_code;
+			this->size = tpl_link;
+			*prev = this;
+			prev = &this->next;
+			printk(KERN_DEBUG
+			       "%s: queuing CIS tuple 0x%02x length %u\n",
+			       mmc_hostname(card->host), tpl_code, tpl_link);
+			/* keep on analyzing tuples */
+			ret = 0;
 		}
 
 		ptr += tpl_link;
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH v2] sdio: recognize io card without powercycle
From: Albert Herranz @ 2009-09-11  9:54 UTC (permalink / raw)
  To: akpm, pierre, linux-mmc; +Cc: bcm43xx-dev, linux-wireless, Albert Herranz

SDIO Simplified Specification V2.00 states that it is strongly recommended
that the host executes either a power reset or issues a CMD52 (I/O Reset) to
re-initialize an I/O only card or the I/O portion of a combo card.
Additionally, the CMD52 must be issued first because it cannot be issued
after a CMD0.

With this patch the Nintendo Wii SDIO-based WLAN card is detected after a
system reset, without requiring a complete system powercycle.

Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
---
v1
- reworded commit message
- CC'd akpm as suggested by mb

v2
- renamed sdio_go_idle to sdio_reset as suggested by Pierre Ossman

 drivers/mmc/core/core.c     |    1 +
 drivers/mmc/core/sdio_ops.c |   36 ++++++++++++++++++++++++++++++------
 drivers/mmc/core/sdio_ops.h |    1 +
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 2649117..dd746d9 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -868,6 +868,7 @@ void mmc_rescan(struct work_struct *work)
 		mmc_claim_host(host);
 
 		mmc_power_up(host);
+		sdio_reset(host);
 		mmc_go_idle(host);
 
 		mmc_send_if_cond(host, host->ocr_avail);
diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
index 4eb7825..dea36d9 100644
--- a/drivers/mmc/core/sdio_ops.c
+++ b/drivers/mmc/core/sdio_ops.c
@@ -67,13 +67,13 @@ int mmc_send_io_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
 	return err;
 }
 
-int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
-	unsigned addr, u8 in, u8* out)
+static int mmc_io_rw_direct_host(struct mmc_host *host, int write, unsigned fn,
+	unsigned addr, u8 in, u8 *out)
 {
 	struct mmc_command cmd;
 	int err;
 
-	BUG_ON(!card);
+	BUG_ON(!host);
 	BUG_ON(fn > 7);
 
 	/* sanity check */
@@ -90,11 +90,11 @@ int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
 	cmd.arg |= in;
 	cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_AC;
 
-	err = mmc_wait_for_cmd(card->host, &cmd, 0);
+	err = mmc_wait_for_cmd(host, &cmd, 0);
 	if (err)
 		return err;
 
-	if (mmc_host_is_spi(card->host)) {
+	if (mmc_host_is_spi(host)) {
 		/* host driver already reported errors */
 	} else {
 		if (cmd.resp[0] & R5_ERROR)
@@ -106,7 +106,7 @@ int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
 	}
 
 	if (out) {
-		if (mmc_host_is_spi(card->host))
+		if (mmc_host_is_spi(host))
 			*out = (cmd.resp[0] >> 8) & 0xFF;
 		else
 			*out = cmd.resp[0] & 0xFF;
@@ -115,6 +115,13 @@ int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
 	return 0;
 }
 
+int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
+	unsigned addr, u8 in, u8 *out)
+{
+	BUG_ON(!card);
+	return mmc_io_rw_direct_host(card->host, write, fn, addr, in, out);
+}
+
 int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
 	unsigned addr, int incr_addr, u8 *buf, unsigned blocks, unsigned blksz)
 {
@@ -182,3 +189,20 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
 	return 0;
 }
 
+int sdio_reset(struct mmc_host *host)
+{
+	int ret;
+	u8 abort;
+
+	/* SDIO Simplified Specification V2.0, 4.4 Reset for SDIO */
+
+	ret = mmc_io_rw_direct_host(host, 0, 0, SDIO_CCCR_ABORT, 0, &abort);
+	if (ret)
+		abort = 0x08;
+	else
+		abort |= 0x08;
+
+	ret = mmc_io_rw_direct_host(host, 1, 0, SDIO_CCCR_ABORT, abort, NULL);
+	return ret;
+}
+
diff --git a/drivers/mmc/core/sdio_ops.h b/drivers/mmc/core/sdio_ops.h
index e2e74b0..12a4d3a 100644
--- a/drivers/mmc/core/sdio_ops.h
+++ b/drivers/mmc/core/sdio_ops.h
@@ -17,6 +17,7 @@ int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
 	unsigned addr, u8 in, u8* out);
 int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
 	unsigned addr, int incr_addr, u8 *buf, unsigned blocks, unsigned blksz);
+int sdio_reset(struct mmc_host *host);
 
 #endif
 
-- 
1.6.0.4


^ permalink raw reply related

* Re: iwlagn: order 2 page allocation failures
From: Mel Gorman @ 2009-09-11  8:47 UTC (permalink / raw)
  To: reinette chatre
  Cc: Frans Pop, Larry Finger, John W. Linville, Pekka Enberg,
	linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
	ipw3945-devel@lists.sourceforge.net, Andrew Morton,
	cl@linux-foundation.org, Krauss, Assaf, Johannes Berg,
	Abbas, Mohamed
In-Reply-To: <1252617290.30150.321.camel@rc-desk>

On Thu, Sep 10, 2009 at 02:14:50PM -0700, reinette chatre wrote:
> On Thu, 2009-09-10 at 02:02 -0700, Mel Gorman wrote:
> 
> > 
> > As a total aside, there is still the problem that the driver is depending on
> > order-2 allocations. On systems without swap, the allocation problem could be
> > more severe as there are fewer pages the system can use to regain contiguity.
> 
> I looked more at the implementation and hardware interface but I do not
> see a way around this. We have to provide 8k buffer to device, and we
> have to make sure it is aligned. 
> 

That would imply an order-1 allocation instead of an order-2 though so
it would appear than we are being worse than we have to. It would appear
to be because of this +256 bytes that goes onto every buffer.

> Do you have any suggestions?
> 

Nothing concrete. Finding an alternative to having the socket buffer
8192+256 to make it an order-1 allocation would be an improvement but I
don't know how that should be tackled. Lacking the hardware, I can't
experiment myself :(

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

^ permalink raw reply

* Re: iwlagn: order 2 page allocation failures
From: Mel Gorman @ 2009-09-11  8:45 UTC (permalink / raw)
  To: reinette chatre
  Cc: Frans Pop, Larry Finger, John W. Linville, Pekka Enberg,
	linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
	ipw3945-devel@lists.sourceforge.net, Andrew Morton,
	cl@linux-foundation.org, Krauss, Assaf, Johannes Berg,
	Abbas, Mohamed
In-Reply-To: <1252606547.30150.304.camel@rc-desk>

On Thu, Sep 10, 2009 at 11:15:47AM -0700, reinette chatre wrote:
> > > We can thus use ___GFP_NOWARN for the allocations in
> > > iwl_rx_allocate and leave it to the restocking to find the needed memory
> > > when it tried its allocations using GFP_KERNEL.
> > > 
> > 
> > Would it be possible to use __GFP_NOWARN *unless* this allocation is
> > necessary to receive the packet?
> 
> I think so.
> 
> > > I do think it is useful to let user know about these allocation
> > > failures, even if it does not result in packet loss. Wrapping it in
> > > net_ratelimit() will help though.
> > > 
> > 
> > If it does not distinguish between failures causing packet loss and just a
> > temporary issue, I'd be worried the messages would generate bug reports and
> > we genuinely won't know if it's a real problem or not.
> 
> Good point.
> 
> > 
> > As a total aside, there is still the problem that the driver is depending on
> > order-2 allocations. On systems without swap, the allocation problem could be
> > more severe as there are fewer pages the system can use to regain contiguity.
> 
> It seems that somebody did think about this in the initialization of
> max_pkt_size (which is priv->hw_params.rx_buf_size - 256). If we use
> max_pkt_size in the code that allocates the skb then the 256 added for
> alignment will not cause it to go to an order-2 allocation. I do not
> know why max_pkt_size is not used at the moment and will have to do some
> digging to find out.
> 

Thanks

> > > How about the patch below?
> > > 
> > > diff --git a/drivers/net/wireless/iwlwifi/iwl-rx.c b/drivers/net/wireless/iwlwifi/iwl-rx.c
> > > index b90adcb..f0ee72e 100644
> > > --- a/drivers/net/wireless/iwlwifi/iwl-rx.c
> > > +++ b/drivers/net/wireless/iwlwifi/iwl-rx.c
> > > @@ -252,10 +252,11 @@ void iwl_rx_allocate(struct iwl_priv *priv, gfp_t priority)
> > >  
> > >  		/* Alloc a new receive buffer */
> > >  		skb = alloc_skb(priv->hw_params.rx_buf_size + 256,
> > > -						priority);
> > > +				priority | __GFP_NOWARN);
> > >  
> > 
> > So, would it be possible here to only remove __GFP_NOWARN if this is GFP_ATOMIC
> > (implying we have to refill now) and the rxq->free_count is really small
> > e.g. <= 2?
> 
> I like your suggestion. Considering the issue I would like to remove
> __GFP_NOWARN even if it is GFP_KERNEL ... I think it is actually even
> more of a problem if we are in GFP_KERNEL and not able to allocate
> memory when running low on buffers. Also, with the queue size of 256 I
> think we can use RX_LOW_WATERMARK here, which is 8.
> 

RX_LOW_WATERMARK sounds reasonable as if that watermark is reached, the
buffer count is pretty low. With order-2 allocations, I bet the system is
beginning to grind a bit to find contiguous pages at that point as well.

I agree that it's a greater problem if the system is unable to allocate
the pages as GFP_KERNEL - prehaps to the extent where it's worth
distinguishing between GFP_KERNEL and GFP_ATOMIC failures. If GFP_KERNEL
allocations are failure, packet loss is likely and the system may not
recover, particularly if there is no swap configured.

> > 
> 
> > 
> > >  		if (!skb) {
> > > -			IWL_CRIT(priv, "Can not allocate SKB buffers\n");
> > > +			if (net_ratelimit())
> > > +				IWL_CRIT(priv, "Can not allocate SKB buffer.\n");
> > 
> > Similarly, could the message either be supressed when there is enough
> > buffers in the RX queue or differenciate between enough buffers and
> > things getting tight possibly causing packet loss?
> 
> Frans also had comments in this regard. Will do. 
> 
> > 
> > The IWL_CRIT() part even is a hint - there is no guarantee that the allocation
> > failure is really a critical problem.
> 
> Right.
> 
> How about this: 
> 
> >From bd2153dd9e4a0ad588adec38c580d67023d5587e Mon Sep 17 00:00:00 2001
> From: Reinette Chatre <reinette.chatre@intel.com>
> Date: Wed, 9 Sep 2009 15:41:00 -0700
> Subject: [PATCH] iwlwifi: reduce noise when skb allocation fails
> 
> Replenishment of receive buffers is done in the tasklet handling
> received frames as well as in a workqueue. When we are in the tasklet
> we cannot sleep and thus attempt atomic skb allocations. It is generally
> not a big problem if this fails since iwl_rx_allocate is always followed
> by a call to iwl_rx_queue_restock which will queue the work to replenish
> the buffers at a time when sleeping is allowed.
> 
> We thus add the __GFP_NOWARN to the skb allocation in iwl_rx_allocate to
> reduce the noise if such an allocation fails while we still have enough
> buffers. We do maintain the warning and the error message when we are low
> on buffers to communicate to the user that there is a potential problem with
> memory availability on system
> 
> This addresses issue reported upstream in thread "iwlagn: order 2 page
> allocation failures" in
> http://thread.gmane.org/gmane.linux.kernel.wireless.general/39187
> 
> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-rx.c       |   12 +++++++++---
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    8 +++++++-
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-rx.c b/drivers/net/wireless/iwlwifi/iwl-rx.c
> index b90adcb..cb50cc7 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-rx.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-rx.c
> @@ -250,12 +250,18 @@ void iwl_rx_allocate(struct iwl_priv *priv, gfp_t priority)
>  		}
>  		spin_unlock_irqrestore(&rxq->lock, flags);
>  
> +		if (rxq->free_count > RX_LOW_WATERMARK)
> +			priority |= __GFP_NOWARN;

Seems very reasonable.

>  		/* Alloc a new receive buffer */
> -		skb = alloc_skb(priv->hw_params.rx_buf_size + 256,
> -						priority);
> +		skb = alloc_skb(priv->hw_params.rx_buf_size + 256, priority);
>  

This change appears superflous. It don't change any functionality. Looks
like the style is just being made consistent with a similar code block
elsewhere.

>  		if (!skb) {
> -			IWL_CRIT(priv, "Can not allocate SKB buffers\n");
> +			if (net_ratelimit())
> +				IWL_DEBUG_INFO("Failed to allocate SKB buffer.\n");
> +			if ((rxq->free_count <= RX_LOW_WATERMARK) &&
> +			    net_ratelimit())
> +				IWL_CRIT(priv, "Failed to allocate SKB buffer. Only %u free buffers remaining\n",
> +					 rxq->free_count);


To get a good idea of how screwed we really are, how about?

				IWL_CRIT(priv, "Failed to allocate SKB buffer with %s. Only %u free buffers remaining\n",
					priority == GFP_ATOMIC ?  "GFP_ATOMIC" : "GFP_KERNEL",
					 rxq->free_count);

>  			/* We don't reschedule replenish work here -- we will
>  			 * call the restock method and if it still needs
>  			 * more buffers it will schedule replenish */
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index 0909668..0d96b17 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -1146,11 +1146,17 @@ static void iwl3945_rx_allocate(struct iwl_priv *priv, gfp_t priority)
>  		}
>  		spin_unlock_irqrestore(&rxq->lock, flags);
>  
> +		if (rxq->free_count > RX_LOW_WATERMARK)
> +			priority |= __GFP_NOWARN;
>  		/* Alloc a new receive buffer */
>  		skb = alloc_skb(priv->hw_params.rx_buf_size, priority);
>  		if (!skb) {
>  			if (net_ratelimit())
> -				IWL_CRIT(priv, ": Can not allocate SKB buffers\n");
> +				IWL_DEBUG_INFO("Failed to allocate SKB buffer.\n");
> +			if ((rxq->free_count <= RX_LOW_WATERMARK) &&
> +			    net_ratelimit())
> +				IWL_CRIT(priv, "Failed to allocate SKB buffer. Only %u free buffers remaining\n",
> +					 rxq->free_count);
>  			/* We don't reschedule replenish work here -- we will
>  			 * call the restock method and if it still needs
>  			 * more buffers it will schedule replenish */

Otherwise, it looks just the finest and I think it will address the
problem to some extent - in that it won't print alarming messages when
they are not needed.

The additional changes with respect to GFP_ATOMIC are optional. Whether
you do it or not.

Acked-by: Mel Gorman <mel@csn.ul.ie>

Thanks very much.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

^ permalink raw reply

* [PATCH] wireless: default CONFIG_WLAN to y
From: Luis R. Rodriguez @ 2009-09-11  8:43 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Luis R. Rodriguez

When this was added no defaults were set and it seems
this implies n. Default this to y.

Reported-by: Jouni Malinen <jouni.malinen@atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig
index ad89d23..49ea9c9 100644
--- a/drivers/net/wireless/Kconfig
+++ b/drivers/net/wireless/Kconfig
@@ -5,6 +5,7 @@
 menuconfig WLAN
 	bool "Wireless LAN"
 	depends on !S390
+	default y
 	---help---
 	  This section contains all the pre 802.11 and 802.11 wireless
 	  device drivers. For a complete list of drivers and documentation
-- 
1.6.3.3


^ permalink raw reply related

* Re: [PATCH] sdio: pass unknown cis tuples to sdio drivers
From: Albert Herranz @ 2009-09-11  8:28 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-mmc, bcm43xx-dev, linux-wireless
In-Reply-To: <20090911010102.4c7840b0.akpm@linux-foundation.org>

--- El vie, 11/9/09, Andrew Morton <akpm@linux-foundation.org> escribió:
> > > 
> > > ret == -EINVAL
> > > 
> > 
> > At this point ret is not -EINVAL.
> 
> Yes it is.  We just did
> 
>     ret = -EINVAL;
> 
> 
> If that assignment happens, we leak `this'.
> 

Hi Andrew,

I misunderstood you. I thought that you were trying to imply on your original comment that retval was _already_ -EINVAL at that point.

Now I see the issue. `this' should be freed if successfully parsed (!ret) or if invalid and not going to be passed to a SDIO driver (ret == -EINVAL).

Thanks for catching that. I'll send an updated patch.

Cheers,
Albert



      

^ permalink raw reply

* [PATCH] cfg80211: use cfg80211_wext_freq() for freq conversion
From: Holger Schurig @ 2009-09-11  8:13 UTC (permalink / raw)
  To: John W Linville, linux-wireless, Johannes Berg

WEXT's "struct iw_freq" can also be used to handle a channel. This patch now
uses cfg80211_wext_freq() instead of hand-converting the frequency. That
allows user-space to specify channels as well, like with SIOCSIWFREQ.

Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

Index: linux-wl/net/wireless/scan.c
===================================================================
--- linux-wl.orig/net/wireless/scan.c	2009-09-11 09:05:27.000000000 +0200
+++ linux-wl/net/wireless/scan.c	2009-09-11 09:05:39.000000000 +0200
@@ -662,7 +662,7 @@ int cfg80211_wext_siwscan(struct net_dev
 				int k;
 				int wiphy_freq = wiphy->bands[band]->channels[j].center_freq;
 				for (k = 0; k < wreq->num_channels; k++) {
-					int wext_freq = wreq->channel_list[k].m / 100000;
+					int wext_freq = cfg80211_wext_freq(wiphy, &wreq->channel_list[k]);
 					if (wext_freq == wiphy_freq)
 						goto wext_freq_found;
 				}


-- 
M&N Solutions GmbH          Ein Unternehmen der Datagroup AG
Holger Schurig
Raiffeisenstr. 10
61191 Rosbach
Tel: 06003/9141-15          Fax 06003/9141-49
http://www.mn-solutions.de/

Handelsregister Friedberg, HRB 5903
Geschäftsführer: P.Schrittenlocher

^ permalink raw reply

* Re: [PATCH] sdio: pass unknown cis tuples to sdio drivers
From: Albert Herranz @ 2009-09-11  8:21 UTC (permalink / raw)
  To: Pierre Ossman; +Cc: akpm, linux-mmc, bcm43xx-dev, linux-wireless
In-Reply-To: <20090911080659.2ac822fc@mjolnir.ossman.eu>

--- El vie, 11/9/09, Pierre Ossman <pierre@ossman.eu> escribió:
> The description for this patch should be made clearer. The title
> suggests it adds functionality that's already in place. It should be
> something along the lines of "Also pass malformed tuples to
> card drivers".

Hi Pierre,

Thanks for your patch review.

I didn't want to use "malformed" in the first place. I used "unknown" as "unknown to the SDIO core". The SDIO core in Linux only knows about FUNCE tuples of type 1 (with a sane length) as described in the SDIO Simplified Spec V2.00.

I think we just have a language issue here, but if you prefer the "malformed" wording I'm ok with that.

> In the sake of sanity, you might want to add this behaviour to all
> parsers, not just the FUNCE one.

I didn't find an application for the other parsers yet, so I tried to stick to the strictly necessary and just did the FUNCE one which has a direct application for Broadcom cards.

> 
> I'm also unclear on how this is supposed to work. What does the
> broadcom tuple look like? This patch looks like it will silence a lot
> of legitimate warnings, and possibly pollute the card structures with
> bogus data.

The contents of the Broadcom FUNCE (type 0x22) tuple that contains the MAC address of a card looks like the following (tuple size 8):

04 06 00 1d bc 62 79 fd
^^ ^^ ^^^^^^^^^^^^^^^^^
 |  |                 |
 |  |                 +--- MAC address
 |  +--------------------- length (6 bytes for a ethernet MAC address)
 +------------------------ type 4 (CISTPL_FUNCE_LAN_NODE_ID)

If you prefer it, instead of passing al "unknown" (or "malformed") FUNCE tuples to SDIO drivers, I can let through only a subset of whitelisted FUNCE types (starting with type 4).

> 
> > diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c
> > index 963f293..87934ac 100644
> > --- a/drivers/mmc/core/sdio_cis.c
> > +++ b/drivers/mmc/core/sdio_cis.c
> > @@ -123,8 +123,9 @@ static int cistpl_funce_func(struct sdio_func *func,
> >      vsn = func->card->cccr.sdio_vsn;
> >      min_size = (vsn == SDIO_SDIO_REV_1_00) ? 28 : 42;
> >  
> > +    /* let the SDIO driver take care of unknown tuples */
> >      if (size < min_size || buf[0] != 1)
> 
> Misleading comment, the tuple is not unknown.
> 

Same language issue described before.

> > -        return -EINVAL;
> > +        return -EILSEQ;
> >  
> 
> What does this change improve?

-EILSEQ is used to indicate that the tuple was not parsed by the SDIO core and should be passed to the SDIO driver via the SDIO func tuple list.

> 
> >      /* TPLFE_MAX_BLK_SIZE */
> >      func->max_blksize =
> buf[12] | (buf[13] << 8);
> > @@ -154,13 +155,7 @@ static int cistpl_funce(struct mmc_card *card, struct sdio_func *func,
> >      else
> >          ret = cistpl_funce_common(card, buf, size);
> >  
> > -    if (ret) {
> > -        printk(KERN_ERR "%s: bad CISTPL_FUNCE size %u "
> > -               "type %u\n", mmc_hostname(card->host), size, buf[0]);
> > -        return ret;
> > -    }
> > -
> > -    return 0;
> > +    return ret;
> >  }
> >  
> >  typedef int (tpl_parse_t)(struct mmc_card *, struct sdio_func *,
> 
> Silencing a legitimate error.
> 

Yes, I see your point.

I think we can keep this code but prevent displaying the error if ret == -EILSEQ (i.e. the tuple is "unknown"/"malformed" BUT should be passed to the SDIO driver for parsing).

> > +        if (ret == -EILSEQ) {
> > +       
>     /* this tuple is unknown to the core */
> 
> Misleading comment, the tuple might be known but malformed.

Same languange issue again.

> 
> Rgds
> -- 
>      -- Pierre Ossman

Thanks a lot for your comments,
Albert



      

^ permalink raw reply

* [PATCH] cfg80211: minimal error handling for wext-compat freq scanning
From: Holger Schurig @ 2009-09-11  8:13 UTC (permalink / raw)
  To: John W Linville, linux-wireless; +Cc: Johannes Berg

Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

Index: linux-wl/net/wireless/scan.c
===================================================================
--- linux-wl.orig/net/wireless/scan.c	2009-09-11 09:05:39.000000000 +0200
+++ linux-wl/net/wireless/scan.c	2009-09-11 09:05:42.000000000 +0200
@@ -680,6 +680,11 @@ int cfg80211_wext_siwscan(struct net_dev
 		err = -EINVAL;
 		goto out;
 	}
+	/* No channels found? */
+	if (!i) {
+		err = -EINVAL;
+		goto out;
+	}
 
 	/* Set real number of channels specified in creq->channels[] */
 	creq->n_channels = i;

-- 
http://www.holgerschurig.de

^ permalink raw reply

* Re: [PATCH] sdio: pass unknown cis tuples to sdio drivers
From: Andrew Morton @ 2009-09-11  8:01 UTC (permalink / raw)
  To: Albert Herranz; +Cc: linux-mmc, bcm43xx-dev, linux-wireless
In-Reply-To: <354996.83066.qm@web28302.mail.ukl.yahoo.com>

On Fri, 11 Sep 2009 07:52:11 +0000 (GMT) Albert Herranz <albert_herranz@yahoo.es> wrote:

> --- El vie, 11/9/09, Andrew Morton <akpm@linux-foundation.org> escribi__:
> > > Some manufacturers provide vendor information in
> > non-vendor specific CIS
> > > tuples. For example, Broadcom uses an Extended
> > Function tuple to provide
> > > the MAC address on some of their network cards, as in
> > the case of the
> > > Nintendo Wii WLAN daughter card.
> > > 
> > > This patch allows passing correct tuples unknown to
> > the SDIO core to
> > > a matching SDIO driver instead of rejecting them and
> > failing.
> > > 
> > 
> > This looks leaky to me.
> > 
> 
> Hi Andrew, thanks for the review.
> I hope I can clarify a bit what the patch does in the next comments.
> 
> > 
> > : ______ ______ if (i < ARRAY_SIZE(cis_tpl_list)) {

argh, now yahoo mail is converting tabs to 0xa0's as well.  Despair.

> > : ______ ______ ______ const struct cis_tpl *tpl = cis_tpl_list + i;
> > : ______ ______ ______ if (tpl_link < tpl->min_size) {
> > : ______ ______ ______ ______ printk(KERN_ERR
> > : ______ ______ ______ ______ __ __ ______"%s: bad CIS tuple 0x%02x"
> > : ______ ______ ______ ______ __ __ ______" (length = %u, expected >= %u)\n",
> > : ______ ______ ______         ______mmc_hostname(card->host),
> > : ______ ______ ______ ______ __ __ ______tpl_code, tpl_link, tpl->min_size);
> > : ______ ______ ______ ______ ret = -EINVAL;
> > 
> > ret == -EINVAL
> > 
> 
> At this point ret is not -EINVAL.

Yes it is.  We just did

	ret = -EINVAL;


If that assignment happens, we leak `this'.

^ permalink raw reply

* Re: [PATCH] cfg80211: allow scanning on specified frequencies when using wext-compatibility
From: Holger Schurig @ 2009-09-11  7:52 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John W Linville, linux-wireless
In-Reply-To: <1252629664.23427.4.camel@johannes.local>

> This is a bit weird -- this way you don't report errors if the
> user specified frequencies that don't exist.

The old code did this:

Loop over all bands
   Loop over all channels
      Stick channel to scan request

I simply added this:

Loop over all bands
   Loop over all channels
      If scan-request hasn't this channel freq: continue
      Stick channel to scan request


Now, if I want to report an -EINVAL for every possibly invalid 
scan-request channel, I'd have to do this:


If scan-request has freqs:
   Loop over all scan-request freqs
      Loop over all bands
         Loop over all channels
            search for freq
   if found:
       Stick channel to scan request
   else:
       err = -EINVAL
else:
   Loop over all bands
      Loop over all channels
         Stick channel to scan request

This is considerable code-bloat for such a seldom-used function.


I'd rather do it like this:


Loop over all bands
   Loop over all channels
      If scan-request hasn't this channel freq: continue
      Stick channel to scan request
if no channels:
   err = -EINVAL

That's a compromise :-)

-- 
http://www.holgerschurig.de

^ permalink raw reply

* Re: [PATCH] sdio: pass unknown cis tuples to sdio drivers
From: Albert Herranz @ 2009-09-11  7:52 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-mmc, bcm43xx-dev, linux-wireless
In-Reply-To: <20090910203951.041d5c17.akpm@linux-foundation.org>

--- El vie, 11/9/09, Andrew Morton <akpm@linux-foundation.org> escribió:
> > Some manufacturers provide vendor information in
> non-vendor specific CIS
> > tuples. For example, Broadcom uses an Extended
> Function tuple to provide
> > the MAC address on some of their network cards, as in
> the case of the
> > Nintendo Wii WLAN daughter card.
> > 
> > This patch allows passing correct tuples unknown to
> the SDIO core to
> > a matching SDIO driver instead of rejecting them and
> failing.
> > 
> 
> This looks leaky to me.
> 

Hi Andrew, thanks for the review.
I hope I can clarify a bit what the patch does in the next comments.

> 
> :         if (i < ARRAY_SIZE(cis_tpl_list)) {
> :             const struct cis_tpl *tpl = cis_tpl_list + i;
> :             if (tpl_link < tpl->min_size) {
> :                 printk(KERN_ERR
> :                        "%s: bad CIS tuple 0x%02x"
> :                        " (length = %u, expected >= %u)\n",
> :                        mmc_hostname(card->host),
> :                        tpl_code, tpl_link, tpl->min_size);
> :                 ret = -EINVAL;
> 
> ret == -EINVAL
> 

At this point ret is not -EINVAL. If it was -EINVAL the code would have had exit at this snipped before:

                if (ret) {
                        kfree(this);
                        break;
                }


> :             } else if (tpl->parse) {
> :                 ret = tpl->parse(card, func,
> :                                  this->data, tpl_link);
> :             }
> :             /* already successfully parsed, not needed anymore */
> :             if (!ret)
> :                 kfree(this);
> 
> `this' doesn't get freed
> 

Yes, that's the whole point of the patch. It must be freed _only_ if the SDIO core parsed it. If the SDIO core cannot parse it then it gets passed to the SDIO driver via the SDIO func struct tuple list (later).

> :         } else {
> :            /* unknown tuple */
> :            ret = -EILSEQ;
> :         }
> :
> :         if (ret == -EILSEQ) {
> 
> `this' doesn't get remembered.
> 

When ret is -EILSEQ `this' is linked to the SDIO func tuple list (later).

> :            /* this tuple is unknown to the core */
> :            this->next = NULL;
> :            this->code = tpl_code;
> :            this->size = tpl_link;
> :            *prev = this;
> :            prev = &this->next;
> :            pr_debug("%s: queuing CIS tuple 0x%02x length %u\n",
> :                     mmc_hostname(card->host), tpl_code, tpl_link);
> :            /* keep on analyzing tuples */
> :            ret = 0;
> :         }
> : 
> :         ptr += tpl_link;
> 
> `this' leaks.
> 

`this' doesn't leak. `this' has been linked to the SDIO func tuple list in:

*prev = this;


> :     } while (!ret);
> 
> Please check?
> 

Thanks a lot for you comments,
Albert



      

^ permalink raw reply

* Re: [PATCH] cfg80211: allow scanning on specified frequencies when using wext-compatibility
From: Holger Schurig @ 2009-09-11  7:35 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John W Linville, linux-wireless
In-Reply-To: <1252629664.23427.4.camel@johannes.local>

> This is a bit weird -- this way you don't report errors if the
> user specified frequencies that don't exist.

Couldn't / shouldn't this done in user-space, e.g. after checking
IWRANGE.


> Also, are you sure that it has to be a frequency as opposed to
> a channel number?

I'm not totally sure, because there's nowhere any documentation
about the exact meanings of fields in "struct iw_freq". My patch
for wpa_supplicant's driver_wext.c only put's a frequency there:

+        req.channel_list[i].m = params->freqs[i] * 100000;
+        req.channel_list[i].e = 1;
+        req.channel_list[i].i = i;
+        req.channel_list[i].flags = 0;

But now that you bring this into my mind, I should have looked
at another consumer of "struct iw_freq", e.g. I should use
cfg80211_wext_freq() like cfg80211_wext_siwfreq() ?!?

I'll prepare some patch.

-- 
http://www.holgerschurig.de

^ permalink raw reply

* Re: [PATCH 3/4] ath5k: define ath_common ops
From: Luis R. Rodriguez @ 2009-09-11  7:23 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Nick Kossifidis, devel, ath9k-devel, linux-wireless, Alan Cox,
	Linus Torvalds, Jeff Garzik
In-Reply-To: <4AA9F22C.3090007@gmail.com>

On Thu, Sep 10, 2009 at 11:46 PM, Jiri Slaby <jirislaby@gmail.com> wrote:
> On 09/11/2009 08:16 AM, Nick Kossifidis wrote:
>>>  static inline u32 ath5k_hw_reg_read(struct ath5k_hw *ah, u16 reg)
>>>  {
>>> -       return ioread32(ah->ah_iobase + reg);
>>> +       return ath5k_hw_common(ah)->ops->read(ah, reg);
>>>  }
>>>
>>> -/*
>>> - * Write to a register
>>> - */
>>>  static inline void ath5k_hw_reg_write(struct ath5k_hw *ah, u32 val, u16 reg)
>>>  {
>>> -       iowrite32(val, ah->ah_iobase + reg);
>>> +       ath5k_hw_common(ah)->ops->write(ah, reg, val);
> ...
>> Since each driver will use it's own reg read/write functions (ath5k hw
>> code still uses ath5k_hw_reg_read/write), why do we need to have
>> common reg read/write functions like that used in the driver code ?
>> This makes the code more complex that it needs to be and i don't see a
>> reason why we need it. I understand why we need a way to handle
>> read/write functions from common ath code but i don't see a reason to
>> use these functions on ath5k, we can just fill ath_ops struct so that
>> common code can use them and leave ath5k_hw_read/write untouched.
>
> I definitely agree with Nick here. Althought whole ath_ops will be hot
> cache after the first operation, there is no need to prolong hot paths
> by computing the op address and a call. Ok, read/write on PCI is pretty
> slow, but still...

That is the way I had it originally before submission, and I
completely agree its reasonable to not incur additional cost at the
expense of having two separate read/write paths, and perhaps we should
only incur the extra cost on routines shared between
ath9k/ath9k/ath9k_htc. But -- is there really is a measurable cost
penalty?

This is why I asked if someone can test and give measurable
differences over this. If there really isn't then that's not strong
point against it.

For example, long ago I had argued over the cost incurred over the
unnecessary branching on ioread()/iowrite() when you know you have
MMIO devices [1] -- the defense then, and IMHO reasonable now, was
that the benefits of allowing cleaner drivers through the new
interfaces outweigh the theoretical penalties imposed by them.

Granted you can argue these new interfaces between
ath5k/ath9k/ath9k_htc would make things a little more complex, but I
would expect sharing the code will help in the end. And if these
interfaces are not acceptable I'm completely open to better suggested
alternatives.

[1] http://lkml.indiana.edu/hypermail/linux/kernel/0709.2/1068.html

  Luis

^ permalink raw reply

* Re: [PATCH 3/4] ath5k: define ath_common ops
From: Jiri Slaby @ 2009-09-11  6:46 UTC (permalink / raw)
  To: Nick Kossifidis; +Cc: Luis R. Rodriguez, devel, ath9k-devel, linux-wireless
In-Reply-To: <40f31dec0909102316q7902098jbee7fd8d17c3f622@mail.gmail.com>

On 09/11/2009 08:16 AM, Nick Kossifidis wrote:
>>  static inline u32 ath5k_hw_reg_read(struct ath5k_hw *ah, u16 reg)
>>  {
>> -       return ioread32(ah->ah_iobase + reg);
>> +       return ath5k_hw_common(ah)->ops->read(ah, reg);
>>  }
>>
>> -/*
>> - * Write to a register
>> - */
>>  static inline void ath5k_hw_reg_write(struct ath5k_hw *ah, u32 val, u16 reg)
>>  {
>> -       iowrite32(val, ah->ah_iobase + reg);
>> +       ath5k_hw_common(ah)->ops->write(ah, reg, val);
...
> Since each driver will use it's own reg read/write functions (ath5k hw
> code still uses ath5k_hw_reg_read/write), why do we need to have
> common reg read/write functions like that used in the driver code ?
> This makes the code more complex that it needs to be and i don't see a
> reason why we need it. I understand why we need a way to handle
> read/write functions from common ath code but i don't see a reason to
> use these functions on ath5k, we can just fill ath_ops struct so that
> common code can use them and leave ath5k_hw_read/write untouched.

I definitely agree with Nick here. Althought whole ath_ops will be hot
cache after the first operation, there is no need to prolong hot paths
by computing the op address and a call. Ok, read/write on PCI is pretty
slow, but still...

^ permalink raw reply

* Re: [PATCH 06/10] ath5k: use common curbssid, bssidmask and macaddr
From: Nick Kossifidis @ 2009-09-11  6:20 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linville, linux-wireless, ath9k-devel, devel
In-Reply-To: <1252611027-5285-7-git-send-email-lrodriguez@atheros.com>

2009/9/10 Luis R. Rodriguez <lrodriguez@atheros.com>:
>
> -
> -/****************\
> -* BSSID handling *
> -\****************/
> -

No need to remove that unless all bssid related functions are gone.

-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

^ permalink raw reply

* Re: [b43] About supporting of BCM4312 [14e4:4315] with Low Power PHY
From: Gábor Stefanik @ 2009-09-11  6:18 UTC (permalink / raw)
  To: Bryan Wu; +Cc: mb, stefano.brivio, linux-wireless
In-Reply-To: <4AA9B46E.8020100@canonical.com>

This chip works (though not quite "supported", that is, can't
guarantee that it will work for you, and speed is not up to par with
wl_hybrid) in wireless-testing. It should also work in
compat-wireless, though compat-wireless is having problems with 64-bit
DMA lately (probably also affects the G-PHY 4311/02). Specifically,
the Dell 1397 (half-mini version of the 1395) and the HP 459263-002
are known to work.

On Fri, Sep 11, 2009 at 4:22 AM, Bryan Wu <bryan.wu@canonical.com> wrote:
> Dear Michael and Stefano,
>
> I have a project which integrate Broadcom Wifi chip. But the mainline b43 still does not support this chip, because it has Low Power PHY.
>
> Here is my lspci -vvnn output for this device:
> ------
> 07:00.0 Network controller [0280]: Broadcom Corporation BCM4312 802.11b/g [14e4:4315] (rev 01)
>        Subsystem: Dell Device [1028:000c]
>        Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>        Latency: 0, Cache Line Size: 32 bytes
>        Interrupt: pin A routed to IRQ 17
>        Region 0: Memory at f0100000 (64-bit, non-prefetchable) [size=16K]
>        Capabilities: [40] Power Management version 3
>                Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
>                Status: D0 PME-Enable- DSel=0 DScale=2 PME-
>        Capabilities: [58] Vendor Specific Information <?>
>        Capabilities: [e8] Message Signalled Interrupts: Mask- 64bit+ Queue=0/0 Enable-
>                Address: 0000000000000000  Data: 0000
>        Capabilities: [d0] Express (v1) Endpoint, MSI 00
>                DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <4us, L1 unlimited
>                        ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
>                DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
>                        RlxdOrd- ExtTag+ PhantFunc- AuxPwr- NoSnoop-
>                        MaxPayload 128 bytes, MaxReadReq 128 bytes
>                DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr- TransPend-
>                LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <4us, L1 <64us
>                        ClockPM+ Suprise- LLActRep- BwNot-
>                LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk+
>                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>                LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
>        Kernel driver in use: b43-pci-bridge
>        Kernel modules: ssb
> ------
>
> Do you guys know how to support this device in 2.6.31 kernel? Need I backport some code from wireless-testing? I enabled the PHY_LP config manually in 2.6.31 kernel and b43 driver recognized the hardware wifi device, but it still
> does not work at all.
>
> Or there is no choice but Broadcom's STA driver? I do not like such non-GPL stuff.
>
> Thanks a lot
> --
> Bryan Wu <bryan.wu@canonical.com>
> Kernel Developer    +86.138-1617-6545 Mobile
> Ubuntu Kernel Team | Hardware Enablement Team
> Canonical Ltd.      www.canonical.com
> Ubuntu - Linux for human beings | www.ubuntu.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

^ permalink raw reply

* Re: [PATCH 3/4] ath5k: define ath_common ops
From: Nick Kossifidis @ 2009-09-11  6:16 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linville, linux-wireless, ath9k-devel, devel
In-Reply-To: <1252632895-11914-4-git-send-email-lrodriguez@atheros.com>

2009/9/11 Luis R. Rodriguez <lrodriguez@atheros.com>:
> All read/write ops now go through the common ops.
>
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> ---
>  drivers/net/wireless/ath/ath5k/ath5k.h |   20 ++++++++++++--------
>  drivers/net/wireless/ath/ath5k/base.c  |   17 +++++++++++++++++
>  drivers/net/wireless/ath/ath5k/base.h  |   11 -----------
>  3 files changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
> index 29ce868..997101b 100644
> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
> @@ -1315,20 +1315,24 @@ static inline unsigned int ath5k_hw_clocktoh(unsigned int clock, bool turbo)
>        return turbo ? (clock / 80) : (clock / 40);
>  }
>
> -/*
> - * Read from a register
> - */
> +static inline struct ath_common *ath5k_hw_common(struct ath5k_hw *ah)
> +{
> +        return &ah->common;
> +}
> +
> +static inline struct ath_regulatory *ath5k_hw_regulatory(struct ath5k_hw *ah)
> +{
> +        return &(ath5k_hw_common(ah)->regulatory);
> +}
> +
>  static inline u32 ath5k_hw_reg_read(struct ath5k_hw *ah, u16 reg)
>  {
> -       return ioread32(ah->ah_iobase + reg);
> +       return ath5k_hw_common(ah)->ops->read(ah, reg);
>  }
>
> -/*
> - * Write to a register
> - */
>  static inline void ath5k_hw_reg_write(struct ath5k_hw *ah, u32 val, u16 reg)
>  {
> -       iowrite32(val, ah->ah_iobase + reg);
> +       ath5k_hw_common(ah)->ops->write(ah, reg, val);
>  }
>
>  #if defined(_ATH5K_RESET) || defined(_ATH5K_PHY)
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> index 3cb0752..535ea72 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -437,6 +437,22 @@ ath5k_chip_name(enum ath5k_srev_type type, u_int16_t val)
>
>        return name;
>  }
> +static unsigned int ath5k_ioread32(void *hw_priv, u32 reg_offset)
> +{
> +       struct ath5k_hw *ah = (struct ath5k_hw *) hw_priv;
> +       return ioread32(ah->ah_iobase + reg_offset);
> +}
> +
> +static void ath5k_iowrite32(void *hw_priv, u32 reg_offset, u32 val)
> +{
> +       struct ath5k_hw *ah = (struct ath5k_hw *) hw_priv;
> +       iowrite32(val, ah->ah_iobase + reg_offset);
> +}
> +
> +static struct ath_ops ath5k_common_ops = {
> +       .read = ath5k_ioread32,
> +       .write = ath5k_iowrite32,
> +};
>
>  static int __devinit
>  ath5k_pci_probe(struct pci_dev *pdev,
> @@ -576,6 +592,7 @@ ath5k_pci_probe(struct pci_dev *pdev,
>        sc->ah->ah_sc = sc;
>        sc->ah->ah_iobase = sc->iobase;
>        common = ath5k_hw_common(sc->ah);
> +       common->ops = &ath5k_common_ops;
>        common->cachelsz = csz << 2; /* convert to bytes */
>
>        /* Initialize device */
> diff --git a/drivers/net/wireless/ath/ath5k/base.h b/drivers/net/wireless/ath/ath5k/base.h
> index 005d25f..b14ba07 100644
> --- a/drivers/net/wireless/ath/ath5k/base.h
> +++ b/drivers/net/wireless/ath/ath5k/base.h
> @@ -201,15 +201,4 @@ struct ath5k_softc {
>  #define ath5k_hw_hasveol(_ah) \
>        (ath5k_hw_get_capability(_ah, AR5K_CAP_VEOL, 0, NULL) == 0)
>
> -static inline struct ath_common *ath5k_hw_common(struct ath5k_hw *ah)
> -{
> -       return &ah->common;
> -}
> -
> -static inline struct ath_regulatory *ath5k_hw_regulatory(struct ath5k_hw *ah)
> -{
> -       return &(ath5k_hw_common(ah)->regulatory);
> -
> -}
> -
>  #endif

Since each driver will use it's own reg read/write functions (ath5k hw
code still uses ath5k_hw_reg_read/write), why do we need to have
common reg read/write functions like that used in the driver code ?
This makes the code more complex that it needs to be and i don't see a
reason why we need it. I understand why we need a way to handle
read/write functions from common ath code but i don't see a reason to
use these functions on ath5k, we can just fill ath_ops struct so that
common code can use them and leave ath5k_hw_read/write untouched.

-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

^ permalink raw reply

* Re: [PATCH] sdio: pass unknown cis tuples to sdio drivers
From: Pierre Ossman @ 2009-09-11  6:06 UTC (permalink / raw)
  To: Albert Herranz
  Cc: akpm, linux-mmc, bcm43xx-dev, linux-wireless, Albert Herranz
In-Reply-To: <1252587402-7382-2-git-send-email-albert_herranz@yahoo.es>

[-- Attachment #1: Type: text/plain, Size: 2616 bytes --]

On Thu, 10 Sep 2009 14:56:42 +0200
Albert Herranz <albert_herranz@yahoo.es> wrote:

> Some manufacturers provide vendor information in non-vendor specific CIS
> tuples. For example, Broadcom uses an Extended Function tuple to provide
> the MAC address on some of their network cards, as in the case of the
> Nintendo Wii WLAN daughter card.
> 
> This patch allows passing correct tuples unknown to the SDIO core to
> a matching SDIO driver instead of rejecting them and failing.
> 
> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
> ---

The description for this patch should be made clearer. The title
suggests it adds functionality that's already in place. It should be
something along the lines of "Also pass malformed tuples to card
drivers".

In the sake of sanity, you might want to add this behaviour to all
parsers, not just the FUNCE one.

I'm also unclear on how this is supposed to work. What does the
broadcom tuple look like? This patch looks like it will silence a lot
of legitimate warnings, and possibly pollute the card structures with
bogus data.

> diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c
> index 963f293..87934ac 100644
> --- a/drivers/mmc/core/sdio_cis.c
> +++ b/drivers/mmc/core/sdio_cis.c
> @@ -123,8 +123,9 @@ static int cistpl_funce_func(struct sdio_func *func,
>  	vsn = func->card->cccr.sdio_vsn;
>  	min_size = (vsn == SDIO_SDIO_REV_1_00) ? 28 : 42;
>  
> +	/* let the SDIO driver take care of unknown tuples */
>  	if (size < min_size || buf[0] != 1)

Misleading comment, the tuple is not unknown.

> -		return -EINVAL;
> +		return -EILSEQ;
>  

What does this change improve?

>  	/* TPLFE_MAX_BLK_SIZE */
>  	func->max_blksize = buf[12] | (buf[13] << 8);
> @@ -154,13 +155,7 @@ static int cistpl_funce(struct mmc_card *card, struct sdio_func *func,
>  	else
>  		ret = cistpl_funce_common(card, buf, size);
>  
> -	if (ret) {
> -		printk(KERN_ERR "%s: bad CISTPL_FUNCE size %u "
> -		       "type %u\n", mmc_hostname(card->host), size, buf[0]);
> -		return ret;
> -	}
> -
> -	return 0;
> +	return ret;
>  }
>  
>  typedef int (tpl_parse_t)(struct mmc_card *, struct sdio_func *,

Silencing a legitimate error.

> +		if (ret == -EILSEQ) {
> +			/* this tuple is unknown to the core */

Misleading comment, the tuple might be known but malformed.

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH] sdio: recognize io card without powercycle
From: Pierre Ossman @ 2009-09-11  5:58 UTC (permalink / raw)
  To: Albert Herranz
  Cc: akpm, linux-mmc, bcm43xx-dev, linux-wireless, Albert Herranz
In-Reply-To: <1252587402-7382-1-git-send-email-albert_herranz@yahoo.es>

[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]

On Thu, 10 Sep 2009 14:56:41 +0200
Albert Herranz <albert_herranz@yahoo.es> wrote:

> SDIO Simplified Specification V2.00 states that it is strongly recommended
> that the host executes either a power reset or issues a CMD52 (I/O Reset) to
> re-initialize an I/O only card or the I/O portion of a combo card.
> Additionally, the CMD52 must be issued first because it cannot be issued
> after a CMD0.
> 
> With this patch the Nintendo Wii SDIO-based WLAN card is detected after a
> system reset, without requiring a complete system powercycle.
> 
> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
> ---

I had problems with cards already in the idle state crashing when I
sent them a reset. I suggest a big round of testing before merging this.

> @@ -182,3 +189,20 @@ int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
>  	return 0;
>  }
>  
> +int sdio_go_idle(struct mmc_host *host)
> +{

It might be clearer if you use the terminology from the spec, so either
sdio_abort() or sdio_reset().

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] input: Add KEY_RFKILL
From: Dmitry Torokhov @ 2009-09-11  5:13 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-input, linux-wireless, marcel
In-Reply-To: <1252603292-20830-1-git-send-email-mjg@redhat.com>

On Thu, Sep 10, 2009 at 06:21:31PM +0100, Matthew Garrett wrote:
> Most laptops have keys that are intended to toggle all device state, not
> just wifi. These are currently generally mapped to KEY_WLAN. As a result,
> rfkill will only kill or enable wifi in response to the key press. This
> confuses users and can make it difficult for them to enable bluetooth
> and wwan devices.
> 
> This patch adds a new keycode, KEY_RFKILL_ALL. It indicates that the

The new name in description does not match the code...

> system should toggle the state of all rfkillable devices.

Hmm, rfkill changes usually go through net tree so unless they want me
to push it though I'll just ACK the new keycode.

> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>

Acked-by: Dmitry Torokhov <dtor@mail.ru>

> ---
>  include/linux/input.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 8b3bc3e..20a622e 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -595,6 +595,8 @@ struct input_absinfo {
>  #define KEY_NUMERIC_STAR	0x20a
>  #define KEY_NUMERIC_POUND	0x20b
>  
> +#define KEY_RFKILL		0x20c /* Key that controls all radios */
> +
>  /* We avoid low common keys in module aliases so they don't get huge. */
>  #define KEY_MIN_INTERESTING	KEY_MUTE
>  #define KEY_MAX			0x2ff

-- 
Dmitry

^ permalink raw reply

* Re: Differences between wireless-testing and mainline
From: Pavel Roskin @ 2009-09-11  4:37 UTC (permalink / raw)
  To: John Ranson; +Cc: linux-wireless
In-Reply-To: <bcd1ba70909101959u4581537ci684a2c45bc17c965@mail.gmail.com>

Quoting John Ranson <ranson@analager.net>:

> Is there a good way to keep track of the differences between the
> mainline kernel and wireless testing?  I'm running wireless testing to
> minimize firmware errors with my iwl4965 card. When mainline
> incorporates the code that I'm running, I would like to switch back.

git whatchanged wireless-testing/base..wireless-testing/master

wireless-testing/base is usually the latest release or release  
candidate from the mainline.

If you want a better granularity, e.g. you are ready to switch to the  
mainline before rc1, you'll need to track the mainline and look for  
the patches you care about.

You can also pick up the patches you need from wireless-testing and  
apply them to the mainline repository.  If you do it with STGit, it  
will tell you that the patches have been merged by telling you that  
the local patches have become empty after the update.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: [PATCH] sdio: pass unknown cis tuples to sdio drivers
From: Andrew Morton @ 2009-09-11  3:39 UTC (permalink / raw)
  To: Albert Herranz; +Cc: linux-mmc, bcm43xx-dev, linux-wireless
In-Reply-To: <1252587402-7382-2-git-send-email-albert_herranz@yahoo.es>

On Thu, 10 Sep 2009 14:56:42 +0200 Albert Herranz <albert_herranz@yahoo.es> wrote:

> Some manufacturers provide vendor information in non-vendor specific CIS
> tuples. For example, Broadcom uses an Extended Function tuple to provide
> the MAC address on some of their network cards, as in the case of the
> Nintendo Wii WLAN daughter card.
> 
> This patch allows passing correct tuples unknown to the SDIO core to
> a matching SDIO driver instead of rejecting them and failing.
> 

This looks leaky to me.


: 		if (i < ARRAY_SIZE(cis_tpl_list)) {
: 			const struct cis_tpl *tpl = cis_tpl_list + i;
: 			if (tpl_link < tpl->min_size) {
: 				printk(KERN_ERR
: 				       "%s: bad CIS tuple 0x%02x"
: 				       " (length = %u, expected >= %u)\n",
: 				       mmc_hostname(card->host),
: 				       tpl_code, tpl_link, tpl->min_size);
: 				ret = -EINVAL;

ret == -EINVAL

: 			} else if (tpl->parse) {
: 				ret = tpl->parse(card, func,
: 						 this->data, tpl_link);
: 			}
: 			/* already successfully parsed, not needed anymore */
: 			if (!ret)
: 				kfree(this);

`this' doesn't get freed

: 		} else {
: 			/* unknown tuple */
: 			ret = -EILSEQ;
: 		}
: 
: 		if (ret == -EILSEQ) {

`this' doesn't get remembered.

: 			/* this tuple is unknown to the core */
: 			this->next = NULL;
: 			this->code = tpl_code;
: 			this->size = tpl_link;
: 			*prev = this;
: 			prev = &this->next;
: 			pr_debug("%s: queuing CIS tuple 0x%02x length %u\n",
: 				 mmc_hostname(card->host), tpl_code, tpl_link);
: 			/* keep on analyzing tuples */
: 			ret = 0;
: 		}
: 
: 		ptr += tpl_link;

`this' leaks.

: 	} while (!ret);

Please check?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox