[U-Boot] Preparing a patch for u-boot

[U-Boot] Preparing a patch for u-boot

[Daniel Jabbour]

Hi,

After reading over the U-Boot patch submission guidelines, I want to  
ask the list before I go ahead and prepare a patch. I am working with  
a hardware vendor using the XBurst CPU SOC (system on chip). It's  
basically a MIPS32 compatible CPU with various integrated functions  
made by Ingenic Semiconductor. A number of low-cost tiny laptops are  
using the chipset, as well as some embedded systems.

The vendor has done a reasonable job releasing all their work under  
GPL and getting the boards to run on Linux 2.4/2.6 series kernels.  
They also patched U-Boot to work with their specific chipset and  
boards. The patch is released here: ftp://ftp.ingenic.cn/3sw/01linux/01loader/u-boot/u-boot-1.1.6-jz-20080530.patch.gz

After looking through the patch, it seems to do the following major  
things (against U-Boot 1.1.6):
  * Add board support for the various boards that use their XBurst  
chipset
  * Added LCD support for their boards
  * Add specific CPU code to the MIPS CPU for their XBurst chipset
  * Modified the NAND read/write/erase commands to enable bad block  
handling

My question is essentially: besides updating the patch against the  
latest GIT version of U-Boot, what would you recommend I do with the  
code to assist in integrating it into the U-Boot mainline? I realize  
there are some stylistic changes, but overall, if you could briefly  
review the patch and give me some pointers it would be most helpful  
(i.e. would you like this split into several patches, does the patch  
make any changes that you would not consider for introduction into the  
mainline, etc)? Thank you very much in advance,

Daniel

--
Daniel Jabbour
Software Engineer
Laptouch, Inc.

[U-Boot] Preparing a patch for u-boot

[Jerry Van Baren]

Daniel Jabbour wrote:
> Hi,
> 
> After reading over the U-Boot patch submission guidelines, I want to  

That's novel, reading the manual first.  ;-)

> ask the list before I go ahead and prepare a patch. I am working with  
> a hardware vendor using the XBurst CPU SOC (system on chip). It's  
> basically a MIPS32 compatible CPU with various integrated functions  
> made by Ingenic Semiconductor. A number of low-cost tiny laptops are  
> using the chipset, as well as some embedded systems.

Excellent.

> The vendor has done a reasonable job releasing all their work under  
> GPL and getting the boards to run on Linux 2.4/2.6 series kernels.  
> They also patched U-Boot to work with their specific chipset and  
> boards. The patch is released here: ftp://ftp.ingenic.cn/3sw/01linux/01loader/u-boot/u-boot-1.1.6-jz-20080530.patch.gz

Its BIG. B.I.G.  You need to split it up into reasonable sized 
orthogonal pieces.  Your list (below) is probably a good start at where 
to split the pieces.

> After looking through the patch, it seems to do the following major  
> things (against U-Boot 1.1.6):
>   * Add board support for the various boards that use their XBurst  
> chipset

Split: a patchset per board.

>   * Added LCD support for their boards

If this is used on multiple boards, it should be part of the first board 
that uses it.  Don't throw it out on the list without a board that uses it.

>   * Add specific CPU code to the MIPS CPU for their XBurst chipset

OK, that probably goes with the first board patches or first followed by 
the board(s).

>   * Modified the NAND read/write/erase commands to enable bad block  
> handling

I'm not a NAND expert, but I would review this to make sure it doesn't 
duplicate existing code (your patch file lump is pretty old at 1.1.6).

> My question is essentially: besides updating the patch against the  
> latest GIT version of U-Boot, what would you recommend I do with the  
> code to assist in integrating it into the U-Boot mainline? I realize  

0) Update against the head in git.
1) Break it up into reviewable patchsets (preferably under 40K, 
definitely under 100K unless you have really good pastries and 
permission from the proper $deity).
2) Fix all the coding style issues you can find.  It's OK, we will still 
find some more. ;-)
    * I saw some // commented out lines, looked like debug hacks.
3) Start slinging patchsets onto the list.  You should start with one 
board (the SIMPLEST one) and whatever pieces are necessary for it to be 
operation.  It will likely take a few patchset iterations to get it in 
u-boot.  Don't get discouraged, improve the code and iterate.  After 
getting the first board in, you will have learned a lot, we will have 
learned a lot, and the rest will go a lot smoother.

> there are some stylistic changes, but overall, if you could briefly  
> review the patch and give me some pointers it would be most helpful  
> (i.e. would you like this split into several patches, does the patch  

YES.

> make any changes that you would not consider for introduction into the  
> mainline, etc)? Thank you very much in advance,

I don't know, it is too big to do a decent review on.  My breezing 
through gave me the impression that it is mostly additions of boards and 
related stuff which should go in OK.  Additive is easier than subtractive.

> 
> Daniel

Best regards,
gvb