Hi Stefano,
Le Sun, 30 Mar 2014 18:20:49 +0200,
Stefano Babic <sba...@denx.de> a écrit :
> I jump directly to V4 ;-) Sorry for late review, I was not in office
> last days.
>
> Added Ben in CC. He sent a first version for the Marsboard.
>
> On 29/03/2014 22:29, Eric Bénard wrote:
> > this board is produced by Embest/Element 14 and is based on i.MX6 Solo
> > The following features are tested :
> > - UART2 (console)
> > - eMMC
> > - SDCard
> > - uSDCard
> > - Ethernet
> > - USB Host (through 4 ports hub)
> > - HDMI output
> > - I2C 1/2/3
> > - LVDS TFT with LCD8000-97C from Embest/Element 14
> >
> > Boot on eMMC and through USB loader are tested.
> >
> > For more informations on this board : http://www.riotboard.org/
> >
> > Signed-off-by: Eric Bénard <e...@eukrea.com>
> > ---
>
> A general remark. I agree by reading the whole thread about checking at
> runtime which is the running board (you do it getting the cpu type).
>
> However, you use also a compiler switch mechanism, adding RIOTBOARD or
> MARSBOARD in the boards.cfg. You have implemented two ways to for the
> same thing. This makes in principle your runtime detection useless,
> because you can use #if CONFIG_MARSBOARD instead of "if board_type ==
> BOARD_IS_MARSBOARD)".
True, as said in the log, anyway at the moment the same code can't run
on both boards because of the different CPU (Solo vs Dual - and not Dual
Lite).
> Is it possible to use only the runtime detection ?
> I think the main problem is CONFIG_ENV_IS_*, that is different for the
> two boards. What do you think about it ?
>
I'll see how we can handle the 2 CONFIG_ENV_IS with runtime detection.
> IMHO you could also squash the two patches together. You add new files,
> and patch 2/2 changes some of them. I think in this case having a single
> patch makes review easier.
>
OK no problem.
> > + /* from linux/arch/arm/mach-imx/mach-imx6q.c :
>
> Codestyle in U-Boot for multiline comments is:
>
> /*
> * ...
> */
>
OK will fix. FWIW checkpatch doesn't provide any warning concerning
this problem.
> > +int board_video_skip(void)
> > +{
> > ..././//
> We have already discussed in the past about this function. Each board
> (at least, imx6 board) want to have such of them, and code is
> duplicated. What about to factorize it ? I am not against to move it
> into imx-common, if we generally agree, but I would like to avoid to
> duplicate this function for each board.
>
OK, if I understand correctly you want me to factorize it ? ;-)
I'll see what I can do there.
Thanks,
Eric
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
