Hi Phil,
On 23.12.2015 12:30, Phil Sutter wrote:
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.
Nice! ;)
+#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.
Thanks.
+/* 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?
Yes. How about CONFIG_DDR_32BIT? Its using in some FSL MPC8xxx board
already.
IMHO the
division between Kconfig symbols and board header defines is a bit
inconsistent.
It definitely is, yes.
Thanks,
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
