Hi Stefan, On Wed, Dec 23, 2015 at 08:56:39AM +0100, Stefan Roese wrote: > >>> diff --git a/arch/arm/mach-mvebu/serdes/axp/board_env_spec.h > >>> b/arch/arm/mach-mvebu/serdes/axp/board_env_spec.h > >>> index f00f327..3dca6a1 100644 > >>> --- a/arch/arm/mach-mvebu/serdes/axp/board_env_spec.h > >>> +++ b/arch/arm/mach-mvebu/serdes/axp/board_env_spec.h > >>> @@ -43,8 +43,9 @@ > >>> #define RD_78460_SERVER_REV2_ID (DB_78X60_PCAC_REV2_ID > >>> + 1) > >>> #define DB_784MP_GP_ID (RD_78460_SERVER_REV2_ID + 1) > >>> #define RD_78460_CUSTOMER_ID (DB_784MP_GP_ID + 1) > >>> -#define MV_MAX_BOARD_ID (RD_78460_CUSTOMER_ID + 1) > >>> -#define INVALID_BAORD_ID 0xFFFFFFFF > >>> +#define SYNOLOGY_DS414_ID (RD_78460_CUSTOMER_ID + 1) > >>> +#define MV_MAX_BOARD_ID (SYNOLOGY_DS414_ID + 1) > >>> +#define INVALID_BOARD_ID 0xFFFFFFFF > >> > >> Do you really need these changes here? > > > > Sadly, yes. See high_speed_env_lib.c for details: There it is needed by > > serdes_phy_config() to get the right satr11 value via board_id_get(). > > Maybe this should be refactored to always use board_sat_r_get() and the > > latter return a static value from a macro which board configs may define > > instead of reading from i2c. > > Yes, a refactorization would be good here. One idea is, to provide a > _weak version of board_sat_r_get() in high_speed_env_lib.c used on > all of these Marvell boards with a BOARD_ID. And custom boards, like > your DS414 can provide a board specific version of board_sat_r_get() > overwriting the weak default. And returning the board specific value. > This way, all new custom boards would not have to touch this file > any more. > > Could you try to implement it this way?
Actually, it's already done. ;) I started yesterday and it turned out to be much more straightforward than I had expected. And yes, the __weak trick came to my mind then, too. > >>> +#define CONFIG_MV78230 /* SoC Version */ > >>> +#define CONFIG_SYNOLOGY_DS414 /* board target name for DDR > >>> training and PCIe init */ > > > > I will review those as well. Maybe the CONFIG_ARMADA_XP solution could > > be implemented for SoC types, too? > > Yes, please. I also thought about this. Perhaps something like this > in the mach-mvebu/Kconfig: > > config ARMADA_38X > bool > > config ARMADA_XP > bool > > config MV78230 > select ARMADA_XP > bool > > config MV78260 > select ARMADA_XP > bool > > config MV78460 > select ARMADA_XP > bool > > config 88F6810 > select ARMADA_38X > bool > > config 88F6820 > select ARMADA_38X > bool > > config 88F6828 > select ARMADA_38X > bool > > config ARMADA_38X > bool > > config ARMADA_XP > bool > > ... > > config TARGET_DB_MV784MP_GP > bool "Support db-mv784mp-gp" > select MV78460 > > ... > > What do you think? Looks good, I'll check. > >>> +/* Enable DDR support in SPL (DDR3 training from Marvell bin_hdr) */ > >>> +#define CONFIG_SYS_MVEBU_DDR_AXP > >>> +#define MV_DDR_32BIT > >> > >> These 2 lines can also be removed with the new patches. > > > > OK, CONFIG_SYS_MVEBU_DDR_AXP seems not to be used anymore at all. But > > without MV_DDR_32BIT, BUS_WIDTH defaults to 64 which is wrong on DS414. > > Ah, okay. Maybe this should be renamed to into a CONFIG_* macro then? IMHO the division between Kconfig symbols and board header defines is a bit inconsistent. Thanks, Phil _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
