On Tue, Sep 22, 2026 at 11:27:32 +0100, David Lechner wrote: > On 9/14/26 8:35 AM, Carlo Caione wrote: > > From: Julien Masson <[email protected]> > > > > Genio 510 and 700 need a provisioning and recovery path that remains > > usable when eMMC is blank or its boot firmware is unusable. Add a RAM- > > based fastboot download agent so boards can be flashed through the > > MediaTek BootROM download protocol without a working on-device > > bootloader. > > > > Running the agent in SPL avoids a dependency on BL31 and U-Boot proper > > during provisioning. > > It would be helpful to have some more explanation here about why the > different parts of this patch are doing what they are doing.
Yeah, I agree. I'll add the explanation of the why in the commit message. > > > > Signed-off-by: Julien Masson <[email protected]> > > Signed-off-by: Carlo Caione <[email protected]> > > --- > > arch/arm/dts/mt8188-genio-da-u-boot.dtsi | 55 ++++++++++++++++++ > > arch/arm/dts/mt8370-genio-510-evk-u-boot.dtsi | 4 ++ > > arch/arm/dts/mt8390-genio-700-evk-u-boot.dtsi | 4 ++ > > arch/arm/dts/mtk-genio-da-binman-u-boot.dtsi | 16 ++++++ > > .../dts/mtk-genio-loader-spl-binman-u-boot.dtsi | 6 ++ > > arch/arm/mach-mediatek/Kconfig | 26 +++++++++ > > arch/arm/mach-mediatek/Makefile | 1 + > > arch/arm/mach-mediatek/spl_fastboot.c | 65 > > ++++++++++++++++++++++ > > board/mediatek/MAINTAINERS | 5 ++ > > common/spl/Kconfig | 2 +- > > configs/mt8188-genio-da-spl-fastboot.config | 15 +++++ > > configs/mt8188-genio-da.config | 19 +++++++ > > I think these should use mt8370 prefix like the other common genio 510/700 > stuff. mt8188 is for chromebooks, so I don't think it would make sense to > use that number with genio-specific configs. :( yup, makes sense. > > configs/mt8370_genio_510_evk_da_spl_defconfig | 2 + > > configs/mt8390_genio_700_evk_da_spl_defconfig | 2 + > > 14 files changed, 221 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/dts/mt8188-genio-da-u-boot.dtsi > > b/arch/arm/dts/mt8188-genio-da-u-boot.dtsi > > new file mode 100644 > > index 00000000000..e9078f37157 > > --- /dev/null > > +++ b/arch/arm/dts/mt8188-genio-da-u-boot.dtsi > > @@ -0,0 +1,55 @@ > > +// SPDX-License-Identifier: (GPL-2.0-only OR MIT) > > +/* > > + * Copyright (c) 2026 BayLibre, SAS. > > + * Author: Carlo Caione <[email protected]> > > + */ > > I would add a comment here explaining that da is download agent and why we > have this special devicetree that is different from the normal one. I'll add a comment. > > + > > +#include "mtk-genio-binman-u-boot.dtsi" > > +#include "mtk-genio-da-binman-u-boot.dtsi" > > + > > +&ssusb0 { > > + dr_mode = "peripheral"; > > > + mediatek,force-vbus; > > This is not an upstream DT property. > > It seems like dr_mode = "peripheral" should already imply this anyway. So > hopefully we can just modify the USB driver and leave this property out. Good point. I'll make the change to the MTU3 driver. > > +}; > > + > > +&ssusb1 { > > + status = "disabled"; > > +}; > > + > > +&ssusb2 { > > + status = "disabled"; > > +}; > > + > > +#ifdef CONFIG_MTK_BROM_DOWNLOAD_AGENT_SPL_FASTBOOT > > Why do these matter for fastboot but not the ones above? Because only the fastboot-in-SPL variant uses USB during SPL execution, in the fastboot-in-U-Boot-proper case these nodes are already in the regular U-Boot DT (no need for 'bootph-pre-ram'). > > +&pericfg_ao { > > + bootph-pre-ram; > > +}; > > + > > +&ssusb0 { > > + bootph-pre-ram; > > +}; > > + > > +&u3phy0 { > > + bootph-pre-ram; > > +}; > > + > > +&u2port0 { > > + bootph-pre-ram; > > +}; > > + > > +&usb_default_pins { > > + bootph-pre-ram; > > + > > + pins-iddig { > > + bootph-pre-ram; > > + }; > > + > > + pins-valid { > > + bootph-pre-ram; > > + }; > > + > > + pins-vbus { > > + bootph-pre-ram; > > + }; > > +}; > > +#endif [...] > > --- /dev/null > > +++ b/arch/arm/dts/mtk-genio-da-binman-u-boot.dtsi > > @@ -0,0 +1,16 @@ > > +// SPDX-License-Identifier: (GPL-2.0-only OR MIT) > > +/* > > + * Copyright (c) 2026 BayLibre, SAS. > > + * Author: Carlo Caione <[email protected]> > > + */ > > + > > +&binman { > > + da: da { > > + filename = "da.bin"; > > + pad-byte = <0>; > > + > > +#define MTK_GENIO_SIMPLE_SPL > > Could we pick a name that describe more precicly what this does? SIMPLE > is vague. Fair enough. [...] > > obj-$(CONFIG_TARGET_MT7622) += mt7622/ > > obj-$(CONFIG_TARGET_MT7623) += mt7623/ > > diff --git a/arch/arm/mach-mediatek/spl_fastboot.c > > b/arch/arm/mach-mediatek/spl_fastboot.c > > new file mode 100644 > > index 00000000000..0ba76d47dfb > > --- /dev/null > > +++ b/arch/arm/mach-mediatek/spl_fastboot.c > > @@ -0,0 +1,65 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2024 BayLibre, SAS. > > 2026 uh, time flies. > > + * Author: Julien Masson <[email protected]> > > + */ > > + > > +#include <dm/uclass.h> > > +#include <fastboot.h> > > +#include <mmc.h> > > Suggest also <stdio.h> for printf and <linux/errno.h> for ENODEV. Ok > > + > > +static int mtk_mmc_boot_prepare(void) > > +{ > > + struct udevice *dev; > > + struct mmc *mmc; > > + int ret; > > + > > + ret = uclass_get_device_by_seq(UCLASS_MMC, > > + CONFIG_FASTBOOT_FLASH_MMC_DEV, &dev); > > Should this be find_mmc_device(CONFIG_VAL(FASTBOOT_FLASH_MMC_DEV))? No, we need this because we need to actually probe the device. [...] > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > > index f9b453df766..77da2f4c8aa 100644 > > --- a/common/spl/Kconfig > > +++ b/common/spl/Kconfig > > @@ -194,7 +194,7 @@ config SPL_SHOW_ERRORS > > config SPL_BINMAN_SYMBOLS > > bool "Declare binman symbols in SPL" > > depends on SPL_FRAMEWORK && BINMAN > > - default n if MTK_GENIO_BOOT_IMAGES > > + default n if MTK_GENIO_IMAGE_CONTRACT > > Should we do something to make sure we don't select MTK_GENIO_BOOT_IMAGES > and MTK_BROM_DOWNLOAD_AGENT at the same time? I'll add a depends on !MTK_GENIO_BOOT_IMAGES > > default y > > help > > This enables use of symbols in SPL which refer to other entries in > > diff --git a/configs/mt8188-genio-da-spl-fastboot.config > > b/configs/mt8188-genio-da-spl-fastboot.config > > new file mode 100644 > > index 00000000000..01b35f1144e > > --- /dev/null > > +++ b/configs/mt8188-genio-da-spl-fastboot.config > > @@ -0,0 +1,15 @@ > > +#include <configs/mt8188-genio-da.config> > > + > > +CONFIG_MTK_BROM_DOWNLOAD_AGENT_SPL_FASTBOOT=y > > +# Sparse writes need an 8 MiB DMA buffer in addition to the fastboot state. > > +CONFIG_SPL_SYS_MALLOC_SIZE=0x1000000 > > +CONFIG_SPL_MMC_WRITE=y > > +CONFIG_SPL_PHY=y > > +CONFIG_SPL_DM_USB_GADGET=y > > +CONFIG_SPL_USB_GADGET=y > > +CONFIG_SPL_USB_MTU3=y > > +CONFIG_SPL_FASTBOOT=y > > +CONFIG_SPL_FASTBOOT_REBOOT=y > > +CONFIG_SPL_FASTBOOT_FLASH_MMC=y > > +CONFIG_SPL_FASTBOOT_MMC_BOOT_SUPPORT=y > > +CONFIG_SPL_FASTBOOT_MMC_USER_SUPPORT=y > > diff --git a/configs/mt8188-genio-da.config b/configs/mt8188-genio-da.config > > Do these really need to be two separate files? As mentioned above, there > isn't really such thing as a mt8188-genio seprate from a mt8370-genio-common. > mt8188.config already contains things that really are genio-specific that > would be more "correct" to move to a separate file. But that is a bigger > cleanup that affects all MediaTek Genio boards that we can look at later. ok, I can slightly make this better by splitting: - mt8370-genio-da.config: settings shared by both DA variants - mt8370-genio-da-spl-fastboot.config: SPL fastboot settings - mt8370-genio-da-uboot-fastboot.config: U-Boot-proper fastboot settings Thanks! -- Carlo Caione
