On Sun, 6 Sep 2020 20:48:57 +0200 Andre Heider <a.hei...@gmail.com> wrote:
> On 06/09/2020 18:12, Marek Behun wrote: > > On Sun, 6 Sep 2020 11:32:47 +0200 > > Pali Rohár <p...@kernel.org> wrote: > > > >> Adding Marek to loop. > >> > >> On Saturday 05 September 2020 14:07:44 Andre Heider wrote: > >>> Required for the generic distro mechanism. > >>> > >>> Linux ships with 4 variants: > >>> marvell/armada-3720-espressobin-v7-emmc.dtb > >>> marvell/armada-3720-espressobin-v7.dtb > >>> marvell/armada-3720-espressobin-emmc.dtb > >>> marvell/armada-3720-espressobin.dtb > >>> > >>> Use available information to determine the appropriate filename. > >>> > >>> Tested on a v5 board without eMMC. > >>> > >>> Signed-off-by: Andre Heider <a.hei...@gmail.com> > >>> --- > >>> arch/arm/mach-mvebu/Kconfig | 1 + > >>> board/Marvell/mvebu_armada-37xx/board.c | 42 +++++++++++++++++++++++++ > >>> 2 files changed, 43 insertions(+) > >>> > >>> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig > >>> index 0d8e0922a2..31f5d26dc2 100644 > >>> --- a/arch/arm/mach-mvebu/Kconfig > >>> +++ b/arch/arm/mach-mvebu/Kconfig > >>> @@ -100,6 +100,7 @@ config TARGET_HELIOS4 > >>> config TARGET_MVEBU_ARMADA_37XX > >>> bool "Support Armada 37xx platforms" > >>> select ARMADA_3700 > >>> + select BOARD_LATE_INIT > >> > >> This should go into espressobin defconfig file. Other Armada 37xx board > >> do not need to have this option enabled. > > > > I agree with this. > > > >>> imply SCSI > >>> > >>> config TARGET_DB_88F6720 > >>> diff --git a/board/Marvell/mvebu_armada-37xx/board.c > >>> b/board/Marvell/mvebu_armada-37xx/board.c > >>> index 90bfc139aa..3bf0a08897 100644 > >>> --- a/board/Marvell/mvebu_armada-37xx/board.c > >>> +++ b/board/Marvell/mvebu_armada-37xx/board.c > >>> @@ -5,6 +5,7 @@ > >>> > >>> #include <common.h> > >>> #include <dm.h> > >>> +#include <env.h> > >>> #include <i2c.h> > >>> #include <init.h> > >>> #include <phy.h> > >>> @@ -50,6 +51,22 @@ DECLARE_GLOBAL_DATA_PTR; > >>> #define MVEBU_G2_SMI_PHY_CMD_REG (24) > >>> #define MVEBU_G2_SMI_PHY_DATA_REG (25) > >>> > >>> +/* > >>> + * Memory Controller Registers > >>> + * > >>> + * Assembled based on public information: > >>> + * > >>> https://gitlab.nic.cz/turris/mox-boot-builder/-/blob/master/wtmi/main.c#L332-336 > >>> + * > >>> https://github.com/MarvellEmbeddedProcessors/mv-ddr-marvell/blob/mv_ddr-armada-18.12/drivers/mv_ddr_mc6.h#L309-L332 > >>> + * > >>> + * And checked against the written register values for the various > >>> topologies: > >>> + * > >>> https://github.com/MarvellEmbeddedProcessors/mv-ddr-marvell/blob/mv_ddr-armada-atf-mainline/a3700/mv_ddr_tim.h > >>> + */ > >>> +#define A3700_CH0_MC_CTRL2_REG MVEBU_REGISTER(0x002c4) > >>> +#define A3700_MC_CTRL2_SDRAM_TYPE_MASK 0xf > >>> +#define A3700_MC_CTRL2_SDRAM_TYPE_OFFS 4 > >>> +#define A3700_MC_CTRL2_SDRAM_TYPE_DDR3 2 > >>> +#define A3700_MC_CTRL2_SDRAM_TYPE_DDR4 3 > >>> + > >>> int board_early_init_f(void) > >>> { > >>> return 0; > >>> @@ -63,6 +80,31 @@ int board_init(void) > >>> return 0; > >>> } > >>> > >>> +int board_late_init(void) > >>> +{ > >>> + bool ddr4, emmc; > >>> + > >>> + if (!of_machine_is_compatible("globalscale,espressobin")) > >>> + return 0; > >>> + > >>> + /* If the memory controller has been configured for DDR4, we're running > >>> on v7 */ > >>> + ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> > >>> A3700_MC_CTRL2_SDRAM_TYPE_OFFS) > >>> + & A3700_MC_CTRL2_SDRAM_TYPE_MASK) == > >>> A3700_MC_CTRL2_SDRAM_TYPE_DDR4; > >> > >> Marek, is this correct way to determining V5 vs V7? > > > > If for all ESPRESSObin board versions it is true that > > "all v7 boards are DDR4 only and all v5 boards are DDR3 only" > > then yes, you can differentiate with this code. > > I believe that's the case. > There may be other ways to detect the board, this was just the most > obvious one to me. > > > This register is set > > early in boot process, by the code in TIM image, and if it was set > > incorrectly, the board would not boot into U-Boot. > > > >>> + > >>> + emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); > >>> + > >>> + if (ddr4 && emmc) > >>> + env_set("fdtfile", > >>> "marvell/armada-3720-espressobin-v7-emmc.dtb"); > >>> + else if (ddr4) > >>> + env_set("fdtfile", "marvell/armada-3720-espressobin-v7.dtb"); > >>> + else if (emmc) > >>> + env_set("fdtfile", "marvell/armada-3720-espressobin-emmc.dtb"); > >>> + else > >>> + env_set("fdtfile", "marvell/armada-3720-espressobin.dtb"); > >> > >> This code would overwrite user's value of fdtfile variable. And any > >> change done by saveenv for fdtfile would be lost. I do not think this is > >> correct way as it would break booting other distributions/forks (e.g. > >> Marvell one) which DTB file has different name. > >> > >> Also user would not be able to adjust usage of its own DTB file if this > >> code would overwrite it at every boot. > >> > >> Cannot be put value of this variable only to default set? Like all other > >> variables? So value would be restored/overwritten only by 'env default' > >> call. > > > > There are special U-Boot variables $soc, $board, and $boardver. > > In include/config_distro_bootcmd.h look at line > > "setenv efi_fdtfile ${soc}-${board}${boardver}.dtb; " > > I think you should be setting $boardver env variable in your code, and > > have default bootscript somehow build fdtfile name from this. > > That's protected by > #if defined(CONFIG_ARM) && !defined(CONFIG_ARM64) > though, so 32bit only. Hi, I meant it as an inspiration, not that that line of code should be used by your board... Marek