Hi Daniel,
On Mon, Mar 09, 2026 at 09:41:16AM +0100, Daniel Lezcano wrote:
[...]
> On 3/8/26 17:46, Leo Yan wrote:
> > Initialize CFLAGS to the default value before including
> > tools/scripts/Makefile.include.
> >
> > Defer appending EXTRA_CFLAGS to CFLAGS until after including
> > Makefile.include, as it may extend EXTRA_CFLAGS in the future.
> >
> > Signed-off-by: Leo Yan <leo.yan@arm.com>
> > ---
> > tools/thermal/lib/Makefile | 16 +++++++++-------
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/thermal/lib/Makefile b/tools/thermal/lib/Makefile
> > index 056d212f25cf51cd8c02260fbe2ef28dda5e4acb..1890779f1574ebd9015f3001b9bb31d4bc0ae5ce 100644
> > --- a/tools/thermal/lib/Makefile
> > +++ b/tools/thermal/lib/Makefile
> > @@ -23,6 +23,14 @@ INSTALL = install
> > DESTDIR ?=
> > DESTDIR_SQ = '$(subst ','\'',$(DESTDIR))'
> > +# Defer assigning EXTRA_CFLAGS to CFLAGS until after including
> > +# tools/scripts/Makefile.include, as it may add flags to EXTRA_CFLAGS.
> > +ifdef EXTRA_CFLAGS
> > + CFLAGS :=
> > +else
> > + CFLAGS := -g -Wall
> > +endif
> > +
>
> Sorry, I don't get the comment :/
>
> Can you clarify the intended purpose with this change ?
Sure. Since this series sets EXTRA_CFLAGS in Makefile.include (patch 05).
Without this patch, the init behavior of the thermal Makefile may change.
For example:
make -C tools/thermal/lib/
Before this series, the Makefile initializes:
CFLAGS := -g -Wall
If _only_ patch 05 is applied, EXTRA_CFLAGS is implicitly set in
tools/scripts/Makefile.include to work around compiler bugs. The
Makefile would initialize:
CFLAGS := $(EXTRA_CFLAGS)
This patch preserves the original initialization of CFLAGS, but changes
the sequence:
CFLAGS := -g -Wall
CFLAGS += $(EXTRA_CFLAGS)
If $(EXTRA_CFLAGS) is set by Makefile.include, it will be appended.
Otherwise, if $(EXTRA_CFLAGS) is empty, the behavior remains unchanged.
> and the 'override' directive below ?
The override directive is used to override any CFLAGS set on the make
command line. This patch does not introduce the directive; it simply
follows the existing style. As you can see, the following lines
already use the directive when appending options.
Thanks,
Leo
> > include $(srctree)/tools/scripts/Makefile.include
> > include $(srctree)/tools/scripts/Makefile.arch
> > @@ -39,13 +47,6 @@ libdir = $(prefix)/$(libdir_relative)
> > libdir_SQ = $(subst ','\'',$(libdir))
> > libdir_relative_SQ = $(subst ','\'',$(libdir_relative))
> > -# Set compile option CFLAGS
> > -ifdef EXTRA_CFLAGS
> > - CFLAGS := $(EXTRA_CFLAGS)
> > -else
> > - CFLAGS := -g -Wall
> > -endif
> > -
> > INCLUDES = \
> > -I/usr/include/libnl3 \
> > -I$(srctree)/tools/lib/thermal/include \
> > @@ -56,6 +57,7 @@ INCLUDES = \
> > -I$(srctree)/tools/include/uapi
> > # Append required CFLAGS
> > +override CFLAGS += $(EXTRA_CFLAGS)
> > override CFLAGS += $(EXTRA_WARNINGS)
> > override CFLAGS += -Werror -Wall
> > override CFLAGS += -fPIC
> >
>