All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mattijs Korpershoek <mkorpershoek@kernel.org>
To: Tom Rini <trini@konsulko.com>,
	Mattijs Korpershoek <mkorpershoek@kernel.org>
Cc: u-boot@lists.denx.de, Lukasz Majewski <lukma@denx.de>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>
Subject: Re: [PATCH] dfu: Make the DFU_WRITE_ALT symbol available outside of DFU
Date: Fri, 13 Mar 2026 09:59:51 +0100	[thread overview]
Message-ID: <87zf4cksu0.fsf@kernel.org> (raw)
In-Reply-To: <20260312225819.GC502704@bill-the-cat>

Hi Tom,

On Thu, Mar 12, 2026 at 16:58, Tom Rini <trini@konsulko.com> wrote:

> On Thu, Mar 12, 2026 at 10:10:06AM +0100, Mattijs Korpershoek wrote:
>> Hi Tom,
>>
>> Thank you for the patch.
>>
>> On Tue, Mar 10, 2026 at 10:26, Tom Rini <trini@konsulko.com> wrote:
>>
>> > The DFU_WRITE_ALT symbol is used both directly and indirectly (via
>> > UPDATE_COMMON) for EFI capsule updates (FIT or raw), but does not depend
>> > on DFU itself. Move this symbol outside of "if DFU" to remove a Kconfig
>> > dependency problem.
>>
>> Looking at drivers/dfu/dfu_alt.c both dfu_write_by_name() and
>> dfu_write_by_alt() seem to rely on functions from drivers/dfu/dfu.c such
>> as dfu_init_env_entities(), dfu_get_entity() and more.
>>
>> Looking at UPDATE_COMMON, I see:
>> config UPDATE_COMMON
>> 	bool
>> 	select DFU_WRITE_ALT
>> 	imply CMD_TFTPBOOT
>>
>> And if we check the code in common/update.c, we can see that
>> dfu_write_by_name() is called.
>>
>> So fit_update() calls
>>    dfu_write_by_name() which calls
>>        dfu_init_env_entities() which might no longer be defined when
>>                                applying this patch.
>>
>> I'm not sure how is this supposed to work. Do we have to stub
>> dfu_init_env_entities() ?
>
> So, to me the challenge is that DFU_TFTP, UPDATE_FIT and UPDATE_TFTP all
> are the three users of UPDATE_COMMON and so DFU_WRITE_ALT. The first two
> depend on DFU, but the latter does not. And there's nothing in-tree
> using it, either (but it does compile I believe, or allyesconfig would
> have failed).

I see, thank you for explaining. I've done some testing on sandbox with
the following diff:

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 280f9c9cfe77..5bc114bad064 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -16,9 +16,6 @@ CONFIG_EFI_SECURE_BOOT=y
 CONFIG_EFI_RT_VOLATILE_STORE=y
 CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
-CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
-CONFIG_EFI_CAPSULE_AUTHENTICATE=y
-CONFIG_EFI_CAPSULE_CRT_FILE="board/sandbox/capsule_pub_key_good.crt"
 CONFIG_BUTTON_CMD=y
 CONFIG_FIT=y
 CONFIG_FIT_CIPHER=y
@@ -50,6 +47,7 @@ CONFIG_LOGF_FUNC=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 # CONFIG_BOARD_INIT is not set
 CONFIG_STACKPROTECTOR=y
+CONFIG_UPDATE_TFTP=y
 CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_UFETCH=y
@@ -201,7 +199,6 @@ CONFIG_AES_SOFTWARE=y
 CONFIG_DM_DEMO=y
 CONFIG_DM_DEMO_SIMPLE=y
 CONFIG_DM_DEMO_SHAPE=y
-CONFIG_DFU_SF=y
 CONFIG_DMA=y
 CONFIG_DMA_CHANNELS=y
 CONFIG_SANDBOX_DMA=y
@@ -362,7 +359,6 @@ CONFIG_CONSOLE_ROTATION=y
 CONFIG_CONSOLE_TRUETYPE=y
 CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
 CONFIG_I2C_EDID=y
-CONFIG_VIDEO_SANDBOX_SDL=y
 CONFIG_VIDEO_BRIDGE=y
 CONFIG_VIDEO_BRIDGE_LVDS_CODEC=y
 CONFIG_VIDEO_DSI_HOST_SANDBOX=y

We indeed have:

$ grep -e CONFIG_DFU -e DFU_TFTP -e UPDATE_FIT -e UPDATE_TFTP .config
CONFIG_UPDATE_TFTP=y
CONFIG_UPDATE_TFTP_CNT_MAX=0
CONFIG_UPDATE_TFTP_MSEC_MAX=100
CONFIG_DFU_WRITE_ALT=y

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

>
> --
> Tom

  reply	other threads:[~2026-03-13  9:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-10 16:26 [PATCH] dfu: Make the DFU_WRITE_ALT symbol available outside of DFU Tom Rini
2026-03-12  9:10 ` Mattijs Korpershoek
2026-03-12 22:58   ` Tom Rini
2026-03-13  8:59     ` Mattijs Korpershoek [this message]
2026-03-13 12:56 ` Ilias Apalodimas
2026-03-16  9:39 ` Mattijs Korpershoek

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=87zf4cksu0.fsf@kernel.org \
    --to=mkorpershoek@kernel.org \
    --cc=ilias.apalodimas@linaro.org \
    --cc=lukma@denx.de \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=xypron.glpk@gmx.de \
    /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.