[U-Boot] [PATCH] arm: at91: at91sam9n12ek: add nandflash/spiflash/mmc/lcd support

[Josh Wu]

Hi, Andreas

On 3/19/2013 7:27 PM, Andreas Bießmann wrote:
> 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 at 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).

Thanks for the information. from the readme, maybe the mtest will be 
removed in later code.
I kept the it in 9n12. And it works fine now. It only test memory: 
0x2000 0000 - 0x26e0 0000

>
> <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.

There are really many useful script for omap3_beagle board. But for the 
9n12 I just want to make the boot command
simple and functional.
For the current senario, I assume the first partition is FAT and kernel 
& dtb binary is there, so when call 'fatload', it will
initialize the mmc then load kernel and dtb.

>
> Best regards
>
> Andreas Bießmann

Best Regards,
Josh Wu