[PATCH v2 1/5] cyclic: Add a symbol for SPL

[Tom Rini]

On Wed, Dec 13, 2023 at 12:50:02PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Sat, 9 Dec 2023 at 08:48, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Sat, Dec 02, 2023 at 08:33:48AM -0700, Simon Glass wrote:
> >
> > > The cyclic subsystem is currently enabled either in all build phases
> > > or none. For tools this should not be enabled, but since lib/shc256.c
> > > and other files include watchdog.h in the host build, we must make
> > > sure that it is not enabled there.
> >
> > This part is what I see as the Wrong Direction. There's some code we
> > really need to share with our user space tools, in order to not
> > copy/paste the same code. In turn, this code must be as user-space
> > friendly as possible. Maybe even we re-factor things a little more, if
> > needed, so that we _just_ have the library functions in common files,
> > and u-boot or user space only files have the make use of logic. I don't
> > feel bad about tools/ needing:
> > void sha256_csum_wd(const unsigned char *input, unsigned int ilen,
> >                 unsigned char *output, unsigned int chunk_sz)
> > {
> >         sha256_context ctx;
> >         sha256_starts(&ctx);
> >         sha256_update(&ctx, input, ilen);
> >         sha256_finish(&ctx, output);
> > }
> >
> > (and so on for other algos) as a duplicate bit of code. Much less so
> > than I do about adding <linux/kconfig.h> to a direct include list (since
> > we should never be doing that) so that later on we can if
> > (IS_ENABLED(..)) the existing code.
> 
> Bear in mind that we have the CONFIG_TOOLS_... options entirely to
> deal with the need for tools to enable features in common code. This
> SHA thing is a very small part of the code, compared to common code in
> boot/ for example.
> 
> So is this really a win?

I don't follow you here, sorry. We share lib/sha*.c with host tools for
generic mkimage functionality. I'm fine with continuing to use
USE_HOSTCC as the guard for U-Boot / userspace here. And I don't think
switching these files from:
#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
to:
	if (IS_ENABLED(CONFIG_HW_WATCHDOG) || IS_ENABLED(CONFIG_WATCHDOG)) {
improves readability of the code.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231213/78907d7c/attachment.sig>