[U-Boot-Users] [PATCH] Add support for AT91RM9200EK board

[Wolfgang Denk]

In message <010601c89ed1$e51c4dd0$070514ac at atmel.com> you wrote:
>
> > I guess you are aware that the merge window is not open, i. e. you are
> > submitting this only for discussion here...
> 
> The goal is to have the board included when the merge window reopens.

OK, so this submission was for review only.

> >> @@ -0,0 +1,142 @@
> >> +/*
> >> + * (C) Copyright 2002
> >> + * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
> >> + * Marius Groeger <mgroeger at sysgo.de>
> > 
> > A new file, and these are the only copyright holders? Really? Ditto
> > for the other files.
> > 
> 
> Not a new file, a 3+ years old file.

OK, but the copyright entries are still obviously not up to date.
Please fix.

In all similar files, too.

> >> --- u-boot.cmp/board/atmel/at91rm9200ek/flash.c 1970-01-01 01:00:00.000000000 +0100
> >> +++ u-boot/board/atmel/at91rm9200ek/flash.c 2008-04-12 21:15:08.000000000 +0200
> >> @@ -0,0 +1,509 @@
> >> +/*
> >> + * (C) Copyright 2002
> >> + * Lineo, Inc. <www.lineo.com>
> >> + * Bernhard Kuhn <bkuhn at lineo.com>
> > 
> > I think this is CFI conformant flash, isn't it? So please use the CFI
> > driver instead.
> 
> This code has been tested on the AT91RM9200EK board (not by me)
> and CFI code has not been tested on the AT91RM9200EK board.

Then please change the implementation to use the CFI driver, and test
it. There is enough time to make it for the next merge window.

> So I see four possible choices.
> 1) Do not add AT91RM9200EK support in u-boot.
> 2) Add current *tested* support for AT91RM9200EK
> 3) Add AT91RM9200EK with CFI support without more than rudimentary
> testing
> 4) Wait until someone else tests CFI support on AT91RM9200EK and submits
> ----
> Option 5) 
>     "Add AT91RM9200EK after me thorougly testing CFI support" 
>     is not going to happen due to time constraints and/or other
> priorities.

6) Submit patches for AT91RM9200EK with CFI support and wait for
   community feedback.
> 
> The flash driver already exists in the AT91RM9200DK board support
> package.

Yes, this is on the remove list, too.

> This board is obsolete since 4 years or so.

That doesn't really matter. We're talking about adding new code to
the public tree, and we have certain rules doing this. One of these
rules is not to repeat old mistakes - in this case not to add
proprietary flash drivers when the CFI driver can be used.

> I think the most practical approach to allow getting rid of the Atmel
> specific
> version is to include the legacy stuff (AT91RM9200 related) first.
> Then move the shared stuff to an board/atmel/common directory
> to make it easy to clean up for those interested later.

Sorry, we all know how the "clean up later" approach (doesn't) work.

Please change the implementation to use the CFI driver. I will reject
the proprietary flash driver unless there are strikt technical
reasons why the CFI driver cannot be used.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I may be synthetic, but I'm not stupid"  -  the  artificial  person,
from _Aliens_