Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH] Wifi led does not work on Acer Aspire One D250 (ath5k driver)
From: Bob Copeland @ 2009-11-06 16:21 UTC (permalink / raw)
  To: John W. Linville; +Cc: Carlo Parata, linux-wireless
In-Reply-To: <20091106160926.GA2782@tuxdriver.com>

On Fri, Nov 6, 2009 at 11:09 AM, John W. Linville
<linville@tuxdriver.com> wrote:

> "... but can you by chance put a contact address in the comment?  I've been
> collecting them in case we one day figure out a better way to do this so it's
> easy to find the people to retest."
>
> Would you mind complying with his request?  Or have I mistakend his applicability?

It's still applicable in the general case, but I think the new patch
is for the same HW as the earlier one posted (Carlo can confirm)
so we should be good already.

-- 
Bob Copeland %% www.bobcopeland.com

^ permalink raw reply

* Re: Getting random regulatory domains on boot-up with ath9k
From: Luis R. Rodriguez @ 2009-11-06 16:30 UTC (permalink / raw)
  To: Jeffrey Baker; +Cc: linux-wireless
In-Reply-To: <43e72e890911060821r2703c186lbb75c40b449b2c55@mail.gmail.com>

On Fri, Nov 6, 2009 at 8:21 AM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:

>  * The fix for the regression would be to either use an alpha2 which
> does have an entry and put that first in the array *or* (my
> preference) to check first the alpha2 set on cfg80211 and see if that
> maps to a country allowed by your region code and if so get cfg80211
> to request that regulatory domain to CRDA.

I just thought of another possible solution for region code stuff --
just make them static world roaming regulatory domains. But to do this
it means we'd enable the world roaming features on cfg80211 and
channels which would typically be disabled would then just become
passive-scan/no-ibss. I'd have to get buy-in through our internal
regulatory guys first. This also seems like a clean solution but it
would mean more static tables define for each major region code we
have and I am not sure how many of those we have.

  Luis

^ permalink raw reply

* Re: Getting random regulatory domains on boot-up with ath9k
From: Luis R. Rodriguez @ 2009-11-06 16:44 UTC (permalink / raw)
  To: Jeffrey Baker; +Cc: linux-wireless
In-Reply-To: <43e72e890911060830k56f9cdcbi14a03b1389351197@mail.gmail.com>

On Fri, Nov 6, 2009 at 8:30 AM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> On Fri, Nov 6, 2009 at 8:21 AM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>
>>  * The fix for the regression would be to either use an alpha2 which
>> does have an entry and put that first in the array *or* (my
>> preference) to check first the alpha2 set on cfg80211 and see if that
>> maps to a country allowed by your region code and if so get cfg80211
>> to request that regulatory domain to CRDA.
>
> I just thought of another possible solution for region code stuff --
> just make them static world roaming regulatory domains. But to do this
> it means we'd enable the world roaming features on cfg80211 and
> channels which would typically be disabled would then just become
> passive-scan/no-ibss. I'd have to get buy-in through our internal
> regulatory guys first. This also seems like a clean solution but it
> would mean more static tables define for each major region code we
> have and I am not sure how many of those we have.

Oh and the other solution is to just add AW to db.txt with the same
mappings as any other country in the region code group.

I'll out line this in an e-mail followup, still reviewing what option to take.

  Luis

^ permalink raw reply

* Re: [PATCH] Wifi led does not work on Acer Aspire One D250 (ath5k  driver)
From: Carlo Parata @ 2009-11-06 16:46 UTC (permalink / raw)
  To: linux-wireless
In-Reply-To: <b6c5339f0911060821q48f71d55rf6e5eabc3ce14504@mail.gmail.com>

Yes I confirm that the earlier patch worked for my hardware.
Thank you very much. 


^ permalink raw reply

* Changing the way we handle region codes on Linux (public thread)
From: Luis R. Rodriguez @ 2009-11-06 16:57 UTC (permalink / raw)
  To: linux-wireless, Vivek Natarajan, Vivek Natarajan
  Cc: Jeffrey Baker, John W. Linville, David Quan, Michael Green

Note: this e-mail is on a public mailing list.

So Jeffrey pointed out a regression introduced by a recent patch to
update our regulatory domain information on the Atheros driver common
module, ath.ko. Of relevance the patch added Aruba "AW" and made "AW"
the first alpha2 to which the ETSI1_WORLD region code maps to. This
region code used to first map to "AT". The wireless-regdb did not
however yet get an update for "AW" so the next logical step is to add
it to wireless-regdb based on the "AT" for example (or any other
country on the region, as all of those countries are expected to be on
the same region and hence the same regulatory domain). That missing
patch would address the issues reported by Jeffrey, which is that he
was unable to use AP mode of operation as no channels when world
roaming are allowed to start IBSS/AP mode unless you are world roaming
(not the case when a region code is used) and find a nearby AP.

Users tend to get confused by the current region code mapping though
so alternatives we can consider are:

1) If a user selects a regulatory domain manually (either before or
after the wireless driver is loaded) check to see if the alpha2 the
user selected fits in the region of the currently programmed region
code. This would be to search the allCountries[] array, and see if the
programmed region code has a match for the alpha2 the user selected at
all in the array. If this is true then allow the usage of that
regulatory domain for the driver.

2) Treat region codes as world roaming regulatory domains and define
them statically in the ath.ko module as we do with the other 12 world
regulatory domains. The advantage to this solution is these regions
would then *world roam* and this means being able to use the world
roaming features of finding a channel on a passive-scan/no-beaconing
channel and lifting those restrictions. The down side to this is we'd
increase ath.ko in size with however many region codes we support.

I think that sums it up. I'd like to hear input on these on
linux-wireless. I'll address this internally at Atheros also to to see
what we think too and get back to this thread with a path. In th
meantime I'll send a patch for wireless-regdb for AW just to get a
quick resolution to this.

  Luis

^ permalink raw reply

* Re: [PATCH] SSB: fix 32 bit PCMCIA access.
From: Michael Buesch @ 2009-11-06 17:10 UTC (permalink / raw)
  To: Martin Fuzzey; +Cc: linux-wireless
In-Reply-To: <20091106155605.20585.15955.stgit@srv002.fuzzey.net>

On Friday 06 November 2009 16:56:05 Martin Fuzzey wrote:
> The scan function was using 32 bit access which does not
> work on 16bit CF cards.
> 
> This patch corrects this by doing two 16 bit reads like
> ssb_pcmcia_read32 already does (unfortunately we don't
> have a struct ssb_device to use that directly).
> 
> Signed-off-by: Martin Fuzzey <mfuzzey@gmail.com>
> 
> ---
> 
>  drivers/ssb/scan.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/ssb/scan.c b/drivers/ssb/scan.c
> index b74212d..6721fa8 100644
> --- a/drivers/ssb/scan.c
> +++ b/drivers/ssb/scan.c
> @@ -162,6 +162,9 @@ static u8 chipid_to_nrcores(u16 chipid)
>  static u32 scan_read32(struct ssb_bus *bus, u8 current_coreidx,
>  		       u16 offset)
>  {
> +	unsigned long flags;
> +	u32 lo, hi;
> +
>  	switch (bus->bustype) {
>  	case SSB_BUSTYPE_SSB:
>  		offset += current_coreidx * SSB_CORE_SIZE;
> @@ -174,7 +177,11 @@ static u32 scan_read32(struct ssb_bus *bus, u8 current_coreidx,
>  			offset -= 0x800;
>  		} else
>  			ssb_pcmcia_switch_segment(bus, 0);
> -		break;
> +		spin_lock_irqsave(&bus->bar_lock, flags);
> +		lo = readw(bus->mmio + offset);
> +		hi = readw(bus->mmio + offset + 2);
> +		spin_unlock_irqrestore(&bus->bar_lock, flags);
> +		return lo | (hi << 16);
>  	case SSB_BUSTYPE_SDIO:
>  		offset += current_coreidx * SSB_CORE_SIZE;
>  		return ssb_sdio_scan_read32(bus, offset);

Thanks for spotting this. This really looks like a bug.
I'll test this patch on my device and remove the unnecessary locks and then resubmit upstream.

Next time, please make sure to Cc maintainers. See the MAINTAINERS file.

-- 
Greetings, Michael.

^ permalink raw reply

* Re: b43: firmware loading problem and sleeping BUG
From: Michael Buesch @ 2009-11-06 17:22 UTC (permalink / raw)
  To: Martin Fuzzey; +Cc: linux-wireless
In-Reply-To: <ba4215e10911060819y1175e003g695281fe084e07ab@mail.gmail.com>

On Friday 06 November 2009 17:19:51 Martin Fuzzey wrote:
> I posted a couple of days ago to the bcm43xx_dev list about problems
> getting b43 to run on a
> arm (MX21 SoC). That list appears to have died recently (at least
> nothing added to the archives
> since last month)

Well, it never really worked. It's hosted on berlios. That's why it's crap. ;)

> However I now get:
> pcmcia_socket pcmcia_socket0: pccard: PCMCIA card inserted into slot 0
> pcmcia 0.0: pcmcia: registering new device pcmcia0.0
> ssb: Core 0 found: ChipCommon (cc 0x800, rev 0x0D, vendor 0x4243)
> ssb: Core 1 found: IEEE 802.11 (cc 0x812, rev 0x09, vendor 0x4243)
> ssb: Core 2 found: PCI (cc 0x804, rev 0x0C, vendor 0x4243)
> ssb: Core 3 found: PCMCIA (cc 0x80D, rev 0x07, vendor 0x4243)
> BUG: sleeping function called from invalid context at mm/vmalloc.c:1367
> in_atomic(): 1, irqs_disabled(): 128, pid: 1361, name: modprobe
> 2 locks held by modprobe/1361:
>  #0:  (buses_mutex){+.+.+.}, at: [<bf03761c>] ssb_bus_register+0x48/0x1a4 [ssb]
>  #1:  (&bus->bar_lock){......}, at: [<bf038e38>]
> ssb_pcmcia_read32+0x24/0x74 [ssb]
> irq event stamp: 105917
> hardirqs last  enabled at (105916): [<c027cf34>]
> __mutex_unlock_slowpath+0x120/0x150
> hardirqs last disabled at (105917): [<c027eee0>] _spin_lock_irqsave+0x20/0x60
> softirqs last  enabled at (105853): [<c0042f60>] irq_exit+0x50/0x64
> softirqs last disabled at (105832): [<c0042f60>] irq_exit+0x50/0x64
> ------------[ cut here ]------------
> WARNING: at kernel/lockdep.c:2465 lockdep_trace_alloc+0xac/0xec()
> Modules linked in: b43(+) ssb mac80211 mxc_pcmcia
> ---[ end trace 7a542bbcadb0bb88 ]---

These logs look weird. Is that the full log from "dmesg" command?
I guess the pcmcia_access_configuration_register() is doing nonatomic vmalloc stuff.

I think this is easy to fix, because we can replace the spinlock by a mutex, as
the b43 driver (which is the only user of the code) always allows sleeping now.
I'll send a patch for testing soon.

> ssb: Sonics Silicon Backplane found on PCMCIA device pcmcia0.0
> b43-phy0: Broadcom 4318 WLAN found (core revision 9)
> b43-phy0 debug: Found PHY: Analog 3, Type 2, Revision 7
> b43-phy0 debug: Found Radio: Manuf 0x17F, Version 0x2050, Revision 8
> phy0: Selected rate control algorithm 'minstrel'
> Broadcom 43xx driver loaded [ Features: M, Firmware-ID: FW13 ]
> 
> And then when I do ifconfig wlan0 up:
> 
> b43 ssb0:0: firmware: requesting b43/ucode5.fw
> b43 ssb0:0: firmware: requesting b43/pcm5.fw
> b43 ssb0:0: firmware: requesting b43/b0g0initvals5.fw
> b43 ssb0:0: firmware: requesting b43/b0g0bsinitvals5.fw
> b43-phy0: Loading firmware version 410.2160 (2007-05-26 15:32:10)
> b43-phy0 ERROR: Initial Values Firmware file-format error.
> b43-phy0 ERROR: You must go to
> http://wireless.kernel.org/en/users/Drivers/b43#devicefirmware
> and download the correct firmware for this driver version. Please
> carefully read all instructions on this website.
> 
> I am using firmware broadcom-wl-4.150.10.5 extracted with
> b43-fwcutter-012 as described
> http://wireless.kernel.org/en/users/Drivers/b43#devicefirmware
> 
> 
> Any ideas why the versions don't match?

These are two different version numbers. One for the broadcom driver
and one for the firmware.

> The firmware files are arch independant right? (since I run fwcutter
> on x86 and run the kernel on ARM)

Yes.

> Kernel version is 2.6.32-rc5
> The PCMCIA controller driver is new (I recently posted it to
> linux-pcmcia and linux-arm)

Thanks for testing. The ssb-pcmcia code is not tested a lot, so
I'm not surprised that there are bugs.

-- 
Greetings, Michael.

^ permalink raw reply

* Re: Changing the way we handle region codes on Linux (public thread)
From: Bob Copeland @ 2009-11-06 17:29 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: linux-wireless, Vivek Natarajan, Vivek Natarajan, Jeffrey Baker,
	John W. Linville, David Quan, Michael Green
In-Reply-To: <43e72e890911060857l6e383578nc1c7d0f2ba63529f@mail.gmail.com>

On Fri, Nov 6, 2009 at 11:57 AM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:

> I think that sums it up.

I personally still like the idea of pushing the vendor-specific
codes out to user space and having psuedo-country codes for
those (e.g. "ATH_37").  Then the driver doesn't need all of the
static rules loaded all the time and it would drop a lot of
policy code from the driver.  CRDA could be enhanced to load
multiple databases, one for pure iso-3166 codes, one with
Atheros codes, one with Intel, etc.

-- 
Bob Copeland %% www.bobcopeland.com

^ permalink raw reply

* [PATCH] b43: Rewrite TX bounce buffer handling
From: Michael Buesch @ 2009-11-06 17:32 UTC (permalink / raw)
  To: John W. Linville
  Cc: Broadcom Wireless, linux-wireless, Larry Finger,
	Christian Casteyde

Do not mess with the original skb, but allocate an independent bouncebuffer.
This protects against bad interference with mac80211's assumptions about
the skb (which already caused bugs).

Signed-off-by: Michael Buesch <mb@bu3sch.de>

---

Might also be worth applying to b43legacy.


Index: wireless-testing/drivers/net/wireless/b43/dma.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/dma.c	2009-11-06 16:55:02.000000000 +0100
+++ wireless-testing/drivers/net/wireless/b43/dma.c	2009-11-06 18:26:23.000000000 +0100
@@ -1157,18 +1157,17 @@ struct b43_dmaring *parse_cookie(struct 
 }
 
 static int dma_tx_fragment(struct b43_dmaring *ring,
-			   struct sk_buff **in_skb)
+			   struct sk_buff *skb)
 {
-	struct sk_buff *skb = *in_skb;
 	const struct b43_dma_ops *ops = ring->ops;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	struct b43_private_tx_info *priv_info = b43_get_priv_tx_info(info);
 	u8 *header;
 	int slot, old_top_slot, old_used_slots;
 	int err;
 	struct b43_dmadesc_generic *desc;
 	struct b43_dmadesc_meta *meta;
 	struct b43_dmadesc_meta *meta_hdr;
-	struct sk_buff *bounce_skb;
 	u16 cookie;
 	size_t hdrsize = b43_txhdr_size(ring->dev);
 
@@ -1212,34 +1211,28 @@ static int dma_tx_fragment(struct b43_dm
 
 	meta->skb = skb;
 	meta->is_last_fragment = 1;
+	priv_info->bouncebuffer = NULL;
 
 	meta->dmaaddr = map_descbuffer(ring, skb->data, skb->len, 1);
 	/* create a bounce buffer in zone_dma on mapping failure. */
 	if (b43_dma_mapping_error(ring, meta->dmaaddr, skb->len, 1)) {
-		bounce_skb = __dev_alloc_skb(skb->len, GFP_ATOMIC | GFP_DMA);
-		if (!bounce_skb) {
+		priv_info->bouncebuffer = kmalloc(skb->len, GFP_ATOMIC | GFP_DMA);
+		if (!priv_info->bouncebuffer) {
 			ring->current_slot = old_top_slot;
 			ring->used_slots = old_used_slots;
 			err = -ENOMEM;
 			goto out_unmap_hdr;
 		}
+		memcpy(priv_info->bouncebuffer, skb->data, skb->len);
 
-		memcpy(skb_put(bounce_skb, skb->len), skb->data, skb->len);
-		memcpy(bounce_skb->cb, skb->cb, sizeof(skb->cb));
-		bounce_skb->dev = skb->dev;
-		skb_set_queue_mapping(bounce_skb, skb_get_queue_mapping(skb));
-		info = IEEE80211_SKB_CB(bounce_skb);
-
-		dev_kfree_skb_any(skb);
-		skb = bounce_skb;
-		*in_skb = bounce_skb;
-		meta->skb = skb;
-		meta->dmaaddr = map_descbuffer(ring, skb->data, skb->len, 1);
+		meta->dmaaddr = map_descbuffer(ring, priv_info->bouncebuffer, skb->len, 1);
 		if (b43_dma_mapping_error(ring, meta->dmaaddr, skb->len, 1)) {
+			kfree(priv_info->bouncebuffer);
+			priv_info->bouncebuffer = NULL;
 			ring->current_slot = old_top_slot;
 			ring->used_slots = old_used_slots;
 			err = -EIO;
-			goto out_free_bounce;
+			goto out_unmap_hdr;
 		}
 	}
 
@@ -1256,8 +1249,6 @@ static int dma_tx_fragment(struct b43_dm
 	ops->poke_tx(ring, next_slot(ring, slot));
 	return 0;
 
-out_free_bounce:
-	dev_kfree_skb_any(skb);
 out_unmap_hdr:
 	unmap_descbuffer(ring, meta_hdr->dmaaddr,
 			 hdrsize, 1);
@@ -1362,11 +1353,7 @@ int b43_dma_tx(struct b43_wldev *dev, st
 	 * static, so we don't need to store it per frame. */
 	ring->queue_prio = skb_get_queue_mapping(skb);
 
-	/* dma_tx_fragment might reallocate the skb, so invalidate pointers pointing
-	 * into the skb data or cb now. */
-	hdr = NULL;
-	info = NULL;
-	err = dma_tx_fragment(ring, &skb);
+	err = dma_tx_fragment(ring, skb);
 	if (unlikely(err == -ENOKEY)) {
 		/* Drop this packet, as we don't have the encryption key
 		 * anymore and must not transmit it unencrypted. */
@@ -1413,12 +1400,17 @@ void b43_dma_handle_txstatus(struct b43_
 		B43_WARN_ON(!(slot >= 0 && slot < ring->nr_slots));
 		desc = ops->idx2desc(ring, slot, &meta);
 
-		if (meta->skb)
-			unmap_descbuffer(ring, meta->dmaaddr, meta->skb->len,
-					 1);
-		else
+		if (meta->skb) {
+			struct b43_private_tx_info *priv_info =
+				b43_get_priv_tx_info(IEEE80211_SKB_CB(meta->skb));
+
+			unmap_descbuffer(ring, meta->dmaaddr, meta->skb->len, 1);
+			kfree(priv_info->bouncebuffer);
+			priv_info->bouncebuffer = NULL;
+		} else {
 			unmap_descbuffer(ring, meta->dmaaddr,
 					 b43_txhdr_size(dev), 1);
+		}
 
 		if (meta->is_last_fragment) {
 			struct ieee80211_tx_info *info;
Index: wireless-testing/drivers/net/wireless/b43/xmit.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/xmit.h	2009-11-06 16:55:02.000000000 +0100
+++ wireless-testing/drivers/net/wireless/b43/xmit.h	2009-11-06 18:25:56.000000000 +0100
@@ -2,6 +2,8 @@
 #define B43_XMIT_H_
 
 #include "main.h"
+#include <net/mac80211.h>
+
 
 #define _b43_declare_plcp_hdr(size) \
 	struct b43_plcp_hdr##size {		\
@@ -332,4 +334,21 @@ static inline u8 b43_kidx_to_raw(struct 
 	return raw_kidx;
 }
 
+/* struct b43_private_tx_info - TX info private to b43.
+ * The structure is placed in (struct ieee80211_tx_info *)->rate_driver_data
+ *
+ * @bouncebuffer: DMA Bouncebuffer (if used)
+ */
+struct b43_private_tx_info {
+	void *bouncebuffer;
+};
+
+static inline struct b43_private_tx_info *
+b43_get_priv_tx_info(struct ieee80211_tx_info *info)
+{
+	BUILD_BUG_ON(sizeof(struct b43_private_tx_info) >
+		     sizeof(info->rate_driver_data));
+	return (struct b43_private_tx_info *)info->rate_driver_data;
+}
+
 #endif /* B43_XMIT_H_ */

-- 
Greetings, Michael.

^ permalink raw reply

* Re: Changing the way we handle region codes on Linux (public thread)
From: Luis R. Rodriguez @ 2009-11-06 17:45 UTC (permalink / raw)
  To: Bob Copeland
  Cc: linux-wireless, Vivek Natarajan, Vivek Natarajan, Jeffrey Baker,
	John W. Linville, David Quan, Michael Green
In-Reply-To: <b6c5339f0911060929w78d16192o6c48e432e8e8c830@mail.gmail.com>

On Fri, Nov 6, 2009 at 9:29 AM, Bob Copeland <me@bobcopeland.com> wrote:
> On Fri, Nov 6, 2009 at 11:57 AM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>
>> I think that sums it up.
>
> I personally still like the idea of pushing the vendor-specific
> codes out to user space and having psuedo-country codes for
> those (e.g. "ATH_37").  Then the driver doesn't need all of the
> static rules loaded all the time and it would drop a lot of
> policy code from the driver.  CRDA could be enhanced to load
> multiple databases, one for pure iso-3166 codes, one with
> Atheros codes, one with Intel, etc.

There is obviously an added complexity added to userspace for this,
but it seems that having that complexity in userspace is better than
in kernel space. Only reason for not doing this I think would be
perhaps if we can really avoid such custom region codes.

Ultimately what makes more sense to me is to see more trust on the
user's location based on alternative inputs like geoclue but since
this year the Google Summer of Code project failed miserly even by the
midterm who knows when we'll be able to get proper feeds we can use in
the kernel.

But with that said -- I think the region-code scheme is overly complex
and am not sure if aiding it is something we should focus energy and
resources on. It would seem better to me to focus on more cleaner
solutions and leave that old stuff as legacy solutions.

  Luis

^ permalink raw reply

* Re: Prism54/p54pci
From: Christian Lamparter @ 2009-11-06 17:48 UTC (permalink / raw)
  To: James Grossmann; +Cc: linux-wireless
In-Reply-To: <167ae39b0911060900i2c8f4a78w4edecdcec36f1a38@mail.gmail.com>

On Friday 06 November 2009 18:00:54 James Grossmann wrote:
> It connects up with the patch!
> I'll attach a dmesg for any info you might be able to glean from that.
>  THANKS!

it's a start, but there are some serious implications.

but first: what is your signal (dBm) reading (iwconfig / iw dev wlanX info)?

I'm asking this because your device is only fine-tunable for
channel 1, 7, 13 & 14 with the current data points.

some performance figures would be nice as well.
e.g.: iperf (tcp) should be > 18Mbit/s,
      if you are close & free line of sight

second: what should we do about these devices?

Surely, the best thing would be to simply reprogram the eeprom,
but this (being complicated work) requires soldering skills/tools,
a steady hand and enough boredom. Therefor this is likely the least
ideal option to put into the WIKI @ wireless.kernel.org .
Any suitable ideas?

Regards,
	Chr

---
> [ 5593.622225] cfg80211: Calling CRDA to update world regulatory
> domain
> [ 5593.756965] p54pci 0000:03:00.0: PCI INT A -> Link[LNKA] -> GSI 11
> (level, low) -> IRQ 11
> [ 5593.757087] p54pci 0000:03:00.0: firmware: requesting isl3886pci
> [ 5593.799682] cfg80211: World regulatory domain updated:
> [ 5593.799693]  (start_freq - end_freq @ bandwidth),
> (max_antenna_gain, max_eirp)
> [ 5593.799701]  (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi,
> 2000 mBm)
> [ 5593.799708]  (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi,
> 2000 mBm)
> [ 5593.799714]  (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi,
> 2000 mBm)
> [ 5593.799721]  (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
> [ 5593.799728]  (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
> [ 5593.826106] phy0: p54 detected a LM86 firmware
> [ 5593.826115] p54: rx_mtu reduced from 3240 to 2376
> [ 5593.826121] phy0: FW rev 2.13.12.0 - Softmac protocol 5.9
> [ 5593.826127] phy0: cryptographic accelerator WEP:YES, TKIP:YES, CCMP:YES
> [ 5593.943095] phy0: hwaddr 00:xx:xx:xx:xx:xx, MAC:isl3880 RF:Frisbee
>
> [ 5593.943532] phy0: Selected rate control algorithm 'minstrel'
> [ 5593.945386] Registered led device: p54-phy0::assoc
> [ 5593.945426] Registered led device: p54-phy0::tx
> [ 5593.945466] Registered led device: p54-phy0::rx
> [ 5593.945507] Registered led device: p54-phy0::radio
> [ 5593.945521] p54pci 0000:03:00.0: is registered as 'phy0'
> [ 5593.973148] udev: renamed network interface wlan0 to wlan1
> [ 5594.105670] ADDRCONF(NETDEV_UP): wlan1: link is not ready
> [ 5601.380939] wlan1: deauthenticating from 00:xx:xx:xx:xx:xx by local
> choice (reason=3)
> [ 5601.381046] wlan1: direct probe to AP 00:xx:xx:xx:xx:xx (try 1)
> [ 5601.385723] wlan1: direct probe responded
> [ 5601.385738] wlan1: authenticate with AP 00:xx:xx:xx:xx:xx (try 1)
> [ 5601.387961] wlan1: authenticated
> [ 5601.388049] wlan1: associate with AP 00:xx:xx:xx:xx:xx (try 1)
> [ 5601.390237] wlan1: RX AssocResp from 00:xx:xx:xx:xx:xx (capab=0x401
> status=0 aid=2)
> [ 5601.390246] wlan1: associated
> [ 5601.391891] ADDRCONF(NETDEV_CHANGE): wlan1: link becomes ready
> [ 5612.024163] wlan1: no IPv6 routers present
> 

^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Ivo van Doorn @ 2009-11-06 17:58 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Ingo Molnar, Bartlomiej Zolnierkiewicz, linux-wireless,
	linux-kernel, netdev, Randy Dunlap, Luis Correia,
	John W. Linville, Johannes Berg, Jarek Poplawski, Pekka Enberg,
	David Miller
In-Reply-To: <20091106074643.GA5562@ucw.cz>

Hi,

> > > Look at the diffstat of Bart's driver:
> > > 
> > >    15 files changed, 4036 insertions(+), 7158 deletions(-)
> > > 
> > > He reduced your 5.2 KLOC non-working driver into a 1.8 KLOC _working_ 
> > > driver.
> > 
> > Bullshit, read the mails again.
> 
> This was uncalled for, right?

The way I said it, yes.

But I was frustrated by Ingo with his comment that the driver was magically fixed,
while even Bartlomiej indicated that was not the case. With these comments,
and those were he were his ignorance about the Ralink drivers in the kernel
was apparent, he managed to get me completely upset because I really
got the impression I was talking to a wall or something and overreacted.

> > > And _still_ your complaint about Bart's series is that he updated the 
> > > MAINTAINERS entry and added an entry for rt2800? Heck _sure_ he should 
> > > update it, he is the one doing the hard work of trying to bring it to 
> > > users, trying to clean up a messy driver space, trying to turn crap into 
> > > gold.
> > 
> > So if I want to focus on something different in the kernel, I just send 1 patch,
> > and a second to claim the maintainership of it even though there is an active
> > maintainer available?
> 
> What about listing yourself as a maintainer for a start?

The rt2x00 project is listed as maintainer for everything in the
drivers/net/wireless/rt2x00 folder.

> > > The thing is, if you dont have the time or interest to listen to and act 
> > > upon review feedback, be constructive about it and fix (obvious) 
> > > structural problems in your rt2800 code, you should just step aside and 
> > > let Bart maintain what he is apparently more capable of maintaining than 
> > > you are.
> > >
> > > What you are doing here is a thinly veiled land-grab: you did a minimal 
> > > token driver for rt2800 that doesnt work, kept it in your private tree 
> > > for _1.5 years_, and the moment someone _else_ came along and did 
> > > something better and more functional in drivers/staging/, you discovered 
> > > your sudden interest for it and moved the crappy driver upstream at 
> > > lightning's speed (it is already in net-next AFAICS, despite negative 
> > > test and review feedback) - ignoring and throwing away all the work that 
> > > Bart has done.
> > 
> > Get your facts straight, the bullshit level in your mail is staggering.
> > 
> > You have no fucking clue who wrote the rt2800 driver which is in
> > drivers/staging/,
> 
> Perhaps you should not be a maintainer if you can't behave yourself?

Ok let me explain clearer then:

drivers/net/staging/rt28{6,7}0 was developed by Ralink and I acked
the merged for those drivers after the asurance that it was only merged
to please the users so developers could focus on the rt2x00 version of
the driver.

Anyway, I'll do what John requested and go back into my hole since I
have absolutely no desire to continue this flamewar.

Ivo

^ permalink raw reply

* Re: [PATCH 02/41] rt2800pci: make Kconfig help entry more helpful
From: Bartlomiej Zolnierkiewicz @ 2009-11-06 16:13 UTC (permalink / raw)
  To: Gertjan van Wingerde
  Cc: linux-wireless, Ivo van Doorn, linux-kernel, John W. Linville
In-Reply-To: <14add3d10911041026r54a0abd1h953609dd7cb7224a@mail.gmail.com>

On Wednesday 04 November 2009 19:26:16 Gertjan van Wingerde wrote:
> On Wed, Nov 4, 2009 at 6:32 PM, Bartlomiej Zolnierkiewicz
> <bzolnier@gmail.com> wrote:
> > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > Subject: [PATCH] rt2800pci: make Kconfig help entry more helpful
> >
> > Document known issues with the driver to aid distribution makers,
> > users and developers in making informed decisions instead of wasting
> > their time needlessly.
> >
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> > This reflects the actual state of affairs for driver in net-next tree.
> >
> 
> Maybe we should consider making this one depend on BROKEN, for the
> moment at least?

Sounds fine with me, would you be willing to take care of this?

Thanks.
-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: [PATCH 10/41] rt2800pci: add rt2800_register_[read,write]() wrappers
From: Bartlomiej Zolnierkiewicz @ 2009-11-06 16:13 UTC (permalink / raw)
  To: Gertjan van Wingerde
  Cc: linux-wireless, Ivo van Doorn, linux-kernel, John W. Linville
In-Reply-To: <14add3d10911041116q1bb293a9hbd6c5361d59fe588@mail.gmail.com>

On Wednesday 04 November 2009 20:16:26 Gertjan van Wingerde wrote:
> On Wed, Nov 4, 2009 at 6:33 PM, Bartlomiej Zolnierkiewicz
> <bzolnier@gmail.com> wrote:
> > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > Subject: [PATCH] rt2800pci: add rt2800_register_[read,write]() wrappers
> >
> > Part of preparations for later code unification.
> >
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> >  drivers/net/wireless/rt2x00/rt2800pci.c |  479 ++++++++++++++++----------------
> >  drivers/net/wireless/rt2x00/rt2800pci.h |   21 +
> >  2 files changed, 261 insertions(+), 239 deletions(-)
> >
> > Index: b/drivers/net/wireless/rt2x00/rt2800pci.c
> > ===================================================================
> > --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> > +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> > @@ -57,7 +57,7 @@ MODULE_PARM_DESC(nohwcrypt, "Disable har
> >  /*
> >  * Register access.
> >  * All access to the CSR registers will go through the methods
> > - * rt2x00pci_register_read and rt2x00pci_register_write.
> > + * rt2800_register_read and rt2800_register_write.
> >  * BBP and RF register require indirect register access,
> >  * and use the CSR registers BBPCSR and RFCSR to achieve this.
> >  * These indirect registers work with busy bits,
> > @@ -66,6 +66,7 @@ MODULE_PARM_DESC(nohwcrypt, "Disable har
> >  * between each attampt. When the busy bit is still set at that time,
> >  * the access attempt is considered to have failed,
> >  * and we will print an error.
> > + * The _lock versions must be used if you already hold the csr_mutex
> >  */
> >  #define WAIT_FOR_BBP(__dev, __reg) \
> >        rt2x00pci_regbusy_read((__dev), BBP_CSR_CFG, BBP_CSR_CFG_BUSY, (__reg))
> 
> The change to the _lock variant seems a bit odd. See below.
> 
> <snip>
> 
> > Index: b/drivers/net/wireless/rt2x00/rt2800pci.h
> > ===================================================================
> > --- a/drivers/net/wireless/rt2x00/rt2800pci.h
> > +++ b/drivers/net/wireless/rt2x00/rt2800pci.h
> > @@ -27,6 +27,27 @@
> >  #ifndef RT2800PCI_H
> >  #define RT2800PCI_H
> >
> > +static inline void rt2800_register_read(struct rt2x00_dev *rt2x00dev,
> > +                                       const unsigned int offset,
> > +                                       u32 *value)
> > +{
> > +       rt2x00pci_register_read(rt2x00dev, offset, value);
> > +}
> > +
> > +static inline void rt2800_register_write(struct rt2x00_dev *rt2x00dev,
> > +                                        const unsigned int offset,
> > +                                        u32 value)
> > +{
> > +       rt2x00pci_register_write(rt2x00dev, offset, value);
> > +}
> > +
> > +static inline void rt2800_register_write_lock(struct rt2x00_dev *rt2x00dev,
> > +                                             const unsigned int offset,
> > +                                             u32 value)
> > +{
> > +       rt2x00pci_register_write(rt2x00dev, offset, value);
> > +}
> > +
> >  /*
> >  * RF chip defines.
> >  *
> 
> Can we add a comment to the _lock variant explaining that this one
> technically isn't
> needed, but is present for alignment purposes with rt2800usb?

I couldn't come with the good comment for it so I just went for
the minimal one in patch #25 (which removed all quoted above inlines):

+static const struct rt2800_ops rt2800pci_rt2800_ops = {
+       .register_read          = rt2x00pci_register_read,
+       .register_write         = rt2x00pci_register_write,
+       .register_write_lock    = rt2x00pci_register_write, /* same for PCI */
+
+       .register_multiread     = rt2x00pci_register_multiread,
+       .register_multiwrite    = rt2x00pci_register_multiwrite,
+
+       .regbusy_read           = rt2x00pci_regbusy_read,
+};

but it certainly can be expanded if somebody has a better idea how
the comment should look like.

-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: [PATCH 13/41] rt2800usb: add rt2800_regbusy_read() wrapper
From: Bartlomiej Zolnierkiewicz @ 2009-11-06 16:23 UTC (permalink / raw)
  To: Ivo van Doorn
  Cc: linux-wireless, Gertjan van Wingerde, linux-kernel,
	John W. Linville
In-Reply-To: <200911051949.28617.IvDoorn@gmail.com>

On Thursday 05 November 2009 19:49:28 Ivo van Doorn wrote:
> On Wednesday 04 November 2009, Bartlomiej Zolnierkiewicz wrote:
> > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > Subject: [PATCH] rt2800usb: add rt2800_regbusy_read() wrapper
> > 
> > Part of preparations for later code unification.
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 
> The macros are the same for rt2800pci and rt2800usb.
> So it might be better to move the entire define set into rt2800lib.h

Indeed, it wasn't possible before patches #13-14 but it is now.

Would you like to send an incremental (on top the series) fix for it?

Thanks.
-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: [PATCH 23/41] rt2x00: add driver private field to struct rt2x00_dev
From: Bartlomiej Zolnierkiewicz @ 2009-11-06 16:27 UTC (permalink / raw)
  To: Ivo van Doorn
  Cc: linux-wireless, Gertjan van Wingerde, linux-kernel,
	John W. Linville
In-Reply-To: <200911051957.06276.IvDoorn@gmail.com>

On Thursday 05 November 2009 19:57:05 Ivo van Doorn wrote:
> On Thursday 05 November 2009, Ivo van Doorn wrote:
> > On Wednesday 04 November 2009, Bartlomiej Zolnierkiewicz wrote:
> > > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > Subject: [PATCH] rt2x00: add driver private field to struct rt2x00_dev
> > > 
> > > Enhance rt2x00 infrastructure by adding driver specific field to
> > > struct rt2x00_dev.
> > > 
> > > The new field will be used by rt2800 drivers for chipset registers
> > > access abstraction layer.
> > > 
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > 
> > Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
> 
> Sorry I have a better idea, please the pointer into struct rt2x00_ops,
> that way you can assign it statically like the rest of that structure.
> That makes actually sense since all other function ops structures
> are assigned like that as well.

I thought about doing it this way initially but since:

- rt2800_ops are used by rt2800 drivers only and rt2x00dev->priv can be
  reused by other drivers for different purposes

- I couldn't come up with the good name for new rt2x00_ops field :)

I decided on the more flexible IMO for the whole rt2x00 stack solution
(however there is some minor difference between them that it can be
changed if you feel strongly about it).

-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: [PATCH 00/41] rewritten rt2800 drivers
From: Bartlomiej Zolnierkiewicz @ 2009-11-06 16:28 UTC (permalink / raw)
  To: Gertjan van Wingerde
  Cc: Luis R. Rodriguez, linux-wireless, Ivo van Doorn, linux-kernel,
	John W. Linville
In-Reply-To: <4AF340CE.5000709@gmail.com>

On Thursday 05 November 2009 22:17:02 Gertjan van Wingerde wrote:
> On 11/05/09 22:06, Luis R. Rodriguez wrote:
> > On Thu, Nov 5, 2009 at 12:59 PM, Gertjan van Wingerde
> > <gwingerde@gmail.com> wrote:
> >> On 11/04/09 21:19, Gertjan van Wingerde wrote:
> >>> On Wed, Nov 4, 2009 at 6:31 PM, Bartlomiej Zolnierkiewicz
> >>> <bzolnier@gmail.com> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> Here is the rt2800 rewrite in the form of patches (I've trimmed cc:
> >>>> list considerably since I'm not sure whether most people really want
> >>>> to see 40+ patches in their mailboxes).
> >>>>
> >>>>
> >>>> There were some minor changes since yesterday:
> >>>>
> >>>> - two new patches were added at the top of tree fixing Kconfig help
> >>>>  entries of rt2800[pci,usb] drivers to be more helpful and to prevent
> >>>>  wasting people's time (I think that patch #1 should go to Linus'
> >>>>  tree as soon as possible, ditto for patch #2 and net-next tree)
> >>>>
> >>>> - patch descriptions were improved for many patches
> >>>>  (suggestion from Ingo)
> >>>>
> >>>> - rt2x00_intf_is_[pci,usb]() helpers for commonly used checks were
> >>>>  added to "rt2x00: add support for different chipset interfaces" patch
> >>>>  (suggestion from Ivo)
> >>>>
> >>>> - addition of separate rt2800 MAINTAINERS entry was dropped for now
> >>>>  because it stirred needless controversies distracting people from
> >>>>  technical issues (thanks to Julian Calaby for pointing this to me),
> >>>>  the goal of change was to make sure that people won't bother busy
> >>>>  rt2x00 maintainers about rt2800 tree so it is not a big deal anyway
> >>>>
> >>>>
> >>>> The rt2800 tree has also been updated to reflect those changes
> >>>> (old branch is still available as rt2800-v1 for reference):
> >>>>
> >>>> The following changes since commit fa867e7355a1bdcd9bf7d55ebe9296f5b9c4028a:
> >>>>  Juuso Oikarinen (1):
> >>>>        wl1271: Generalize command response reading
> >>>>
> >>>> are available in the git repository at:
> >>>>
> >>>>  git://git.kernel.org/pub/scm/linux/kernel/git/bart/misc.git rt2800
> >>>>
> >>>> Bartlomiej Zolnierkiewicz (41):
> >>>>      rt2800usb: make Kconfig help entry more helpful
> >>>>      rt2800pci: make Kconfig help entry more helpful
> >>>>      rt2800usb: fix rt2800usb_rfcsr_read()
> >>>>      rt2800pci: fix crypto in TX frame
> >>>>      rt2800pci: fix comment about register access
> >>>>      rt2800pci: fix comment about IV/EIV fields
> >>>>      rt2x00: fix rt2x00usb_register_read() comment
> >>>>      rt2800usb: use rt2x00usb_register_multiwrite() to set key entries
> >>>>      rt2800usb: add rt2800_register_[read,write]() wrappers
> >>>>      rt2800pci: add rt2800_register_[read,write]() wrappers
> >>>>      rt2800usb: add rt2800_register_multi[read,write]() wrappers
> >>>>      rt2800pci: add rt2800_register_multi[read,write]() wrappers
> >>>>      rt2800usb: add rt2800_regbusy_read() wrapper
> >>>>      rt2800pci: add rt2800_regbusy_read() wrapper
> >>>>      rt2800usb: add rt2800_bbp_[read,write]() wrappers
> >>>>      rt2800pci: add rt2800_bbp_[read,write]() wrappers
> >>>>      rt2800usb: add rt2800_rfcsr_[read,write]() wrappers
> >>>>      rt2800pci: add rt2800_rfcsr_[read,write]() wrappers
> >>>>      rt2800usb: add rt2800_rf_[read,write]() wrappers
> >>>>      rt2800pci: add rt2800_rf_[read,write]() wrappers
> >>>>      rt2800usb: add rt2800_mcu_request() wrapper
> >>>>      rt2800pci: add rt2800_mcu_request() wrapper
> >>>>      rt2x00: add driver private field to struct rt2x00_dev
> >>>>      rt2800usb: convert to use struct rt2800_ops methods
> >>>>      rt2800pci: convert to use struct rt2800_ops methods
> >>>>      rt2x00: fix rt2x00usb_register_multiwrite() arguments
> >>>>      rt2x00: fix rt2x00usb_regbusy_read() arguments
> >>>>      rt2x00: fix rt2x00pci_register_multi[read,write]() arguments
> >>>>      rt2800: add rt2800lib.h
> >>>>      rt2800usb: fix comments in rt2800usb.h
> >>>>      rt2800usb: add RXINFO_DESC_SIZE definition
> >>>>      rt2800: fix duplication in header files
> >>>>      rt2800: fix comments in rt2800.h
> >>>>      rt2x00: add support for different chipset interfaces
> >>>>      rt2800: prepare for rt2800lib addition
> >>>>      rt2800: add rt2800lib (part one)
> >>>>      rt2x00: remove needless ifdefs from rt2x00leds.h
> >>>>      rt2800: add rt2800lib (part two)
> >>>>      rt2x00: move REGISTER_BUSY_* definitions to rt2x00.h
> >>>>      rt2800: add rt2800lib (part three)
> >>>>      rt2800: add rt2800lib (part four)
> >>>>
> >>>>  drivers/net/wireless/rt2x00/Kconfig      |   16 +-
> >>>>  drivers/net/wireless/rt2x00/Makefile     |    1 +
> >>>>  drivers/net/wireless/rt2x00/rt2800.h     | 1816 ++++++++++++++++++++++++++++
> >>>>  drivers/net/wireless/rt2x00/rt2800lib.c  | 1817 ++++++++++++++++++++++++++++
> >>>>  drivers/net/wireless/rt2x00/rt2800lib.h  |  134 +++
> >>>>  drivers/net/wireless/rt2x00/rt2800pci.c  | 1908 +++---------------------------
> >>>>  drivers/net/wireless/rt2x00/rt2800pci.h  | 1780 ----------------------------
> >>>>  drivers/net/wireless/rt2x00/rt2800usb.c  | 1828 ++---------------------------
> >>>>  drivers/net/wireless/rt2x00/rt2800usb.h  | 1818 +----------------------------
> >>>>  drivers/net/wireless/rt2x00/rt2x00.h     |   43 +
> >>>>  drivers/net/wireless/rt2x00/rt2x00leds.h |    4 -
> >>>>  drivers/net/wireless/rt2x00/rt2x00pci.h  |   24 +-
> >>>>  drivers/net/wireless/rt2x00/rt2x00usb.c  |    2 +-
> >>>>  drivers/net/wireless/rt2x00/rt2x00usb.h  |   17 +-
> >>>>  14 files changed, 4048 insertions(+), 7160 deletions(-)
> >>>>  create mode 100644 drivers/net/wireless/rt2x00/rt2800.h
> >>>>  create mode 100644 drivers/net/wireless/rt2x00/rt2800lib.c
> >>>>  create mode 100644 drivers/net/wireless/rt2x00/rt2800lib.h
> >>>>
> >>>
> >>> Hi Bart,
> >>>
> >>> Many thanks for these patches.
> >>>
> >>> So far I have been able to go through the first 30 patches of the
> >>> series. I'll finish the other 11 tomorrow (when I'm back home and have
> >>> better facilities to review these more elaborate patches of the
> >>> series).
> >>> They all look fine to me, I only had a comment for patch 10.
> >>>
> >>> I've sent my ACKs for the remaining 29 I reviewed.
> >>>
> >>> I guess Ivo still has to review them as well, as, so far, he has been
> >>> the one of the rt2x00 project to give the final ACK for inclusion,
> >>> unless he indicates that he is fine with my assessments.
> >>>
> >>> ---
> >>> Gertjan
> >>> rt2x00 developer
> >>>
> >>
> >> OK. I have now completed my review of the remaining patches and have sent my ACKs for those.
> > 
> > A little summary would be nice for those who don't want to read single
> > line acks/nacks.
> > 
> 
> I'm sorry. In my mind the message was clear that I Acked all of the remaining patches in the series.
> Apparently not :-(
> 
> So, basically, from my side all patches have been acked, with the exception of path 10 of the series (which was only a minor issue).

Thanks a lot for quick reaction and review!

I've updated relevant patches w/ your and Ivo's ACKs.

I have also small process related suggestion: if possible please group ACKs
together into the reply for 00/ mail (like you just did now), this way there
is a higher chance that I will not lose some of ACKs or review comments.

Thanks.
-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: [PATCH 00/41] rewritten rt2800 drivers
From: Bartlomiej Zolnierkiewicz @ 2009-11-06 18:15 UTC (permalink / raw)
  To: Julian Calaby
  Cc: Gertjan van Wingerde, linux-wireless, Ivo van Doorn, linux-kernel,
	John W. Linville
In-Reply-To: <646765f40911041455m7a6f55b8ka3ce95de7dd6ac6c@mail.gmail.com>

On Wednesday 04 November 2009 23:55:39 Julian Calaby wrote:
> Bart,
> 
> FWIW, this all looks good to me, except for these comments:
> 
> 1. When you introduce struct rt2800_ops, it may telegraph your
> intentions more clearly if you introduce rt2800lib.h at the same time
> - this also means that we don't have (if only for a single patch)
> duplicate versions of this structure and it's associated code.

The current order is mostly the result of incremental steps leading to
the final conclusions so indeed it can be polished a bit now.

> 2. Patches #26-28 should arguably come before the conversions to use
> the struct rt2800_ops methods.

Done, also the patch adding rt2800lib.h has been moved in front the ones
adding rt2800_ops.

[ I've kept all ACKs in affected patches, I hope people are fine with it. ]

> 3. I don't get the reasoning behind patch #37 (remove useless ifdefs
> from rt2x00leds.h) but I'm going to assume that it's all right.

struct rt2x00_led is referenced in rt2800lib.h so instead of adding more
ifdefs to fix build I removed needless ones.

> 4. Patch #39 should arguably come earlier in the patch set as it's a
> general cleanup.

This was also needed to fix build (for patch #40 IIRC).

I've moved #37 and #39 near the beginning of the patch series (after
"rt2x00: fix rt2x00usb_register_read() comment" patch).

The rt2800 tree has been updated to reflect above changes (rt2800-v2.1
branch is now the current one), if somebody would like to see patches
please ping me (I think that such minor updates don't justify spamming
mailing list w/ 41 patches but that's just me).

Thanks.
-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: [PATCH 13/41] rt2800usb: add rt2800_regbusy_read() wrapper
From: Ivo van Doorn @ 2009-11-06 18:20 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: linux-wireless, Gertjan van Wingerde, linux-kernel,
	John W. Linville
In-Reply-To: <200911061723.10099.bzolnier@gmail.com>

On Friday 06 November 2009, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 05 November 2009 19:49:28 Ivo van Doorn wrote:
> > On Wednesday 04 November 2009, Bartlomiej Zolnierkiewicz wrote:
> > > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > Subject: [PATCH] rt2800usb: add rt2800_regbusy_read() wrapper
> > > 
> > > Part of preparations for later code unification.
> > > 
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > 
> > The macros are the same for rt2800pci and rt2800usb.
> > So it might be better to move the entire define set into rt2800lib.h
> 
> Indeed, it wasn't possible before patches #13-14 but it is now.
> 
> Would you like to send an incremental (on top the series) fix for it?

Considering the amount of patches, and incremental patch is fine.
So you can just add my:
Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
for this patch. 

ivo

^ permalink raw reply

* Re: [PATCH 23/41] rt2x00: add driver private field to struct rt2x00_dev
From: Ivo van Doorn @ 2009-11-06 18:22 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: linux-wireless, Gertjan van Wingerde, linux-kernel,
	John W. Linville
In-Reply-To: <200911061727.50118.bzolnier@gmail.com>

On Friday 06 November 2009, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 05 November 2009 19:57:05 Ivo van Doorn wrote:
> > On Thursday 05 November 2009, Ivo van Doorn wrote:
> > > On Wednesday 04 November 2009, Bartlomiej Zolnierkiewicz wrote:
> > > > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > > Subject: [PATCH] rt2x00: add driver private field to struct rt2x00_dev
> > > > 
> > > > Enhance rt2x00 infrastructure by adding driver specific field to
> > > > struct rt2x00_dev.
> > > > 
> > > > The new field will be used by rt2800 drivers for chipset registers
> > > > access abstraction layer.
> > > > 
> > > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > > 
> > > Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
> > 
> > Sorry I have a better idea, please the pointer into struct rt2x00_ops,
> > that way you can assign it statically like the rest of that structure.
> > That makes actually sense since all other function ops structures
> > are assigned like that as well.
> 
> I thought about doing it this way initially but since:
> 
> - rt2800_ops are used by rt2800 drivers only and rt2x00dev->priv can be
>   reused by other drivers for different purposes
> 
> - I couldn't come up with the good name for new rt2x00_ops field :)
>
> I decided on the more flexible IMO for the whole rt2x00 stack solution
> (however there is some minor difference between them that it can be
> changed if you feel strongly about it).

Ok, keep it as it is now.

Ivo

^ permalink raw reply

* Re: [PATCH 35/41] rt2800: prepare for rt2800lib addition
From: Ivo van Doorn @ 2009-11-06 18:24 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: linux-wireless, Gertjan van Wingerde, linux-kernel,
	John W. Linville
In-Reply-To: <20091104173617.28463.6733.sendpatchset@localhost.localdomain>

Hi,

> @@ -2644,6 +2658,8 @@ static int rt2800usb_probe_hw(struct rt2
>  {
>  	int retval;
>  
> +	rt2x00_set_chip_intf(rt2x00dev, RT2X00_CHIP_INTF_USB);
> +
>  	rt2x00dev->priv = (void *)&rt2800usb_rt2800_ops;
>  
>  	/*
> 

Another afterthought, so perhaps you could make it an incremental patch.
Could you move the rt2x00_set_chip_intf() to rt2x00pci and rt2x00usb?
That way the field will be set automatically for all drivers early during probe.

Thanks,

Ivo

^ permalink raw reply

* Re: b43: firmware loading problem and sleeping BUG
From: Martin Fuzzey @ 2009-11-06 18:27 UTC (permalink / raw)
  To: Michael Buesch; +Cc: linux-wireless
In-Reply-To: <200911061822.17121.mb@bu3sch.de>

On Fri, Nov 6, 2009 at 6:22 PM, Michael Buesch <mb@bu3sch.de> wrote:
> These logs look weird. Is that the full log from "dmesg" command?
Yes - I've just stripped the timestamps

> I guess the pcmcia_access_configuration_register() is doing nonatomic vmalloc stuff.
>
Indeed looks like its iounmap() that eventually gets called from there.

> I think this is easy to fix, because we can replace the spinlock by a mutex, as
> the b43 driver (which is the only user of the code) always allows sleeping now.
> I'll send a patch for testing soon.
>
Great - happy to test it.

>> b43-phy0: Loading firmware version 410.2160 (2007-05-26 15:32:10)
>> b43-phy0 ERROR: Initial Values Firmware file-format error.
>> b43-phy0 ERROR: You must go to
>>
>> Any ideas why the versions don't match?
>
> These are two different version numbers. One for the broadcom driver
> and one for the firmware.
>
Ok so I do have the correct firmware.

I seem to be having very non deterministic behaviour regarding
the firmware loading.

Sometimes I get the behaviour above (which I trace to the first test
in b43_write_initvals().

And sometimes I get this behaviour:

pcmcia driver load:
[  102.850000] ssb: Sonics Silicon Backplane found on PCMCIA device pcmcia0.0

[  102.880000] b43-phy0: Broadcom 4318 WLAN found (core revision 9)

[  102.990000] b43-phy0 debug: Found PHY: Analog 3, Type 2, Revision 7

[  103.010000] b43-phy0 debug: Found Radio: Manuf 0x17F, Version
0x2050, Revision 8

[  105.610000] phy0: Selected rate control algorithm 'minstrel'

[  105.650000] Broadcom 43xx driver loaded [ Features: M, Firmware-ID: FW13 ]


First ifconfig wlan0 up (note it's trying to use the open firmware -
which I don't have):
[  108.770000] b43 ssb0:0: firmware: requesting b43/ucode5.fw

[  108.910000] b43 ssb0:0: firmware: requesting b43/pcm5.fw

[  109.170000] b43 ssb0:0: firmware: requesting b43-open/ucode5.fw

[  109.330000] b43 ssb0:0: firmware: requesting b43-open/pcm5.fw

[  109.520000] b43 ssb0:0: firmware: requesting b43-open/b0g0initvals5.fw

[  109.690000] b43-phy0 ERROR: Firmware file "b43/pcm5.fw" format error.

[  109.760000] b43-phy0 ERROR: Firmware file
"b43-open/b0g0initvals5.fw" not found

[  109.960000] b43-phy0 ERROR: You must go to
http://wireless.kernel.org/en/users/Drivers/b43#devicefirmware and
download the correct firmware for this driver version. Please
carefully read all instructions on this website.


Second ifconfig wlan0 up (no reboot):
[  157.730000] b43 ssb0:0: firmware: requesting b43/ucode5.fw

[  157.860000] b43 ssb0:0: firmware: requesting b43/pcm5.fw

[  158.040000] b43 ssb0:0: firmware: requesting b43/b0g0initvals5.fw

[  158.310000] b43 ssb0:0: firmware: requesting b43/b0g0bsinitvals5.fw

[  158.940000] b43-phy0: Loading firmware version 410.2160 (2007-05-26 15:32:10)

[  159.010000] b43-phy0 debug: Chip initialized

[  159.030000] b43-phy0 debug: PIO initialized

[  159.040000] b43-phy0 debug: QoS enabled

[  159.070000] b43-phy0 debug: Wireless interface started

[  159.130000] b43-phy0 debug: Adding Interface type 2


Yay - and iwlist scanning works (haven't tried associating yet)

And sometimes even after mutiple ups it gives up and the microcode
doesn't reply...

So any suggestions on figuring out what is going on?
>
> Thanks for testing. The ssb-pcmcia code is not tested a lot, so
> I'm not surprised that there are bugs.
>
You're welcome

Cheers,

Martin

^ permalink raw reply

* Re: b43: firmware loading problem and sleeping BUG
From: Michael Buesch @ 2009-11-06 18:31 UTC (permalink / raw)
  To: Martin Fuzzey; +Cc: linux-wireless
In-Reply-To: <ba4215e10911061027t413d2c34g8ad66a7982f7c43c@mail.gmail.com>

On Friday 06 November 2009 19:27:17 Martin Fuzzey wrote:
> [  109.690000] b43-phy0 ERROR: Firmware file "b43/pcm5.fw" format error.

> So any suggestions on figuring out what is going on?

Your userspace firmware helper (I guess it's not udev on your device?)
might be throwing crap at the kernel.

-- 
Greetings, Michael.

^ permalink raw reply

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

On Friday 06 November 2009 18:58:56 Ivo van Doorn wrote:

> drivers/net/staging/rt28{6,7}0 was developed by Ralink and I acked

The original vendor drivers were:
- rt2860
- rt2870
- rt3070
- rt3090

Each weighting ~100 KLOC.

The current staging drivers (rt2860 w/ RT3090 support and rt2870 w/ RT3070
support) are the result of my work on getting staging drivers under control
(+ trying some new strategies of dealing with ugly code) and weight ~75 KLOC
_together_ (they share the wireless stack code).

> the merged for those drivers after the asurance that it was only merged
> to please the users so developers could focus on the rt2x00 version of
> the driver.

Could somebody please explain me (in the public or in the private) what is
the reason behind whole affair about staging drivers because all the time
I feel like I'm missing some important detail here.  It would a lot more
productive than all the things that I could hear about my agenda, my work
or my intellectual abilities in the past.

[ Like I said before I got only interested into them in April this year
  while doing casual staging cleanups and I'm not affiliated with any
  distribution vendor. ]

Thanks.
-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: Changing the way we handle region codes on Linux (public thread)
From: John W. Linville @ 2009-11-06 18:38 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Bob Copeland, linux-wireless, Vivek Natarajan, Vivek Natarajan,
	Jeffrey Baker, David Quan, Michael Green
In-Reply-To: <43e72e890911060945s3b00e056u1efe483bd88ebd30@mail.gmail.com>

On Fri, Nov 06, 2009 at 09:45:29AM -0800, Luis R. Rodriguez wrote:
> On Fri, Nov 6, 2009 at 9:29 AM, Bob Copeland <me@bobcopeland.com> wrote:
> > On Fri, Nov 6, 2009 at 11:57 AM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> >
> >> I think that sums it up.
> >
> > I personally still like the idea of pushing the vendor-specific
> > codes out to user space and having psuedo-country codes for
> > those (e.g. "ATH_37").  Then the driver doesn't need all of the
> > static rules loaded all the time and it would drop a lot of
> > policy code from the driver.  CRDA could be enhanced to load
> > multiple databases, one for pure iso-3166 codes, one with
> > Atheros codes, one with Intel, etc.

<snip>

> But with that said -- I think the region-code scheme is overly complex
> and am not sure if aiding it is something we should focus energy and
> resources on. It would seem better to me to focus on more cleaner
> solutions and leave that old stuff as legacy solutions.

That's the thing about "legacy" stuff -- it doesn't go away just from
ignoring it!

FWIW, I think Bob's suggestion makes a lot of sense.

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


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