[U-Boot-Users] U-boot code etiquette

[nbryan at embebidos.com]

Dear Wolfgang,

I thought my suggestions may prove unpopular,
but I did not expect quite such a roasting ;-)

Please read my defence below.

u-boot-users at lists.sourceforge.net

To:             	Neil Bryan <nbryan at embebidos.com>
Copies to:      	u-boot-users at lists.sourceforge.net, pperez at embebidos.com
From:           	Wolfgang Denk <wd at denx.de>
Subject:        	Re: [U-Boot-Users] U-boot code etiquette 
Date sent:      	Mon, 24 Jan 2005 11:12:17 +0100

> Dear Neil,
> 
> in message <1106558355.3698.47.camel at daroca> you wrote:
> > 
> > The board has a limited EEPROM storage area and it has been decided that
> > U-boot environment variables are to be stored in EEPROM.  
> 
> Don't do that. Using  an  EEPROM  for  the  environment  has  several
> serious   disadvantages   (slow   boot  speed,  risk  of  losing  the
> environment). I strongly  recommend  to  use  flash,  porobably  even
> redundand flash sectors.

For some reason, which I do not know, a policy decision has 
been made to use EEPROM for all configuration data.  This 
includes U-Boot environment variables.  Now this is a 
particularly weak argument on my part, but in this context, 
I am just blindly following orders.  However, I can ask  
the question to my policy-makers and make a strong 
argument to them based on your reservations NOT to use 
EEPROM for U-boot environment variables.  This point
shall be followed up.

> 
> > U-boot uses quite a large amount of environment variable space,
> 
> This depends on what you do with it. If you want,  you  can  do  with
> just  128  bytes or so. But then a lot of the power of U-Boot results
> from clever use of the environment, so you  have  to  decide  if  you
> really want to castrate U-Boot.
No, the idea was to leave most of U-Boot's environment space  
untouched, only remove the parts that are common to both U-Boot
and the OS that is to be loaded.  This results in a very small
saving in space, but ties in with the proposed pseudo-file 
structure scheme.
> 
> > in fact the default size for the AM91RM9200DK CFG_ENV_SIZE is 0x2000
> 
> The default isto use flash, too, where this is not a problem at all.
At this point, lets just assume that we are using EEPROM!

> 
> > (8kbytes). So all my EEPROM disappears to U-boot leaving non for Linux.
> 
> Who says that Linux and U-Boot cannot share the same storage?
Good point, but parsing can be a pain with the way that U-Boot 
stores environment variables.  It throws everything into one big 
pile and passes all the work of finding what you need to the driver.
My proposed scheme is to help simplify parsing.

> 
> > BUT, cetain variables such as the board IP address need to be common 
> > to both Linux and U-boot.  So if the board IP address is changed by the
> > OS or by Linux, the change needs to be visible to the other.
> 
> Right. So share the environment.
Yes, if the environment was nice to use.  I get the impression that
as it was designed to use FLASH, U-Boot can be very generous in the
amount of space it reserves for a simple value.  We have a small 
EEPROM and I cannot justify this expense.
> 
> > SOLUTION:
> > To divide the EEPROM into a psuedo-file structure.  We have data 
> 
> What a overkill.
How can you say this with such little knowledge of what is proposed?
I value your opinion as this is why I asked for it, but _I_ find 
it very hard to discount an idea until I at least understand the
detail.  
> 
> > structures of varying lengths and types to pack more data into the 
> 
> KISS: we already have that. We have  name=value  tuples,  encoded  in
> plain text with variable length. What else do you need?
The current U-Boot scheme works very well for the limited number of
variables needed by the environment.  But with other OSes (not Linux)
this number can be very large.  If a single flag (1bit) has to be 
stored as an ascii encoded identifier followed by 'yes' or 'no',
we will soon run out of space.  Using a single byte, I can represent
eight flags.  Of course, it is possible to use U-Boot in this way,
but it adds overhead.  However, you have got me thinking on this
point!
> 
> > EEPROM.  We need to support a variety of OSes on the board and will 
> > have several OS-specific data structures.
> 
> Please provide an example that cannot be encoded in the way mentioned
> above.
The difficulty is not to encode the data, but to encode it 
efficiently and easily accessible by the OS driver that has to use 
it.
> 
> > One data structure would be reserved for U-boot, length 2kBytes,
> > 
> > However, common values such as IP address will be stored in
> > this psuedo-file structure.
> 
> Doesn't make sense. Don't duplicate the same data if you can avoid it.
This is exactly the idea.  To have common data shared between an OS 
and U-Boot, but not using the U-Boot method for storing the common 
data.
> 
> > When U-boot reads the environment variables, it needs to check in two
> > places in EEPROM:
> 
> Such a modifcation is avoidable.
> 
> >  o I don't want to make board-specific changes in common code areas.
> >    I can see my proposal affecting cmd_nvedit.c.  This code does
> >    not have pre-processing for multiple board-types and so 
> >    architecturaly it feels wrong. 
> 
> I don't see what  sort  of  board  specific  preprocessing  might  be
> needed. Maube you can elucidate.
If my board uses a different data structure for storing NV data,
then when U-Boot attempts to read, for example ipaddr, it will be 
routed through a different interface to access the value stored on
the EEPROM.  The value returned to U-Boot will be the same, just
it will not be stored in the long chain of strings as is the
current scheme.  This requires board-specific code to be added
to functions like getenv().  But I don't want to do this as it
breaks the architecture of U-Boot.

> 
> >  o I need to hook into functions getenv() and setenv().  I will read
> >    a table of constant strings and compare with the environment 
> >    variable which is being sought and then parse it appropriately.
> 
> Don't do that. Don't duplicate function, don't duplicate code,  don't
> duplicate data. Use one set of data.
My proposal is to use one set of data.  The intention is to extend 
not duplicate.  The long and the short is that U-Boot stores data in 
a way that is friendly only to U-Boot.  Any other driver that needs
access to this data has an horrendous job of parsing it.

> 
> > I don't want to spend lots of time making theses suggested changes
> > to have it rejected for inclusion into the mainline code.  Will 
> > it be accepted or is it just too far outside of the way U-boot 
> > evolves?  Is there a better solution?
> 
> If you have the choice, then provide access functions to  the  U-Boot
> environment  for  your other components that need to access (read and
> write) configuration data. For Linux this is trivial - see  the  code
> in tools/env as example. If you really *must* use a different storage
> format,  then  use  this  for  everything  and provide a complete new
> access method for U-Boot.
I have to support several other OSes aside from Linux.  I am happy to 
change the EEPROM interface to U-Boot if it makes for an improved 
U-Boot architecture.  I really don't want to hack it, I want to keep
the same interface and only change areas that are specific to my 
board.  The problem is, that the env_x and cmd_x code is in the 
common area.

My final point is, if I make the changes in a board-specific 
directory, would you include it in the U-Boot tree?  If not,
then I shall stop working on this problem now.

Thanks for the discussion.

Regards,

Neil.
> 
> Best regards,
> 
> Wolfgang Denk
> 
> -- 
> Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> Why don't you have a Linux partition installed so you can be  working
> in  a  programmer-friendly environment instead of a keep-gates'-bank-
> account-happy one? :-)                            -- Tom Christiansen
> 


************************************************************
 Neil Bryan
 Ingeniero de Desarrollo.
 Sistemas Embebidos, S.A. 
 C/ Calvo Sotelo, 1 1º Dcha, 26003 Logroño
 Tfno: ++34 941 270060, Fax: ++34 941 237770
************************************************************