[PATCH v2] xyz-modem: Select CRC16

[Quentin Schulz]

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,
- 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 <daniel at thingy.jp>
> ---
> 
> 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.

Cheers,
Quentin