On 6/8/26 4:04 PM, Tom Rini wrote:
On Mon, Jun 08, 2026 at 03:59:11PM +0200, Quentin Schulz wrote:
Hi Daniel,
On 6/8/26 3:06 PM, Daniel Palmer wrote:
- CONFIG_LOADB causes the xyzModem code to get built and that code depends
on CONFIG_CRC16. If you don't have something else that selects CONFIG_CRC16
you can unselect it and then the final link will fail:
LD u-boot
/usr/bin/m68k-linux-gnu-ld.bfd: common/xyzModem.o: in function
`xyzModem_get_hdr':
u-boot/common/xyzModem.c:388:(.text.xyzModem_get_hdr+0x17a): undefined
reference to `crc16_ccitt'
- Add CONFIG_YMODEM_SUPPORT, make that select CONFIG_CRC16 and make
CONFIG_LOADB select CONFIG_YMODEM_SUPPORT.
- SPL/TPL already have Kconfig symbols for ymodem support but the spl
ymodem code has issues building in the TPL phase and doesn't seem to
be used:
/usr/bin/aarch64-linux-gnu-ld.bfd: common/spl/spl_ymodem.o: in function
`spl_ymodem_load_image':
u-boot/common/spl/spl_ymodem.c:138:(.text.spl_ymodem_load_image+0xdc):
undefined reference to `spl_load_simple_fit'
u-boot/common/spl/spl_ymodem.c:138:(.text.spl_ymodem_load_image+0xdc):
relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
`spl_load_simple_fit'
make[1]: *** [scripts/Makefile.xpl:546: tpl/u-boot-tpl] Error 1
- Remove the ymodem symbol entirely for TPL.
- Select CONFIG_SPL_CRC16 for SPL.
- Finally, since everyone is now using the ymodem support symbols remove
the special rule for loadb in the makefile.
When you need to list what you're doing in a commit log, it usually means
you should be splitting the content into multiple separate patches (in a
single series is usually fine/recommended).
What I could identify (slightly reworked, which better match how *I* would
write the patches):
- changes made to common/spl/Kconfig.tpl with the justification given above,
And I also see a mention of CONFIG_TPL_YMODEM_SUPPORT=y in
doc/usage/spl_boot.rst that would need removal
- adding the missing CRC16 dependency to CMD_LOADB,
- adding missing SPL_CRC16 dependency to SPL_YMODEM_SUPPORT (possibly
squashed with the CMD_LOADB+CRC16 patch above, I'm undecided on what's best
here),
- create new YMODEM_SUPPORT symbol and migrate CMD_LOADB + common/Makefile
to use that,
This makes it clearer what the purpose of each individual commit is:
- remove dead code
- fix existing support for CMD_LOADB,
- fix existing support for SPL_YMODEM_SUPPORT,
- quality of life improvement by migrating the CRC16 dependency from
CMD_LOADB into a new symbol + making use of that new symbol in
common/Makefile instead of "hardlinking" CMD_LOADB with building xyzModem.o
(which is quite honestly confusing, so good to see this fixed!),
What do you think?
Signed-off-by: Daniel Palmer <[email protected]>
---
v2: Fix the SPL/TPL stuff as brought up by Tom. This opened a can of
worms when I tested if the TPL builds or not so I decided to just
remove the TPL ymodem stuff. It didn't seem to be used in any
defconfig and there isn't a CONFIG_TPL_CRC16 to enable the needed
crc16 code anyhow so maybe it hasn't been used for a while?
cmd/Kconfig | 1 +
common/Kconfig | 4 ++++
common/Makefile | 1 -
common/spl/Kconfig | 1 +
common/spl/Kconfig.tpl | 11 +----------
5 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index c71c6824a196..503fc51f6bb4 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1412,6 +1412,7 @@ config CMD_W1
config CMD_LOADB
bool "loadb"
default y
+ select YMODEM_SUPPORT
help
Load a binary file over serial line.
diff --git a/common/Kconfig b/common/Kconfig
index 8e8c733aa295..0db74aa580e4 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1274,3 +1274,7 @@ config IO_TRACE
confirm that the driver behaves the same way before and after a code
change. To add support for your architecture, add '#include
<iotrace.h>' to the bottom of arch/<arch>/include/asm/io.h and test.
+
+config YMODEM_SUPPORT
+ bool
+ select CRC16
Is there any reason we wouldn't want the user to be able to select this
independently from CMD_LOADB?
I'm asking because this is both invisible (no prompt) and has no help text.
I think this should stay hidden as it's *not* hidden for xPL as it's the
equivalent of CMD_LOADB but it's the symbol for the library portion. As
a maybe clearer result, maybe the library symbol should be
XYZMODEM_SUPPORT / xPL_XYZMODEM_SUPPORT, to control building
common/xyzModem.o ?
I'm not sure to understand how the rename would help here? I'm assuming
we have an issue of the symbol having a different meaning in xPL and in
proper. If I understood correctly, YMODEM_SUPPORT in proper is for
building the library and using it, while in xPL it's both for building
the library and add support for loading the next U-Boot stage over UART.
Is this what you meant? Are you suggesting we split "build library code"
and "load next stage from UART" into separate symbols?
Also, lib/Makefile has
obj-$(CONFIG_SPL_YMODEM_SUPPORT) += crc16-ccitt.o
which is.... odd? Shouldn't this handle the case of "missing" SPL_CRC16
dependency? So: does it really not build if SPL_CRC16=n but
SPL_YMODEM_SUPPORT=y? If it does not build, is bringing SPL_CRC16 too
much? We only need crc16-ccitt.o I think, and this will also bring
crc16.o into the SPL, which may not be desired (size increase).
Cheers,
Quentin
