Couple of clang warnings for Rockchip boards

Fri May 8 18:09:56 CEST 2026

On Fri, May 08, 2026 at 11:33:11AM +0200, Quentin Schulz wrote:
> Hi Simon,
> 
> On 5/4/26 10:40 PM, Simon Glass wrote:
> > On Mon, 4 May 2026 at 11:35, Tom Rini <trini at konsulko.com> wrote:
> > > 
> > > On Mon, May 04, 2026 at 05:14:13AM -0600, Simon Glass wrote:
> > > > Hi Quentin,
> > > > 
> > > > On Wed, 29 Apr 2026 at 10:32, Quentin Schulz <quentin.schulz at cherry.de> wrote:
> > > > > 
> > > > > On 4/29/26 4:25 PM, Tom Rini wrote:
> > > > > > On Wed, Apr 29, 2026 at 08:18:14AM -0600, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > > 
> > > > > > > On Mon, 27 Apr 2026 at 08:46, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > 
> > > > > > > > On Mon, Apr 27, 2026 at 11:54:09PM +1200, Simon Glass wrote:
> > > > > > > > > Hi Quentin,
> > > > > > > > > 
> > > > > > > > > On Thu, 23 Apr 2026 at 22:24, Quentin Schulz <quentin.schulz at cherry.de> wrote:
> > > > > > > > > > 
> > > > > > > > > > Hi Simon,
> > > > > > > > > > 
> > > > > > > > > > On 4/22/26 11:57 PM, Simon Glass wrote:
> > > > > > > > > > > Hi Quentin,
> > > > > > > > > > > 
> > > > > > > > > > > On Thu, 23 Apr 2026 at 01:26, Quentin Schulz <quentin.schulz at cherry.de> wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > Hi all,
> > > > > > > > > > > > 
> > > > > > > > > > > > I'm looking into force-enabling CONFIG_WERROR for all Rockchip SoCs.
> > > > > > > > > > > > First, let me know if this is a bad idea :)
> > > > > > > > > > > 
> > > > > > > > > > > I don't like it, as it makes development slower. I like to see all the
> > > > > > > > > > > warnings created by a change and tend to fix them iteratively. We
> > > > > > > > > > > already have a warning check in CI. A better approach is to
> > > > > > > > > > > 
> > > > > > > > > > > Can you use: KCFLAGS=-Werror
> > > > > > > > > > > 
> > > > > > > > > > > either in your environment or on the 'make' cmdline?
> > > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Well, if it's during development, you can always disable WERROR? The
> > > > > > > > > > fact that I've never actually enabled -Werror locally in about a decade
> > > > > > > > > > in U-Boot probably isn't a good sign. One of the things that make me
> > > > > > > > > > wary though is compiler updates with new warnings. I've tested with
> > > > > > > > > > clang 22, don't know when clang 23 is planned to be released. GCC 16 is
> > > > > > > > > > about to be released as well.
> > > > > > > > > > 
> > > > > > > > > > By having the check in CI, we have a longer-than-necessary feedback loop
> > > > > > > > > > for contributors. Has this been an issue in the past? /me shrugs
> > > > > > > > > 
> > > > > > > > > This is strange though, because people should see the warnings during
> > > > > > > > > development. I wonder if it is scrolling off the screen.
> > > > > > > > > 
> > > > > > > > > One my bugbears is non-actionable output. When the build succeeds
> > > > > > > > > (with no warnings) I see no output, using 'buildman -I --board xxx -w
> > > > > > > > > -o /tmp/b/xxx' or in fact these days 'um build xxx'. This uses 'make
> > > > > > > > > -s' under the hood, so that it doesn't print a huge list of filenames,
> > > > > > > > > etc. I also sometimes do a 'fresh' build with 'um build -F xxx' to
> > > > > > > > > check everything is clean ('buildman -m' does something similar).
> > > > > > > > > 
> > > > > > > > > Note that buildman returns exit code 101 if there are any warnings, so
> > > > > > > > > the build basically fails.
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > No defconfig seems to be setting this option, not sure exactly why that is.
> > > > > > > > > 
> > > > > > > > > To me this is a difference between 'functionality' options and
> > > > > > > > > development options. Putting development options in Kconfig makes it
> > > > > > > > > harder to turn them on and off, since you must either edit the
> > > > > > > > > defconfig or edit the .config file. It is easier to just pass an
> > > > > > > > > environment variable.
> > > > > > > > 
> > > > > > > > The point of a configuration is that it configures the build. Stuff in
> > > > > > > > environment (directly or make FOO=bar) is a workaround and anti-pattern.
> > > > > > > > Having to run one of the config front-end to change the config is a
> > > > > > > > feature, and good thing.
> > > > > > > 
> > > > > > > To give an example, we have boards which enable LTO, but it is
> > > > > > > sometimes useful to disable it during development - it used to be a
> > > > > > > huge time sync but something seems to have changed recently, as I
> > > > > > > don't see much of a performance gap now. We don't want people
> > > > > > > disabling LTO in the defconfig, though. The same applies for things
> > > > > > > like V=1
> > > > > > > 
> > > > > > > So I think there are build features which are of no use to end-users,
> > > > > > > but are helpful for development. That is the distinction we have had
> > > > > > > for a while and it would be a shame to lose it.
> > > > > > 
> > > > > > Yes, the LTO thing is a good example of something we shouldn't have done
> > > > > > really. Outside of sandbox you cannot just disable it.
> > > > 
> > > > Are you sure about that?
> > > 
> > > Yes. Outside of sandbox LTO is enabled because the platform binaries
> > > will exceed their non-optional size constraints.
> > 
> > That is certainly true for some boards, but I'm not sure it is very many.
> > 
> > > 
> > > > I seldom build with LTO, except for CI. For
> > > > an incremental build the non-LTO is often twice as fast. Most of the
> > > > time when developing new features I am just trying to make things
> > > > work, so it is big time-saver in the IDE. To some extent this is
> > > > changing, though (LSPs and AI tools). The main impact for me is with
> > > > sandbox incremental builds.
> > > 
> > > We could maybe just drop LTO from sandbox, now that it's enabled more
> > > widely on hardware.
> > 
> > The nice thing about LTO for sandbox is that it tests a different
> > build path. We could perhaps disable it for just the 'sandbox' board?
> > 
> > > 
> > > > > > 
> > > > > 
> > > > > FYI, the Linux kernel defaults to CONFIG_WERROR=y. See commit
> > > > > 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
> > > > > (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3fe617ccafd6f5bb33c2391d6f4eeb41c1fd0151).
> > > > 
> > > > Do we suffer from this same problem?
> > > > 
> > > > > 
> > > > > I wouldn't necessarily recommend to switch it on right now, but I think
> > > > > it could be a good idea in the mid-term. We're also very far behind in
> > > > > terms of warn flags compared to the kernel. Syncing will take some time
> > > > > though. For now I'm looking into enabling -Werror for host tools and
> > > > > this resulted in (so far) 21 patches.
> > > > 
> > > > I must be misunderstanding what you are referring to here. I don't see
> > > > any warnings when building tools-only - certainly I would not put up
> > > > with that. Do you mean that you are enabling additional warnings?
> > > > 
> > > > We already have CI checking (which fails on the first warning) and we
> > > > already show the warnings for normal builds. Importantly, without
> > > > -Werror, we show *all* the warnings rather than just stopping at the
> > > > first one depending on -j, etc.
> > > > 
> > > > So forcing this option seems to be a tax on people who fix warnings,
> > > > imposed by those who don't. It really should be the other way around
> > > > :-)
> > > > 
> > > > BTW we do have KCFLAGS which can be used to set Werror (and also
> > > > perhaps other things).
> > > 
> > > I'll note that I just had to request changes on another patch because
> > > they didn't realize warnings had to be fixed. It's quite likely that
> > > making CONFIG_WERROR=y will save more people more time (maybe document
> > > make -k? I don't know what people don't know about these days, but I do
> > > "make -kj" a lot when doing the randconfig stuff for example) than what
> > > we do today.
> > 
> > Yes, could be, particular if people are not using -s with make. I
> > really dislike warnings quite a lot. I'd be happy enough with
> > CONFIG_WERROR if we have an env var to disable -Werror like we have
> > for LTO.
> > 
> 
> I'll let Tom speak his mind whether that's a good idea.
> 
> I really don't want to be touching scripts/Makefile.extrawarn once sync'ed
> with the one in Linux kernel's, so adding some logic in there is a no-no in
> my book. So it means we can only change the CONFIG_WERROR symbol. But we
> kinda don't want to rename it as it'll likely be used throughout the source
> code (hopefully spreading more and more over time) since we also want to
> sync as much as possible kbuild/makefiles from the Linux kernel. So what I
> thought could be a way to make this more palatable to you would be to
> introduce an invisible symbol which depends on an env variable.
> 
> Something like
> 
> config DISABLE_WERROR_FROM_ENV
>   def_bool $(success, test "$(DISABLE_WERROR)" != "1" )
>   help
>     Allow to override CONFIG_WERROR from .config via the DISABLE_WERROR
> environment variable.
> 
> config WERROR
>   depends on !DISABLE_WERROR_FROM_ENV
> 
> for example (untested).

Yes, I thought I was clear enough earlier in the thread. Changing this
behavior from the environment isn't something we should do and is bad
design. We're stuck with the LTO one for the moment and should not add
more.

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