On 06/09/2020 11:32, Pali Rohár 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.

Right you are, fixed locally.


        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?

+
+       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.

Indeed, it shouldn't wipe a saved $fdtfile. Fixed locally with adding this hunk to the start of the function:
+       if (env_get("fdtfile"))
+               return 0;

CC'ed Baruch, since I looked at the clearfog implementation which has the same bug.

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.

It would be possible to avoid the above runtime detection and just add:
"fdtfile=marvell/" CONFIG_DEFAULT_DEVICE_TREE ".dtb\0"
to CONFIG_EXTRA_ENV_SETTINGS instead. But for that, the u-boot dtb variants need to match Linux', which they currently do not.

I posted a set which adds "-emmc.dts". But "-v7.dts" and "v7-emmc.dts" are still missing here, and I don't think it makes sense to add them, because all they're doing is relabeling the switch ports.

Or maybe it's desired to have all dts files from Linux here too, even though we technically don't need a binary for each board?


+       return 0;
+}
+
  /* Board specific AHCI / SATA enable code */
  int board_ahci_enable(void)
  {
--
2.28.0


Thanks,
Andre

Reply via email to