[PATCH] rtl8187se staging: Fix compilation warnings and procfs directory leak

[PATCH] rtl8187se staging: Fix compilation warnings and procfs directory leak

[Larry Finger]

Fix some warnings during compilation and correct a programming error
that was leaking a directory in /proc.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Tested-by: Bernhard Schiffner <bernhard@schiffner-limbach.de>
---

Greg,

Please incorporate this patch in wireless/staging as soon as is possible.
I'm not sure what the rules are concerning such changes.

I have a number of patches that clean up the vendor code; however, I think
I will hold them for the moment as they do not change the function of this
driver and only improve the readability.

I am now working on porting this driver to use mac80211 so that it may be
included in mainline.

Larry
---

Index: linux-2.6/drivers/staging/rtl8187se/r8180.h
===================================================================
--- linux-2.6.orig/drivers/staging/rtl8187se/r8180.h
+++ linux-2.6/drivers/staging/rtl8187se/r8180.h
@@ -19,7 +19,7 @@
 #define R8180H
 
 
-#define RTL8180_MODULE_NAME "rtl8180"
+#define RTL8180_MODULE_NAME "r8180"
 #define DMESG(x,a...) printk(KERN_INFO RTL8180_MODULE_NAME ": " x "\n", ## a)
 #define DMESGW(x,a...) printk(KERN_WARNING RTL8180_MODULE_NAME ": WW:" x "\n", ## a)
 #define DMESGE(x,a...) printk(KERN_WARNING RTL8180_MODULE_NAME ": EE:" x "\n", ## a)
Index: linux-2.6/drivers/staging/rtl8187se/r8180_core.c
===================================================================
--- linux-2.6.orig/drivers/staging/rtl8187se/r8180_core.c
+++ linux-2.6/drivers/staging/rtl8187se/r8180_core.c
@@ -640,11 +640,9 @@ void rtl8180_proc_init_one(struct net_de
 {
 	struct proc_dir_entry *e;
 	struct r8180_priv *priv = (struct r8180_priv *)ieee80211_priv(dev);
-	priv->dir_dev = create_proc_entry(dev->name,
-					  S_IFDIR | S_IRUGO | S_IXUGO,
-					  rtl8180_proc);
+	priv->dir_dev = rtl8180_proc;
 	if (!priv->dir_dev) {
-		DMESGE("Unable to initialize /proc/net/rtl8180/%s\n",
+		DMESGE("Unable to initialize /proc/net/r8180/%s\n",
 		      dev->name);
 		return;
 	}
@@ -654,7 +652,7 @@ void rtl8180_proc_init_one(struct net_de
 
 	if (!e) {
 		DMESGE("Unable to initialize "
-		      "/proc/net/rtl8180/%s/stats-hw\n",
+		      "/proc/net/r8180/%s/stats-hw\n",
 		      dev->name);
 	}
 
@@ -663,7 +661,7 @@ void rtl8180_proc_init_one(struct net_de
 
 	if (!e) {
 		DMESGE("Unable to initialize "
-		      "/proc/net/rtl8180/%s/stats-rx\n",
+		      "/proc/net/r8180/%s/stats-rx\n",
 		      dev->name);
 	}
 
@@ -673,7 +671,7 @@ void rtl8180_proc_init_one(struct net_de
 
 	if (!e) {
 		DMESGE("Unable to initialize "
-		      "/proc/net/rtl8180/%s/stats-tx\n",
+		      "/proc/net/r8180/%s/stats-tx\n",
 		      dev->name);
 	}
 	#if 0
@@ -702,7 +700,7 @@ void rtl8180_proc_init_one(struct net_de
 
 	if (!e) {
 		DMESGE("Unable to initialize "
-		      "/proc/net/rtl8180/%s/registers\n",
+		      "/proc/net/r8180/%s/registers\n",
 		      dev->name);
 	}
 }
@@ -977,13 +975,6 @@ void check_tx_ring(struct net_device *de
 			      *tmp & (1<<15)? "ok": "err", *(tmp+4));
 	}
 
-	DMESG("nic at %d",
-		(nic-nicbegin) / 8 /4);
-	DMESG("tail at %d", ((int)tail - (int)begin) /8 /4);
-	DMESG("head at %d", ((int)head - (int)begin) /8 /4);
-	DMESG("check free desc returns %d", check_nic_enought_desc(dev,pri));
-	DMESG("free desc is %d\n", get_curr_tx_free_desc(dev,pri));
-	//rtl8180_reset(dev);
 	return;
 }
 
@@ -1736,17 +1727,7 @@ short alloc_tx_desc_ring(struct net_devi
 		 * descriptor's buffer must be 256 byte aligned
 		 * we shouldn't be here, since we set DMA mask !
 		 */
-		DMESGW("Fixing TX alignment");
-		desc = (u32*)((u8*)desc + 256);
-#if (defined(CONFIG_HIGHMEM64G) || defined(CONFIG_64BIT_PHYS_ADDR))
-		desc = (u32*)((u64)desc &~ 0xff);
-		dma_desc = (dma_addr_t)((u8*)dma_desc + 256);
-		dma_desc = (dma_addr_t)((u64)dma_desc &~ 0xff);
-#else
-		desc = (u32*)((u32)desc &~ 0xff);
-		dma_desc = (dma_addr_t)((u8*)dma_desc + 256);
-		dma_desc = (dma_addr_t)((u32)dma_desc &~ 0xff);
-#endif
+		WARN(1, "DMA buffer is not aligned\n");
 	}
 	tmp=desc;
 	for (i=0;i<count;i++)
@@ -1984,18 +1965,7 @@ short alloc_rx_desc_ring(struct net_devi
 		 * descriptor's buffer must be 256 byte aligned
 		 * should never happen since we specify the DMA mask
 		 */
-
-		DMESGW("Fixing RX alignment");
-		desc = (u32*)((u8*)desc + 256);
-#if (defined(CONFIG_HIGHMEM64G) || defined(CONFIG_64BIT_PHYS_ADDR))
-		desc = (u32*)((u64)desc &~ 0xff);
-		dma_desc = (dma_addr_t)((u8*)dma_desc + 256);
-		dma_desc = (dma_addr_t)((u64)dma_desc &~ 0xff);
-#else
-		desc = (u32*)((u32)desc &~ 0xff);
-		dma_desc = (dma_addr_t)((u8*)dma_desc + 256);
-		dma_desc = (dma_addr_t)((u32)dma_desc &~ 0xff);
-#endif
+		WARN(1, "DMA buffer is not aligned\n");
 	}
 
 	priv->rxring=desc;

Re: [PATCH] rtl8187se staging: Fix compilation warnings and procfs directory leak

[Greg KH]

On Sat, Apr 18, 2009 at 09:09:08PM -0500, Larry Finger wrote:
> Fix some warnings during compilation and correct a programming error
> that was leaking a directory in /proc.
> 
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Tested-by: Bernhard Schiffner <bernhard@schiffner-limbach.de>
> ---
> 
> Greg,
> 
> Please incorporate this patch in wireless/staging as soon as is possible.
> I'm not sure what the rules are concerning such changes.

I take stuff through my staging tree, it has nothing to do with the
wireless tree.

> I have a number of patches that clean up the vendor code; however, I think
> I will hold them for the moment as they do not change the function of this
> driver and only improve the readability.

Why wait?

> I am now working on porting this driver to use mac80211 so that it may be
> included in mainline.

That's great!

A few questions:

> --- linux-2.6.orig/drivers/staging/rtl8187se/r8180.h
> +++ linux-2.6/drivers/staging/rtl8187se/r8180.h
> @@ -19,7 +19,7 @@
>  #define R8180H
>  
>  
> -#define RTL8180_MODULE_NAME "rtl8180"
> +#define RTL8180_MODULE_NAME "r8180"

Why change the name?

> --- linux-2.6.orig/drivers/staging/rtl8187se/r8180_core.c
> +++ linux-2.6/drivers/staging/rtl8187se/r8180_core.c
> @@ -640,11 +640,9 @@ void rtl8180_proc_init_one(struct net_de
>  {
>  	struct proc_dir_entry *e;
>  	struct r8180_priv *priv = (struct r8180_priv *)ieee80211_priv(dev);
> -	priv->dir_dev = create_proc_entry(dev->name,
> -					  S_IFDIR | S_IRUGO | S_IXUGO,
> -					  rtl8180_proc);
> +	priv->dir_dev = rtl8180_proc;
>  	if (!priv->dir_dev) {
> -		DMESGE("Unable to initialize /proc/net/rtl8180/%s\n",
> +		DMESGE("Unable to initialize /proc/net/r8180/%s\n",
>  		      dev->name);
>  		return;
>  	}

So put the files in the root proc dir?

> @@ -1736,17 +1727,7 @@ short alloc_tx_desc_ring(struct net_devi
>  		 * descriptor's buffer must be 256 byte aligned
>  		 * we shouldn't be here, since we set DMA mask !
>  		 */
> -		DMESGW("Fixing TX alignment");
> -		desc = (u32*)((u8*)desc + 256);
> -#if (defined(CONFIG_HIGHMEM64G) || defined(CONFIG_64BIT_PHYS_ADDR))
> -		desc = (u32*)((u64)desc &~ 0xff);
> -		dma_desc = (dma_addr_t)((u8*)dma_desc + 256);
> -		dma_desc = (dma_addr_t)((u64)dma_desc &~ 0xff);
> -#else
> -		desc = (u32*)((u32)desc &~ 0xff);
> -		dma_desc = (dma_addr_t)((u8*)dma_desc + 256);
> -		dma_desc = (dma_addr_t)((u32)dma_desc &~ 0xff);
> -#endif
> +		WARN(1, "DMA buffer is not aligned\n");
>  	}
>  	tmp=desc;
>  	for (i=0;i<count;i++)

What replaces this logic?  Is it not needed anymore?

thanks,

greg k-h

Re: [PATCH] rtl8187se staging: Fix compilation warnings and procfs directory leak

[Larry Finger]

Greg KH wrote:
> On Sat, Apr 18, 2009 at 09:09:08PM -0500, Larry Finger wrote:
>> Please incorporate this patch in wireless/staging as soon as is possible.
>> I'm not sure what the rules are concerning such changes.
> 
> I take stuff through my staging tree, it has nothing to do with the
> wireless tree.

That was a slip of the keyboard.

>> I have a number of patches that clean up the vendor code; however, I think
>> I will hold them for the moment as they do not change the function of this
>> driver and only improve the readability.
> 
> Why wait?

The only reason is that I'm hoping that the port will allow the staging driver
to be deleted. If you want the bigger changes now, I would be happy to oblige.
FYI, the bigger patches will change 14 files with a total of 233 insertions and
1930 deletions.

>> -#define RTL8180_MODULE_NAME "rtl8180"
>> +#define RTL8180_MODULE_NAME "r8180"
> 
> Why change the name?

I want to distinguish this one from the mainline driver for the RTL8180/RTL8185
that uses the name "rtl8180". Perhaps rtl8187se would have been better; however,
that is the name I will use for the new mainline driver.

>> --- linux-2.6.orig/drivers/staging/rtl8187se/r8180_core.c
>> +++ linux-2.6/drivers/staging/rtl8187se/r8180_core.c
>> @@ -640,11 +640,9 @@ void rtl8180_proc_init_one(struct net_de
>>  {
>>  	struct proc_dir_entry *e;
>>  	struct r8180_priv *priv = (struct r8180_priv *)ieee80211_priv(dev);
>> -	priv->dir_dev = create_proc_entry(dev->name,
>> -					  S_IFDIR | S_IRUGO | S_IXUGO,
>> -					  rtl8180_proc);
>> +	priv->dir_dev = rtl8180_proc;
>>  	if (!priv->dir_dev) {
>> -		DMESGE("Unable to initialize /proc/net/rtl8180/%s\n",
>> +		DMESGE("Unable to initialize /proc/net/r8180/%s\n",
>>  		      dev->name);
>>  		return;
>>  	}
> 
> So put the files in the root proc dir?

Sure, if you think that would be better. The problem that I am fixing is that
the vendor code put the files in /proc/net/rtl8180/wlan0/XXX and then failed to
delete the wlan0 directory. I wanted to make the minimum changes for it to work
correctly.

>> @@ -1736,17 +1727,7 @@ short alloc_tx_desc_ring(struct net_devi
>>  		 * descriptor's buffer must be 256 byte aligned
>>  		 * we shouldn't be here, since we set DMA mask !
>>  		 */
>> -		DMESGW("Fixing TX alignment");
>> -		desc = (u32*)((u8*)desc + 256);
>> -#if (defined(CONFIG_HIGHMEM64G) || defined(CONFIG_64BIT_PHYS_ADDR))
>> -		desc = (u32*)((u64)desc &~ 0xff);
>> -		dma_desc = (dma_addr_t)((u8*)dma_desc + 256);
>> -		dma_desc = (dma_addr_t)((u64)dma_desc &~ 0xff);
>> -#else
>> -		desc = (u32*)((u32)desc &~ 0xff);
>> -		dma_desc = (dma_addr_t)((u8*)dma_desc + 256);
>> -		dma_desc = (dma_addr_t)((u32)dma_desc &~ 0xff);
>> -#endif
>> +		WARN(1, "DMA buffer is not aligned\n");
>>  	}
>>  	tmp=desc;
>>  	for (i=0;i<count;i++)
> 
> What replaces this logic?  Is it not needed anymore?

AFAIK, it was only needed if the kernel's DMA memory allocation failed. As we
know, that code is now perfect ;), I thought that a simple kernel warning would
be sufficient. Besides, I'm not smart enough to get the old code to compile
without warnings on x86_64 architecture. I'm not sure about i386 machines. So
far in testing, the warning has not triggered.

Larry