On 17.12.2020 17:09, Tim Small wrote:

Hi,

Hello,

I'm in the process of adding support to U-Boot for an AT91 SAMA5D27 based board. There may be a USB port configuration change pending, but otherwise it's functionally complete.

The design is quite similar to the Microchip SAMA5D2 Xplained Board, to the extent that I have only 6 lines of code changes out of the ~180 lines of (non-comment) code in board/atmel/sama5d2_xplained/sama5d2_xplained.c

In particular the clocks and memory setup code is non-trivial, but identical. With this in mind I was wondering whether (and to what extent) an acceptable patch to support this board would require factoring out this common code?

The (non-comment) diff between my interim init code for the new board, and eval/reference board is currently:

--- board/atmel/sama5d2_xplained/sama5d2_xplained.c 2020-12-15 10:52:32.175616057 +0000 +++ board/scs/cubemb1/cubemb1.c 2020-12-17 14:42:42.399610818 +0000 @@ -30,10 +30,6 @@ #ifdef CONFIG_BOARD_LATE_INIT int board_late_init(void) { -#ifdef CONFIG_DM_VIDEO

•   at91_video_show_board_info();
  

-#endif

Does your board select DM_VIDEO ? If you do not require DM_VIDEO you can just disable it from your configuration

•   at91_pda_detect();
  

Does this impact your board? It should not do anything, provided that you do not have the W1 subsystem enabled in your board configuration

     return 0;

} #endif @@ -83,13 +79,13 @@ return 0; }

-#define AT24MAC_MAC_OFFSET 0x9a +#define MAC24AA_MAC_OFFSET 0xfa

#ifdef CONFIG_MISC_INIT_R int misc_init_r(void) { #ifdef CONFIG_I2C_EEPROM

•   at91_set_ethaddr(AT24MAC_MAC_OFFSET);
  

•   at91_set_ethaddr(MAC24AA_MAC_OFFSET);
  

  #endif

Here you have a different EEPROM on your board, this is correct ? For this, it may be an idea to take out that offset into some type of header configuration... Other than this, you can completely reuse the sama5d2_xplained.c ? (considering also my comments above )

     return 0;

I'm also uncertain about file naming - there seem to be a number of different conventions in use for file and directory names (different combinations of SoC, board, architecture, and manufacturer).

The new board is:

mfr: South Coast Science model: Cube MB v1

Do the following file and directory names look reasonable?

arch/arm/dts/Makefile | 3 +++ arch/arm/dts/at91-sama5d27_cubemb1.dts | 263 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ arch/arm/mach-at91/Kconfig | 8 +++++++ board/scs/cubemb1/Kconfig | 15 +++++++++++++ board/scs/cubemb1/MAINTAINERS | 6 ++++++ board/scs/cubemb1/Makefile | 8 +++++++ board/scs/cubemb1/cubemb1.c | 198 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ configs/cubemb1_emmc_defconfig | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ configs/cubemb1_sdcard_defconfig | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/configs/cubemb1.h | 9 ++++++++ 10 files changed, 718 insertions(+)

Cheers,

Tim.

-- South East Open Source Solutions Limited Registered in England and Wales with company number 06134732. Registered Office: 2 Powell Gardens, Redhill, Surrey, RH1 1TQ VAT number: GB 900 6633 53 http://seoss.co.uk/ +44-(0)1273-808309