Re: [PATCH v3 04/30] tools/thermal: Initialize CFLAGS before including Makefile.include

[Leo Yan]

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@xxxxxxx>
> > ---
> >   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
> > 
>