Manufacturer specific code in a bothersome place
I'm porting u-boot to the Sharp KEV7A400 development board. The CPU has an ARM9 core with some Sharp and some ARM cells. The existing ARM920T cpu code has quite a bit of Samsung specific code in it. So, the question is this: how do we best handle development like this? For time time being, I'm creating a new CPU directory to eliminate a ridiculous set of ifdefs. However, this may not be a good long-term solution.
I've been musing about solving the problem and have come up with some ideas, though nothing yet comprehensivve.
1) It seems to be a mistake to put manufacturer specific code in the CPU directory unless we equate CPU not to the *core* but to the implementation of the core. For example, not x86, but AMD Elan. Not ARM920T but Sharp LH7a400. In the embedded market, this makes more sense than it does for x86 PCs.
2) There are some board-specific driver choices, e.g. serial driver, that sometimes appear in the CPU directory. Perhaps these choices are best made in the board directory instead.
3) Drivers, such as serial, could be made to export a function table. This would allow driver selection by choosing which file to compile and link.
At this point, I'm looking to start a discussion about this. I've seen the same kinds of problems twice, now. I'd like to find out how best to fix them.
Cheers.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 10 lines · Wolfgang Denk
Quoted 4 lines · In message <[email protected]> you wrote:
I'm porting u-boot to the Sharp KEV7A400 development board. The CPU has an ARM9 core with some Sharp and some ARM cells. The existing ARM920T cpu code has quite a bit of Samsung specific code in it. So, the question is this: how do we best handle development like this?
If this really becomes a problem I think we might create processor specific sub-directories below the CPU/* directory, i.e. something like
Quoted 6 lines
cpu/arm920t/s3c24x0/
cpu/arm920t/other_1/
cpu/arm920t/other_2/...What do you think?
OK. Does this mean that the configuration line in Makefile says:
... arm arm920t/<subcpu> <boardname>
Quoted 8 lines
Quoted 4 lines
2) There are some board-specific driver choices, e.g. serial driver, that sometimes appear in the CPU directory. Perhaps these choices are best made in the board directory instead.
There should be no _board_-specific drivers in CPU directories. We had this discussion before, and all such cases that I'm aware of are really _processor_-specific drivers.
Perhaps I am misspeaking. These serial drivers are chip specific. ARM920T, however, is *not* a chip. The serial driver in the ARM920T directory is specific to the Samsung implementation.
Quoted 7 lines
Please name the files if you disagree.
Quoted 3 lines
3) Drivers, such as serial, could be made to export a function table. This would allow driver selection by choosing which file to compile and link.
Isn't this how it works right now?
The serial driver for the samsung chip exports serial_getc(void) and like routines. It could export more generic routines using a jump table. I think this is the same thing that happened with the x86 port.
Cheers.
Quoted 11 lines · In message <[email protected]> you wrote:
Quoted 7 lines
cpu/arm920t/s3c24x0/
cpu/arm920t/other_1/
cpu/arm920t/other_2/...What do you think?
OK. Does this mean that the configuration line in
Makefilesays:... arm arm920t/<subcpu> <boardname>
Probably not. Maybe we can solve this by a #define in the board config file.
Quoted 7 lines
Quoted 3 lines
There should be no _board_-specific drivers in CPU directories. We had this discussion before, and all such cases that I'm aware of are really _processor_-specific drivers.
Perhaps I am misspeaking. These serial drivers are chip specific. ARM920T, however, is *not* a chip. The serial driver in the ARM920T directory is specific to the Samsung implementation.
Understood, and agreed. Should be cleaned up once we implement such a change.
Quoted 10 lines
Quoted 5 lines
Quoted 3 lines
3) Drivers, such as serial, could be made to export a function table. This would allow driver selection by choosing which file to compile and link.
Isn't this how it works right now?
The serial driver for the samsung chip exports
serial_getc(void)and like routines. It could export more generic routines using a jump table. I think this is the same thing that happened with the x86 port.
Which advantage do we get from an additional level of indirection when the functions are known (getc, tstc, putc, puts) anyway? IMHO it just adds additional complexity (needs manual relocation) and wastes memory.
Best regards,
Wolfgang Denk
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de How many NASA managers does it take to screw in a lightbulb? "That's a known problem... don't worry about it."
Quoted 7 lines · Wolfgang Denk
Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 6 lines
cpu/arm920t/s3c24x0/
cpu/arm920t/other_1/
cpu/arm920t/other_2/...
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
What do you think?
OK. Does this mean that the configuration line in
Makefilesays:... arm arm920t/<subcpu> <boardname>
Probably not. Maybe we can solve this by a #define in the board config file.
Now, I'm confused as to how this would work.
In the CPU directory, there is a Makefile and several source files. The Makefile staticly builds the sources and links a library. This library includes the primary entry point for u-boot.
If we want to have a sub-cpu directory, either we implement something in the Makefile that selects source files or we use a goofy #ifdef over the whole contents of a source file--tough for me not to editorialize.
What I'd like to see is something much more flexible. I'd like to let the board directory & Makefile control the whole build. In it, we select the libraries and object file to link in order to get the services we need. This would allow, for example, a serial_lh7a400.c, serial_ns16550.c, and so on to coexist where only one is compiled and linked.
In another example, it seems that we don't share as much of the flash code as we could. Since most flash programming is always the same, we could have a driver that is passed descriptive parameters for the flash device and then handles all of the details. And, here is where the jump table comes in: my board has two kinds of flash. With jump tables, I can use both drivers without using function name tricks.
Quoted 9 lines
Quoted 4 lines
The serial driver for the samsung chip exports
serial_getc(void)and like routines. It could export more generic routines using a jump table. I think this is the same thing that happened with the x86 port.Which advantage do we get from an additional level of indirection when the functions are known (getc, tstc, putc, puts) anyway? IMHO it just adds additional complexity (needs manual relocation) and wastes memory.
It seems to me that there is already some of this going on. In drivers/serial.c, there are calls to the NS16650_ routines. Perhaps there's some middle ground that makes sense.
Chiefly, I'm interested in making this all simpler. I think that the whole-file #ifdefs are not the best method for selecting code. The Linux kernel configuration stuff works well in selecting object files for linking.
obj-$(CONFIG_SERIAL_16550) += serial_16550.o obj-$(CONFIG_SERIAL_LH7A400) += serial_lh7a400.o
This way, we only compile and link the files necessary for each configuration option. It means that we can eliminate redundant code (e.g. serial.c as well as serial-ns16550.c) as well as putting more code into central locations.
That's for the serial driver.
There are other drivers, however, for which we may need more than one. For example, I have two kinds of flash on my board. I'd like to be able to choose from a common set of flash drivers. In this case, jump tables make it possible to link more than one driver of the same type, letting the board specific code handle board-specific setup.
BTW, I don't think that jump-table relocation should be a problem as long as the file is linked to the target (post-relocation) load address. Still, I agree that for the serial driver there is no reason for the dynamic dispatch afforded by a jump table.
Cheers.
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 3 lines
Probably not. Maybe we can solve this by a #define in the board config file.
Now, I'm confused as to how this would work.
Like allthe other source files are woking at the moment.
Quoted 4 lines
If we want to have a sub-cpu directory, either we implement something in the
Makefilethat selects source files or we use a goofy #ifdef over the whole contents of a source file--tough for me not to editorialize.
It's not exactly beautiful, but it's simple, and it works.
Quoted 1 line
What I'd like to see is something much more flexible. I'd like to let
You pay a price for such flexibility.
Quoted 5 lines
the board directory &
Makefilecontrol the whole build. In it, we select the libraries and object file to link in order to get the services we need. This would allow, for example, aserial_lh7a400.c,serial_ns16550.c, and so on to coexist where only one is compiled and linked.
There have been several discussions about things like this, or better config tools, etc.
I need to see a working example before I can comment on such a suggestion.
Quoted 4 lines
In another example, it seems that we don't share as much of the flash code as we could. Since most flash programming is always the same, we could have a driver that is passed descriptive parameters for the flash device and then handles all of the details. And, here is where
Again, flexibility does not come for free. If you try t make the flash driver compile-time configurable, it may easily end up being unreadable. If you configure at run-time you will pay with memory footprint.
We all know that automatic detection can be implemented, and where to find existing code which could be reused. This is theory. In practice, we're better off by just cloning and adapting an existing, highly specialized driver. YMMV.
Again: I'll be happy to accept a better solution, but it must meet two requirements:
* It shall work on all existing boards.
* It shall not have a bigger memory footprint.
Quoted 2 lines
the jump table comes in: my board has two kinds of flash. With jump tables, I can use both drivers without using function name tricks.
No tricks are needed. There are boards with two kinds of flash. This works fine as is.
Quoted 7 lines
Chiefly, I'm interested in making this all simpler. I think that the whole-file #ifdefs are not the best method for selecting code. The Linux kernel configuration stuff works well in selecting object files for linking.
obj-$(CONFIG_SERIAL_16550) += serial_16550.o obj-$(CONFIG_SERIAL_LH7A400) += serial_lh7a400.o
I see, another round of config tool discussion. But this time buttom up :-)
Quoted 2 lines
This way, we only compile and link the files necessary for each configuration option. It means that we can eliminate redundant code
...
Quoted 3 lines
There are other drivers, however, for which we may need more than one. For example, I have two kinds of flash on my board. I'd like to be able to choose from a common set of flash drivers. In this case, jump
Feel free to submit a tested patch. I need to see working code to have something I can criticize ;-)
Quoted 3 lines
BTW, I don't think that jump-table relocation should be a problem as long as the file is linked to the target (post-relocation) load address. Still, I agree that for the serial driver there is no reason
No, it doesn't work that way. U-Boot is always linked to the pre- relocation address in flash. The post-relocation address is not known at compile time, as it may depend on things like RAM size.
Best regards,
Wolfgang Denk
Signature · 5 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de This is a story about sex and drugs and Music with Rocks In. Well... ...one out of three ain't bad. Actually, it's only thirty-three per- cent, but it could be worse. - Terry Pratchett, _Soul Music_
Quoted 5 lines · Wolfgang Denk
Quoted 2 lines
obj-$(CONFIG_SERIAL_16550) += serial_16550.o obj-$(CONFIG_SERIAL_LH7A400) += serial_lh7a400.oI see, another round of config tool discussion. But this time buttom up :-)
No config tool necessary.
I'll make it work for ARM920T and then you can gripe.
Quoted 7 lines
Quoted 3 lines
BTW, I don't think that jump-table relocation should be a problem as long as the file is linked to the target (post-relocation) load address. Still, I agree that for the serial driver there is no reason
No, it doesn't work that way. U-Boot is always linked to the pre- relocation address in flash. The post-relocation address is not known at compile time, as it may depend on things like RAM size.
That's one way. On arm, there isn't any reason to link to the flash address since it does position independent addressing. For the x86 implementation, I reworked the u-boot.lds to locate the pre-relocation code to the flash address and the rest at the runtime address. At least on the targets (three) I've looked at, there is no reason to pick a RAM address at runtime. Why waste the instructions? ;-)
Cheers.