[U-Boot] [PATCH v2 4/4] arm, at91: add siemens corvus board

Heiko Schocher
2013-11-04T09:53:15Z

Hello Andreas,

Am 04.11.2013 09:53, schrieb Andreas Bie?mann:
> Hi Heiko,
>
> On 11/04/2013 07:40 AM, Heiko Schocher wrote:
>> enable support for the siemens AT91SAM9G20 based board corvus.
>>
>> Signed-off-by: Boris Schmidt<boris.schmidt@siemens.com>
>> Reviewed-by: Heiko Schocher<hs@denx.de>
>> Cc: Andreas Bie?mann<andreas.devel@googlemail.com>
>> Cc: Bo Shen<voice.shen@atmel.com>
>>
>> ---
>> - changes for v2:
>> - add comments from Bo Shen
>> - use gpio api
>> - remove unneccessary comment
>> - use at91_wait_for_reset()
>> - remove unneccessary code in board file
>> - Coding Style cleanup (tabs and unneccessary 1 after config define
>> removed, use ".xxx = x" notation for initializing structs)
>> - remove reset_phy()
>> - remove CONFIG_SYS_MEMTEST_x defines, as mtest command
>> is not used on this board.
>> - changes load address
>> - delete lcd support
>> - add comments from Andreas Bie?mann:
>> - rearrange some init calls
>> - remove some unneeded ifdef
>> ---
>> board/siemens/corvus/Makefile | 39 ++++++++
>> board/siemens/corvus/board.c | 216 ++++++++++++++++++++++++++++++++++++++++++
>> boards.cfg | 1 +
>> include/configs/corvus.h | 165 ++++++++++++++++++++++++++++++++
>> 4 files changed, 421 insertions(+)
>> create mode 100644 board/siemens/corvus/Makefile
>> create mode 100644 board/siemens/corvus/board.c
>> create mode 100644 include/configs/corvus.h
>>
>> diff --git a/board/siemens/corvus/board.c b/board/siemens/corvus/board.c
>> new file mode 100644
>> index 0000000..11f0d49
>> --- /dev/null
>> +++ b/board/siemens/corvus/board.c
>> @@ -0,0 +1,216 @@
[...]
>> +#ifdef CONFIG_MACB
>> +static void corvus_macb_hw_init(void)
>> +{
>> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
>> + struct at91_rstc *rstc = (struct at91_rstc *)ATMEL_BASE_RSTC;
>> + unsigned long erstl;
>> +
>> + /* Enable clock */
>> + writel(1<< ATMEL_ID_EMAC,&pmc->pcer);
>> +
>> + /*
>> + * Disable pull-up on:
>> + * RXDV (PA15) => PHY normal mode (not Test mode)
>> + * ERX0 (PA12) => PHY ADDR0
>> + * ERX1 (PA13) => PHY ADDR1 => PHYADDR = 0x0
>> + *
>> + * PHY has internal pull-down
>> + */
>> + at91_set_pio_pullup(AT91_PIO_PORTA, 15, 0);
>> + at91_set_pio_pullup(AT91_PIO_PORTA, 12, 0);
>> + at91_set_pio_pullup(AT91_PIO_PORTA, 13, 0);
>
> Here applies the same statement as for the taurus board: Could you
> please wait for 'ATMEL_LEGACY' PIO API change or provide at91 portmux
> API (I think the AVR32 portmux API is a good starting point).

Yes.

>> +
>> + erstl = readl(&rstc->mr)& AT91_RSTC_MR_ERSTL_MASK;
>> +
>> + /* Need to reset PHY -> 500ms reset */
>> + writel(AT91_RSTC_KEY | AT91_RSTC_MR_ERSTL(13) |
>> + AT91_RSTC_MR_URSTEN,&rstc->mr);
>> +
>> + writel(AT91_RSTC_KEY | AT91_RSTC_CR_EXTRST,&rstc->cr);
>> +
>> + /* Wait for end of reset */
>> + at91_wait_for_reset(100);
>
> You say above, that this will be 500ms reset pulse ... but wait just
> 100ms. Is that Ok?
> Please also check the taurus board.

We should rework this as Wolfgang suggested, or?

>> + /* Restore NRST value */
>> + writel(AT91_RSTC_KEY | erstl | AT91_RSTC_MR_URSTEN,
>> + &rstc->mr);
>> +
>> + /* Re-enable pull-up */
>> + at91_set_pio_pullup(AT91_PIO_PORTA, 15, 1);
>> + at91_set_pio_pullup(AT91_PIO_PORTA, 12, 1);
>> + at91_set_pio_pullup(AT91_PIO_PORTA, 13, 1);
>> +
>> + /* And the pins. */
>> + at91_macb_hw_init();
>> +}
>> +#endif
>> +
>> +int board_early_init_f(void)
>> +{
>> + at91_seriald_hw_init();
>> + return 0;
>> +}
>> +
>> +int board_init(void)
>> +{
>> + /* address of boot parameters */
>> + gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100;
>> +
>> +#ifdef CONFIG_CMD_NAND
>> + corvus_nand_hw_init();
>> +#endif
>> +#ifdef CONFIG_ATMEL_SPI
>> + at91_spi0_hw_init(1<< 4);
>> +#endif
>> +#ifdef CONFIG_HAS_DATAFLASH
>> + at91_spi0_hw_init(1<< 0);
>> +#endif
>> +#ifdef CONFIG_MACB
>> + corvus_macb_hw_init();
>> +#endif
>> +#ifdef CONFIG_CMD_USB
>> + at91sam9m10g45ek_usb_hw_init();
>
> NAK, this is located in another board file (at91sam9m10g45ek), please adopt.

reworked.

>> +#endif
>> + return 0;
>> +}
>> +
>> +#ifdef CONFIG_RESET_PHY_R
>
> Why provide empty reset_phy? Just remove the CONFIG_RESET_PHY_R in board
> config.

removed.

[...]
>> diff --git a/include/configs/corvus.h b/include/configs/corvus.h
>> new file mode 100644
>> index 0000000..b864562
>> --- /dev/null
>> +++ b/include/configs/corvus.h
>> @@ -0,0 +1,165 @@
[...]
>> +/* LED */
>> +#define CONFIG_AT91_LED
>> +#define CONFIG_RED_LED AT91_PIN_PD31 /* this is the user1 led */
>> +#define CONFIG_GREEN_LED AT91_PIN_PD0 /* this is the user2 led */
>
> Could you please remove the<tab> between 'define' and defined name.

removed.

[...]

bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany