[U-Boot] [PATCH v2] [ARM] at91: Add support for taskit AT91SAM9G20 boards

[Tom Rix <tom@bumblecow.com>]

Achim Ehrlich wrote:
> Tom wrote:
>> Achim Ehrlich wrote:
>>> This adds support for the AT91SAM9G20 boards by taskit GmbH.
>>> Both boards, Stamp9G20 and PortuxG20, are integrated in one file.
>>> PortuxG20 is basically a SBC built around Stamp9G20.
>>>
>>> Signed-off-by: Achim Ehrlich <aehrlich@taskit.de>
>>> ---
>>>  MAINTAINERS                        |    5 +
>>>  MAKEALL                            |    1 +
>>>  Makefile                           |    4 +
>>>  board/taskit/stamp9G20/Makefile    |   50 +++++++++
>>>  board/taskit/stamp9G20/config.mk   |    1 +
>>>  board/taskit/stamp9G20/stamp9G20.c |  203 
>>> ++++++++++++++++++++++++++++++++++++
>>>  include/configs/stamp9G20.h        |  180 
>>> ++++++++++++++++++++++++++++++++
>>>  7 files changed, 444 insertions(+), 0 deletions(-)
>>>  create mode 100644 board/taskit/stamp9G20/Makefile
>>>  create mode 100644 board/taskit/stamp9G20/config.mk
>>>  create mode 100644 board/taskit/stamp9G20/stamp9G20.c
>>>  create mode 100644 include/configs/stamp9G20.h
>>>
>> Are more taskit based boards expected?
>> If not taskit/stamp9G20 could be reduced to just taskit
>>
> 
> Yes, there are more taskit boards expected.
>  

Ok

>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 7f40ebd..c4259a0 100644
<snip>

>>> +    writel(AT91_SMC_MODE_RM_NRD | AT91_SMC_MODE_WM_NWE |
>>> +        AT91_SMC_MODE_EXNW_DISABLE |
>>> +#ifdef CONFIG_SYS_NAND_DBW_16
>>> +               AT91_SMC_MODE_DBW_16 |
>>> +#else /* CONFIG_SYS_NAND_DBW_8 */
>>> +               AT91_SMC_MODE_DBW_8 |
>>> +#endif
>> CONFIG_SYS_NAND_DBW_16 is unused.
>> The if-def should be removed
>>
Expecting dead code to be removed

>>> +               AT91_SMC_MODE_TDF_CYCLE(2),
>>> +        &smc->cs[3].mode);
>>> +

<snip>

>>> +    /* adress of boot parameters */
>>> +    gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100;
>>> +
>>> +    at91_set_pio_output(AT91_PIO_PORTC, 9, 1);
>>> +    at91_set_pio_output(AT91_PIO_PORTC, 5, 1);
>> Change immediates to logical #defines
> 
> Sorry, I don't get that clearly. In the moment I would take that to mean
> that I should do something like
> #define CONFIG_BOOT_PARAMS CONFIG_SYS_SDRAM_BASE + 0x100
> in the appropriate place
> and use here:
> gd->bd->bi_boot_params = CONFIG_BOOT_PARAMS
> or am I wrong?

I was referring to 9 and 5 arguments to at91_set_pio_output
You could also change the bi_boot_params but that isn't as
important.

> 
>>> +
>>> +    at91_serial_hw_init();

<snip>

>>> +
>>> +#ifdef CONFIG_USE_IRQ
>>> +#error CONFIG_USE_IRQ not supported
>>> +#endif
>>> +
>>> +#endif
> 
> Thanks for your attention
> 
> Kind regards
> 
> Achim
> 
Tom