From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Andreas_Bie=DFmann?= Date: Tue, 19 Mar 2013 12:27:36 +0100 Subject: [U-Boot] [PATCH] arm: at91: at91sam9n12ek: add nandflash/spiflash/mmc/lcd support In-Reply-To: <514844C3.10105@atmel.com> References: <1363342624-2939-1-git-send-email-josh.wu@atmel.com> <51471B46.5000900@gmail.com> <514844C3.10105@atmel.com> Message-ID: <51484BA8.7040509@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Josh, On 03/19/2013 11:58 AM, Josh Wu wrote: > Hi, Andreas > > thanks for the review. > > On 3/18/2013 9:48 PM, Andreas Bie?mann wrote: >> Dear Josh Wu, >> >> this is an additional review. I left out MAINTAINERS, alphabetical >> ordering, copyright stuff a.s.o. mentioned before. >> >> On 03/15/2013 11:17 AM, Josh Wu wrote: >>> This patch adds at91sam9n12ek support, it enables: >>> - dbgu >>> - nand with pmecc >>> - spi flash >>> - mmc >>> - lcd >>> >>> TODO: >>> - usb >>> - ethernet >>> >>> Signed-off-by: Josh Wu >>> --- >>> +void spi_cs_activate(struct spi_slave *slave) >>> +{ >>> + switch (slave->cs) { >>> + case 0: >>> + at91_set_pio_output(AT91_PIO_PORTA, 14, 0); >> Ouch ... before you setup these as peripherial lines, here you use it as >> PIO. Please a) setup as PIO or b) do not set the line here cause it >> should be set by SPI IP automagically on transfer (havn't checked that, >> but should work). > > I prefer to choose a) setup as PIO. for b), it may impact many other > boards. I'm fine with this solution. >>> --- a/drivers/spi/atmel_spi.c >>> +++ b/drivers/spi/atmel_spi.c >>> @@ -92,7 +92,8 @@ struct spi_slave *spi_setup_slave(unsigned int bus, >>> unsigned int cs, >>> as->slave.cs = cs; >>> as->regs = regs; >>> as->mr = ATMEL_SPI_MR_MSTR | ATMEL_SPI_MR_MODFDIS >>> -#if defined(CONFIG_AT91SAM9X5) || defined(CONFIG_AT91SAM9M10G45) >>> +#if defined(CONFIG_AT91SAM9X5) || defined(CONFIG_AT91SAM9M10G45) \ >>> + || defined(CONFIG_AT91SAM9N12) >> I mentioned that before in a mail to Bo, can we please find some better >> solution here like 'CPU_HAS_MCIx' (like the CPU_HAS_PIO3) or some other >> identifier? > > for the SPI ip, I will include a extra patch in next version, which will > use a run-time ip detect for SPI. > so those macro can be removed. Wow, great. This will be a step in the right direction. >>> +#define CONFIG_SYS_MEMTEST_START CONFIG_SYS_SDRAM_BASE >>> +#define CONFIG_SYS_MEMTEST_END 0x26e00000 >> Wasn't there some change in mtest lately? Are these configs correct then? > > hmm, I don't know the mtest well, this END address is just align with > 9x5 config file. Can you please read the doc/README.memory-test introduced in a2681707b2478abef34b8c403e7ab52daae9c331 Please check that we haven't placed the exception table at CONFIG_SYS_SDRAM_BASE and that we will not scratch the relocated u-boot copy at 0x26e00000 (110MiB, i think it is ok). >>> +#else /* CONFIG_SYS_USE_MMC */ >>> + >>> +/* bootstrap + u-boot + env + linux in mmc */ >>> +#define CONFIG_ENV_IS_IN_MMC >>> +/* For FAT system, most cases it should be in the reserved sector */ >>> +#define CONFIG_ENV_OFFSET 0x2000 >>> +#define CONFIG_ENV_SIZE 0x1000 >>> +#define CONFIG_SYS_MMC_ENV_DEV 0 >>> +#define CONFIG_BOOTCOMMAND \ >>> + "mmcinfo;fatload mmc 0:1 0x21000000 dtb;" \ >> Isn't mmcinfo the old command? AFAIR this is obsolete with >> CONFIG_MMC_GENERIC, please fix and use the newer commands. > > I checked the source, seems not found the information about the mmcinfo > is old command. > But if we remove ;mmcinfo' in the bootcommand, it still works well. so I > will remove 'mmcinfo' here. Ok, it was my fault. mmcinfo is available in the 'generic' implementation, but this is to display information of the card (see common/cmd_mmc.c). The old commands are 'mmc init' (initialize) and 'mmc device' (show card info). You should however add some 'mmc rescan' (initialize) and maybe 'mmc dev $mmcdev $mmcpart' (set active mmc and partition) before to ensure setup the correct mmc device and have it enabled before accessing it. I found the omap3_beagle script for CONFIG_BOOTCOMMAND quite useful. Best regards Andreas Bie?mann