2005-02-19 08:34:57

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] drivers/net/smc-mca.c: cleanups

This patch contains the following cleanups:
- make a needlessly global function static
- make three needlessly global structs static

Since after moving the now-static stucts to smc-mca.c the file smc-mca.h
was empty except for two #define's, I've also killed the rest of
smc-mca.h .

Signed-off-by: Adrian Bunk <[email protected]>

---

drivers/net/smc-mca.c | 60 +++++++++++++++++++++++++++++++++++++++--
drivers/net/smc-mca.h | 61 ------------------------------------------
2 files changed, 58 insertions(+), 63 deletions(-)

--- linux-2.6.11-rc3-mm2-full/drivers/net/smc-mca.h 2004-12-24 22:35:23.000000000 +0100
+++ /dev/null 2004-11-25 03:16:25.000000000 +0100
@@ -1,61 +0,0 @@
-/*
- * djweis [email protected]
- * most of this file was taken from ps2esdi.h
- */
-
-struct {
- unsigned int base_addr;
-} addr_table[] = {
- { 0x0800 },
- { 0x1800 },
- { 0x2800 },
- { 0x3800 },
- { 0x4800 },
- { 0x5800 },
- { 0x6800 },
- { 0x7800 },
- { 0x8800 },
- { 0x9800 },
- { 0xa800 },
- { 0xb800 },
- { 0xc800 },
- { 0xd800 },
- { 0xe800 },
- { 0xf800 }
-};
-
-#define MEM_MASK 64
-
-struct {
- unsigned char mem_index;
- unsigned long mem_start;
- unsigned char num_pages;
-} mem_table[] = {
- { 16, 0x0c0000, 40 },
- { 18, 0x0c4000, 40 },
- { 20, 0x0c8000, 40 },
- { 22, 0x0cc000, 40 },
- { 24, 0x0d0000, 40 },
- { 26, 0x0d4000, 40 },
- { 28, 0x0d8000, 40 },
- { 30, 0x0dc000, 40 },
- {144, 0xfc0000, 40 },
- {148, 0xfc8000, 40 },
- {154, 0xfd0000, 40 },
- {156, 0xfd8000, 40 },
- { 0, 0x0c0000, 20 },
- { 1, 0x0c2000, 20 },
- { 2, 0x0c4000, 20 },
- { 3, 0x0c6000, 20 }
-};
-
-#define IRQ_MASK 243
-struct {
- unsigned char new_irq;
- unsigned char old_irq;
-} irq_table[] = {
- { 3, 3 },
- { 4, 4 },
- { 10, 10 },
- { 14, 15 }
-};
--- linux-2.6.11-rc3-mm2-full/drivers/net/smc-mca.c.old 2005-02-16 18:44:29.000000000 +0100
+++ linux-2.6.11-rc3-mm2-full/drivers/net/smc-mca.c 2005-02-16 18:47:24.000000000 +0100
@@ -49,7 +49,6 @@
#include <asm/system.h>

#include "8390.h"
-#include "smc-mca.h"

#define DRV_NAME "smc-mca"

@@ -100,6 +99,63 @@
MODULE_PARM_DESC(ultra_io, "SMC Ultra/EtherEZ MCA I/O base address(es)");
MODULE_PARM_DESC(ultra_irq, "SMC Ultra/EtherEZ MCA IRQ number(s)");

+static struct {
+ unsigned int base_addr;
+} addr_table[] = {
+ { 0x0800 },
+ { 0x1800 },
+ { 0x2800 },
+ { 0x3800 },
+ { 0x4800 },
+ { 0x5800 },
+ { 0x6800 },
+ { 0x7800 },
+ { 0x8800 },
+ { 0x9800 },
+ { 0xa800 },
+ { 0xb800 },
+ { 0xc800 },
+ { 0xd800 },
+ { 0xe800 },
+ { 0xf800 }
+};
+
+#define MEM_MASK 64
+
+static struct {
+ unsigned char mem_index;
+ unsigned long mem_start;
+ unsigned char num_pages;
+} mem_table[] = {
+ { 16, 0x0c0000, 40 },
+ { 18, 0x0c4000, 40 },
+ { 20, 0x0c8000, 40 },
+ { 22, 0x0cc000, 40 },
+ { 24, 0x0d0000, 40 },
+ { 26, 0x0d4000, 40 },
+ { 28, 0x0d8000, 40 },
+ { 30, 0x0dc000, 40 },
+ {144, 0xfc0000, 40 },
+ {148, 0xfc8000, 40 },
+ {154, 0xfd0000, 40 },
+ {156, 0xfd8000, 40 },
+ { 0, 0x0c0000, 20 },
+ { 1, 0x0c2000, 20 },
+ { 2, 0x0c4000, 20 },
+ { 3, 0x0c6000, 20 }
+};
+
+#define IRQ_MASK 243
+static struct {
+ unsigned char new_irq;
+ unsigned char old_irq;
+} irq_table[] = {
+ { 3, 3 },
+ { 4, 4 },
+ { 10, 10 },
+ { 14, 15 }
+};
+
static short smc_mca_adapter_ids[] __initdata = {
0x61c8,
0x61c9,
@@ -126,7 +182,7 @@

static int ultra_found = 0;

-int __init ultramca_probe(struct device *gen_dev)
+static int __init ultramca_probe(struct device *gen_dev)
{
unsigned short ioaddr;
struct net_device *dev;


2005-02-19 08:44:59

by Jeff Garzik

[permalink] [raw]
Subject: Re: [2.6 patch] drivers/net/smc-mca.c: cleanups

Adrian Bunk wrote:
> This patch contains the following cleanups:
> - make a needlessly global function static
> - make three needlessly global structs static
>
> Since after moving the now-static stucts to smc-mca.c the file smc-mca.h
> was empty except for two #define's, I've also killed the rest of
> smc-mca.h .

It looks like the structs should be 'static const', not just 'static'.

This comment is applicable to similar changes, also. Use 'const'
whenever possible.

Jeff



2005-02-19 09:20:17

by Jeff Garzik

[permalink] [raw]
Subject: Re: [2.6 patch] drivers/net/smc-mca.c: cleanups

Arjan van de Ven wrote:
> On Sat, 2005-02-19 at 03:41 -0500, Jeff Garzik wrote:
>
>>Adrian Bunk wrote:
>>
>>>This patch contains the following cleanups:
>>>- make a needlessly global function static
>>>- make three needlessly global structs static
>>>
>>>Since after moving the now-static stucts to smc-mca.c the file smc-mca.h
>>>was empty except for two #define's, I've also killed the rest of
>>>smc-mca.h .
>>
>>It looks like the structs should be 'static const', not just 'static'.
>>
>>This comment is applicable to similar changes, also. Use 'const'
>>whenever possible.
>
>
> does that even have meaning in C? In C++ it does, but afaik in C it
> doesn't.

Of course it has meaning in C.

Jeff



2005-02-19 09:11:28

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [2.6 patch] drivers/net/smc-mca.c: cleanups

On Sat, 2005-02-19 at 03:41 -0500, Jeff Garzik wrote:
> Adrian Bunk wrote:
> > This patch contains the following cleanups:
> > - make a needlessly global function static
> > - make three needlessly global structs static
> >
> > Since after moving the now-static stucts to smc-mca.c the file smc-mca.h
> > was empty except for two #define's, I've also killed the rest of
> > smc-mca.h .
>
> It looks like the structs should be 'static const', not just 'static'.
>
> This comment is applicable to similar changes, also. Use 'const'
> whenever possible.

does that even have meaning in C? In C++ it does, but afaik in C it
doesn't.


2005-02-19 15:26:25

by Willy Tarreau

[permalink] [raw]
Subject: Re: [2.6 patch] drivers/net/smc-mca.c: cleanups

On Sat, Feb 19, 2005 at 10:09:00AM +0100, Arjan van de Ven wrote:
> On Sat, 2005-02-19 at 03:41 -0500, Jeff Garzik wrote:
> > Adrian Bunk wrote:
> > > This patch contains the following cleanups:
> > > - make a needlessly global function static
> > > - make three needlessly global structs static
> > >
> > > Since after moving the now-static stucts to smc-mca.c the file smc-mca.h
> > > was empty except for two #define's, I've also killed the rest of
> > > smc-mca.h .
> >
> > It looks like the structs should be 'static const', not just 'static'.
> >
> > This comment is applicable to similar changes, also. Use 'const'
> > whenever possible.
>
> does that even have meaning in C? In C++ it does, but afaik in C it
> doesn't.

Yes it does. Often the variables declared this way will go into the text
section which is marked read-only. I've used this technique in a few very
small programs to reduce their size (I could strip off both their bss and
data sections to save space). Also, I believe that the compiler is able
to optimize code using consts, but this is pure speculation, I've not
verified it.

Willy

2005-02-19 15:44:04

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [2.6 patch] drivers/net/smc-mca.c: cleanups


> > > This comment is applicable to similar changes, also. Use 'const'
> > > whenever possible.
> >
> > does that even have meaning in C? In C++ it does, but afaik in C it
> > doesn't.
>
> Yes it does. Often the variables declared this way will go into the text
> section which is marked read-only.

true. Doesn't mean too much for the kernel right now (in kernel space
not a lot of memory is really read only) though.

> I've used this technique in a few very
> small programs to reduce their size (I could strip off both their bss and
> data sections to save space). Also, I believe that the compiler is able
> to optimize code using consts, but this is pure speculation, I've not
> verified it.

Afaik that's the main difference between C and C++; in C you can still
change "const" variables... in C++ thats illegal (at least that's what I
remember and google seems to support somewhat ;)


2005-02-20 10:45:33

by Herbert Xu

[permalink] [raw]
Subject: Re: [2.6 patch] drivers/net/smc-mca.c: cleanups

Arjan van de Ven <[email protected]> wrote:
>
>> I've used this technique in a few very
>> small programs to reduce their size (I could strip off both their bss and
>> data sections to save space). Also, I believe that the compiler is able
>> to optimize code using consts, but this is pure speculation, I've not
>> verified it.
>
> Afaik that's the main difference between C and C++; in C you can still
> change "const" variables... in C++ thats illegal (at least that's what I
> remember and google seems to support somewhat ;)

The compiler does use the const modifier on a static object to optimise
code. Try compiling this program:

const int x;

int bar(int);

int foo(void)
{
bar(x);
return bar(x);
}

With the const gcc (3.3.4) will only load x once while it'll reload
it after calling bar if you remove the const modifier.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2005-02-21 01:27:20

by Jeff Garzik

[permalink] [raw]
Subject: Re: [2.6 patch] drivers/net/smc-mca.c: cleanups

Adrian Bunk wrote:
> This patch contains the following cleanups:
> - make a needlessly global function static
> - make three needlessly global structs static
>
> Since after moving the now-static stucts to smc-mca.c the file smc-mca.h
> was empty except for two #define's, I've also killed the rest of
> smc-mca.h .
>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> ---
>
> drivers/net/smc-mca.c | 60 +++++++++++++++++++++++++++++++++++++++--
> drivers/net/smc-mca.h | 61 ------------------------------------------
> 2 files changed, 58 insertions(+), 63 deletions(-)
>
> --- linux-2.6.11-rc3-mm2-full/drivers/net/smc-mca.h 2004-12-24 22:35:23.000000000 +0100
> +++ /dev/null 2004-11-25 03:16:25.000000000 +0100
> @@ -1,61 +0,0 @@
> -/*
> - * djweis [email protected]
> - * most of this file was taken from ps2esdi.h
> - */
> -
> -struct {
> - unsigned int base_addr;
> -} addr_table[] = {
> - { 0x0800 },
> - { 0x1800 },
> - { 0x2800 },
> - { 0x3800 },
> - { 0x4800 },
> - { 0x5800 },
> - { 0x6800 },
> - { 0x7800 },
> - { 0x8800 },
> - { 0x9800 },
> - { 0xa800 },
> - { 0xb800 },
> - { 0xc800 },
> - { 0xd800 },
> - { 0xe800 },
> - { 0xf800 }
> -};
> -
> -#define MEM_MASK 64
> -
> -struct {
> - unsigned char mem_index;
> - unsigned long mem_start;
> - unsigned char num_pages;
> -} mem_table[] = {
> - { 16, 0x0c0000, 40 },
> - { 18, 0x0c4000, 40 },
> - { 20, 0x0c8000, 40 },
> - { 22, 0x0cc000, 40 },
> - { 24, 0x0d0000, 40 },
> - { 26, 0x0d4000, 40 },
> - { 28, 0x0d8000, 40 },
> - { 30, 0x0dc000, 40 },
> - {144, 0xfc0000, 40 },
> - {148, 0xfc8000, 40 },
> - {154, 0xfd0000, 40 },
> - {156, 0xfd8000, 40 },
> - { 0, 0x0c0000, 20 },
> - { 1, 0x0c2000, 20 },
> - { 2, 0x0c4000, 20 },
> - { 3, 0x0c6000, 20 }
> -};
> -
> -#define IRQ_MASK 243
> -struct {
> - unsigned char new_irq;
> - unsigned char old_irq;
> -} irq_table[] = {
> - { 3, 3 },
> - { 4, 4 },
> - { 10, 10 },
> - { 14, 15 }
> -};
> --- linux-2.6.11-rc3-mm2-full/drivers/net/smc-mca.c.old 2005-02-16 18:44:29.000000000 +0100
> +++ linux-2.6.11-rc3-mm2-full/drivers/net/smc-mca.c 2005-02-16 18:47:24.000000000 +0100
> @@ -49,7 +49,6 @@
> #include <asm/system.h>
>
> #include "8390.h"
> -#include "smc-mca.h"
>
> #define DRV_NAME "smc-mca"
>
> @@ -100,6 +99,63 @@
> MODULE_PARM_DESC(ultra_io, "SMC Ultra/EtherEZ MCA I/O base address(es)");
> MODULE_PARM_DESC(ultra_irq, "SMC Ultra/EtherEZ MCA IRQ number(s)");
>
> +static struct {
> + unsigned int base_addr;
> +} addr_table[] = {
> + { 0x0800 },
> + { 0x1800 },
> + { 0x2800 },
> + { 0x3800 },
> + { 0x4800 },
> + { 0x5800 },
> + { 0x6800 },
> + { 0x7800 },
> + { 0x8800 },
> + { 0x9800 },
> + { 0xa800 },
> + { 0xb800 },
> + { 0xc800 },
> + { 0xd800 },
> + { 0xe800 },
> + { 0xf800 }
> +};
> +
> +#define MEM_MASK 64
> +
> +static struct {
> + unsigned char mem_index;
> + unsigned long mem_start;
> + unsigned char num_pages;
> +} mem_table[] = {
> + { 16, 0x0c0000, 40 },
> + { 18, 0x0c4000, 40 },
> + { 20, 0x0c8000, 40 },
> + { 22, 0x0cc000, 40 },
> + { 24, 0x0d0000, 40 },
> + { 26, 0x0d4000, 40 },
> + { 28, 0x0d8000, 40 },
> + { 30, 0x0dc000, 40 },
> + {144, 0xfc0000, 40 },
> + {148, 0xfc8000, 40 },
> + {154, 0xfd0000, 40 },
> + {156, 0xfd8000, 40 },
> + { 0, 0x0c0000, 20 },
> + { 1, 0x0c2000, 20 },
> + { 2, 0x0c4000, 20 },
> + { 3, 0x0c6000, 20 }
> +};
> +
> +#define IRQ_MASK 243
> +static struct {

these tables should be const-ified


2005-02-21 14:52:10

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] drivers/net/smc-mca.c: cleanups

On Sun, Feb 20, 2005 at 08:26:42PM -0500, Jeff Garzik wrote:

> these tables should be const-ified


Updated patch:


<-- snip -->


This patch contains the following cleanups:
- make a needlessly global function static
- make three needlessly global structs static const

Since after moving the now-static stucts to smc-mca.c the file smc-mca.h
was empty except for two #define's, I've also killed the rest of
smc-mca.h .

Signed-off-by: Adrian Bunk <[email protected]>

---

drivers/net/smc-mca.c | 60 +++++++++++++++++++++++++++++++++++++++--
drivers/net/smc-mca.h | 61 ------------------------------------------
2 files changed, 58 insertions(+), 63 deletions(-)

--- linux-2.6.11-rc3-mm2-full/drivers/net/smc-mca.h 2004-12-24 22:35:23.000000000 +0100
+++ /dev/null 2004-11-25 03:16:25.000000000 +0100
@@ -1,61 +0,0 @@
-/*
- * djweis [email protected]
- * most of this file was taken from ps2esdi.h
- */
-
-struct {
- unsigned int base_addr;
-} addr_table[] = {
- { 0x0800 },
- { 0x1800 },
- { 0x2800 },
- { 0x3800 },
- { 0x4800 },
- { 0x5800 },
- { 0x6800 },
- { 0x7800 },
- { 0x8800 },
- { 0x9800 },
- { 0xa800 },
- { 0xb800 },
- { 0xc800 },
- { 0xd800 },
- { 0xe800 },
- { 0xf800 }
-};
-
-#define MEM_MASK 64
-
-struct {
- unsigned char mem_index;
- unsigned long mem_start;
- unsigned char num_pages;
-} mem_table[] = {
- { 16, 0x0c0000, 40 },
- { 18, 0x0c4000, 40 },
- { 20, 0x0c8000, 40 },
- { 22, 0x0cc000, 40 },
- { 24, 0x0d0000, 40 },
- { 26, 0x0d4000, 40 },
- { 28, 0x0d8000, 40 },
- { 30, 0x0dc000, 40 },
- {144, 0xfc0000, 40 },
- {148, 0xfc8000, 40 },
- {154, 0xfd0000, 40 },
- {156, 0xfd8000, 40 },
- { 0, 0x0c0000, 20 },
- { 1, 0x0c2000, 20 },
- { 2, 0x0c4000, 20 },
- { 3, 0x0c6000, 20 }
-};
-
-#define IRQ_MASK 243
-struct {
- unsigned char new_irq;
- unsigned char old_irq;
-} irq_table[] = {
- { 3, 3 },
- { 4, 4 },
- { 10, 10 },
- { 14, 15 }
-};
--- linux-2.6.11-rc3-mm2-full/drivers/net/smc-mca.c.old 2005-02-16 18:44:29.000000000 +0100
+++ linux-2.6.11-rc3-mm2-full/drivers/net/smc-mca.c 2005-02-16 18:47:24.000000000 +0100
@@ -49,7 +49,6 @@
#include <asm/system.h>

#include "8390.h"
-#include "smc-mca.h"

#define DRV_NAME "smc-mca"

@@ -100,6 +99,63 @@
MODULE_PARM_DESC(ultra_io, "SMC Ultra/EtherEZ MCA I/O base address(es)");
MODULE_PARM_DESC(ultra_irq, "SMC Ultra/EtherEZ MCA IRQ number(s)");

+static const struct {
+ unsigned int base_addr;
+} addr_table[] = {
+ { 0x0800 },
+ { 0x1800 },
+ { 0x2800 },
+ { 0x3800 },
+ { 0x4800 },
+ { 0x5800 },
+ { 0x6800 },
+ { 0x7800 },
+ { 0x8800 },
+ { 0x9800 },
+ { 0xa800 },
+ { 0xb800 },
+ { 0xc800 },
+ { 0xd800 },
+ { 0xe800 },
+ { 0xf800 }
+};
+
+#define MEM_MASK 64
+
+static const struct {
+ unsigned char mem_index;
+ unsigned long mem_start;
+ unsigned char num_pages;
+} mem_table[] = {
+ { 16, 0x0c0000, 40 },
+ { 18, 0x0c4000, 40 },
+ { 20, 0x0c8000, 40 },
+ { 22, 0x0cc000, 40 },
+ { 24, 0x0d0000, 40 },
+ { 26, 0x0d4000, 40 },
+ { 28, 0x0d8000, 40 },
+ { 30, 0x0dc000, 40 },
+ {144, 0xfc0000, 40 },
+ {148, 0xfc8000, 40 },
+ {154, 0xfd0000, 40 },
+ {156, 0xfd8000, 40 },
+ { 0, 0x0c0000, 20 },
+ { 1, 0x0c2000, 20 },
+ { 2, 0x0c4000, 20 },
+ { 3, 0x0c6000, 20 }
+};
+
+#define IRQ_MASK 243
+static const struct {
+ unsigned char new_irq;
+ unsigned char old_irq;
+} irq_table[] = {
+ { 3, 3 },
+ { 4, 4 },
+ { 10, 10 },
+ { 14, 15 }
+};
+
static short smc_mca_adapter_ids[] __initdata = {
0x61c8,
0x61c9,
@@ -126,7 +182,7 @@

static int ultra_found = 0;

-int __init ultramca_probe(struct device *gen_dev)
+static int __init ultramca_probe(struct device *gen_dev)
{
unsigned short ioaddr;
struct net_device *dev;