All of lore.kernel.org
 help / color / mirror / Atom feed
From: Carlo Caione <ccaione@baylibre.com>
To: David Lechner <dlechner@baylibre.com>
Cc: GSS_MTK_Uboot_upstream <GSS_MTK_Uboot_upstream@mediatek.com>,
	u-boot@lists.u-boot-project.org, "Tom Rini" <trini@konsulko.com>,
	"Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
	"Ryder Lee" <ryder.lee@mediatek.com>,
	"Weijie Gao" <weijie.gao@mediatek.com>,
	"Chunfeng Yun" <chunfeng.yun@mediatek.com>,
	"Igor Belwon" <igor.belwon@mentallysanemainliners.org>,
	"Julien Stephan" <jstephan@baylibre.com>,
	"Julien Masson" <jmasson@baylibre.com>,
	"Johan Jonker" <jbx6244@gmail.com>,
	"Macpaul Lin" <macpaul.lin@mediatek.com>,
	"Chris-QJ Chen" <chris-qj.chen@mediatek.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Bastien Curutchet" <bastien.curutchet@bootlin.com>,
	"Alexey Charkov" <alchark@gmail.com>,
	"Vitor Sato Eschholz" <vsatoes@baylibre.com>,
	"Pablo Sun (孫毓翔)" <pablo.sun@mediatek.com>,
	"Arnaud Ferraris" <arnaud.ferraris@collabora.com>,
	"Suhrid Subramaniam" <Suhrid.Subramaniam@mediatek.com>
Subject: Re: [PATCH 1/3] arm: mediatek: enable SPL fastboot DAs on Genio 510 and 700
Date: Wed, 23 Sep 2026 16:15:07 +0200	[thread overview]
Message-ID: <arPV4uDU7S-SbPKs@macbook> (raw)
In-Reply-To: <81b22e9a-5522-42e8-b9f8-a8cd509f23bc@baylibre.com>

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 <jmasson@baylibre.com>
> > 
> > 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 <jmasson@baylibre.com>
> > Signed-off-by: Carlo Caione <ccaione@baylibre.com>
> > ---
> >  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 <ccaione@baylibre.com>
> > + */
> 
> 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 <ccaione@baylibre.com>
> > + */
> > +
> > +&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 <jmasson@baylibre.com>
> > + */
> > +
> > +#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

  reply	other threads:[~2026-09-23 14:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-14 13:35 [PATCH 0/3] Add Genio 510 and 700 BootROM download agents Carlo Caione
2026-09-14 13:35 ` [PATCH 1/3] arm: mediatek: enable SPL fastboot DAs on Genio 510 and 700 Carlo Caione
2026-09-22 16:27   ` David Lechner
2026-09-23 14:15     ` Carlo Caione [this message]
2026-09-14 13:35 ` [PATCH 2/3] arm: mediatek: enable U-Boot " Carlo Caione
2026-09-22 16:27   ` David Lechner
2026-09-23 14:25     ` Carlo Caione
2026-09-23 14:55       ` David Lechner
2026-09-23 16:14         ` Carlo Caione
2026-09-14 13:35 ` [PATCH 3/3] doc: board: mediatek: document Genio download agents Carlo Caione

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arPV4uDU7S-SbPKs@macbook \
    --to=ccaione@baylibre.com \
    --cc=GSS_MTK_Uboot_upstream@mediatek.com \
    --cc=Suhrid.Subramaniam@mediatek.com \
    --cc=alchark@gmail.com \
    --cc=arnaud.ferraris@collabora.com \
    --cc=bastien.curutchet@bootlin.com \
    --cc=chris-qj.chen@mediatek.com \
    --cc=chunfeng.yun@mediatek.com \
    --cc=dlechner@baylibre.com \
    --cc=igor.belwon@mentallysanemainliners.org \
    --cc=ilias.apalodimas@linaro.org \
    --cc=jbx6244@gmail.com \
    --cc=jmasson@baylibre.com \
    --cc=jstephan@baylibre.com \
    --cc=macpaul.lin@mediatek.com \
    --cc=neil.armstrong@linaro.org \
    --cc=pablo.sun@mediatek.com \
    --cc=ryder.lee@mediatek.com \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.u-boot-project.org \
    --cc=vsatoes@baylibre.com \
    --cc=weijie.gao@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.