[U-Boot-Users] u-boot patch submission process

[Dirk Behme]

Todd Fischer wrote:
> I am working on a patch to add DM355 SoC support to u-boot git tree.  I 
> am following the steps listed at 
> http://www.denx.de/wiki/UBoot/DevelopmentProcess.  Since the DM355 has 
> an ARM9 processor, I am creating a patch for the u-boot-arm git tree.
> 
> Since one of the steps is to verify MAKEALL builds cleanly, I first ran 
> MAKEALL before applying my patch to make sure all is well.  Since I am 
> focusing on ARM9, I only did a MAKEALL for ARM9 (I don't have a PCC 
> toolchain).  Specifically, I did:
> 
> git clone git://www.denx.de/git/u-boot-arm.git u-boot-arm
> cd u-boot-arm
> CROSS_COMPILE=arm-linux-  BUILD_DIR=/tmp/build MAKEALL_LOGDIR=/tmp/log 
> ./MAKEALL ARM9
> 
> Much to my surprise, I not only got warnings, I got errors.  The main 
> error being:
> 
> mpc8xxx_spi.c:26:29: error: asm/mpc8xxx_spi.h: No such file or 
> directory.  I see a patch to fix this error has been submitted.
> 
> The second error I  got relates to PUEN() / __REG2() macros.  I am not 
> sure if that is a real error or caused by the toolchain I am using.  I 
> looked at the code, but got lost with the use of __builtin_constant_p() 
> in asm-arm/arch-pxa/hardware.h where __REG2 is defined as
> 
> # define __REG2(x,y)    \
>         ( __builtin_constant_p(y) ? (__REG((x) + (y))) \
>                                   : (*(volatile u32 *)((u32)&__REG(x) + 
> (y))) )
> 
> I have no way to check that any change I propose works so I don't feel 
> comfortable providing a patch, (plus the above code reinforces my 
> believe that accessing hardware should be made obvious using inl() 
> outl() type API). Does anyone else see this error when using MAKEALL for 
> ARM9?
> 
> Questions:
> 
> 1) Did I do something wrong on how I invoked MAKEALL or maybe have a 
> toolchain issue?  

Regarding the spi.h most probably not.

> Should MAKEALL ARM9 build cleanly?

No. The u-boot-arm.git is broken. I got the (unfortunately) private 
answer from Peter (ARM custodian) regarding

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/36022/

-- cut --
Yup - my mail with subject

[U-Boot-Users] [PATCH] [DRIVERS] <asm/mpc8xxx_spi.h> is onlyavailable
for ppc boards.

I'm assuming this will eventually be properly patched in the main tree
- meanwhile anyone building ARM code can apply this patch
-- cut --

This is quite annoying, particularly if you should test patches 
already in that tree or against that. IMHO the arm.git should contain 
the fix temporarily to be able to build cleanly instead of staying 
with a broken tree.

To workaround I would propose that you test MAKEALL against mainline 
U-Boot git.

> 2) I am looking at providing two patchs.  One to add  DM355 SoC support 
> and a second to add DFU USB functionality to the u-boot git tree.  
> Should I post the USB patch based on u-boot-arm git tree or the 
> u-boot-usb git tree.  I have to have DM355 SoC support in order to test 
> the DFU USB functionality so I would rather have the patches apply to 
> the same git tree.  The USB code in both git trees is nearly identical.  
> Any suggestions?

Same as above: Test your patches against mainline U-Boot git and then 
send them as a series with anything like

[PATCH 1/2] ARM: DAVINCI: DM355 SoC
[PATCH 2/2] USB: DFU USB functionality

Nice to hear that you work on DM355.

Dirk

> 3) There are common USB UDC API definitions in usbdcore_omap1510.h and 
> usbdcore_mpc8xx.h  (and they don't agree on the API).  I am adding 
> usbdcore_musb.h that supports the same API.  Should I pull out this 
> common code in a separate patch or as part of the USB UDC support for 
> DM355 patch?
> 
> The API I found to be common includes:
> 
> int udc_init(void);
> void udc_irq(void);
> int udc_endpoint_write(struct usb_endpoint_instance *endpoint);
> void udc_setup_ep(struct usb_device_instance *device, unsigned int ep,
>                   struct usb_endpoint_instance *endpoint);
> void udc_connect(void);
> void udc_disconnect(void);
> void udc_enable(struct usb_device_instance *device);
> void udc_disable(void);
> void udc_startup_events(struct usb_device_instance *device);
> void udc_set_nak(int epid);
> void udc_unset_nak (int epid);
> 
> Note that omap has udc_endpoint_write() not returning a value, but 
> drivers/serial/usbtty.c expects udc_endpoint_write() to return a result 
> code.
> 
> Basically I am confused on how to submit patches when one patch requires 
> another patch and the patches could be applied to different git 
> custodian trees.
> 
> Thanks,
> 
> Todd