[U-Boot] [PATCH 2/2] TT-01: add basic board support for HALE TT-01

[Helmut Raiger]

Hi Stefano,

>> +include $(TOPDIR)/config.mk
>> +
>> +LIB	= $(obj)lib$(BOARD).o
>> +
>> +COBJS	:= tt01.o
>> +# reuse the mx31pdk low-level setup
>> +SOBJS	:= ../../freescale/mx31pdk/lowlevel_init.o
> It is always a good idea to reuse code, but taking it to another board
> seems hackish. Your board could become broken if the mx31pdk's
> maintainer change his code.
>
> Reading this file I do not see (except setting the AIPS) no good reason
> to write this part in assembly. Everything can be done for example in
> board_early_init_f, and even better we can rationalize this code and put
> it into arch/cpu/arm1136/mx31.

As far as I understood this is called from arch/arm/cpu/arm1136/start.S
before stack is setup. I don't know much about C-calling convention
on the arm1136, but this might be the reason why it's done in assembly.
I'd rather not touch start.S, so I'll copy the file over from mx31pdk?

>> +#define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + 10*1024*1024)
> 10 MB for heap in bootloader ? Is it ok ? I am only asking if it is
> really wanted.

We are about to display large compressed bitmaps in u-boot, that's why
the heap is that large. The frame buffer driver patch
http://patchwork.ozlabs.org/patch/113341/ is still being reviewed,
that is why I left it out here (but kept the heap size).
Additionally I don't care much about time and space here. The production
units will boot from NAND and we'll use a different setup there.
That's why I reserved 1MB for u-boot, I simply didn't want it to overwrite
my environment when being reprogrammed.

>> +#define CONFIG_SYS_FLASH_CFI		/* Flash memory is CFI compliant */
>> +#define CONFIG_FLASH_CFI_DRIVER		/* Use drivers/cfi_flash.c */
>> +#define CONFIG_FLASH_SPANSION_S29WS_N
>> +/* TODO: bluetechnix did undefine these for some purpose
> if you do not need to undefine, you can drop this comment. Maybe there
> is no issues with lock/unlock mechanism with the flash you have chosen.

Bluetechnix is the supplier of the SOM we are using. Their original
version of u-boot (1.2 or so) defined these values. So the flash is
definitely the same. I'd like to review this later, therefore the TODO.

>> +#define CONFIG_ENV_SECT_SIZE	(128 * 1024)
>> +#define CONFIG_ENV_SIZE		CONFIG_ENV_SECT_SIZE
> Regarding your previous comment: you could set CONFIG_ENV_SIZE to a
> smaller value as CONFIG_ENV_SECT_SIZE, and this can speed up get/set of
> the environment. Or you could save the environment in tha last (smaller)
> sectors.
>
I'll look into the speed change, but as described above I don't
really care about size.
>> + * on TT-01 UART1 pins are used by Audio, so we use UART2
>> + *   make sure that the transceiver is enabled during PL=1 for testing!
> What does it mean PL=1 ?
>
Nothing that concerns u-boot, it means P(ower)L(evel)=1. The TT-01
implements a hardware that turns off components depending on
the said power level. In PL=1 the RS232 transceiver is usually off.

>> +/* this is currently not supported, mxc_nand.c is too incomplete for it */
> Only for my understanding: Which is the issue with mxc_nand.c ? At the
> moment, we have several boards using it, and I wonder it is incomplete.
> What do you mean ?

Part of this whole mess is, that I actually wrote this board support 2
years ago and simply rebased to finally contribute the stuff. Probably
there is no issue with mxc_nand.c any more (and I don't remember
what was the problem).

Thanks for your thorough review, I'll pass along V2 when we come
to a solution about low_level.S

Helmut


--
Scanned by MailScanner.