Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [RFC] usb: Check results of dma_map_single
From: Paulius Zaleckas @ 2009-11-04 12:50 UTC (permalink / raw)
  To: Larry Finger; +Cc: Greg Kroah-Hartman, linux-wireless, linux-usb
In-Reply-To: <4af115c3.JUDAYfcydqPYCYyH%Larry.Finger@lwfinger.net>

On 11/04/2009 07:48 AM, Larry Finger wrote:
> At http://marc.info/?l=linux-wireless&m=125695331205062&w=2, a problem
> with DMA buffer processing was corrected for the libertas driver. Because
> routine usb_fill_bulk_urb() does not check that DMA is possible when
> dma_map_single() is called, this condition was not detected until the buffer
> was unmapped. By this time memory corruption had occurred.
>
> The situation is fixed by testing the returned DMA address. If not a legal
> address, a WARN_ON(1) is executed to provide traceback and the error is
> returned.
>
> Signed-off-by: Larry Finger<Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
> ---
>
> Index: linux-2.6/drivers/usb/core/hcd.c
> ===================================================================
> --- linux-2.6.orig/drivers/usb/core/hcd.c
> +++ linux-2.6/drivers/usb/core/hcd.c
> @@ -1281,6 +1281,13 @@ static int map_urb_for_dma(struct usb_hc
>   					urb->setup_packet,
>   					sizeof(struct usb_ctrlrequest),
>   					DMA_TO_DEVICE);
> +			ret = dma_mapping_error(hcd->self.controller,
> +						urb->setup_dma);
> +			/* warn if DMA mapping failed */
> +			if (ret) {
> +				WARN_ON(1);
> +				return ret;
> +			}

First of all you forgot to add { } around everything under if statement.

I don't think WARN_ON is needed...
dma_mapping_error under most architectures return 0 or 1 so it would be
better to make some real error value. EAGAIN seems to be proper error,
since documentation says that driver should try again later.

I would write this error handler like this:

if (dma_mapping_error(hcd->self.controller, urb->setup_dma))
	ret = -EAGAIN;

>   		else if (hcd->driver->flags&  HCD_LOCAL_MEM)
>   			ret = hcd_alloc_coherent(
>   					urb->dev->bus, mem_flags,
> @@ -1299,6 +1306,13 @@ static int map_urb_for_dma(struct usb_hc
>   					urb->transfer_buffer,
>   					urb->transfer_buffer_length,
>   					dir);
> +			ret = dma_mapping_error(hcd->self.controller,
> +						urb->transfer_dma);
> +			/* warn if DMA mapping failed */
> +			if (ret) {
> +				WARN_ON(1);
> +				return ret;
> +			}

ditto

>   		else if (hcd->driver->flags&  HCD_LOCAL_MEM) {
>   			ret = hcd_alloc_coherent(
>   					urb->dev->bus, mem_flags,

^ permalink raw reply

* Re: pull request: wireless-next-2.6 2009-10-28
From: Jiri Kosina @ 2009-11-04 13:03 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Luis Correia, John W. Linville, Ingo Molnar, Johannes Berg,
	Jarek Poplawski, Bartlomiej Zolnierkiewicz, Pekka Enberg,
	David Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <20091102211629.GA1322@ucw.cz>

On Mon, 2 Nov 2009, Pavel Machek wrote:

> > I've searched on my GMail archives and the only patch Bart has
> > provided so far for the rt2x00 project is this:
> > 
> > [PATCH] MAINTAINERS: rt2x00 list is moderated
> > 
> > Which, while technically correct, adds nothing to the project.
> > 
> > So, I will personally continue to ignore Bart's comments, regards and
> > rants, until he provides patches for rt2x00 that actually make the
> > driver better.
> 
> Well, he provided review feedback... he should be thanked for that,
> not flamed for it.

He has actually written real patches (and quite non-trivial both in amount 
and in functionality), see

	http://lkml.org/lkml/2009/11/3/372

But yes, he got flamed for this for some odd reason. I got the impression 
that the community around rt2x00 doesn't like (or understand) the way how 
opensource development happens.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.


^ permalink raw reply

* Re: Marvell 88w8385 Support
From: tsr @ 2009-11-04 12:40 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-wireless, mangoo
In-Reply-To: <1231959472.14909.73.camel@localhost.localdomain>

Hi,

So, once again, I try to get my FSG-3 up and running with Debian and I
wonder how thing went with this.

>From what I can tell there was a patch (referenced here:
http://marc.info/?l=linux-wireless&m=123782768023390&w=2) to fix this
but I also get indications that this is not the case since there is both
a USB and miniPCI version of the 88w8385.


You write something
(http://marc.info/?l=linux-wireless&m=124334594302472&w=2) that is
cryptic to me.

Also searching the kernel gittree I found
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/net/wireless/libertas/if_spi.c;h=06a46d7b3d6c2460aecd793d2827033c7c0348f6;hb=cadeba315cc91ae1b57632e61b0cec3a4ed7088d
which seems to include a driver for the miniPCI libertas 88w8385.

Could you please enlighten me?

Also CC:ing Tomasz Chmielewski since he expressed interest in this too.

Hopefully,
Tomas

Dan Williams escribió:
> On Wed, 2009-01-14 at 19:35 +0100, tsr wrote:
>> John Daiker wrote:
>>> On 01/14/2009 08:47 AM, Dan Williams wrote:
>>>> On Tue, 2009-01-13 at 09:52 -0500, John W. Linville wrote:
>>>>   
>>>>> On Tue, Jan 13, 2009 at 01:09:12PM +0100, tsr wrote:
>>>>>     
>>>>>> Hi,
>>>>>>
>>>>>> I have a FSG-3 that includes a Marvell 88w8385 card. I wonder if there
>>>>>> is support for it in recent kernels. According to
>>>>>> http://marc.info/?l=linux-wireless&m=123178916308198&w=2 (search for
>>>>>> 8385) there seems to be support but I don't want to start messing 
>>>>>> around
>>>>>> with kernels and stuff without being sure.
>>>>>>        
>>>>> That driver is for SDIO hardware.  It looks like your FSG-3 has
>>>>> mini-PCI?  Either way, we don't have a driver merged yet that would
>>>>> cover your hardware.
>>>>>      
>>>> Looks like it's a CB35, which is actually an 88w8335 MiniPCI card.
>>>> Would be supported by the mwl8k driver ideally.  I've started on trying
>>>> to add support to mwl8k for the 8335 parts, but we'll need to split the
>>>> driver so that we can create IO hooks for the different parts.  It'll be
>>>> a bit, but we'll get there.
>>>>
>>>> Dan
>>>>    
>>> Very interested in the progress of 8335 support at some point.  My 
>>> Netgear wg311v3 is sitting on my desk... waiting for some Linux bits to 
>>> get it running!
>>>
>>> I can assist in testing whenever bits become available ( officially or 
>>> unofficially, hint, hint! :-P )
>>>
>>> Thanks for the hard work, and keep them bits a comin'
>>>
>>> JD
>>>
>> Thanks for the info, I've found something that is said to be a working 
>> drivers for 'my' card in this package: 
>> http://www.seagate.com/staticfiles/maxtor/en_us/downloads/MSSII_3.1.2.src.tgz
>>
>> The kernel source says it's some kind of 2.6 kernel, grepping for it I 
>> found the highest number to be 2.6.14 so maybe it is possible to use 
>> something from it.
>>
>> AFAIK it's also GPL.
> 
> Wow, that is the right driver...  haven't seen a driver for the 8335
> before, but there it is, and it's GPL too.  Got the firmware too, which
> I also haven't seen separately.
> 
> The path forward with this is to port the support for the hardware over
> to mwl8k since the firmware interface is almost the same.
> 
> That said, the only reason I chose that as the path forward was that we
> couldn't make any progress with the standalone driver, and figured that
> Marvell's driver would actually work where mrv8k didn't.  I still think
> it makes sense to use the same driver core for both but if that turns
> out to be too hard, we can just do a mac80211 port for this.
> 
> Dan
> 
> 


^ permalink raw reply

* [ANN] wireless-regdb: master-2009-11-04
From: wireless @ 2009-11-04 13:39 UTC (permalink / raw)
  To: linux-wireless


A new release of wireless-regdb (master-2009-11-04) is available at:

http://wireless.kernel.org/download/wireless-regdb/wireless-regdb-2009.11.04.tar.bz2

SHA1 sum: c490f2ae0e8262057825a613699016eb040367cd


For your convenience the individual regulatory.bin file can be downloaded from:

http://wireless.kernel.org/download/wireless-regdb/regulatory.bins/2009.11.04-regulatory.bin

SHA1 sum a22829e58aae59841519a45ceb0cc3cd73679062

Here is the short log of the changes included in this 
release:

John W. Linville (1):
      wireless-regdb: _correctly_ update regulatory.bin based on preceding changes.


^ permalink raw reply

* [PATCH] mac80211: make ieee80211_find_sta per virtual interface
From: Johannes Berg @ 2009-11-04 13:42 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless, Jouni Malinen

Since we have a TODO item to make all station
management dependent on virtual interfaces, I
figured I'd start with pushing such a change
to drivers before more drivers start using the
ieee80211_find_sta() API with a hw pointer and
cause us grief later on.

For now continue exporting the old API in form
of ieee80211_find_sta_by_hw(), but discourage
its use strongly.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/wireless/ath/ath9k/recv.c      |    3 ++-
 drivers/net/wireless/ath/ath9k/xmit.c      |    3 ++-
 drivers/net/wireless/iwlwifi/iwl-3945-rs.c |    3 ++-
 drivers/net/wireless/iwlwifi/iwl-core.c    |    2 +-
 drivers/net/wireless/iwlwifi/iwl-sta.c     |    2 +-
 include/net/mac80211.h                     |   25 +++++++++++++++++++++++--
 net/mac80211/sta_info.c                    |   18 ++++++++++++++++--
 7 files changed, 47 insertions(+), 9 deletions(-)

--- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-3945-rs.c	2009-11-04 14:14:46.000000000 +0100
+++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-3945-rs.c	2009-11-04 14:15:25.000000000 +0100
@@ -913,7 +913,8 @@ void iwl3945_rate_scale_init(struct ieee
 
 	rcu_read_lock();
 
-	sta = ieee80211_find_sta(hw, priv->stations[sta_id].sta.sta.addr);
+	sta = ieee80211_find_sta(priv->vif,
+				 priv->stations[sta_id].sta.sta.addr);
 	if (!sta) {
 		rcu_read_unlock();
 		return;
--- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-core.c	2009-11-04 14:14:49.000000000 +0100
+++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-core.c	2009-11-04 14:15:25.000000000 +0100
@@ -2301,7 +2301,7 @@ static void iwl_ht_conf(struct iwl_priv 
 	switch (priv->iw_mode) {
 	case NL80211_IFTYPE_STATION:
 		rcu_read_lock();
-		sta = ieee80211_find_sta(priv->hw, priv->bssid);
+		sta = ieee80211_find_sta(priv->vif, priv->bssid);
 		if (sta) {
 			struct ieee80211_sta_ht_cap *ht_cap = &sta->ht_cap;
 			int maxstreams;
--- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-sta.c	2009-11-04 14:14:47.000000000 +0100
+++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-sta.c	2009-11-04 14:15:25.000000000 +0100
@@ -1017,7 +1017,7 @@ int iwl_rxon_add_station(struct iwl_priv
 	 */
 	if (priv->current_ht_config.is_ht) {
 		rcu_read_lock();
-		sta = ieee80211_find_sta(priv->hw, addr);
+		sta = ieee80211_find_sta(priv->vif, addr);
 		if (sta) {
 			memcpy(&ht_config, &sta->ht_cap, sizeof(ht_config));
 			cur_ht_config = &ht_config;
--- wireless-testing.orig/include/net/mac80211.h	2009-11-04 14:14:50.000000000 +0100
+++ wireless-testing/include/net/mac80211.h	2009-11-04 14:15:25.000000000 +0100
@@ -2106,16 +2106,37 @@ void ieee80211_stop_tx_ba_cb_irqsafe(str
 /**
  * ieee80211_find_sta - find a station
  *
- * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @vif: virtual interface to look for station on
  * @addr: station's address
  *
  * This function must be called under RCU lock and the
  * resulting pointer is only valid under RCU lock as well.
  */
-struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_hw *hw,
+struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif,
 					 const u8 *addr);
 
 /**
+ * ieee80211_find_sta_by_hw - find a station on hardware
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @addr: station's address
+ *
+ * This function must be called under RCU lock and the
+ * resulting pointer is only valid under RCU lock as well.
+ *
+ * NOTE: This function should not be used! When mac80211 is converted
+ *	 internally to properly keep track of stations on multiple
+ *	 virtual interfaces, it will not always know which station to
+ *	 return here since a single address might be used by multiple
+ *	 logical stations (e.g. consider a station connecting to another
+ *	 BSSID on the same AP hardware without disconnecting first).
+ *
+ * DO NOT USE THIS FUNCTION.
+ */
+struct ieee80211_sta *ieee80211_find_sta_by_hw(struct ieee80211_hw *hw,
+					       const u8 *addr);
+
+/**
  * ieee80211_beacon_loss - inform hardware does not receive beacons
  *
  * @vif: &struct ieee80211_vif pointer from &struct ieee80211_if_init_conf.
--- wireless-testing.orig/net/mac80211/sta_info.c	2009-11-04 14:14:50.000000000 +0100
+++ wireless-testing/net/mac80211/sta_info.c	2009-11-04 14:33:56.000000000 +0100
@@ -801,8 +801,8 @@ void ieee80211_sta_expire(struct ieee802
 		sta_info_destroy(sta);
 }
 
-struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_hw *hw,
-					 const u8 *addr)
+struct ieee80211_sta *ieee80211_find_sta_by_hw(struct ieee80211_hw *hw,
+					       const u8 *addr)
 {
 	struct sta_info *sta = sta_info_get(hw_to_local(hw), addr);
 
@@ -810,4 +810,18 @@ struct ieee80211_sta *ieee80211_find_sta
 		return NULL;
 	return &sta->sta;
 }
+EXPORT_SYMBOL_GPL(ieee80211_find_sta_by_hw);
+
+struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif,
+					 const u8 *addr)
+{
+	struct ieee80211_sub_if_data *sdata;
+
+	if (!vif)
+		return NULL;
+
+	sdata = vif_to_sdata(vif);
+
+	return ieee80211_find_sta_by_hw(&sdata->local->hw, addr);
+}
 EXPORT_SYMBOL(ieee80211_find_sta);
--- wireless-testing.orig/drivers/net/wireless/ath/ath9k/recv.c	2009-11-04 14:14:46.000000000 +0100
+++ wireless-testing/drivers/net/wireless/ath/ath9k/recv.c	2009-11-04 14:16:40.000000000 +0100
@@ -202,7 +202,8 @@ static int ath_rx_prepare(struct sk_buff
 	}
 
 	rcu_read_lock();
-	sta = ieee80211_find_sta(sc->hw, hdr->addr2);
+	/* XXX: use ieee80211_find_sta! */
+	sta = ieee80211_find_sta_by_hw(sc->hw, hdr->addr2);
 	if (sta) {
 		an = (struct ath_node *) sta->drv_priv;
 		if (ds->ds_rxstat.rs_rssi != ATH9K_RSSI_BAD &&
--- wireless-testing.orig/drivers/net/wireless/ath/ath9k/xmit.c	2009-11-04 14:14:46.000000000 +0100
+++ wireless-testing/drivers/net/wireless/ath/ath9k/xmit.c	2009-11-04 14:16:40.000000000 +0100
@@ -282,7 +282,8 @@ static void ath_tx_complete_aggr(struct 
 
 	rcu_read_lock();
 
-	sta = ieee80211_find_sta(sc->hw, hdr->addr1);
+	/* XXX: use ieee80211_find_sta! */
+	sta = ieee80211_find_sta_by_hw(sc->hw, hdr->addr1);
 	if (!sta) {
 		rcu_read_unlock();
 		return;



^ permalink raw reply

* Re: [RFC] usb: Check results of dma_map_single
From: Larry Finger @ 2009-11-04 14:06 UTC (permalink / raw)
  To: Paulius Zaleckas; +Cc: Greg Kroah-Hartman, linux-wireless, linux-usb
In-Reply-To: <4AF17884.5000200@gmail.com>

Paulius Zaleckas wrote:
> On 11/04/2009 07:48 AM, Larry Finger wrote:
>> At http://marc.info/?l=linux-wireless&m=125695331205062&w=2, a problem
>> with DMA buffer processing was corrected for the libertas driver. Because
>> routine usb_fill_bulk_urb() does not check that DMA is possible when
>> dma_map_single() is called, this condition was not detected until the
>> buffer
>> was unmapped. By this time memory corruption had occurred.
>>
>> The situation is fixed by testing the returned DMA address. If not a
>> legal
>> address, a WARN_ON(1) is executed to provide traceback and the error is
>> returned.
>>
>> Signed-off-by: Larry
>> Finger<Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
>> ---
>>
>> Index: linux-2.6/drivers/usb/core/hcd.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/usb/core/hcd.c
>> +++ linux-2.6/drivers/usb/core/hcd.c
>> @@ -1281,6 +1281,13 @@ static int map_urb_for_dma(struct usb_hc
>>                       urb->setup_packet,
>>                       sizeof(struct usb_ctrlrequest),
>>                       DMA_TO_DEVICE);
>> +            ret = dma_mapping_error(hcd->self.controller,
>> +                        urb->setup_dma);
>> +            /* warn if DMA mapping failed */
>> +            if (ret) {
>> +                WARN_ON(1);
>> +                return ret;
>> +            }
> 
> First of all you forgot to add { } around everything under if statement.
> 
> I don't think WARN_ON is needed...
> dma_mapping_error under most architectures return 0 or 1 so it would be
> better to make some real error value. EAGAIN seems to be proper error,
> since documentation says that driver should try again later.
> 
> I would write this error handler like this:
> 
> if (dma_mapping_error(hcd->self.controller, urb->setup_dma))
>     ret = -EAGAIN;
> 
>>           else if (hcd->driver->flags&  HCD_LOCAL_MEM)
>>               ret = hcd_alloc_coherent(
>>                       urb->dev->bus, mem_flags,
>> @@ -1299,6 +1306,13 @@ static int map_urb_for_dma(struct usb_hc
>>                       urb->transfer_buffer,
>>                       urb->transfer_buffer_length,
>>                       dir);
>> +            ret = dma_mapping_error(hcd->self.controller,
>> +                        urb->transfer_dma);
>> +            /* warn if DMA mapping failed */
>> +            if (ret) {
>> +                WARN_ON(1);
>> +                return ret;
>> +            }
> 
> ditto
> 
>>           else if (hcd->driver->flags&  HCD_LOCAL_MEM) {
>>               ret = hcd_alloc_coherent(
>>                       urb->dev->bus, mem_flags,
> 

Thank you for your review and comments. I'll wait a bit to see what
other comments are offered, but I have implemented all your changes.

Larry

^ permalink raw reply

* Re: Marvell 88w8385 Support
From: Holger Schurig @ 2009-11-04 15:29 UTC (permalink / raw)
  To: tsr; +Cc: Dan Williams, linux-wireless, mangoo
In-Reply-To: <4AF1763C.4010901@tsr.se>

> From what I can tell there was a patch (referenced here:
> http://marc.info/?l=linux-wireless&m=123782768023390&w=2) to fix this
> but I also get indications that this is not the case since there is both
> a USB and miniPCI version of the 88w8385.

This patch is only for the CF/PCMCIA versions of this those chips.

There is no if_pci.c yet inside drivers/net/wireless/libertas/, if you
are certain you've a PCI card you're welcome to write one, should be
pretty easy. You can get help by looking at the other if_*.c files for
other physical interfaces.


> You write something
> (http://marc.info/?l=linux-wireless&m=124334594302472&w=2) that is
> cryptic to me.

Can you elaborate?  What exactly here is cryptic?

CF/SDIO/GSPO/USB ?
8015/8385 ?


> Also searching the kernel gittree I found
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/net/wireless/libertas/if_spi.c;h=06a46d7b3d6c2460aecd793d2827033c7c0348f6;hb=cadeba315cc91ae1b57632e61b0cec3a4ed7088d
> which seems to include a driver for the miniPCI libertas 88w8385.

No, if_spi.c is an indicator that this is  for an SPI or GSPI.

There's to my knowledge no such thing as a PCI-based Marvell 8385
chip/card/whatever.

There is, however, some support for other Marvel 8xxx PCI-based cards
in linux, see drivers/net/wireless/mwl8k.c.

^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: John W. Linville @ 2009-11-04 14:38 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Ivo van Doorn, Bartlomiej Zolnierkiewicz, linux-wireless,
	linux-kernel, netdev, Randy Dunlap, Luis Correia, Johannes Berg,
	Jarek Poplawski, Pekka Enberg, David Miller
In-Reply-To: <20091104083737.GA16978@elte.hu>

On Wed, Nov 04, 2009 at 09:37:37AM +0100, Ingo Molnar wrote:

<snip>

> Really, you should listen to contrary opinion and _you_ should work 
> _hard_ to integrate Bart socially and open up your close circle of 
> wireless insiders instead of fighting his 'outsider' contributions every 
> which way. We dont care if people are rough, express displeasure and 
> show strong opinion about crappy code - but the moment you are 
> _excluding_ capable people and playing petty office politics (like you 
> are very clearly doing it with Bart here) everyone loses.

Ivo almost immediately ACKed 10 of Bart's patches (despite Bart's
failure to post them to the mailing list directly per convention),
so you criticize Ivo for saying he needed time to review the rest?
And you accuse him of excluding Bart's contributions when they have
only just arrived?

> Guys, show some minimal amount of honesty, openness and critical 
> thinking please ...

I would ask the same of you, Ingo.  You have burst onto the scene
only recently, full of accusations and blame for one side and
apparently unable to recognize the faults of the other.  You fail to
recognize that this set of patches is essentially the _first_ time
Bart has actually made any attempt to work on the rt2x00 drivers.
Your partisanship is clear.

At least Bart has finally made some rt2x00 patches available.  That is
how things get done.  You continuing to defend him as some sort of
martyr is completely unhelpful.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [RFC] usb: Check results of dma_map_single
From: Michael Buesch @ 2009-11-04 14:51 UTC (permalink / raw)
  To: Larry Finger
  Cc: Paulius Zaleckas, Greg Kroah-Hartman, linux-wireless, linux-usb
In-Reply-To: <4AF18A61.20909@lwfinger.net>

On Wednesday 04 November 2009 15:06:25 Larry Finger wrote:
> Thank you for your review and comments. I'll wait a bit to see what
> other comments are offered, but I have implemented all your changes.

If you use a WARN_ON, please use WARN_ON_ONCE to avoid possible spamming of the logs.

-- 
Greetings, Michael.

^ permalink raw reply

* Re: [PATCH] rtl8187: Fix kernel oops when device is removed when LEDS enabled (Bugzilla #14539)
From: John W. Linville @ 2009-11-04 15:11 UTC (permalink / raw)
  To: Larry Finger
  Cc: Herton Ronaldo Krzesinski, Hin-Tak Leung, sidhayn, linux-wireless
In-Reply-To: <4af11879./IumKJ+RAbw7Zkq6%Larry.Finger@lwfinger.net>

On Wed, Nov 04, 2009 at 12:00:25AM -0600, Larry Finger wrote:
> As reported by Rick Farina (sidhayn@gmail.com), removing the RTL8187 USB
> stick, or unloading the driver rtl8187 using rmmod will cause a kernel oops.
> There are at least two forms of the failure, (1) BUG: Scheduling while atomic,
> and (2) a fatal kernel page fault. This problem is reported in Bugzilla #14539.
> 
> This problem does not occur for kernel 2.6.31, but does for 2.6.32-rc2, thus
> it is technically a regression; however, bisection did not locate any faulty
> patch. The fix was found by comparing the faulty code in rtl8187 with p54usb.
> My interpretation is that the handling of work queues in mac80211 changed
> enough to the LEDs to be unregistered before tasks on the work queues are
> cancelled. Previously, these actions could be done in either order.
> 
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Reported-and-tested by: Rick Farina <sidhayn@gmail.com>
> ---
> 
> John,
> 
> This is 2.6.32 material. Sorry to take so long to get a patch, but it was
> difficult for me to locate the problem. Fortunately, I had the postings of the
> two flame wars to amuse me while all the kernel compilations were happening.
> 
> Larry
> ---
> 
> Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_leds.c
> +++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
> @@ -210,10 +210,10 @@ void rtl8187_leds_exit(struct ieee80211_
>  
>  	/* turn the LED off before exiting */
>  	ieee80211_queue_delayed_work(dev, &priv->led_off, 0);
> -	cancel_delayed_work_sync(&priv->led_off);
> -	cancel_delayed_work_sync(&priv->led_on);
>  	rtl8187_unregister_led(&priv->led_rx);
>  	rtl8187_unregister_led(&priv->led_tx);
> +	cancel_delayed_work_sync(&priv->led_off);
> +	cancel_delayed_work_sync(&priv->led_on);
>  }
>  #endif /* def CONFIG_RTL8187_LED */
>  

This seems like a band-aid.  If anything, the original order would
seem to make more sense.

Do you have a link to the original backtrace?  I don't see one in
the bugzilla entry.

Thanks,

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: John W. Linville @ 2009-11-04 15:15 UTC (permalink / raw)
  To: Gertjan van Wingerde
  Cc: Bartlomiej Zolnierkiewicz, linux-wireless, linux-kernel, netdev,
	Ivo van Doorn, Randy Dunlap, Luis Correia, Ingo Molnar,
	Johannes Berg, Jarek Poplawski, Pekka Enberg, David Miller
In-Reply-To: <4AF09A15.3060504@gmail.com>

On Tue, Nov 03, 2009 at 10:01:09PM +0100, Gertjan van Wingerde wrote:

> However, it is good to see that you have put effort in providing a patch series
> for these issues.

Yes, indeed.  Thanks to Bart for making patches available!

> Now, since I believe that it is better to work with people, rather than against
> them, would it be possible to post this patch series somewhere as a set of 
> separate patches, so that they can be reviewed as such?

I must echo this request.

Thanks!

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [RFC] usb: Check results of dma_map_single
From: Larry Finger @ 2009-11-04 15:16 UTC (permalink / raw)
  To: Michael Buesch
  Cc: Paulius Zaleckas, Greg Kroah-Hartman, linux-wireless, linux-usb
In-Reply-To: <200911041551.52914.mb@bu3sch.de>

On 11/04/2009 08:51 AM, Michael Buesch wrote:
> On Wednesday 04 November 2009 15:06:25 Larry Finger wrote:
>> Thank you for your review and comments. I'll wait a bit to see what
>> other comments are offered, but I have implemented all your changes.
> 
> If you use a WARN_ON, please use WARN_ON_ONCE to avoid possible spamming of the logs.

Good point.

Larry

^ permalink raw reply

* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Andrew Morton @ 2009-11-04 15:23 UTC (permalink / raw)
  To: David Miller
  Cc: johannes, marcel, dmitry.torokhov, torvalds, linville,
	linux-kernel, linux-wireless
In-Reply-To: <20091104.004157.86565200.davem@davemloft.net>

On Wed, 04 Nov 2009 00:41:57 -0800 (PST) David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Tue, 3 Nov 2009 22:34:33 -0800
> 
> > Problem is, subsystem maintainers are very unreliable.
> 
> This is common, but not universal Andrew.

That's what I said too.

> Or, if it is, what patches from you have I been sitting on? :-)

zippo.  Although if you can help these:

isdn-hisax-fix-lock-imbalance.patch
hfc_usb-fix-read-buffer-overflow.patch
misdn-fix-reversed-if-in-st_own_ctrl.patch
isdn-eicon-use-offsetof.patch
isdn-eicon-return-on-error.patch
hisax-fix-test-in-waitforxfw.patch

move forward it'd be nice!

^ permalink raw reply

* Re: [PATCH] rtl8187: Fix kernel oops when device is removed when LEDS enabled (Bugzilla #14539)
From: Christian Lamparter @ 2009-11-04 15:30 UTC (permalink / raw)
  To: John W. Linville
  Cc: Larry Finger, Herton Ronaldo Krzesinski, Hin-Tak Leung, sidhayn,
	linux-wireless
In-Reply-To: <20091104151132.GD12965@tuxdriver.com>

On Wednesday 04 November 2009 16:11:33 John W. Linville wrote:
> On Wed, Nov 04, 2009 at 12:00:25AM -0600, Larry Finger wrote:
> > As reported by Rick Farina (sidhayn@gmail.com), removing the RTL8187 USB
> > stick, or unloading the driver rtl8187 using rmmod will cause a kernel oops.
> > There are at least two forms of the failure, (1) BUG: Scheduling while atomic,
> > and (2) a fatal kernel page fault. This problem is reported in Bugzilla #14539.
> > 
> > This problem does not occur for kernel 2.6.31, but does for 2.6.32-rc2, thus
> > it is technically a regression; however, bisection did not locate any faulty
> > patch. The fix was found by comparing the faulty code in rtl8187 with p54usb.
> > My interpretation is that the handling of work queues in mac80211 changed
> > enough to the LEDs to be unregistered before tasks on the work queues are
> > cancelled. Previously, these actions could be done in either order.
> > 
> > Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> > Reported-and-tested by: Rick Farina <sidhayn@gmail.com>
> > ---
> > 
> > John,
> > 
> > This is 2.6.32 material. Sorry to take so long to get a patch, but it was
> > difficult for me to locate the problem. Fortunately, I had the postings of the
> > two flame wars to amuse me while all the kernel compilations were happening.
> > 
> > Larry
> > ---
> > 
> > Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
> > ===================================================================
> > --- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_leds.c
> > +++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
> > @@ -210,10 +210,10 @@ void rtl8187_leds_exit(struct ieee80211_
> >  
> >  	/* turn the LED off before exiting */
> >  	ieee80211_queue_delayed_work(dev, &priv->led_off, 0);
> > -	cancel_delayed_work_sync(&priv->led_off);
> > -	cancel_delayed_work_sync(&priv->led_on);
> >  	rtl8187_unregister_led(&priv->led_rx);
> >  	rtl8187_unregister_led(&priv->led_tx);
> > +	cancel_delayed_work_sync(&priv->led_off);
> > +	cancel_delayed_work_sync(&priv->led_on);
> >  }
> >  #endif /* def CONFIG_RTL8187_LED */
> >  
> 
> This seems like a band-aid.  If anything, the original order would
> seem to make more sense.

really?

take this code from led-class.c

void led_classdev_unregister(struct led_classdev *led_cdev)
{
        device_remove_file(led_cdev->dev, &dev_attr_max_brightness);
        device_remove_file(led_cdev->dev, &dev_attr_brightness);
#ifdef CONFIG_LEDS_TRIGGERS
        device_remove_file(led_cdev->dev, &dev_attr_trigger);
        down_write(&led_cdev->trigger_lock);
        if (led_cdev->trigger)
                led_trigger_set(led_cdev, NULL);
        up_write(&led_cdev->trigger_lock);
#endif

        device_unregister(led_cdev->dev);

        down_write(&leds_list_lock);
        list_del(&led_cdev->node);
        up_write(&leds_list_lock);
}

as you can see the led is switched-off right before the device is unregistered.
but rtl8187, p54 & ar9170 led-triggers are timed & asynchronous. So
we really need a cancel_delayed_work_sync after the unregister routine
finished... else the timed trigger might fire when the device/module
is _faded_ from memory.
 

^ permalink raw reply

* Re: compat-wireless and minstrel
From: Christian Lamparter @ 2009-11-04 15:53 UTC (permalink / raw)
  To: Adam Wozniak; +Cc: linux-wireless, nbd, derek
In-Reply-To: <4AF0D54D.4090303@irobot.com>

On Wednesday 04 November 2009 02:13:49 Adam Wozniak wrote:
> I have two systems under test, both Dell laptops (a Latitude D630 and an 
> Inspiron 600m) both running Ubuntu 9.10 with the latest updates, and 
> bleeding edge compat-wireless-2009-11-02.  I'm using identical AR9170 
> based D-Link DWA-160 USB 802.11adapters.  I'm using nuttcp to measure 
> throughput.  I'm running in ad-hoc mode.  Both machines have the same 
> ar9170 files in /lib/firmware.  The machines are sitting about 5 feet 
> apart in my office.
> 
> I'm having occasional problems where throughput drops through the floor 
> (0.5Mbps - 1.5Mbps).  When I cat 
> /sys/kernel/debug/ieee80211/*/stations/*/rc_stats, one of the machines 
> lists the full set of rates, but the other only lists 1M and 54M.  After 
> a period of time, that machine drops 54M and lists only one rate 
> (1Mbps), and the throughput listed by nuttcp drops accordingly.  I 
> assume that, for whatever reason, the rates drop off the list and 
> minstrel uses the only one left available to it.
> 
> If I modify include/net/mac80211.h and force the inline function 
> rate_supported to always return 1, this fixes the problem.  However, I 
> think this is a band aid around some other issue.
> 
> Any clues or ideas what the real issue might be here?
no, but there are a few knobs you can play with...

First of: more debug data

1. enable some ibss-related verbose messages
   open config.mk (in your compat-wireless directory)
   and get rid of the leading "# " <-- "sharp + space"
   in front of # CONFIG_MAC80211_IBSS_DEBUG=y
   (rebuild, reload, retest & checkout dmesg for a _flood_
    of potential interesting data)

2. do you think you can get a capture any beacons, auth, assoc, reassoc 
   management frames form the misbehaving setup?

3. cat /sys/kernel/debug/ieee80211/*/stations/*/rc_stats perhaps?
   (yeah, I know... sort of pointless, but people have different believes
    and somehow they have the bad tendency to really _stick_ to it!)

Regards,
	Chr

^ permalink raw reply

* Re: [PATCH] rtl8187: Fix kernel oops when device is removed when LEDS enabled (Bugzilla #14539)
From: Larry Finger @ 2009-11-04 15:54 UTC (permalink / raw)
  To: John W. Linville
  Cc: Herton Ronaldo Krzesinski, Hin-Tak Leung, sidhayn, linux-wireless
In-Reply-To: <20091104151132.GD12965@tuxdriver.com>

On 11/04/2009 09:11 AM, John W. Linville wrote:
> On Wed, Nov 04, 2009 at 12:00:25AM -0600, Larry Finger wrote:
>> As reported by Rick Farina (sidhayn@gmail.com), removing the RTL8187 USB
>> stick, or unloading the driver rtl8187 using rmmod will cause a kernel oops.
>> There are at least two forms of the failure, (1) BUG: Scheduling while atomic,
>> and (2) a fatal kernel page fault. This problem is reported in Bugzilla #14539.
>>
>> This problem does not occur for kernel 2.6.31, but does for 2.6.32-rc2, thus
>> it is technically a regression; however, bisection did not locate any faulty
>> patch. The fix was found by comparing the faulty code in rtl8187 with p54usb.
>> My interpretation is that the handling of work queues in mac80211 changed
>> enough to the LEDs to be unregistered before tasks on the work queues are
>> cancelled. Previously, these actions could be done in either order.
>>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> Reported-and-tested by: Rick Farina <sidhayn@gmail.com>
>> ---
>>
>> John,
>>
>> This is 2.6.32 material. Sorry to take so long to get a patch, but it was
>> difficult for me to locate the problem. Fortunately, I had the postings of the
>> two flame wars to amuse me while all the kernel compilations were happening.
>>
>> Larry
>> ---
>>
>> Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
>> ===================================================================
>> --- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_leds.c
>> +++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
>> @@ -210,10 +210,10 @@ void rtl8187_leds_exit(struct ieee80211_
>>  
>>  	/* turn the LED off before exiting */
>>  	ieee80211_queue_delayed_work(dev, &priv->led_off, 0);
>> -	cancel_delayed_work_sync(&priv->led_off);
>> -	cancel_delayed_work_sync(&priv->led_on);
>>  	rtl8187_unregister_led(&priv->led_rx);
>>  	rtl8187_unregister_led(&priv->led_tx);
>> +	cancel_delayed_work_sync(&priv->led_off);
>> +	cancel_delayed_work_sync(&priv->led_on);
>>  }
>>  #endif /* def CONFIG_RTL8187_LED */
>>  
> 
> This seems like a band-aid.  If anything, the original order would
> seem to make more sense.
> 
> Do you have a link to the original backtrace?  I don't see one in
> the bugzilla entry.

I agree that the original order makes more sense, which is why I coded
it that way in the first place; however, something changed during the
post-2.6.31 merge period. I tried to bisect the regression, but gave
up after 4 days of trying. I kept ending up where all the remaining
commits referred to drivers I'm not even using.

I don't have a full backtrace as I have had no success with
netconsole. My hand notes have only limited trace info, but I did note
that none of the rtl8187 or mac80211 routines are mentioned in any
trace I've seen. In the one in my notes, the process that crashed was
ifdown with a "scheduling while atomic" BUG.

I will try once more to get netconsole working to capture the backtrace.

Larry

^ permalink raw reply

* Re: compat-wireless and minstrel
From: Luis R. Rodriguez @ 2009-11-04 15:57 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: Adam Wozniak, linux-wireless, nbd, derek
In-Reply-To: <200911041653.33737.chunkeey@googlemail.com>

On Wed, Nov 4, 2009 at 7:53 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Wednesday 04 November 2009 02:13:49 Adam Wozniak wrote:
>> I have two systems under test, both Dell laptops (a Latitude D630 and an
>> Inspiron 600m) both running Ubuntu 9.10 with the latest updates, and
>> bleeding edge compat-wireless-2009-11-02.  I'm using identical AR9170
>> based D-Link DWA-160 USB 802.11adapters.  I'm using nuttcp to measure
>> throughput.  I'm running in ad-hoc mode.  Both machines have the same
>> ar9170 files in /lib/firmware.  The machines are sitting about 5 feet
>> apart in my office.
>>
>> I'm having occasional problems where throughput drops through the floor
>> (0.5Mbps - 1.5Mbps).  When I cat
>> /sys/kernel/debug/ieee80211/*/stations/*/rc_stats, one of the machines
>> lists the full set of rates, but the other only lists 1M and 54M.  After
>> a period of time, that machine drops 54M and lists only one rate
>> (1Mbps), and the throughput listed by nuttcp drops accordingly.  I
>> assume that, for whatever reason, the rates drop off the list and
>> minstrel uses the only one left available to it.
>>
>> If I modify include/net/mac80211.h and force the inline function
>> rate_supported to always return 1, this fixes the problem.  However, I
>> think this is a band aid around some other issue.
>>
>> Any clues or ideas what the real issue might be here?
> no, but there are a few knobs you can play with...
>
> First of: more debug data
>
> 1. enable some ibss-related verbose messages
>   open config.mk (in your compat-wireless directory)
>   and get rid of the leading "# " <-- "sharp + space"
>   in front of # CONFIG_MAC80211_IBSS_DEBUG=y
>   (rebuild, reload, retest & checkout dmesg for a _flood_
>    of potential interesting data)

If you can just use wireless-testing directly for debugging I
recommend that as we don't use mconf and friends for cofiguration
selection so we cannot be sure the config changes you make to enable
something are indeed agreeable by current upstream Kconfig rules. That
is, unless you know what you are doing I'd advise against using
debugging on compat-wireless.

  Luis

^ permalink raw reply

* Re: [PATCH] ar9170usb: add mode-switching for AVM Fritz!WLAN USB N devices in cdrom mode
From: Alan Stern @ 2009-11-04 16:16 UTC (permalink / raw)
  To: Frank Schaefer; +Cc: USB list, linux-wireless
In-Reply-To: <4AF095C3.5020409@gmx.net>

On Tue, 3 Nov 2009, Frank Schaefer wrote:

> Basically, I have no problem to agree here.
> But what's the price we pay for this solution(s) in this case (and most
> other devices with windows-driver-disc-mode) ?
> You should not ignore the problems coming-up with the discussed
> userspace-solutions.
> This is not ONLY about technical aspects. We already discussed usability
> and maintanance. Think about the whole product, not only the kernel-part.
> Then, please, make a list of ALL pros and cons a make a pragmatic decision.

Here's my first attempt at considering all the pros and cons.  Has
anything important been left out?

First, let's consider the case where the relevant drivers/programs are 
already present when a switchable device is plugged in.  Then 
presumably it all works correctly and there's no problem.  :-)

So second, let's consider the case where the drivers/programs are not
already present.  The device does not get switched automatically.  In
order to do anything with it, the user must upgrade his system.

     1. If the mode-switch code is in the kernel: The user must install
	a new kernel.  This might mean downloading a new version from
	the distro or it might mean installing a patch and rebuilding
	manually.

     2. If the mode-switch code is in userspace: The user must update 
	the mode-switch program.  This might mean adding a line to a 
	udev script file, installing a new binary package, or 
	downloading new source code to usb_modeswitch and rebuilding 
	manually.

     3. If the policy is handled by a userspace program while the
	mechanism is handled by the kernel: The user will have to carry
	out 1 or 2 or possibly both.  Hence this case reduces to the
	earlier cases (plus the fact that the user somehow has to
	determine which steps are necessary).

Either way, once the upgrade has been done the device will work.  So 
the only question is which is easier for everybody, 1 or 2?

Clearly 1 puts more of a burden on the kernel/distro developers while 2
puts more of a burden on the userspace support teams.  But they
comprise a relatively small number of people compared to the total user
population.  For users, then, which is easier?

Downloading a new kernel package is roughly equivalent to downloading a 
new usb_modeswitch binary package.  It takes longer and there might be 
administrative issues (updating the kernel has more repercussions than 
updating a single program), but overall they are similar.

Clearly editing a udev script is very easy.  Downloading patches or 
source code and rebuilding by hand requires a certain amount of skill, 
and it is not something most people would care to do -- although if 
they had to, building a program is much easier than building a kernel.

Thus, in terms of the amount of work required of the user, I'd say that 
1 is worse than 2.  But there's more to consider.  How long does it 
take for new-device support to get disseminated?

User programs like usb_modeswitch can be updated very rapidly.  It's
merely a question of writing the new code and making it publicly
available in the appropriate repository.  Typically this might involve
a delay of a few days.  By contrast, new kernels appear roughly every
three months.  It's possible that new mode-switch code would be allowed
in a stable release (although this is far from certain).  If so, then
the delay could drop as low as a few weeks.

Thus, in terms of the delay time for new support, I'd say again that 1
is worse than 2.  Finally, let's consider adding support for a new 
device to an old system.

An old system is not readily upgradable.  But _some_ sort of upgrade is 
needed to support new devices.

	If all that is needed is a modification to a udev script,
	there is no problem.

	Installing a new version of a program might be difficult
	because of library dependencies that aren't up to date.

	Installing a new version of the kernel might be difficult
	because of incompatibilities with old userspace components.

Comparing the last two directly is hard; either could be a
show-stopper.  The ideal solution would be to provide a way to upgrade
a mode-switching program by modifying a text configuration or script
file rather than by changing the code.  For example, if usb_modeswitch
could parse a text file containing device IDs to match and packet
contents to send, then upgrades would be very easy.

In short, for users I don't see any advantage to putting the
mode-switching stuff in the kernel, whereas there may be considerable
advantages to putting it in userspace.

The only exception would be the case that Oliver is worried about,
where a device loses its mode setting during a reset or system sleep.  
In principle the kernel could put it back in the correct mode while
keeping the device's identity intact (although the code to do this
would be extremely ugly, and convincing people to allow it would be
hard).  Even then, I don't see how existing serial/phone connections
could be maintained across a reset, so there might not be any point.

Alan Stern


^ permalink raw reply

* Re: [PATCH] ar9170usb: add mode-switching for AVM Fritz!WLAN USB N devices in cdrom mode
From: Johannes Berg @ 2009-11-04 16:25 UTC (permalink / raw)
  To: Alan Stern; +Cc: Frank Schaefer, USB list, linux-wireless
In-Reply-To: <Pine.LNX.4.44L0.0911041036100.2968-100000@iolanthe.rowland.org>

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

On Wed, 2009-11-04 at 11:16 -0500, Alan Stern wrote:

> Here's my first attempt at considering all the pros and cons.  Has
> anything important been left out?

[snip]

I think you forgot to consider the case where both the driver and the
mode switch code need an update to handle a new device. This is relevant
in the case where a new device shows up that needs mode-switching, but
even after switching has an ID that the current driver doesn't know
about.

It's not clear to me which case typically happens though. I suppose that
often enough it'd be possible to have a device that shows up with a
special driver USB ID, and then after you eject it comes up again with a
regular USB ID that other devices use too.

Input from people who have such switching devices could be helpful.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: David Miller @ 2009-11-04 16:32 UTC (permalink / raw)
  To: akpm
  Cc: johannes, marcel, dmitry.torokhov, torvalds, linville,
	linux-kernel, linux-wireless
In-Reply-To: <20091104072315.18f56322.akpm@linux-foundation.org>

From: Andrew Morton <akpm@linux-foundation.org>
Date: Wed, 4 Nov 2009 07:23:15 -0800

> Although if you can help these:
> 
> isdn-hisax-fix-lock-imbalance.patch
> misdn-fix-reversed-if-in-st_own_ctrl.patch
> isdn-eicon-use-offsetof.patch
> hisax-fix-test-in-waitforxfw.patch
> hfc_usb-fix-read-buffer-overflow.patch

Applied to net-2.6

> isdn-eicon-return-on-error.patch

Also applied to net-2.6, holy crap someone run that driver through
some auto-formatting tool! :-)


^ permalink raw reply

* Re: [PATCH] ar9170usb: add mode-switching for AVM Fritz!WLAN USB N devices in cdrom mode
From: Oliver Neukum @ 2009-11-04 16:41 UTC (permalink / raw)
  To: Alan Stern; +Cc: Frank Schaefer, USB list, linux-wireless
In-Reply-To: <Pine.LNX.4.44L0.0911041036100.2968-100000@iolanthe.rowland.org>

Am Mittwoch, 4. November 2009 17:16:18 schrieb Alan Stern:
> The only exception would be the case that Oliver is worried about,
> where a device loses its mode setting during a reset or system sleep.  
> In principle the kernel could put it back in the correct mode while
> keeping the device's identity intact (although the code to do this
> would be extremely ugly, and convincing people to allow it would be

Oh yes.

> hard).  Even then, I don't see how existing serial/phone connections
> could be maintained across a reset, so there might not be any point.

I am afraid this will spread beyond modems.
For now I'd like those devices which are affected by resets to get
a quirk that marks them as not resettable for purposes of error handling.
But I fear we may have to face this system in the future on a regular
basis.

	Regards
		Oliver


^ permalink raw reply

* Re: [Bugme-new] [Bug 14450] New: [2.6.31] Network interfaces are dead with 2.6.31 (iwlagn and sky2)
From: Bjorn Helgaas @ 2009-11-04 16:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: orion.linbug, bugzilla-daemon, bugme-daemon, netdev, linux-acpi,
	Rafael J. Wysocki, linux-wireless
In-Reply-To: <20091103152146.5785195a.akpm@linux-foundation.org>

On Tuesday 03 November 2009 04:21:46 pm Andrew Morton wrote:
> 
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).

> > http://bugzilla.kernel.org/show_bug.cgi?id=14450

This looks like a duplicate of http://bugzilla.kernel.org/show_bug.cgi?id=13940

There's a patch in that bug that is upstream already:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=15b812f1d0a5ca8f5efe7f5882f468af10682ca8

Please try booting 2.6.31 with "pci=use_crs" and report the result.

If possible, please also try booting an upstream kernel containing
the patch above (without "pci=use_crs") and report that result, too.

^ permalink raw reply

* Re: [PATCH] rtl8187: Fix kernel oops when device is removed when LEDS enabled (Bugzilla #14539)
From: John W. Linville @ 2009-11-04 16:49 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Larry Finger, Herton Ronaldo Krzesinski, Hin-Tak Leung, sidhayn,
	linux-wireless
In-Reply-To: <200911041630.20122.chunkeey@googlemail.com>

On Wed, Nov 04, 2009 at 04:30:19PM +0100, Christian Lamparter wrote:
> On Wednesday 04 November 2009 16:11:33 John W. Linville wrote:

> > This seems like a band-aid.  If anything, the original order would
> > seem to make more sense.
> 
> really?
> 
> take this code from led-class.c
> 
> void led_classdev_unregister(struct led_classdev *led_cdev)
> {
>         device_remove_file(led_cdev->dev, &dev_attr_max_brightness);
>         device_remove_file(led_cdev->dev, &dev_attr_brightness);
> #ifdef CONFIG_LEDS_TRIGGERS
>         device_remove_file(led_cdev->dev, &dev_attr_trigger);
>         down_write(&led_cdev->trigger_lock);
>         if (led_cdev->trigger)
>                 led_trigger_set(led_cdev, NULL);
>         up_write(&led_cdev->trigger_lock);
> #endif
> 
>         device_unregister(led_cdev->dev);
> 
>         down_write(&leds_list_lock);
>         list_del(&led_cdev->node);
>         up_write(&leds_list_lock);
> }
> 
> as you can see the led is switched-off right before the device is unregistered.
> but rtl8187, p54 & ar9170 led-triggers are timed & asynchronous. So
> we really need a cancel_delayed_work_sync after the unregister routine
> finished... else the timed trigger might fire when the device/module
> is _faded_ from memory.

OK, I got it...the unregister can queue-up more work.  Thanks for
the explanation.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [PATCH] rtl8187: Fix kernel oops when device is removed when LEDS enabled (Bugzilla #14539)
From: John W. Linville @ 2009-11-04 16:54 UTC (permalink / raw)
  To: Larry Finger
  Cc: Herton Ronaldo Krzesinski, Hin-Tak Leung, sidhayn, linux-wireless,
	mcgrof, johannes
In-Reply-To: <4AF1A3BD.1020009@lwfinger.net>

On Wed, Nov 04, 2009 at 09:54:37AM -0600, Larry Finger wrote:

> I will try once more to get netconsole working to capture the backtrace.

No need, I think I understand it now.  The new order still sorta
looks/feels "wrong", but it seems fine.  Maybe an alternative would
be to make the brightness_set routine aware of the shutdown and not
queue the work?  Maybe even ieee80211_queue_delayed_work could be
made a bit smarter here?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [PATCH] ar9170usb: add mode-switching for AVM Fritz!WLAN USB N devices in cdrom mode
From: Alan Stern @ 2009-11-04 17:07 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Frank Schaefer, USB list, linux-wireless
In-Reply-To: <1257351909.28469.97.camel@johannes.local>

On Wed, 4 Nov 2009, Johannes Berg wrote:

> On Wed, 2009-11-04 at 11:16 -0500, Alan Stern wrote:
> 
> > Here's my first attempt at considering all the pros and cons.  Has
> > anything important been left out?
> 
> [snip]
> 
> I think you forgot to consider the case where both the driver and the
> mode switch code need an update to handle a new device. This is relevant
> in the case where a new device shows up that needs mode-switching, but
> even after switching has an ID that the current driver doesn't know
> about.

This could be handled by programs like usb_modeswitch.  The USB serial 
drivers allow IDs to be added dynamically.

Of course, if something more than that is required then the situation 
would be different.  However I think this is pretty rare.

Alan Stern


^ 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