Fix some warnings during compilation and correct a programming error
that was leaking a directory in /proc.
Signed-off-by: Larry Finger <[email protected]>
Tested-by: Bernhard Schiffner <[email protected]>
---
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;
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 <[email protected]>
> Tested-by: Bernhard Schiffner <[email protected]>
> ---
>
> 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
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