| Message ID | 1328410966-13946-2-git-send-email-troy.kisky@boundarydevices.com |
|---|---|
| State | RFC |
| Headers | show |
On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: > --- a/drivers/net/phy/broadcom.c > +++ b/drivers/net/phy/broadcom.c > > -static struct phy_driver BCM5461S_driver = { > +struct phy_driver BCM5461S_driver __phy_entry = { why do you have to remove the static ? that shouldn't affect the section name that it gets placed into. > --- a/include/phy.h > +++ b/include/phy.h > > +extern struct phy_driver __phy_entry_start, __phy_entry_end; linker symbols should be declared like: extern char __phy_entry_start[]; > --- a/u-boot-common.lds > +++ b/u-boot-common.lds i'm not seeing this in the u-boot tree ... > + . = ALIGN(4); > + __phy_entry_start = .; > + .phy_entry : { > + KEEP(*(.phy_entry)) > + } > + __phy_entry_end = .; might have to introduce a helper macro like Linux's VMLINUX_SYMBOL() since some targets have a symbol prefix (like an underscore) -mike
Hi Mike, Le 05/02/2012 04:38, Mike Frysinger a écrit : > On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: >> --- a/drivers/net/phy/broadcom.c >> +++ b/drivers/net/phy/broadcom.c >> >> -static struct phy_driver BCM5461S_driver = { >> +struct phy_driver BCM5461S_driver __phy_entry = { > > why do you have to remove the static ? that shouldn't affect the section name > that it gets placed into. > >> --- a/include/phy.h >> +++ b/include/phy.h >> >> +extern struct phy_driver __phy_entry_start, __phy_entry_end; > > linker symbols should be declared like: > extern char __phy_entry_start[]; Why should they? Amicalement,
On Sunday 05 February 2012 08:26:57 Albert ARIBAUD wrote: > Le 05/02/2012 04:38, Mike Frysinger a écrit : > > On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: > >> --- a/include/phy.h > >> +++ b/include/phy.h > >> > >> +extern struct phy_driver __phy_entry_start, __phy_entry_end; > > > > linker symbols should be declared like: > > extern char __phy_entry_start[]; > > Why should they? because that's what the GNU linker documentation says to, and that's how all existing symbols have been handled. look at asm/sections.h in every Linux arch. -mike
On 2/4/2012 8:38 PM, Mike Frysinger wrote: > On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: >> --- a/drivers/net/phy/broadcom.c >> +++ b/drivers/net/phy/broadcom.c >> >> -static struct phy_driver BCM5461S_driver = { >> +struct phy_driver BCM5461S_driver __phy_entry = { > why do you have to remove the static ? that shouldn't affect the section name > that it gets placed into. I had static to start. But the compiler ate all of the code. No references to any of the static symbols. > >> --- a/include/phy.h >> +++ b/include/phy.h >> >> +extern struct phy_driver __phy_entry_start, __phy_entry_end; > linker symbols should be declared like: > extern char __phy_entry_start[]; Why char ? > > >> + . = ALIGN(4); >> + __phy_entry_start = .; >> + .phy_entry : { >> + KEEP(*(.phy_entry)) >> + } >> + __phy_entry_end = .; > might have to introduce a helper macro like Linux's VMLINUX_SYMBOL() since > some targets have a symbol prefix (like an underscore) > -mike Hmmm. Your right, grep ___u_boot_cmd_start 0001-RFC-create-u-boot-common.lds.patch finds that arch/blackfin/cpu/u-boot.lds has an extra "_" Thanks for pointing it out. Troy
On Monday 06 February 2012 13:48:13 Troy Kisky wrote: > On 2/4/2012 8:38 PM, Mike Frysinger wrote: > > On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: > >> --- a/drivers/net/phy/broadcom.c > >> +++ b/drivers/net/phy/broadcom.c > >> > >> -static struct phy_driver BCM5461S_driver = { > >> +struct phy_driver BCM5461S_driver __phy_entry = { > > > > why do you have to remove the static ? that shouldn't affect the section > > name that it gets placed into. > > I had static to start. But the compiler ate all of the code. No > references to any of the static symbols. sounds like you should change the __phy_entry define from "unused" to "used" -mike
On 2/6/2012 12:07 PM, Mike Frysinger wrote: > On Monday 06 February 2012 13:48:13 Troy Kisky wrote: >> On 2/4/2012 8:38 PM, Mike Frysinger wrote: >>> On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: >>>> --- a/drivers/net/phy/broadcom.c >>>> +++ b/drivers/net/phy/broadcom.c >>>> >>>> -static struct phy_driver BCM5461S_driver = { >>>> +struct phy_driver BCM5461S_driver __phy_entry = { >>> why do you have to remove the static ? that shouldn't affect the section >>> name that it gets placed into. >> I had static to start. But the compiler ate all of the code. No >> references to any of the static symbols. > sounds like you should change the __phy_entry define from "unused" to "used" > -mike The would give me compiler warnings for unused variables. How does that help? Is there a keep attribute like the linker has for sections? Troy
Le 05/02/2012 21:40, Mike Frysinger a écrit : > On Sunday 05 February 2012 08:26:57 Albert ARIBAUD wrote: >> Le 05/02/2012 04:38, Mike Frysinger a écrit : >>> On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: >>>> --- a/include/phy.h >>>> +++ b/include/phy.h >>>> >>>> +extern struct phy_driver __phy_entry_start, __phy_entry_end; >>> >>> linker symbols should be declared like: >>> extern char __phy_entry_start[]; >> >> Why should they? > > because that's what the GNU linker documentation says to, and that's how all > existing symbols have been handled. look at asm/sections.h in every Linux > arch. Does it? What I read from <http://sourceware.org/binutils/docs-2.22/ld/Source-Code-Reference.html#Source-Code-Reference> never says that linker-defined symbols should be declared in source code as char[]; actually, it gives examples where linker-defined symbols are defined with types int and char, not char[]. What the section says, OTOH, is that one must remember that the linker will not allocate space for a symbol unless explicitly instructed to, so such symbols my not have meaningful values, only addresses, and the code should access these symbols by address -- which is what is being done in the code of the RFC patch IIUC. > -mike Amicalement,
Le 06/02/2012 21:17, Troy Kisky a écrit : > On 2/6/2012 12:07 PM, Mike Frysinger wrote: >> On Monday 06 February 2012 13:48:13 Troy Kisky wrote: >>> On 2/4/2012 8:38 PM, Mike Frysinger wrote: >>>> On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: >>>>> --- a/drivers/net/phy/broadcom.c >>>>> +++ b/drivers/net/phy/broadcom.c >>>>> >>>>> -static struct phy_driver BCM5461S_driver = { >>>>> +struct phy_driver BCM5461S_driver __phy_entry = { >>>> why do you have to remove the static ? that shouldn't affect the >>>> section >>>> name that it gets placed into. >>> I had static to start. But the compiler ate all of the code. No >>> references to any of the static symbols. >> sounds like you should change the __phy_entry define from "unused" to >> "used" >> -mike > The would give me compiler warnings for unused variables. How does that > help? > Is there a keep attribute like the linker has for sections? No, but indeed not keeping the 'static' keyword has this effect: the object file will keep the phy struct, in case it is referred to by another object file at link time. > Troy Amicalement,
On Monday 06 February 2012 15:17:32 Troy Kisky wrote: > On 2/6/2012 12:07 PM, Mike Frysinger wrote: > > On Monday 06 February 2012 13:48:13 Troy Kisky wrote: > >> On 2/4/2012 8:38 PM, Mike Frysinger wrote: > >>> On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: > >>>> --- a/drivers/net/phy/broadcom.c > >>>> +++ b/drivers/net/phy/broadcom.c > >>>> > >>>> -static struct phy_driver BCM5461S_driver = { > >>>> +struct phy_driver BCM5461S_driver __phy_entry = { > >>> > >>> why do you have to remove the static ? that shouldn't affect the > >>> section name that it gets placed into. > >> > >> I had static to start. But the compiler ate all of the code. No > >> references to any of the static symbols. > > > > sounds like you should change the __phy_entry define from "unused" to > > "used" > > The would give me compiler warnings for unused variables. How does that > help? does gcc issue warnings ? doesn't seem to do so for me. > Is there a keep attribute like the linker has for sections? yes, __attribute__((used)) -mike
Le 06/02/2012 21:57, Mike Frysinger a écrit : >> The would give me compiler warnings for unused variables. How does that >> help? > > does gcc issue warnings ? doesn't seem to do so for me. Some do, and some will. >> Is there a keep attribute like the linker has for sections? > > yes, __attribute__((used)) What is the point in adding a 'static' qualifier and a ((used)) attribute, when not adding them in the first place gives the same result? Amicalement,
On 2/6/2012 1:57 PM, Mike Frysinger wrote: > On Monday 06 February 2012 15:17:32 Troy Kisky wrote: >> On 2/6/2012 12:07 PM, Mike Frysinger wrote: >>> On Monday 06 February 2012 13:48:13 Troy Kisky wrote: >>>> On 2/4/2012 8:38 PM, Mike Frysinger wrote: >>>>> On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: >>>>>> --- a/drivers/net/phy/broadcom.c >>>>>> +++ b/drivers/net/phy/broadcom.c >>>>>> >>>>>> -static struct phy_driver BCM5461S_driver = { >>>>>> +struct phy_driver BCM5461S_driver __phy_entry = { >>>>> why do you have to remove the static ? that shouldn't affect the >>>>> section name that it gets placed into. >>>> I had static to start. But the compiler ate all of the code. No >>>> references to any of the static symbols. >>> sounds like you should change the __phy_entry define from "unused" to >>> "used" >> The would give me compiler warnings for unused variables. How does that >> help? > does gcc issue warnings ? doesn't seem to do so for me. > >> Is there a keep attribute like the linker has for sections? > yes, __attribute__((used)) > -mike Thanks, since the gcc manual I was using didn't list used, I thought you merely meant to remove unused. Seems the gcc version 4.1.2 does not list this option while 4.2.4 does. What level of compiler is required ? Troy
On Monday 06 February 2012 16:01:56 Albert ARIBAUD wrote: > Le 06/02/2012 21:57, Mike Frysinger a écrit : > >> The would give me compiler warnings for unused variables. How does that > >> help? > > > > does gcc issue warnings ? doesn't seem to do so for me. > > Some do, and some will. vague ... be nice to have actual examples > >> Is there a keep attribute like the linker has for sections? > > > > yes, __attribute__((used)) > > What is the point in adding a 'static' qualifier and a ((used)) > attribute, when not adding them in the first place gives the same result? to control the visibility -mike
On Monday 06 February 2012 16:44:36 Troy Kisky wrote: > On 2/6/2012 1:57 PM, Mike Frysinger wrote: > > On Monday 06 February 2012 15:17:32 Troy Kisky wrote: > >> On 2/6/2012 12:07 PM, Mike Frysinger wrote: > >>> On Monday 06 February 2012 13:48:13 Troy Kisky wrote: > >>>> On 2/4/2012 8:38 PM, Mike Frysinger wrote: > >>>>> On Saturday 04 February 2012 22:02:46 Troy Kisky wrote: > >>>>>> --- a/drivers/net/phy/broadcom.c > >>>>>> +++ b/drivers/net/phy/broadcom.c > >>>>>> > >>>>>> -static struct phy_driver BCM5461S_driver = { > >>>>>> +struct phy_driver BCM5461S_driver __phy_entry = { > >>>>> > >>>>> why do you have to remove the static ? that shouldn't affect the > >>>>> section name that it gets placed into. > >>>> > >>>> I had static to start. But the compiler ate all of the code. No > >>>> references to any of the static symbols. > >>> > >>> sounds like you should change the __phy_entry define from "unused" to > >>> "used" > >> > >> The would give me compiler warnings for unused variables. How does that > >> help? > > > > does gcc issue warnings ? doesn't seem to do so for me. > > > >> Is there a keep attribute like the linker has for sections? > > > > yes, __attribute__((used)) > > Thanks, since the gcc manual I was using didn't list used, I thought you > merely meant to > remove unused. > > Seems the gcc version 4.1.2 does not list this option while 4.2.4 does. > > What level of compiler is required ? in looking at how Linux does things, you should include linux/compiler.h and then utilize __maybe_unused rather than specifying the attribute yourself -mike
Le 07/02/2012 16:20, Mike Frysinger a écrit : > On Monday 06 February 2012 16:01:56 Albert ARIBAUD wrote: >> Le 06/02/2012 21:57, Mike Frysinger a écrit : >>>> The would give me compiler warnings for unused variables. How does that >>>> help? >>> >>> does gcc issue warnings ? doesn't seem to do so for me. >> >> Some do, and some will. > > vague ... be nice to have actual examples > >>>> Is there a keep attribute like the linker has for sections? >>> >>> yes, __attribute__((used)) >> >> What is the point in adding a 'static' qualifier and a ((used)) >> attribute, when not adding them in the first place gives the same result? > > to control the visibility I don't understand what you mean with this. Can you please elaborate? > -mike Amicalement,
On Friday 10 February 2012 14:39:12 Albert ARIBAUD wrote: > Le 07/02/2012 16:20, Mike Frysinger a écrit : > > On Monday 06 February 2012 16:01:56 Albert ARIBAUD wrote: > >> Le 06/02/2012 21:57, Mike Frysinger a écrit : > >>>> Is there a keep attribute like the linker has for sections? > >>> > >>> yes, __attribute__((used)) > >> > >> What is the point in adding a 'static' qualifier and a ((used)) > >> attribute, when not adding them in the first place gives the same > >> result? > > > > to control the visibility > > I don't understand what you mean with this. Can you please elaborate? no static means it has global elf visibility (other .c files can "extern" it, and you have to worry about symbol clashes): $ gcc -x c -c - -o test.o <<<'int foo;' && readelf -s test.o | grep foo 7: 0000000000000004 4 OBJECT GLOBAL DEFAULT COM foo static means it has local elf visibility (other files don't get access, and you don't have to worry about symbol clashes): $ gcc -x c -c - -o test.o <<<'static int foo;' && readelf -s test.o | grep foo 5: 0000000000000000 4 OBJECT LOCAL DEFAULT 3 foo imo, anything that should not be externally accessed should have "static". this is just good programming practice. -mike
Le 10/02/2012 21:32, Mike Frysinger a écrit : > On Friday 10 February 2012 14:39:12 Albert ARIBAUD wrote: >> Le 07/02/2012 16:20, Mike Frysinger a écrit : >>> On Monday 06 February 2012 16:01:56 Albert ARIBAUD wrote: >>>> Le 06/02/2012 21:57, Mike Frysinger a écrit : >>>>>> Is there a keep attribute like the linker has for sections? >>>>> >>>>> yes, __attribute__((used)) >>>> >>>> What is the point in adding a 'static' qualifier and a ((used)) >>>> attribute, when not adding them in the first place gives the same >>>> result? >>> >>> to control the visibility >> >> I don't understand what you mean with this. Can you please elaborate? > > no static means it has global elf visibility (other .c files can "extern" it, > and you have to worry about symbol clashes): > $ gcc -x c -c - -o test.o<<<'int foo;'&& readelf -s test.o | grep foo > 7: 0000000000000004 4 OBJECT GLOBAL DEFAULT COM foo > > static means it has local elf visibility (other files don't get access, and you > don't have to worry about symbol clashes): > $ gcc -x c -c - -o test.o<<<'static int foo;'&& readelf -s test.o | grep foo > 5: 0000000000000000 4 OBJECT LOCAL DEFAULT 3 foo > > imo, anything that should not be externally accessed should have "static". > this is just good programming practice. I would agree 100% if the symbol was truly local, i.e. declared *and used* locally. Here, however, it is used globally, by being gathered in a global section to serve as an entry in a global array. The only interest of making the symbol static would indeed be to allow reusing the symbol name elsewhere, which I think is quite improbable considering the symbol was global so far. So we add the static qualifier despite the object actually not being static; and because the object is not actually static, that qualifier causes a legit diagnostic; and to eliminate that diagnostic, we add an 'unused' attribute. This I find less than good programming practice. > -mike Amicalement,
On Friday 10 February 2012 15:57:50 Albert ARIBAUD wrote: > Le 10/02/2012 21:32, Mike Frysinger a écrit : > > On Friday 10 February 2012 14:39:12 Albert ARIBAUD wrote: > >> Le 07/02/2012 16:20, Mike Frysinger a écrit : > >>> On Monday 06 February 2012 16:01:56 Albert ARIBAUD wrote: > >>>> Le 06/02/2012 21:57, Mike Frysinger a écrit : > >>>>>> Is there a keep attribute like the linker has for sections? > >>>>> > >>>>> yes, __attribute__((used)) > >>>> > >>>> What is the point in adding a 'static' qualifier and a ((used)) > >>>> attribute, when not adding them in the first place gives the same > >>>> result? > >>> > >>> to control the visibility > >> > >> I don't understand what you mean with this. Can you please elaborate? > > > > no static means it has global elf visibility (other .c files can "extern" > > it, and you have to worry about symbol clashes): > > $ gcc -x c -c - -o test.o<<<'int foo;'&& readelf -s test.o | grep foo > > > > 7: 0000000000000004 4 OBJECT GLOBAL DEFAULT COM foo > > > > static means it has local elf visibility (other files don't get access, > > and you don't have to worry about symbol clashes): > > $ gcc -x c -c - -o test.o<<<'static int foo;'&& readelf -s test.o | grep > > foo > > > > 5: 0000000000000000 4 OBJECT LOCAL DEFAULT 3 foo > > > > imo, anything that should not be externally accessed should have > > "static". this is just good programming practice. > > I would agree 100% if the symbol was truly local, i.e. declared *and > used* locally. Here, however, it is used globally, by being gathered in > a global section to serve as an entry in a global array. except access is now explicitly gated, and symbol collisions are still prevented > The only interest of making the symbol static would indeed be to allow > reusing the symbol name elsewhere, which I think is quite improbable > considering the symbol was global so far. one or two might be global, but for the most part, they were all local. look at his patch ... he deletes the "static" keyword in many places. this style i'm proposing has been used in the kernel in subsystems, and some of them end up using the same variable name in diff modules. like crypto/ which uses "alg" as the name for all of its shash drivers. > So we add the static qualifier despite the object actually not being > static; and because the object is not actually static, that qualifier > causes a legit diagnostic; and to eliminate that diagnostic, we add an > 'unused' attribute. This I find less than good programming practice. no, the unused attribute was added *after* removing "static". i'm proposing adding "used" so gcc won't strip it regardless of what else happens while retaining all other benefits that "static" brings us. -mike
Le 10/02/2012 22:41, Mike Frysinger a écrit : > On Friday 10 February 2012 15:57:50 Albert ARIBAUD wrote: >> Le 10/02/2012 21:32, Mike Frysinger a écrit : >>> On Friday 10 February 2012 14:39:12 Albert ARIBAUD wrote: >>>> Le 07/02/2012 16:20, Mike Frysinger a écrit : >>>>> On Monday 06 February 2012 16:01:56 Albert ARIBAUD wrote: >>>>>> Le 06/02/2012 21:57, Mike Frysinger a écrit : >>>>>>>> Is there a keep attribute like the linker has for sections? >>>>>>> >>>>>>> yes, __attribute__((used)) >>>>>> >>>>>> What is the point in adding a 'static' qualifier and a ((used)) >>>>>> attribute, when not adding them in the first place gives the same >>>>>> result? >>>>> >>>>> to control the visibility >>>> >>>> I don't understand what you mean with this. Can you please elaborate? >>> >>> no static means it has global elf visibility (other .c files can "extern" >>> it, and you have to worry about symbol clashes): >>> $ gcc -x c -c - -o test.o<<<'int foo;'&& readelf -s test.o | grep foo >>> >>> 7: 0000000000000004 4 OBJECT GLOBAL DEFAULT COM foo >>> >>> static means it has local elf visibility (other files don't get access, >>> and you don't have to worry about symbol clashes): >>> $ gcc -x c -c - -o test.o<<<'static int foo;'&& readelf -s test.o | grep >>> foo >>> >>> 5: 0000000000000000 4 OBJECT LOCAL DEFAULT 3 foo >>> >>> imo, anything that should not be externally accessed should have >>> "static". this is just good programming practice. >> >> I would agree 100% if the symbol was truly local, i.e. declared *and >> used* locally. Here, however, it is used globally, by being gathered in >> a global section to serve as an entry in a global array. > > except access is now explicitly gated, and symbol collisions are still > prevented > >> The only interest of making the symbol static would indeed be to allow >> reusing the symbol name elsewhere, which I think is quite improbable >> considering the symbol was global so far. > > one or two might be global, but for the most part, they were all local. look > at his patch ... he deletes the "static" keyword in many places. "Were" is precisely the point. They were indeed proper locals before the patch. With it, they are not any more -- they are not used locally, they are used globally. > this style i'm proposing has been used in the kernel in subsystems, and some > of them end up using the same variable name in diff modules. like crypto/ > which uses "alg" as the name for all of its shash drivers. > >> So we add the static qualifier despite the object actually not being >> static; and because the object is not actually static, that qualifier >> causes a legit diagnostic; and to eliminate that diagnostic, we add an >> 'unused' attribute. This I find less than good programming practice. > > no, the unused attribute was added *after* removing "static". i'm proposing > adding "used" so gcc won't strip it regardless of what else happens while > retaining all other benefits that "static" brings us. Correct, but it does not change my overall point that there is no point in adding a qualifier and an attribute to avoid name collisions that do not actually happen in the first place and that we can easily avoid by simply giving meaningful names to each of theses structs -- a practice which, again, is currently and successfully applied. Plus, having globals is a good thing, because we can put names, so to speak, in the map file on what was collected in the section, whereas with statics, the section would just be just a black box and, were we to check if such entry was actually put in it, we would have to dig in the source code and build system. > -mike Amicalement,
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 798473d..ad82256 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -30,7 +30,7 @@ static int ar8021_config(struct phy_device *phydev) return 0; } -struct phy_driver AR8021_driver = { +struct phy_driver AR8021_driver __phy_entry = { .name = "AR8021", .uid = 0x4dd040, .mask = 0xfffff0, @@ -39,10 +39,3 @@ struct phy_driver AR8021_driver = { .startup = genphy_startup, .shutdown = genphy_shutdown, }; - -int phy_atheros_init(void) -{ - phy_register(&AR8021_driver); - - return 0; -} diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 427ac60..513931d 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -248,7 +248,7 @@ static int bcm5482_startup(struct phy_device *phydev) return 0; } -static struct phy_driver BCM5461S_driver = { +struct phy_driver BCM5461S_driver __phy_entry = { .name = "Broadcom BCM5461S", .uid = 0x2060c0, .mask = 0xfffff0, @@ -258,7 +258,7 @@ static struct phy_driver BCM5461S_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver BCM5464S_driver = { +struct phy_driver BCM5464S_driver __phy_entry = { .name = "Broadcom BCM5464S", .uid = 0x2060b0, .mask = 0xfffff0, @@ -268,7 +268,7 @@ static struct phy_driver BCM5464S_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver BCM5482S_driver = { +struct phy_driver BCM5482S_driver __phy_entry = { .name = "Broadcom BCM5482S", .uid = 0x143bcb0, .mask = 0xffffff0, @@ -277,12 +277,3 @@ static struct phy_driver BCM5482S_driver = { .startup = &bcm5482_startup, .shutdown = &genphy_shutdown, }; - -int phy_broadcom_init(void) -{ - phy_register(&BCM5482S_driver); - phy_register(&BCM5464S_driver); - phy_register(&BCM5461S_driver); - - return 0; -} diff --git a/drivers/net/phy/davicom.c b/drivers/net/phy/davicom.c index e96a4af..1c61197 100644 --- a/drivers/net/phy/davicom.c +++ b/drivers/net/phy/davicom.c @@ -80,7 +80,7 @@ static int dm9161_startup(struct phy_device *phydev) return 0; } -static struct phy_driver DM9161_driver = { +struct phy_driver DM9161_driver __phy_entry = { .name = "Davicom DM9161E", .uid = 0x181b880, .mask = 0xffffff0, @@ -89,10 +89,3 @@ static struct phy_driver DM9161_driver = { .startup = &dm9161_startup, .shutdown = &genphy_shutdown, }; - -int phy_davicom_init(void) -{ - phy_register(&DM9161_driver); - - return 0; -} diff --git a/drivers/net/phy/lxt.c b/drivers/net/phy/lxt.c index d67bbdd..5b85616 100644 --- a/drivers/net/phy/lxt.c +++ b/drivers/net/phy/lxt.c @@ -69,7 +69,7 @@ static int lxt971_startup(struct phy_device *phydev) return 0; } -static struct phy_driver LXT971_driver = { +struct phy_driver LXT971_driver __phy_entry = { .name = "LXT971", .uid = 0x1378e0, .mask = 0xfffff0, @@ -78,10 +78,3 @@ static struct phy_driver LXT971_driver = { .startup = &lxt971_startup, .shutdown = &genphy_shutdown, }; - -int phy_lxt_init(void) -{ - phy_register(&LXT971_driver); - - return 0; -} diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index e51e799..90072cc 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -395,7 +395,7 @@ static int m88e1149_config(struct phy_device *phydev) } -static struct phy_driver M88E1011S_driver = { +struct phy_driver M88E1011S_driver __phy_entry = { .name = "Marvell 88E1011S", .uid = 0x1410c60, .mask = 0xffffff0, @@ -405,7 +405,7 @@ static struct phy_driver M88E1011S_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver M88E1111S_driver = { +struct phy_driver M88E1111S_driver __phy_entry = { .name = "Marvell 88E1111S", .uid = 0x1410cc0, .mask = 0xffffff0, @@ -415,7 +415,7 @@ static struct phy_driver M88E1111S_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver M88E1118_driver = { +struct phy_driver M88E1118_driver __phy_entry = { .name = "Marvell 88E1118", .uid = 0x1410e10, .mask = 0xffffff0, @@ -425,7 +425,7 @@ static struct phy_driver M88E1118_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver M88E1121R_driver = { +struct phy_driver M88E1121R_driver__phy_entry = { .name = "Marvell 88E1121R", .uid = 0x1410cb0, .mask = 0xffffff0, @@ -435,7 +435,7 @@ static struct phy_driver M88E1121R_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver M88E1145_driver = { +struct phy_driver M88E1145_driver __phy_entry = { .name = "Marvell 88E1145", .uid = 0x1410cd0, .mask = 0xffffff0, @@ -445,7 +445,7 @@ static struct phy_driver M88E1145_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver M88E1149S_driver = { +struct phy_driver M88E1149S_driver __phy_entry = { .name = "Marvell 88E1149S", .uid = 0x1410ca0, .mask = 0xffffff0, @@ -454,15 +454,3 @@ static struct phy_driver M88E1149S_driver = { .startup = &m88e1011s_startup, .shutdown = &genphy_shutdown, }; - -int phy_marvell_init(void) -{ - phy_register(&M88E1149S_driver); - phy_register(&M88E1145_driver); - phy_register(&M88E1121R_driver); - phy_register(&M88E1118_driver); - phy_register(&M88E1111S_driver); - phy_register(&M88E1011S_driver); - - return 0; -} diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index d4e64f2..7508403 100644 --- a/drivers/net/phy/micrel.c +++ b/drivers/net/phy/micrel.c @@ -22,7 +22,7 @@ */ #include <phy.h> -static struct phy_driver KSZ804_driver = { +struct phy_driver KSZ804_driver __phy_entry = { .name = "Micrel KSZ804", .uid = 0x221510, .mask = 0xfffff0, @@ -32,7 +32,7 @@ static struct phy_driver KSZ804_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver KS8721_driver = { +struct phy_driver KS8721_driver __phy_entry = { .name = "Micrel KS8721BL", .uid = 0x221610, .mask = 0xfffff0, @@ -41,11 +41,3 @@ static struct phy_driver KS8721_driver = { .startup = &genphy_startup, .shutdown = &genphy_shutdown, }; - -int phy_micrel_init(void) -{ - phy_register(&KSZ804_driver); - phy_register(&KS8721_driver); - - return 0; -} diff --git a/drivers/net/phy/natsemi.c b/drivers/net/phy/natsemi.c index ea60ac1..3829d74 100644 --- a/drivers/net/phy/natsemi.c +++ b/drivers/net/phy/natsemi.c @@ -78,7 +78,7 @@ static int dp83865_startup(struct phy_device *phydev) } -static struct phy_driver DP83865_driver = { +struct phy_driver DP83865_driver __phy_entry = { .name = "NatSemi DP83865", .uid = 0x20005c70, .mask = 0xfffffff0, @@ -87,10 +87,3 @@ static struct phy_driver DP83865_driver = { .startup = &dp83865_startup, .shutdown = &genphy_shutdown, }; - -int phy_natsemi_init(void) -{ - phy_register(&DP83865_driver); - - return 0; -} diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index eb55180..d3441c1 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -420,40 +420,11 @@ static LIST_HEAD(phy_drivers); int phy_init(void) { -#ifdef CONFIG_PHY_ATHEROS - phy_atheros_init(); -#endif -#ifdef CONFIG_PHY_BROADCOM - phy_broadcom_init(); -#endif -#ifdef CONFIG_PHY_DAVICOM - phy_davicom_init(); -#endif -#ifdef CONFIG_PHY_LXT - phy_lxt_init(); -#endif -#ifdef CONFIG_PHY_MARVELL - phy_marvell_init(); -#endif -#ifdef CONFIG_PHY_MICREL - phy_micrel_init(); -#endif -#ifdef CONFIG_PHY_NATSEMI - phy_natsemi_init(); -#endif -#ifdef CONFIG_PHY_REALTEK - phy_realtek_init(); -#endif -#ifdef CONFIG_PHY_SMSC - phy_smsc_init(); -#endif -#ifdef CONFIG_PHY_TERANETICS - phy_teranetics_init(); -#endif -#ifdef CONFIG_PHY_VITESSE - phy_vitesse_init(); -#endif - + struct phy_driver *entry = &__phy_entry_start; + while (entry < &__phy_entry_end) { + phy_register(entry); + entry++; + } return 0; } diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c index b7e2753..8733e38 100644 --- a/drivers/net/phy/realtek.c +++ b/drivers/net/phy/realtek.c @@ -112,7 +112,7 @@ static int rtl8211b_startup(struct phy_device *phydev) return 0; } -static struct phy_driver RTL8211B_driver = { +struct phy_driver RTL8211B_driver __phy_entry = { .name = "RealTek RTL8211B", .uid = 0x1cc910, .mask = 0xfffff0, @@ -121,10 +121,3 @@ static struct phy_driver RTL8211B_driver = { .startup = &rtl8211b_startup, .shutdown = &genphy_shutdown, }; - -int phy_realtek_init(void) -{ - phy_register(&RTL8211B_driver); - - return 0; -} diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c index 6dee8eb..8bb4204 100644 --- a/drivers/net/phy/smsc.c +++ b/drivers/net/phy/smsc.c @@ -52,7 +52,7 @@ static int smsc_startup(struct phy_device *phydev) return 0; } -static struct phy_driver lan8700_driver = { +struct phy_driver lan8700_driver __phy_entry = { .name = "SMSC LAN8700", .uid = 0x0007c0c0, .mask = 0xffff0, @@ -62,7 +62,7 @@ static struct phy_driver lan8700_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver lan911x_driver = { +struct phy_driver lan911x_driver __phy_entry = { .name = "SMSC LAN911x Internal PHY", .uid = 0x0007c0d0, .mask = 0xffff0, @@ -72,7 +72,7 @@ static struct phy_driver lan911x_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver lan8710_driver = { +struct phy_driver lan8710_driver __phy_entry = { .name = "SMSC LAN8710/LAN8720", .uid = 0x0007c0f0, .mask = 0xffff0, @@ -81,12 +81,3 @@ static struct phy_driver lan8710_driver = { .startup = &smsc_startup, .shutdown = &genphy_shutdown, }; - -int phy_smsc_init(void) -{ - phy_register(&lan8710_driver); - phy_register(&lan911x_driver); - phy_register(&lan8700_driver); - - return 0; -} diff --git a/drivers/net/phy/teranetics.c b/drivers/net/phy/teranetics.c index 78447b7..fd51355 100644 --- a/drivers/net/phy/teranetics.c +++ b/drivers/net/phy/teranetics.c @@ -93,7 +93,7 @@ int tn2020_startup(struct phy_device *phydev) return 0; } -struct phy_driver tn2020_driver = { +struct phy_driver tn2020_driver __phy_entry = { .name = "Teranetics TN2020", .uid = PHY_UID_TN2020, .mask = 0xfffffff0, @@ -105,10 +105,3 @@ struct phy_driver tn2020_driver = { .startup = &tn2020_startup, .shutdown = &gen10g_shutdown, }; - -int phy_teranetics_init(void) -{ - phy_register(&tn2020_driver); - - return 0; -} diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c index d48d4fe..15439aa 100644 --- a/drivers/net/phy/vitesse.c +++ b/drivers/net/phy/vitesse.c @@ -146,7 +146,7 @@ int vsc8601_config(struct phy_device *phydev) return 0; } -static struct phy_driver VSC8211_driver = { +struct phy_driver VSC8211_driver __phy_entry = { .name = "Vitesse VSC8211", .uid = 0xfc4b0, .mask = 0xffff0, @@ -156,7 +156,7 @@ static struct phy_driver VSC8211_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver VSC8221_driver = { +struct phy_driver VSC8221_driver __phy_entry = { .name = "Vitesse VSC8221", .uid = 0xfc550, .mask = 0xffff0, @@ -166,7 +166,7 @@ static struct phy_driver VSC8221_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver VSC8244_driver = { +struct phy_driver VSC8244_driver __phy_entry = { .name = "Vitesse VSC8244", .uid = 0xfc6c0, .mask = 0xffff0, @@ -176,7 +176,7 @@ static struct phy_driver VSC8244_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver VSC8234_driver = { +struct phy_driver VSC8234_driver __phy_entry = { .name = "Vitesse VSC8234", .uid = 0xfc620, .mask = 0xffff0, @@ -186,7 +186,7 @@ static struct phy_driver VSC8234_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver VSC8601_driver = { +struct phy_driver VSC8601_driver __phy_entry = { .name = "Vitesse VSC8601", .uid = 0x70420, .mask = 0xffff0, @@ -196,7 +196,7 @@ static struct phy_driver VSC8601_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver VSC8641_driver = { +struct phy_driver VSC8641_driver __phy_entry = { .name = "Vitesse VSC8641", .uid = 0x70430, .mask = 0xffff0, @@ -207,7 +207,7 @@ static struct phy_driver VSC8641_driver = { }; /* Vitesse bought Cicada, so we'll put these here */ -static struct phy_driver cis8201_driver = { +struct phy_driver cis8201_driver __phy_entry = { .name = "CIS8201", .uid = 0xfc410, .mask = 0xffff0, @@ -217,7 +217,7 @@ static struct phy_driver cis8201_driver = { .shutdown = &genphy_shutdown, }; -static struct phy_driver cis8204_driver = { +struct phy_driver cis8204_driver __phy_entry = { .name = "Cicada Cis8204", .uid = 0xfc440, .mask = 0xffff0, @@ -226,17 +226,3 @@ static struct phy_driver cis8204_driver = { .startup = &vitesse_startup, .shutdown = &genphy_shutdown, }; - -int phy_vitesse_init(void) -{ - phy_register(&VSC8641_driver); - phy_register(&VSC8601_driver); - phy_register(&VSC8234_driver); - phy_register(&VSC8244_driver); - phy_register(&VSC8211_driver); - phy_register(&VSC8221_driver); - phy_register(&cis8201_driver); - phy_register(&cis8204_driver); - - return 0; -} diff --git a/include/phy.h b/include/phy.h index bc522d5..e7a1ea7 100644 --- a/include/phy.h +++ b/include/phy.h @@ -231,4 +231,7 @@ int phy_vitesse_init(void); /* PHY UIDs for various PHYs that are referenced in external code */ #define PHY_UID_TN2020 0x00a19410 +#define __phy_entry __attribute__((unused, section(".phy_entry"), aligned(4))) +extern struct phy_driver __phy_entry_start, __phy_entry_end; + #endif diff --git a/u-boot-common.lds b/u-boot-common.lds index e9a5fc9..db2e9ad 100644 --- a/u-boot-common.lds +++ b/u-boot-common.lds @@ -5,6 +5,13 @@ } __u_boot_cmd_end = .; + . = ALIGN(4); + __phy_entry_start = .; + .phy_entry : { + KEEP(*(.phy_entry)) + } + __phy_entry_end = .; + /* powerpc specific, but harmless for others */ . = ALIGN(4); __start___ex_table = .;
Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> --- drivers/net/phy/atheros.c | 9 +-------- drivers/net/phy/broadcom.c | 15 +++------------ drivers/net/phy/davicom.c | 9 +-------- drivers/net/phy/lxt.c | 9 +-------- drivers/net/phy/marvell.c | 24 ++++++------------------ drivers/net/phy/micrel.c | 12 ++---------- drivers/net/phy/natsemi.c | 9 +-------- drivers/net/phy/phy.c | 39 +++++---------------------------------- drivers/net/phy/realtek.c | 9 +-------- drivers/net/phy/smsc.c | 15 +++------------ drivers/net/phy/teranetics.c | 9 +-------- drivers/net/phy/vitesse.c | 30 ++++++++---------------------- include/phy.h | 3 +++ u-boot-common.lds | 7 +++++++ 14 files changed, 43 insertions(+), 156 deletions(-)