From mboxrd@z Thu Jan 1 00:00:00 1970 From: Helmut Raiger Date: Thu, 06 Oct 2011 15:07:06 +0200 Subject: [U-Boot] [PATCH 2/2] TT-01: add basic board support for HALE TT-01 In-Reply-To: <4E7B3D52.1030805@denx.de> References: <1316693575-20726-1-git-send-email-helmut.raiger@hale.at> <1316693575-20726-3-git-send-email-helmut.raiger@hale.at> <4E7B3D52.1030805@denx.de> Message-ID: <4E8DA7FA.6020304@hale.at> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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.