Couple of clang warnings for Rockchip boards

Mon May 11 20:10:31 CEST 2026

Hi Quentin,

On Fri, 8 May 2026 at 03:33, Quentin Schulz <quentin.schulz at cherry.de> 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).

That's clever!

But Tom is very opposed to being able to disable -Werror from the
environment for boards which enable it. Let's leave it alone.

Regards,
Simon