* [PATCH 1/6] pcmcia/3c574_cs: Fix dubious bitfield warning
@ 2007-12-11 4:32 ` Richard Knutsson
0 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:32 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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;
};
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 2/6] pcmcia/3c574_cs: Fix 'shadow variable' warning
2007-12-11 4:32 ` Richard Knutsson
@ 2007-12-11 4:32 ` Richard Knutsson
-1 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:32 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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);
}
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 2/6] pcmcia/3c574_cs: Fix 'shadow variable' warning
@ 2007-12-11 4:32 ` Richard Knutsson
0 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:32 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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);
}
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 2/6] pcmcia/3c574_cs: Fix 'shadow variable' warning
2007-12-11 4:32 ` Richard Knutsson
@ 2007-12-11 13:22 ` Alan Cox
-1 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2007-12-11 13:22 UTC (permalink / raw)
To: Richard Knutsson; +Cc: linux-pcmcia, kernel-janitors, linux-kernel
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] pcmcia/3c574_cs: Fix 'shadow variable' warning
@ 2007-12-11 13:22 ` Alan Cox
0 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2007-12-11 13:22 UTC (permalink / raw)
To: Richard Knutsson
Cc: linux-pcmcia, kernel-janitors, linux-kernel, Richard Knutsson
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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/6] pcmcia/axnet_cs: Make functions static
2007-12-11 4:32 ` Richard Knutsson
@ 2007-12-11 4:32 ` Richard Knutsson
-1 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:32 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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);
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 3/6] pcmcia/axnet_cs: Make functions static
@ 2007-12-11 4:32 ` Richard Knutsson
0 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:32 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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);
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/6] pcmcia/axnet_cs: Make use of 'max()' instead of handcrafted one
2007-12-11 4:32 ` Richard Knutsson
@ 2007-12-11 4:32 ` Richard Knutsson
-1 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:32 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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,
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/6] pcmcia/axnet_cs: Make use of 'max()' instead of handcrafted one
@ 2007-12-11 4:32 ` Richard Knutsson
0 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:32 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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,
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/6] pcmcia/fmvj18x_cs: Fix 'shadow variable' warning
2007-12-11 4:32 ` Richard Knutsson
@ 2007-12-11 4:32 ` Richard Knutsson
-1 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:32 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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) {
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 5/6] pcmcia/fmvj18x_cs: Fix 'shadow variable' warning
@ 2007-12-11 4:32 ` Richard Knutsson
0 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:32 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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) {
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/6] pcmcia/pcnet_cs: Fix 'shadow variable' warning
2007-12-11 4:32 ` Richard Knutsson
@ 2007-12-11 4:33 ` Richard Knutsson
-1 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:33 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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) &&
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 6/6] pcmcia/pcnet_cs: Fix 'shadow variable' warning
@ 2007-12-11 4:33 ` Richard Knutsson
0 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-11 4:33 UTC (permalink / raw)
To: linux-pcmcia; +Cc: kernel-janitors, linux-kernel, 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) &&
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/6] pcmcia/3c574_cs: Fix dubious bitfield warning
2007-12-11 4:32 ` Richard Knutsson
@ 2007-12-11 13:19 ` Alan Cox
-1 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2007-12-11 13:19 UTC (permalink / raw)
To: Richard Knutsson; +Cc: linux-pcmcia, kernel-janitors, linux-kernel
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.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 1/6] pcmcia/3c574_cs: Fix dubious bitfield warning
@ 2007-12-11 13:19 ` Alan Cox
0 siblings, 0 replies; 18+ messages in thread
From: Alan Cox @ 2007-12-11 13:19 UTC (permalink / raw)
To: Richard Knutsson
Cc: linux-pcmcia, kernel-janitors, linux-kernel, Richard Knutsson
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.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 1/6] pcmcia/3c574_cs: Fix dubious bitfield warning
2007-12-11 13:19 ` Alan Cox
@ 2007-12-12 1:40 ` Richard Knutsson
-1 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-12 1:40 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-pcmcia, kernel-janitors, linux-kernel
[-- 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;
};
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 1/6] pcmcia/3c574_cs: Fix dubious bitfield warning
@ 2007-12-12 1:40 ` Richard Knutsson
0 siblings, 0 replies; 18+ messages in thread
From: Richard Knutsson @ 2007-12-12 1:40 UTC (permalink / raw)
To: Alan Cox; +Cc: linux-pcmcia, kernel-janitors, linux-kernel
[-- 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;
};
^ permalink raw reply related [flat|nested] 18+ messages in thread