[PATCH] WLAN acx100: OOPS fix, KERN_xxx, misc.

[PATCH] WLAN acx100: OOPS fix, KERN_xxx, misc.

[Andreas Mohr]

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

Hi all (and especially Denis),

acx-20060116_misc.diff:
- fix OOPS in acx_l_rxmonitor() (wrong ndev->type setting leading to memcpy
  of negative size) by reworking monitor mode type setup and adding
  missing sanity checks
- rename acx1XX_ie_powermgmt_t to more correct acx1XX_ie_powersave_t
- fix format string compile warnings
- add <linux/compiler.h> include apparently required for __iomem define
  around Linux 2.6.8
- rework eCPU init by replacing static msleep with faster, more intelligent
  (hopefully) busy-wait

acx-20060116_KERN_xxx.diff (too large: gzipped):
- add proper KERN_xxx prefixes to printk() as requested recently

Note that both patches are based on acx-20060116 proper (rediffed from
acx-20060113), smallish conflicts may result; apply acx-20060116_KERN_xxx.diff after acx-20060116_misc.diff.

Andreas Mohr

[-- Attachment #2: acx-20060116_misc.diff --]
[-- Type: text/plain, Size: 9559 bytes --]

diff -urN acx-20060116.orig/acx_struct.h acx-20060116_misc/acx_struct.h
--- acx-20060116.orig/acx_struct.h	2006-01-15 12:03:38.000000000 +0100
+++ acx-20060116_misc/acx_struct.h	2006-01-16 14:12:04.000000000 +0100
@@ -1202,6 +1202,7 @@
 	u8		ap[ETH_ALEN];		/* The AP we want, FF:FF:FF:FF:FF:FF is any */
 	u16		aid;			/* The Association ID sent from the AP / last used AID if we're an AP */
 	u16		mode;			/* mode from iwconfig */
+	int		monitor_type;		/* ARPHRD_IEEE80211 or ARPHRD_IEEE80211_PRISM */
 	u16		status;			/* 802.11 association status */
 	u8		essid_active;		/* specific ESSID active, or select any? */
 	u8		essid_len;		/* to avoid dozens of strlen() */
@@ -1623,7 +1624,7 @@
 #define PS_OPT_TX_PSPOLL	0x02 /* send PSPoll frame to fetch waiting frames from AP (on frame with matching AID) */
 #define PS_OPT_STILL_RCV_BCASTS	0x01
 
-typedef struct acx100_ie_powermgmt {
+typedef struct acx100_ie_powersave {
 	u16	type ACX_PACKED;
 	u16	len ACX_PACKED;
 	u8	wakeup_cfg ACX_PACKED;
@@ -1631,9 +1632,9 @@
 	u8	options ACX_PACKED;
 	u8	hangover_period ACX_PACKED; /* remaining wake time after Tx MPDU w/ PS bit, in values of 1/1024 seconds */
 	u16	enhanced_ps_transition_time ACX_PACKED; /* rem. wake time for Enh. PS */
-} acx100_ie_powermgmt_t;
+} acx100_ie_powersave_t;
 
-typedef struct acx111_ie_powermgmt {
+typedef struct acx111_ie_powersave {
 	u16	type ACX_PACKED;
 	u16	len ACX_PACKED;
 	u8	wakeup_cfg ACX_PACKED;
@@ -1642,7 +1643,7 @@
 	u8	hangover_period ACX_PACKED; /* remaining wake time after Tx MPDU w/ PS bit, in values of 1/1024 seconds */
 	u32	beacon_rx_time ACX_PACKED;
 	u32	enhanced_ps_transition_time ACX_PACKED; /* rem. wake time for Enh. PS */
-} acx111_ie_powermgmt_t;
+} acx111_ie_powersave_t;
 
 
 /***********************************************************************
diff -urN acx-20060116.orig/common.c acx-20060116_misc/common.c
--- acx-20060116.orig/common.c	2006-01-15 12:38:43.000000000 +0100
+++ acx-20060116_misc/common.c	2006-01-16 14:22:02.000000000 +0100
@@ -183,7 +183,7 @@
 		diff -= adev->lock_time;
 		if (diff > max_lock_time) {
 			where = sanitize_str(where);
-			printk("max lock hold time %d CPU ticks from %s "
+			printk("max lock hold time %ld CPU ticks from %s "
 				"to %s\n", diff, adev->last_lock, where);
 			max_lock_time = diff;
 		}
@@ -230,7 +230,7 @@
 		unsigned long diff = jiffies - adev->sem_time;
 		if (diff > max_sem_time) {
 			where = sanitize_str(where);
-			printk("max sem hold time %d jiffies from %s "
+			printk("max sem hold time %ld jiffies from %s "
 				"to %s\n", diff, adev->last_sem, where);
 			max_sem_time = diff;
 		}
@@ -838,7 +838,7 @@
 acx100_ie_len[] = {
 	0,
 	ACX100_IE_ACX_TIMER_LEN,
-	sizeof(acx100_ie_powermgmt_t)-4, /* is that 6 or 8??? */
+	sizeof(acx100_ie_powersave_t)-4, /* is that 6 or 8??? */
 	ACX1xx_IE_QUEUE_CONFIG_LEN,
 	ACX100_IE_BLOCK_SIZE_LEN,
 	ACX1xx_IE_MEMORY_CONFIG_OPTIONS_LEN,
@@ -889,7 +889,7 @@
 acx111_ie_len[] = {
 	0,
 	ACX100_IE_ACX_TIMER_LEN,
-	sizeof(acx111_ie_powermgmt_t)-4,
+	sizeof(acx111_ie_powersave_t)-4,
 	ACX1xx_IE_QUEUE_CONFIG_LEN,
 	ACX100_IE_BLOCK_SIZE_LEN,
 	ACX1xx_IE_MEMORY_CONFIG_OPTIONS_LEN,
@@ -2156,6 +2156,7 @@
 	adev->listen_interval = 100;
 	adev->beacon_interval = DEFAULT_BEACON_INTERVAL;
 	adev->mode = ACX_MODE_2_STA;
+	adev->monitor_type = ARPHRD_IEEE80211_PRISM;
 	adev->dtim_interval = DEFAULT_DTIM_INTERVAL;
 
 	adev->msdu_lifetime = DEFAULT_MSDU_LIFETIME;
@@ -2347,10 +2348,11 @@
 
 	skb_put(skb, skb_len);
 
+	if (adev->ndev->type == ARPHRD_IEEE80211) {
 		/* when in raw 802.11 mode, just copy frame as-is */
-	if (adev->ndev->type == ARPHRD_IEEE80211)
 		datap = skb->data;
-	else { /* otherwise, emulate prism header */
+	} else if (adev->ndev->type == ARPHRD_IEEE80211_PRISM) {
+		/* emulate prism header */
 		msg = (wlansniffrm_t*)skb->data;
 		datap = msg + 1;
 
@@ -2410,8 +2412,20 @@
 		msg->frmlen.status = WLANITEM_STATUS_data_ok;
 		msg->frmlen.len = 4;
 		msg->frmlen.data = skb_len;
+	} else {
+		printk(KERN_ERR "unsupported netdev type %d!\n",
+			adev->ndev->type);
+		dev_kfree_skb(skb);
+		return;
 	}
 
+	/* sanity check (keep it here) */
+	if (unlikely((int)skb_len < 0))
+	{
+		printk(KERN_ERR "skb_len bug (%d)!! Aborting...\n", (int)skb_len);
+		dev_kfree_skb(skb);
+		return;
+	}
 	memcpy(datap, ((unsigned char*)rxbuf)+payload_offset, skb_len);
 
 	skb->dev = adev->ndev;
@@ -2425,6 +2439,7 @@
 
 	adev->stats.rx_packets++;
 	adev->stats.rx_bytes += skb->len;
+
 end:
 	FN_EXIT0;
 }
@@ -5816,8 +5831,8 @@
 {
 	/* merge both structs in a union to be able to have common code */
 	union {
-		acx111_ie_powermgmt_t acx111;
-		acx100_ie_powermgmt_t acx100;
+		acx111_ie_powersave_t acx111;
+		acx100_ie_powersave_t acx100;
 	} pm;
 
 	/* change 802.11 power save mode settings */
@@ -6246,7 +6261,8 @@
 	}
 
 	if (adev->set_mask & GETSET_MODE) {
-		adev->ndev->type = ARPHRD_ETHER;
+		adev->ndev->type = (adev->mode == ACX_MODE_MONITOR) ?
+			adev->monitor_type : ARPHRD_ETHER;
 
 		switch (adev->mode) {
 		case ACX_MODE_3_AP:
@@ -6265,9 +6281,6 @@
 			acx_s_cmd_join_bssid(adev, adev->bssid);
 			break;
 		case ACX_MODE_MONITOR:
-			/* adev->ndev->type = ARPHRD_ETHER; */
-			/* adev->ndev->type = ARPHRD_IEEE80211; */
-			adev->ndev->type = ARPHRD_IEEE80211_PRISM;
 			acx111_s_feature_on(adev, 0, FEATURE2_NO_TXCRYPT|FEATURE2_SNIFFER);
 			/* this stops beacons */
 			acx_s_cmd_join_bssid(adev, adev->bssid);
diff -urN acx-20060116.orig/ioctl.c acx-20060116_misc/ioctl.c
--- acx-20060116.orig/ioctl.c	2006-01-15 12:02:26.000000000 +0100
+++ acx-20060116_misc/ioctl.c	2006-01-16 14:21:46.000000000 +0100
@@ -2078,13 +2078,14 @@
 
 	switch (params[0]) {
 	case 0:
-		adev->ndev->type = ARPHRD_ETHER;
+		/* no monitor mode. hmm, should we simply ignore it
+		 * or go back to enabling adev->netdev->type ARPHRD_ETHER? */
 		break;
 	case 1:
-		adev->ndev->type = ARPHRD_IEEE80211_PRISM;
+		adev->monitor_type = ARPHRD_IEEE80211_PRISM;
 		break;
 	case 2:
-		adev->ndev->type = ARPHRD_IEEE80211;
+		adev->monitor_type = ARPHRD_IEEE80211;
 		break;
 	}
 
diff -urN acx-20060116.orig/pci.c acx-20060116_misc/pci.c
--- acx-20060116.orig/pci.c	2006-01-15 12:01:41.000000000 +0100
+++ acx-20060116_misc/pci.c	2006-01-16 14:21:13.000000000 +0100
@@ -33,6 +33,7 @@
 
 #include <linux/config.h>
 #include <linux/version.h>
+#include <linux/compiler.h> /* required for Lx 2.6.8 ?? */
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
@@ -354,8 +355,9 @@
 		write_flush(adev);
 		write_reg32(adev, IO_ACX_EEPROM_CTL, 1);
 
+		count = 0xffff;
 		while (read_reg16(adev, IO_ACX_EEPROM_CTL)) {
-			if (unlikely(++count > 0xffff)) {
+			if (unlikely(!--count)) {
 				printk("WARNING, DANGER!!! "
 					"Timeout waiting for EEPROM write\n");
 				goto end;
@@ -369,13 +371,13 @@
 	write_flush(adev);
 
 	/* now start a verification run */
-	count = 0xffff;
 	for (i = 0; i < len; i++) {
 		write_reg32(adev, IO_ACX_EEPROM_CFG, 0);
 		write_reg32(adev, IO_ACX_EEPROM_ADDR, addr + i);
 		write_flush(adev);
 		write_reg32(adev, IO_ACX_EEPROM_CTL, 2);
 
+		count = 0xffff;
 		while (read_reg16(adev, IO_ACX_EEPROM_CTL)) {
 			if (unlikely(!--count)) {
 				printk("timeout waiting for EEPROM read\n");
@@ -747,18 +749,17 @@
 	temp = read_reg16(adev, IO_ACX_ECPU_CTRL) | 0x1;
 	write_reg16(adev, IO_ACX_ECPU_CTRL, temp);
 
-	/* now do soft reset of eCPU */
+	/* now do soft reset of eCPU, set bit */
 	temp = read_reg16(adev, IO_ACX_SOFT_RESET) | 0x1;
 	log(L_DEBUG, "%s: enable soft reset...\n", __func__);
 	write_reg16(adev, IO_ACX_SOFT_RESET, temp);
 	write_flush(adev);
 
-	/* now reset bit again */
+	/* now clear bit again: deassert eCPU reset */
 	log(L_DEBUG, "%s: disable soft reset and go to init mode...\n", __func__);
-	/* deassert eCPU reset */
 	write_reg16(adev, IO_ACX_SOFT_RESET, temp & ~0x1);
 
-	/* now start a burst read from initial flash EEPROM */
+	/* now start a burst read from initial EEPROM */
 	temp = read_reg16(adev, IO_ACX_EE_START) | 0x1;
 	write_reg16(adev, IO_ACX_EE_START, temp);
 	write_flush(adev);
@@ -893,6 +894,7 @@
 	int result = NOT_OK;
 	u16 hardware_info;
 	u16 ecpu_ctrl;
+	int count;
 
 	FN_ENTER;
 
@@ -927,12 +929,24 @@
 
 	acx_unlock(adev, flags);
 
-	/* without this delay acx100 may fail to report hardware_info
-	** (see below). Most probably eCPU runs some init code */
-	acx_s_msleep(10);
-
 	/* need to know radio type before fw load */
-	hardware_info = read_reg16(adev, IO_ACX_EEPROM_INFORMATION);
+ 	/* Need to wait for arrival of this information in a loop,
+ 	 * most probably since eCPU runs some init code from EEPROM
+ 	 * (started burst read in reset_mac()) which also
+ 	 * sets the radio type ID */
+ 
+ 	count = 0xffff;
+ 	do {
+ 		hardware_info = read_reg16(adev, IO_ACX_EEPROM_INFORMATION);
+ 		if (!--count)
+ 		{
+ 			msg = "eCPU didn't indicate radio type";
+ 			goto end_fail;
+ 		}
+ 		cpu_relax();
+ 	} while (!(hardware_info & 0xff00)); /* radio type still zero? */
+ 
+ 	/* printk("DEBUG: count %d\n", count); */
 	adev->form_factor = hardware_info & 0xff;
 	adev->radio_type = hardware_info >> 8;
 
@@ -940,11 +954,11 @@
 	if (OK != acxpci_s_upload_fw(adev))
 		goto end_fail;
 
-	acx_s_msleep(10);
+	/* acx_s_msleep(10);	this one really shouldn't be required */
 
 	/* now start eCPU by clearing bit */
-	log(L_DEBUG, "booted eCPU up and waiting for completion...\n");
 	write_reg16(adev, IO_ACX_ECPU_CTRL, ecpu_ctrl & ~0x1);
+	log(L_DEBUG, "booted eCPU up and waiting for completion...\n");
 
 	/* wait for eCPU bootup */
 	if (OK != acxpci_s_verify_init(adev)) {

[-- Attachment #3: acx-20060116_KERN_xxx.diff.gz --]
[-- Type: application/x-gzip, Size: 18122 bytes --]

Re: [PATCH] WLAN acx100: OOPS fix, KERN_xxx, misc.

[Denis Vlasenko]

On Monday 16 January 2006 16:02, Andreas Mohr wrote:
> Hi all (and especially Denis),
> 
> acx-20060116_misc.diff:
> - fix OOPS in acx_l_rxmonitor() (wrong ndev->type setting leading to memcpy
>   of negative size) by reworking monitor mode type setup and adding
>   missing sanity checks
> - rename acx1XX_ie_powermgmt_t to more correct acx1XX_ie_powersave_t
> - fix format string compile warnings
> - add <linux/compiler.h> include apparently required for __iomem define
>   around Linux 2.6.8
> - rework eCPU init by replacing static msleep with faster, more intelligent
>   (hopefully) busy-wait
> 
> acx-20060116_KERN_xxx.diff (too large: gzipped):

Applied, thanks!

> - add proper KERN_xxx prefixes to printk() as requested recently

Please forward me that request.
--
vda


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click