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

[Daniel Lezcano]

Hi Leo,

On 3/9/26 11:07, Leo Yan wrote:
> 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 for the clarification

Acked-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>