[PATCH] arm: mvebu: Espressobin: Set environment variable fdtfile
Andre Heider
a.heider at gmail.com
Mon Sep 7 07:20:10 CEST 2020
Hi Marek,
On 06/09/2020 20:56, Marek Behun wrote:
> On Sun, 6 Sep 2020 20:48:57 +0200
> Andre Heider <a.heider at gmail.com> wrote:
>
>> On 06/09/2020 18:12, Marek Behun wrote:
>>> On Sun, 6 Sep 2020 11:32:47 +0200
>>> Pali Rohár <pali at 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.heider at 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
Oh sorry, I misread the first time.
I see what you mean, and there're boards which do just that, and
there're boards which do it like my patch.
But in the end it would just use another envvar, with the added
complexity of a little scripting. At first glance, plain $fdtfile might
be more straight forward? Is there another advantage of your approach
I'm not seeing yet?
Thanks,
Andre
More information about the U-Boot
mailing list