[PATCH 1/6] pcmcia/3c574_cs: Fix dubious bitfield warning

[PATCH 1/6] pcmcia/3c574_cs: Fix dubious bitfield warning

[Richard Knutsson]

Fixing:
  CHECK   drivers/net/pcmcia/3c574_cs.c
drivers/net/pcmcia/3c574_cs.c:194:13: warning: dubious bitfield without explicit `signed' or `unsigned'
drivers/net/pcmcia/3c574_cs.c:196:14: warning: dubious bitfield without explicit `signed' or `unsigned'

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---
Is there a reason for not doing it this way?


diff --git a/drivers/net/pcmcia/3c574_cs.c b/drivers/net/pcmcia/3c574_cs.c
index ad134a6..97b6daa 100644
--- a/drivers/net/pcmcia/3c574_cs.c
+++ b/drivers/net/pcmcia/3c574_cs.c
@@ -190,10 +190,10 @@ enum Window3 {			/* Window 3: MAC/config bits. */
 union wn3_config {
 	int i;
 	struct w3_config_fields {
-		unsigned int ram_size:3, ram_width:1, ram_speed:2, rom_size:2;
-		int pad8:8;
-		unsigned int ram_split:2, pad18:2, xcvr:3, pad21:1, autoselect:1;
-		int pad24:7;
+		u8 ram_size:3, ram_width:1, ram_speed:2, rom_size:2;
+		u8 pad8;
+		u8 ram_split:2, pad18:2, xcvr:3, pad21:1;
+		u8 autoselect:1, pad24:7;
 	} u;
 };
 

[PATCH 2/6] pcmcia/3c574_cs: Fix 'shadow variable' warning

[Richard Knutsson]

Fixing:
  CHECK   drivers/net/pcmcia/3c574_cs.c
drivers/net/pcmcia/3c574_cs.c:695:7: warning: symbol 'i' shadows an earlier one
drivers/net/pcmcia/3c574_cs.c:636:6: originally declared here

Signed-off-by: Richard Knutson <ricknu-0@student.ltu.se>
---


diff --git a/drivers/net/pcmcia/3c574_cs.c b/drivers/net/pcmcia/3c574_cs.c
index ad134a6..97b6daa 100644
--- a/drivers/net/pcmcia/3c574_cs.c
+++ b/drivers/net/pcmcia/3c574_cs.c
@@ -692,7 +692,7 @@ static void tc574_reset(struct net_device *dev)
 	mdio_write(ioaddr, lp->phys, 4, lp->advertising);
 	if (!auto_polarity) {
 		/* works for TDK 78Q2120 series MII's */
-		int i = mdio_read(ioaddr, lp->phys, 16) | 0x20;
+		i = mdio_read(ioaddr, lp->phys, 16) | 0x20;
 		mdio_write(ioaddr, lp->phys, 16, i);
 	}
 

[PATCH 3/6] pcmcia/axnet_cs: Make functions static

[Richard Knutsson]

Fixing:
  CHECK   drivers/net/pcmcia/axnet_cs.c
drivers/net/pcmcia/axnet_cs.c:994:5: warning: symbol 'ax_close' was not declared. Should it be static?
drivers/net/pcmcia/axnet_cs.c:1017:6: warning: symbol 'ei_tx_timeout' was not declared. Should it be static?

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---


diff --git a/drivers/net/pcmcia/axnet_cs.c b/drivers/net/pcmcia/axnet_cs.c
index 8d910a3..96931cc 100644
--- a/drivers/net/pcmcia/axnet_cs.c
+++ b/drivers/net/pcmcia/axnet_cs.c
@@ -991,7 +991,7 @@ static int ax_open(struct net_device *dev)
  *
  * Opposite of ax_open(). Only used when "ifconfig <devname> down" is done.
  */
-int ax_close(struct net_device *dev)
+static int ax_close(struct net_device *dev)
 {
 	unsigned long flags;
 
@@ -1014,7 +1014,7 @@ int ax_close(struct net_device *dev)
  * completed (or failed) - i.e. never posted a Tx related interrupt.
  */
 
-void ei_tx_timeout(struct net_device *dev)
+static void ei_tx_timeout(struct net_device *dev)
 {
 	long e8390_base = dev->base_addr;
 	struct ei_device *ei_local = (struct ei_device *) netdev_priv(dev);

[PATCH 4/6] pcmcia/axnet_cs: Make use of 'max()' instead of handcrafted one

[Richard Knutsson]

Use 'max(x,y)' instead of 'x < y ? y : x'.

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---


diff --git a/drivers/net/pcmcia/axnet_cs.c b/drivers/net/pcmcia/axnet_cs.c
index 8d910a3..96931cc 100644
--- a/drivers/net/pcmcia/axnet_cs.c
+++ b/drivers/net/pcmcia/axnet_cs.c
@@ -1091,8 +1091,8 @@ static int ei_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	
 	ei_local->irqlock = 1;
 
-	send_length = ETH_ZLEN < length ? length : ETH_ZLEN;
-	
+	send_length = max(length, ETH_ZLEN);
+
 	/*
 	 * We have two Tx slots available for use. Find the first free
 	 * slot, and then perform some sanity checks. With two Tx bufs,

[PATCH 5/6] pcmcia/fmvj18x_cs: Fix 'shadow variable' warning

[Richard Knutsson]

Fixing:
  CHECK   drivers/net/pcmcia/fmvj18x_cs.c
drivers/net/pcmcia/fmvj18x_cs.c:1205:6: warning: symbol 'i' shadows an earlier one
drivers/net/pcmcia/fmvj18x_cs.c:1179:9: originally declared here

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---


diff --git a/drivers/net/pcmcia/fmvj18x_cs.c b/drivers/net/pcmcia/fmvj18x_cs.c
index 8c719b4..4f604ae 100644
--- a/drivers/net/pcmcia/fmvj18x_cs.c
+++ b/drivers/net/pcmcia/fmvj18x_cs.c
@@ -1202,8 +1202,7 @@ static void set_rx_mode(struct net_device *dev)
 	outb(1, ioaddr + RX_MODE);	/* Ignore almost all multicasts. */
     } else {
 	struct dev_mc_list *mclist;
-	int i;
-	
+
 	memset(mc_filter, 0, sizeof(mc_filter));
 	for (i = 0, mclist = dev->mc_list; mclist && i < dev->mc_count;
 	     i++, mclist = mclist->next) {

[PATCH 6/6] pcmcia/pcnet_cs: Fix 'shadow variable' warning

[Richard Knutsson]

Fixing:
  CHECK   drivers/net/pcmcia/pcnet_cs.c
drivers/net/pcmcia/pcnet_cs.c:523:15: warning: symbol 'hw_info' shadows an earlier one
drivers/net/pcmcia/pcnet_cs.c:148:18: originally declared here

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---


diff --git a/drivers/net/pcmcia/pcnet_cs.c b/drivers/net/pcmcia/pcnet_cs.c
index db6a97d..5779344 100644
--- a/drivers/net/pcmcia/pcnet_cs.c
+++ b/drivers/net/pcmcia/pcnet_cs.c
@@ -520,7 +520,7 @@ static int pcnet_config(struct pcmcia_device *link)
     int i, last_ret, last_fn, start_pg, stop_pg, cm_offset;
     int has_shmem = 0;
     u_short buf[64];
-    hw_info_t *hw_info;
+    hw_info_t *local_hw_info;
     DECLARE_MAC_BUF(mac);
 
     DEBUG(0, "pcnet_config(0x%p)\n", link);
@@ -589,23 +589,23 @@ static int pcnet_config(struct pcmcia_device *link)
 	dev->if_port = 0;
     }
 
-    hw_info = get_hwinfo(link);
-    if (hw_info == NULL)
-	hw_info = get_prom(link);
-    if (hw_info == NULL)
-	hw_info = get_dl10019(link);
-    if (hw_info == NULL)
-	hw_info = get_ax88190(link);
-    if (hw_info == NULL)
-	hw_info = get_hwired(link);
-
-    if (hw_info == NULL) {
+    local_hw_info = get_hwinfo(link);
+    if (local_hw_info == NULL)
+	local_hw_info = get_prom(link);
+    if (local_hw_info == NULL)
+	local_hw_info = get_dl10019(link);
+    if (local_hw_info == NULL)
+	local_hw_info = get_ax88190(link);
+    if (local_hw_info == NULL)
+	local_hw_info = get_hwired(link);
+
+    if (local_hw_info == NULL) {
 	printk(KERN_NOTICE "pcnet_cs: unable to read hardware net"
 	       " address for io base %#3lx\n", dev->base_addr);
 	goto failed;
     }
 
-    info->flags = hw_info->flags;
+    info->flags = local_hw_info->flags;
     /* Check for user overrides */
     info->flags |= (delay_output) ? DELAY_OUTPUT : 0;
     if ((link->manf_id == MANFID_SOCKET) &&

Re: [PATCH 1/6] pcmcia/3c574_cs: Fix dubious bitfield warning

[Alan Cox]

On Tue, 11 Dec 2007 05:32:38 +0100 (MET)
Richard Knutsson <ricknu-0@student.ltu.se> wrote:

> Fixing:
>   CHECK   drivers/net/pcmcia/3c574_cs.c
> drivers/net/pcmcia/3c574_cs.c:194:13: warning: dubious bitfield without explicit `signed' or `unsigned'
> drivers/net/pcmcia/3c574_cs.c:196:14: warning: dubious bitfield without explicit `signed' or `unsigned'
> 
> Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
> ---
> Is there a reason for not doing it this way?

How is the endianness handled here (I suspect its always been broken)

> diff --git a/drivers/net/pcmcia/3c574_cs.c b/drivers/net/pcmcia/3c574_cs.c
> index ad134a6..97b6daa 100644
> --- a/drivers/net/pcmcia/3c574_cs.c
> +++ b/drivers/net/pcmcia/3c574_cs.c
> @@ -190,10 +190,10 @@ enum Window3 {			/* Window 3: MAC/config bits. */
>  union wn3_config {
>  	int i;
>  	struct w3_config_fields {
> -		unsigned int ram_size:3, ram_width:1, ram_speed:2, rom_size:2;
> -		int pad8:8;
> -		unsigned int ram_split:2, pad18:2, xcvr:3, pad21:1, autoselect:1;
> -		int pad24:7;
> +		u8 ram_size:3, ram_width:1, ram_speed:2, rom_size:2;
> +		u8 pad8;
> +		u8 ram_split:2, pad18:2, xcvr:3, pad21:1;
> +		u8 autoselect:1, pad24:7;

Just changing the int pad to unsigned int pad would be safer in terms of
not causing changes. Simply delcaring a 32bit field and bit masks to
and/or into it is probably a lot saner in the general case.

Re: [PATCH 2/6] pcmcia/3c574_cs: Fix 'shadow variable' warning

[Alan Cox]

On Tue, 11 Dec 2007 05:32:43 +0100 (MET)
Richard Knutsson <ricknu-0@student.ltu.se> wrote:

> Fixing:
>   CHECK   drivers/net/pcmcia/3c574_cs.c
> drivers/net/pcmcia/3c574_cs.c:695:7: warning: symbol 'i' shadows an earlier one
> drivers/net/pcmcia/3c574_cs.c:636:6: originally declared here
> 
> Signed-off-by: Richard Knutson <ricknu-0@student.ltu.se>

For 2/3/4/5/6:

Acked-by: Alan Cox <alan@redhat.com>

Not sure I agree with the first patch though.

Re: [PATCH 1/6] pcmcia/3c574_cs: Fix dubious bitfield warning

[Richard Knutsson]

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

Alan Cox wrote:
> On Tue, 11 Dec 2007 05:32:38 +0100 (MET)
> Richard Knutsson <ricknu-0@student.ltu.se> wrote:
>
>   
>> Fixing:
>>   CHECK   drivers/net/pcmcia/3c574_cs.c
>> drivers/net/pcmcia/3c574_cs.c:194:13: warning: dubious bitfield without explicit `signed' or `unsigned'
>> drivers/net/pcmcia/3c574_cs.c:196:14: warning: dubious bitfield without explicit `signed' or `unsigned'
>>
>> Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
>> ---
>> Is there a reason for not doing it this way?
>>     
>
> How is the endianness handled here (I suspect its always been broken)
>   
Guess so, but it should not handle the endians differently with such a 
change, does it?
>   
>> diff --git a/drivers/net/pcmcia/3c574_cs.c b/drivers/net/pcmcia/3c574_cs.c
>> index ad134a6..97b6daa 100644
>> --- a/drivers/net/pcmcia/3c574_cs.c
>> +++ b/drivers/net/pcmcia/3c574_cs.c
>> @@ -190,10 +190,10 @@ enum Window3 {			/* Window 3: MAC/config bits. */
>>  union wn3_config {
>>  	int i;
>>  	struct w3_config_fields {
>> -		unsigned int ram_size:3, ram_width:1, ram_speed:2, rom_size:2;
>> -		int pad8:8;
>> -		unsigned int ram_split:2, pad18:2, xcvr:3, pad21:1, autoselect:1;
>> -		int pad24:7;
>> +		u8 ram_size:3, ram_width:1, ram_speed:2, rom_size:2;
>> +		u8 pad8;
>> +		u8 ram_split:2, pad18:2, xcvr:3, pad21:1;
>> +		u8 autoselect:1, pad24:7;
>>     
>
> Just changing the int pad to unsigned int pad would be safer in terms of
> not causing changes. Simply delcaring a 32bit field and bit masks to
> and/or into it is probably a lot saner in the general case.
>   
Thought about it before and with the endian-issue it seem like a better 
solution.
So, something like this?


[-- Attachment #2: drivers_net_pcmcia_3c574_cs.c-dubious-bitfield.patch --]
[-- Type: text/x-patch, Size: 1062 bytes --]

Fixing:
  CHECK   drivers/net/pcmcia/3c574_cs.c
drivers/net/pcmcia/3c574_cs.c:194:13: warning: dubious bitfield without explicit `signed' or `unsigned'
drivers/net/pcmcia/3c574_cs.c:196:14: warning: dubious bitfield without explicit `signed' or `unsigned'

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
---
Moved 'autoselect' to the last line to make every line 8 bits.
Is there a reason for not doing it this way?


diff --git a/drivers/net/pcmcia/3c574_cs.c b/drivers/net/pcmcia/3c574_cs.c
index ad134a6..e549427 100644
--- a/drivers/net/pcmcia/3c574_cs.c
+++ b/drivers/net/pcmcia/3c574_cs.c
@@ -190,10 +190,10 @@ enum Window3 {			/* Window 3: MAC/config bits. */
 union wn3_config {
 	int i;
 	struct w3_config_fields {
-		unsigned int ram_size:3, ram_width:1, ram_speed:2, rom_size:2;
-		int pad8:8;
-		unsigned int ram_split:2, pad18:2, xcvr:3, pad21:1, autoselect:1;
-		int pad24:7;
+		u32 ram_size:3, ram_width:1, ram_speed:2, rom_size:2,
+		    pad8:8,
+		    ram_split:2, pad18:2, xcvr:3, pad21:1,
+		    autoselect:1, pad24:7;
 	} u;
 };