Hi Ahmad, On 23 October 2014 00:08, Ahmad Draidi <ar200...@gmail.com> wrote: > Hello, Mr. Glass. > > On Wednesday 22 October 2014 11:29:00 AM Simon Glass wrote: >> One little nit below but it looks OK to me. I'm assume that no one >> would want to replace the command line completely? >> > > In some setups, one image can be used with several versions, or even > different boards of hardware. The bootloader passes a command line > argument to specify this. Also, the header file for the mkbootimg > specifies that the "kernel_args[] is appended to the kernel > commandline". So I thought this would make it consistent with other > bootloaders, and the spec. > >> I hope you can write a test too. Re your comment about not wanting to >> change the code much - if we go that way the code will get really >> ugly. When you add features you often need to refactor. When there are >> tests, it becomes easier to know you have not broken things. > > Thanks for the tip. > I'll try to write a test if I get the time. > One thing comes to mind. To be able to write a test, we'll need an image > to test against. I think pulling in the mkbootimg tool is the right move. > The other option I can think of is bundling a small dummy image with > U-Boot, which I think is a bit ugly. What do you think?
Yes bringing in the tool sounds good. It should probably be integrated as just another output from mkimage. > >> Reviewed-by: Simon Glass <s...@chromium.org> >> nit: return -ENOMEM - suggest adding a comment to >> android_image_get_kernel() so its args and return value are >> documented. >> > > Thank you. I'll send in a new version soon. > >> Regards, >> Simon > > Regards, > Ahmad Draidi Regards. Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
