| Message ID | 20200911043510.488790-1-a.heider@gmail.com |
|---|---|
| State | Accepted |
| Commit | 68e32e34436f406c60a26c79fdc4821d3b0de0ff |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [v2] arm: mvebu: Espressobin: Set environment variable fdtfile | expand |
Le 11/09/2020 à 06:35, Andre Heider a écrit : > 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. > > Fixes booting GRUB EFI arm64 on Fedora. > > Reported-by: Dennis Gilmore <dennis@ausil.us> > Signed-off-by: Andre Heider <a.heider@gmail.com> > --- > > v2: > - enable BOARD_LATE_INIT only for espressobin, via defconfig > - don't overwrite a set/saved $fdtfile > > This still has the issue that $fdtfile doesn't show up after `env > default -a`. Pali may look into that, but the infrastructure for it > needs to be created first. > > Until then, this can be considered as v2010.10 material as it fixes > booting distros relying on $fdtfile. > > Tested myself on a v5 board without eMMC. > Tested by Gérald on v7 emmc, v7/ddr4 detection confirmed working. > > board/Marvell/mvebu_armada-37xx/board.c | 47 +++++++++++++++++++++ > configs/mvebu_espressobin-88f3720_defconfig | 1 + > 2 files changed, 48 insertions(+) > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c > index 90bfc139aa..73d69e0388 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,36 @@ int board_init(void) > return 0; > } > > +#ifdef CONFIG_BOARD_LATE_INIT > +int board_late_init(void) > +{ > + bool ddr4, emmc; > + > + if (env_get("fdtfile")) > + return 0; > + > + 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; > + > + 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"); > + > + return 0; > +} > +#endif > + > /* Board specific AHCI / SATA enable code */ > int board_ahci_enable(void) > { > diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig > index 5e9fcd1f26..559aeb076f 100644 > --- a/configs/mvebu_espressobin-88f3720_defconfig > +++ b/configs/mvebu_espressobin-88f3720_defconfig > @@ -85,3 +85,4 @@ CONFIG_USB_ETHER_SMSC95XX=y > CONFIG_SHA1=y > CONFIG_SHA256=y > CONFIG_NET_RANDOM_ETHADDR=y > +CONFIG_BOARD_LATE_INIT=y |Tested-by: Gérald Kerma <gandalf@gk2.net>|
On 11.09.20 06:35, 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. > > Fixes booting GRUB EFI arm64 on Fedora. > > Reported-by: Dennis Gilmore <dennis@ausil.us> > Signed-off-by: Andre Heider <a.heider@gmail.com> Applied to u-boot-marvell/master Thanks, Stefan > --- > > v2: > - enable BOARD_LATE_INIT only for espressobin, via defconfig > - don't overwrite a set/saved $fdtfile > > This still has the issue that $fdtfile doesn't show up after `env > default -a`. Pali may look into that, but the infrastructure for it > needs to be created first. > > Until then, this can be considered as v2010.10 material as it fixes > booting distros relying on $fdtfile. > > Tested myself on a v5 board without eMMC. > Tested by Gérald on v7 emmc, v7/ddr4 detection confirmed working. > > board/Marvell/mvebu_armada-37xx/board.c | 47 +++++++++++++++++++++ > configs/mvebu_espressobin-88f3720_defconfig | 1 + > 2 files changed, 48 insertions(+) > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c > index 90bfc139aa..73d69e0388 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,36 @@ int board_init(void) > return 0; > } > > +#ifdef CONFIG_BOARD_LATE_INIT > +int board_late_init(void) > +{ > + bool ddr4, emmc; > + > + if (env_get("fdtfile")) > + return 0; > + > + 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; > + > + 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"); > + > + return 0; > +} > +#endif > + > /* Board specific AHCI / SATA enable code */ > int board_ahci_enable(void) > { > diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig > index 5e9fcd1f26..559aeb076f 100644 > --- a/configs/mvebu_espressobin-88f3720_defconfig > +++ b/configs/mvebu_espressobin-88f3720_defconfig > @@ -85,3 +85,4 @@ CONFIG_USB_ETHER_SMSC95XX=y > CONFIG_SHA1=y > CONFIG_SHA256=y > CONFIG_NET_RANDOM_ETHADDR=y > +CONFIG_BOARD_LATE_INIT=y > Viele Grüße, Stefan
On Friday 11 September 2020 06:35:10 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. > > Fixes booting GRUB EFI arm64 on Fedora. > > Reported-by: Dennis Gilmore <dennis@ausil.us> > Signed-off-by: Andre Heider <a.heider@gmail.com> > --- > > v2: > - enable BOARD_LATE_INIT only for espressobin, via defconfig > - don't overwrite a set/saved $fdtfile > > This still has the issue that $fdtfile doesn't show up after `env > default -a`. Pali may look into that, but the infrastructure for it > needs to be created first. > > Until then, this can be considered as v2010.10 material as it fixes > booting distros relying on $fdtfile. > > Tested myself on a v5 board without eMMC. > Tested by Gérald on v7 emmc, v7/ddr4 detection confirmed working. > > board/Marvell/mvebu_armada-37xx/board.c | 47 +++++++++++++++++++++ > configs/mvebu_espressobin-88f3720_defconfig | 1 + > 2 files changed, 48 insertions(+) > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c > index 90bfc139aa..73d69e0388 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,36 @@ int board_init(void) > return 0; > } > > +#ifdef CONFIG_BOARD_LATE_INIT > +int board_late_init(void) > +{ > + bool ddr4, emmc; > + > + if (env_get("fdtfile")) > + return 0; > + > + 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; > + > + emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); Maybe stupid question, but are not we able to detect if eMMC is connected or not at runtime? That could simplify setup and avoid usage of additional special DTS file for eMMC in U-Boot. > + > + 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"); > + > + return 0; > +} > +#endif > + > /* Board specific AHCI / SATA enable code */ > int board_ahci_enable(void) > { > diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig > index 5e9fcd1f26..559aeb076f 100644 > --- a/configs/mvebu_espressobin-88f3720_defconfig > +++ b/configs/mvebu_espressobin-88f3720_defconfig > @@ -85,3 +85,4 @@ CONFIG_USB_ETHER_SMSC95XX=y > CONFIG_SHA1=y > CONFIG_SHA256=y > CONFIG_NET_RANDOM_ETHADDR=y > +CONFIG_BOARD_LATE_INIT=y > -- > 2.28.0 >
On 25/09/2020 09:46, Pali Rohár wrote: > On Friday 11 September 2020 06:35:10 Andre Heider wrote: ... >> >> +#ifdef CONFIG_BOARD_LATE_INIT >> +int board_late_init(void) >> +{ >> + bool ddr4, emmc; >> + >> + if (env_get("fdtfile")) >> + return 0; >> + >> + 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; >> + >> + emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); > > Maybe stupid question, but are not we able to detect if eMMC is > connected or not at runtime? That could simplify setup and avoid usage > of additional special DTS file for eMMC in U-Boot. At some point I wondered about the same. IIRC armbian just enables it and uses one u-boot binary everywhere. A non-existing chip won't get detected, so that shouldn't be a problem. But I think it has more to do with enabling/powering up hardware blocks, which is not wanted or may even problematic. Regards, Andre
On Saturday 26 September 2020 11:09:59 Andre Heider wrote: > On 25/09/2020 09:46, Pali Rohár wrote: > > On Friday 11 September 2020 06:35:10 Andre Heider wrote: > ... > > > +#ifdef CONFIG_BOARD_LATE_INIT > > > +int board_late_init(void) > > > +{ > > > + bool ddr4, emmc; > > > + > > > + if (env_get("fdtfile")) > > > + return 0; > > > + > > > + 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; > > > + > > > + emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); > > > > Maybe stupid question, but are not we able to detect if eMMC is > > connected or not at runtime? That could simplify setup and avoid usage > > of additional special DTS file for eMMC in U-Boot. > > At some point I wondered about the same. > > IIRC armbian just enables it and uses one u-boot binary everywhere. A > non-existing chip won't get detected, so that shouldn't be a problem. > > But I think it has more to do with enabling/powering up hardware blocks, > which is not wanted or may even problematic. In U-Boot such functionality may be implemented in board_fix_fdt() function which allows U-Boot to modify its Device tree prior using it. You have already wrote code which is doing V5 vs V7 detection, so detecting eMMC is the last thing which is missing to have just one U-Boot DTS file and therefore only one U-Boot binary for Espressobin. > Regards, > Andre >
On 02/10/2020 12:49, Pali Rohár wrote: > On Saturday 26 September 2020 11:09:59 Andre Heider wrote: >> On 25/09/2020 09:46, Pali Rohár wrote: >>> On Friday 11 September 2020 06:35:10 Andre Heider wrote: >> ... >>>> +#ifdef CONFIG_BOARD_LATE_INIT >>>> +int board_late_init(void) >>>> +{ >>>> + bool ddr4, emmc; >>>> + >>>> + if (env_get("fdtfile")) >>>> + return 0; >>>> + >>>> + 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; >>>> + >>>> + emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); >>> >>> Maybe stupid question, but are not we able to detect if eMMC is >>> connected or not at runtime? That could simplify setup and avoid usage >>> of additional special DTS file for eMMC in U-Boot. >> >> At some point I wondered about the same. >> >> IIRC armbian just enables it and uses one u-boot binary everywhere. A >> non-existing chip won't get detected, so that shouldn't be a problem. >> >> But I think it has more to do with enabling/powering up hardware blocks, >> which is not wanted or may even problematic. > > In U-Boot such functionality may be implemented in board_fix_fdt() > function which allows U-Boot to modify its Device tree prior using it. > > You have already wrote code which is doing V5 vs V7 detection, so > detecting eMMC is the last thing which is missing to have just one > U-Boot DTS file and therefore only one U-Boot binary for Espressobin. That implies setting status=okay for the emmc node, which then powers up that block. I don't know if that might be problematic. Can we just do that? The emmc detection would also add some delay to the boot process. At least it should be powered down if no emmc chip has been detected, but I didn't check if that happens right now. Regards, Andre
On Saturday 03 October 2020 09:17:35 Andre Heider wrote: > On 02/10/2020 12:49, Pali Rohár wrote: > > On Saturday 26 September 2020 11:09:59 Andre Heider wrote: > > > On 25/09/2020 09:46, Pali Rohár wrote: > > > > On Friday 11 September 2020 06:35:10 Andre Heider wrote: > > > ... > > > > > +#ifdef CONFIG_BOARD_LATE_INIT > > > > > +int board_late_init(void) > > > > > +{ > > > > > + bool ddr4, emmc; > > > > > + > > > > > + if (env_get("fdtfile")) > > > > > + return 0; > > > > > + > > > > > + 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; > > > > > + > > > > > + emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); > > > > > > > > Maybe stupid question, but are not we able to detect if eMMC is > > > > connected or not at runtime? That could simplify setup and avoid usage > > > > of additional special DTS file for eMMC in U-Boot. > > > > > > At some point I wondered about the same. > > > > > > IIRC armbian just enables it and uses one u-boot binary everywhere. A > > > non-existing chip won't get detected, so that shouldn't be a problem. > > > > > > But I think it has more to do with enabling/powering up hardware blocks, > > > which is not wanted or may even problematic. > > > > In U-Boot such functionality may be implemented in board_fix_fdt() > > function which allows U-Boot to modify its Device tree prior using it. > > > > You have already wrote code which is doing V5 vs V7 detection, so > > detecting eMMC is the last thing which is missing to have just one > > U-Boot DTS file and therefore only one U-Boot binary for Espressobin. > > That implies setting status=okay for the emmc node, which then powers up > that block. I don't know if that might be problematic. Can we just do that? No. I mean to detect presence of eMMC in board_fix_fdt() and then set tatus=okay only if check passed. > The emmc detection would also add some delay to the boot process. > > At least it should be powered down if no emmc chip has been detected, but I > didn't check if that happens right now. > > Regards, > Andre
On 03/10/2020 10:50, Pali Rohár wrote: > On Saturday 03 October 2020 09:17:35 Andre Heider wrote: >> On 02/10/2020 12:49, Pali Rohár wrote: >>> On Saturday 26 September 2020 11:09:59 Andre Heider wrote: >>>> On 25/09/2020 09:46, Pali Rohár wrote: >>>>> On Friday 11 September 2020 06:35:10 Andre Heider wrote: >>>> ... >>>>>> +#ifdef CONFIG_BOARD_LATE_INIT >>>>>> +int board_late_init(void) >>>>>> +{ >>>>>> + bool ddr4, emmc; >>>>>> + >>>>>> + if (env_get("fdtfile")) >>>>>> + return 0; >>>>>> + >>>>>> + 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; >>>>>> + >>>>>> + emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); >>>>> >>>>> Maybe stupid question, but are not we able to detect if eMMC is >>>>> connected or not at runtime? That could simplify setup and avoid usage >>>>> of additional special DTS file for eMMC in U-Boot. >>>> >>>> At some point I wondered about the same. >>>> >>>> IIRC armbian just enables it and uses one u-boot binary everywhere. A >>>> non-existing chip won't get detected, so that shouldn't be a problem. >>>> >>>> But I think it has more to do with enabling/powering up hardware blocks, >>>> which is not wanted or may even problematic. >>> >>> In U-Boot such functionality may be implemented in board_fix_fdt() >>> function which allows U-Boot to modify its Device tree prior using it. >>> >>> You have already wrote code which is doing V5 vs V7 detection, so >>> detecting eMMC is the last thing which is missing to have just one >>> U-Boot DTS file and therefore only one U-Boot binary for Espressobin. >> >> That implies setting status=okay for the emmc node, which then powers up >> that block. I don't know if that might be problematic. Can we just do that? > > No. I mean to detect presence of eMMC in board_fix_fdt() and then set > tatus=okay only if check passed. Yes, but how do you detect the emmc then? Enabling it in u-boot's dts and calling mmc_init() on it would be the easy way, but open coding all the required parts to do that with status=disabled could get rather big and/or unmaintanable (I didn't check what exactly would be required)? > >> The emmc detection would also add some delay to the boot process. >> >> At least it should be powered down if no emmc chip has been detected, but I >> didn't check if that happens right now. >> >> Regards, >> Andre
On Monday 05 October 2020 18:20:17 Andre Heider wrote: > On 03/10/2020 10:50, Pali Rohár wrote: > > On Saturday 03 October 2020 09:17:35 Andre Heider wrote: > > > On 02/10/2020 12:49, Pali Rohár wrote: > > > > On Saturday 26 September 2020 11:09:59 Andre Heider wrote: > > > > > On 25/09/2020 09:46, Pali Rohár wrote: > > > > > > On Friday 11 September 2020 06:35:10 Andre Heider wrote: > > > > > ... > > > > > > > +#ifdef CONFIG_BOARD_LATE_INIT > > > > > > > +int board_late_init(void) > > > > > > > +{ > > > > > > > + bool ddr4, emmc; > > > > > > > + > > > > > > > + if (env_get("fdtfile")) > > > > > > > + return 0; > > > > > > > + > > > > > > > + 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; > > > > > > > + > > > > > > > + emmc = of_machine_is_compatible("globalscale,espressobin-emmc"); > > > > > > > > > > > > Maybe stupid question, but are not we able to detect if eMMC is > > > > > > connected or not at runtime? That could simplify setup and avoid usage > > > > > > of additional special DTS file for eMMC in U-Boot. > > > > > > > > > > At some point I wondered about the same. > > > > > > > > > > IIRC armbian just enables it and uses one u-boot binary everywhere. A > > > > > non-existing chip won't get detected, so that shouldn't be a problem. > > > > > > > > > > But I think it has more to do with enabling/powering up hardware blocks, > > > > > which is not wanted or may even problematic. > > > > > > > > In U-Boot such functionality may be implemented in board_fix_fdt() > > > > function which allows U-Boot to modify its Device tree prior using it. > > > > > > > > You have already wrote code which is doing V5 vs V7 detection, so > > > > detecting eMMC is the last thing which is missing to have just one > > > > U-Boot DTS file and therefore only one U-Boot binary for Espressobin. > > > > > > That implies setting status=okay for the emmc node, which then powers up > > > that block. I don't know if that might be problematic. Can we just do that? > > > > No. I mean to detect presence of eMMC in board_fix_fdt() and then set > > tatus=okay only if check passed. > > Yes, but how do you detect the emmc then? Enabling it in u-boot's dts and > calling mmc_init() on it would be the easy way, but open coding all the > required parts to do that with status=disabled could get rather big and/or > unmaintanable (I didn't check what exactly would be required)? Hm... seems that it would be really easier to enable eMMC in U-Boot DTS, let U-Boot to try initialize eMMC and in case it fails, then disable eMMC drivers & devices and disable it also in FTB blob. For powering up eMMC there is function comphy_emmc_power_up(), so it would be needed to write power_down equivalent. And for xenon_sdhci.c would be needed to write .remove callback (for disabling eMMC). > > > > > The emmc detection would also add some delay to the boot process. > > > > > > At least it should be powered down if no emmc chip has been detected, but I > > > didn't check if that happens right now. > > > > > > Regards, > > > Andre >
diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c index 90bfc139aa..73d69e0388 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,36 @@ int board_init(void) return 0; } +#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{ + bool ddr4, emmc; + + if (env_get("fdtfile")) + return 0; + + 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; + + 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"); + + return 0; +} +#endif + /* Board specific AHCI / SATA enable code */ int board_ahci_enable(void) { diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig index 5e9fcd1f26..559aeb076f 100644 --- a/configs/mvebu_espressobin-88f3720_defconfig +++ b/configs/mvebu_espressobin-88f3720_defconfig @@ -85,3 +85,4 @@ CONFIG_USB_ETHER_SMSC95XX=y CONFIG_SHA1=y CONFIG_SHA256=y CONFIG_NET_RANDOM_ETHADDR=y +CONFIG_BOARD_LATE_INIT=y
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. Fixes booting GRUB EFI arm64 on Fedora. Reported-by: Dennis Gilmore <dennis@ausil.us> Signed-off-by: Andre Heider <a.heider@gmail.com> --- v2: - enable BOARD_LATE_INIT only for espressobin, via defconfig - don't overwrite a set/saved $fdtfile This still has the issue that $fdtfile doesn't show up after `env default -a`. Pali may look into that, but the infrastructure for it needs to be created first. Until then, this can be considered as v2010.10 material as it fixes booting distros relying on $fdtfile. Tested myself on a v5 board without eMMC. Tested by Gérald on v7 emmc, v7/ddr4 detection confirmed working. board/Marvell/mvebu_armada-37xx/board.c | 47 +++++++++++++++++++++ configs/mvebu_espressobin-88f3720_defconfig | 1 + 2 files changed, 48 insertions(+)