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 josh.wu@atmel.com
<snip>
+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.
<snip>
--- 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.
<snip>
+#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).
<snip>
+#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