[U-Boot-Users] Manufacturer specific code in a bothersome place

[U-Boot-Users] Manufacturer specific code in a bothersome place

[Marc Singer]

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.

[U-Boot-Users] Manufacturer specific code in a bothersome place

[Marc Singer]

On Mon, Jul 07, 2003 at 04:20:41PM +0200, Wolfgang Denk wrote:
> In message <20030707002607.GA4106@buici.com> 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
> 
>   	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>

> 
> >   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.

> Please name the files if you disagree.
> 
> >   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.

[U-Boot-Users] Manufacturer specific code in a bothersome place

[Wolfgang Denk]

In message <20030707152907.GA2816@buici.com> you wrote:
>
> >   	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>

Probably not. Maybe we can solve this  by  a  #define  in  the  board
config file.

> > 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.

> > >   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

-- 
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."

[U-Boot-Users] Manufacturer specific code in a bothersome place

[Marc Singer]

On Mon, Jul 07, 2003 at 06:33:32PM +0200, Wolfgang Denk wrote:
> In message <20030707152907.GA2816@buici.com> you wrote:
> >
> > >   	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>
> 
> 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.

> > 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.

[U-Boot-Users] Manufacturer specific code in a bothersome place

[Wolfgang Denk]

In message <20030707184116.GA26444@buici.com> you wrote:
>
> > 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.

> 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.

It's not exactly beautiful, but it's simple, and it works.

> What I'd like to see is something much more flexible.  I'd like to let

You pay a price for such flexibility.

> 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.

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.


> 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.

> 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.

> 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 :-)

> This way, we only compile and link the files necessary for each
> configuration option.  It means that we can eliminate redundant code
...
> 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 ;-)

> 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

-- 
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_

[U-Boot-Users] Manufacturer specific code in a bothersome place

[Marc Singer]

On Tue, Jul 08, 2003 at 12:37:06AM +0200, Wolfgang Denk wrote:
> >   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 :-)

No config tool necessary.

I'll make it work for ARM920T and then you can gripe.

> > 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.