mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* Makefile.lib: cmd_dtc: warning: missing whitespace after the macro name
@ 2022-09-07  8:21 Antony Pavlov
  2022-09-12  8:01 ` Sascha Hauer
  0 siblings, 1 reply; 5+ messages in thread
From: Antony Pavlov @ 2022-09-07  8:21 UTC (permalink / raw)
  To: Sascha Hauer, Trent Piepho, Oleksij Rempel, Ahmad Fatoum, Barebox List

Hi Everyone!

During MIPS ath79_defconfig build I have a 'missing whitespace after the macro name' warning:

  AS [P]  arch/mips/dts/ar9331_tl_mr3020.dtb.pbl.o
  DTC     arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb
<stdin>:1:9: warning: missing whitespace after the macro name
  XZKERN  arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb.z

E.g. see https://gitlab.com/frantony/barebox/-/jobs/2969826747#L47

The reason is the scripts/Makefile.lib generates a C macro with the point symbol in the macro name because the arch/mips/dts/ar9344-tl-wdr4300-v1.7.dts file name contains the point symbol before '.dts', as a result we have:

  #define ar9344_tl_wdr4300_v1.7_dts 1

e.g.

  barebox$ grep -RHn -o "define ar9344_tl_wdr4300_v1.* 1" . 2>/dev/null
  ./arch/mips/dts/.ar9344-tl-wdr4300-v1.7.dtb.cmd:1:define ar9344_tl_wdr4300_v1.7_dts 1

cmd_dtc in scripts/Makefile.lib substitutes the '-' symbols with the '_' symbols but do nothing with other unwanted C preprocessor macro name symbols.

It looks like the linux kernel has no problems with extra point symbols in dts file names, there are several files with extra dot in dts:

  barebox$ find dts/ -iname '*.*.dts' | wc -l
  33

So we have to fix Makefile.lib.

This simple patch fixes the warning problem:

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 16308497b84..2f79656c1e9 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -363,7 +363,7 @@ $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
 dts-frags = $(subst $(quote),,$(CONFIG_EXTERNAL_DTS_FRAGMENTS))
 quiet_cmd_dtc = DTC     $@
 # For compatibility between make 4.2 and 4.3
-cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
+cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(subst .,_,$(*F)))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
 	$(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) - ; \
 	$(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 \
 		-i $(srctree)/arch/$(SRCARCH)/dts $(DTC_FLAGS) \

I suppose that this simple patch may lead to some undesirable side effects.

Any suggestions?

-- 
Best regards,
  Antony Pavlov



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Makefile.lib: cmd_dtc: warning: missing whitespace after the macro name
  2022-09-07  8:21 Makefile.lib: cmd_dtc: warning: missing whitespace after the macro name Antony Pavlov
@ 2022-09-12  8:01 ` Sascha Hauer
  2022-09-12  9:07   ` Antony Pavlov
  0 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2022-09-12  8:01 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: Trent Piepho, Oleksij Rempel, Ahmad Fatoum, Barebox List

On Wed, Sep 07, 2022 at 11:21:18AM +0300, Antony Pavlov wrote:
> Hi Everyone!
> 
> During MIPS ath79_defconfig build I have a 'missing whitespace after the macro name' warning:
> 
>   AS [P]  arch/mips/dts/ar9331_tl_mr3020.dtb.pbl.o
>   DTC     arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb
> <stdin>:1:9: warning: missing whitespace after the macro name
>   XZKERN  arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb.z
> 
> E.g. see https://gitlab.com/frantony/barebox/-/jobs/2969826747#L47
> 
> The reason is the scripts/Makefile.lib generates a C macro with the point symbol in the macro name because the arch/mips/dts/ar9344-tl-wdr4300-v1.7.dts file name contains the point symbol before '.dts', as a result we have:
> 
>   #define ar9344_tl_wdr4300_v1.7_dts 1
> 
> e.g.
> 
>   barebox$ grep -RHn -o "define ar9344_tl_wdr4300_v1.* 1" . 2>/dev/null
>   ./arch/mips/dts/.ar9344-tl-wdr4300-v1.7.dtb.cmd:1:define ar9344_tl_wdr4300_v1.7_dts 1
> 
> cmd_dtc in scripts/Makefile.lib substitutes the '-' symbols with the '_' symbols but do nothing with other unwanted C preprocessor macro name symbols.
> 
> It looks like the linux kernel has no problems with extra point symbols in dts file names, there are several files with extra dot in dts:
> 
>   barebox$ find dts/ -iname '*.*.dts' | wc -l
>   33
> 
> So we have to fix Makefile.lib.
> 
> This simple patch fixes the warning problem:
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 16308497b84..2f79656c1e9 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -363,7 +363,7 @@ $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
>  dts-frags = $(subst $(quote),,$(CONFIG_EXTERNAL_DTS_FRAGMENTS))
>  quiet_cmd_dtc = DTC     $@
>  # For compatibility between make 4.2 and 4.3
> -cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
> +cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(subst .,_,$(*F)))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
>  	$(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) - ; \
>  	$(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 \
>  		-i $(srctree)/arch/$(SRCARCH)/dts $(DTC_FLAGS) \
> 
> I suppose that this simple patch may lead to some undesirable side effects.

One side effect is that this gets even less readable.

Another one would be that two dts filenames which only differ in the
usage of '.' and '_' would result in the same define, but I think that
case is negligible as this define is unused in barebox itself. It could
be used by external dts fragments passed in via CONFIG_EXTERNAL_DTS_FRAGMENTS.

Other than that, what side effects are you afraid of?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Makefile.lib: cmd_dtc: warning: missing whitespace after the macro name
  2022-09-12  8:01 ` Sascha Hauer
@ 2022-09-12  9:07   ` Antony Pavlov
  2022-09-19  9:14     ` Sascha Hauer
  0 siblings, 1 reply; 5+ messages in thread
From: Antony Pavlov @ 2022-09-12  9:07 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Trent Piepho, Oleksij Rempel, Ahmad Fatoum, Barebox List

On Mon, 12 Sep 2022 10:01:52 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

Hi Sascha!

> On Wed, Sep 07, 2022 at 11:21:18AM +0300, Antony Pavlov wrote:
> > Hi Everyone!
> > 
> > During MIPS ath79_defconfig build I have a 'missing whitespace after the macro name' warning:
> > 
> >   AS [P]  arch/mips/dts/ar9331_tl_mr3020.dtb.pbl.o
> >   DTC     arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb
> > <stdin>:1:9: warning: missing whitespace after the macro name
> >   XZKERN  arch/mips/dts/ar9344-tl-wdr4300-v1.7.dtb.z
> > 
> > E.g. see https://gitlab.com/frantony/barebox/-/jobs/2969826747#L47
> > 
> > The reason is the scripts/Makefile.lib generates a C macro with the point symbol in the macro name because the arch/mips/dts/ar9344-tl-wdr4300-v1.7.dts file name contains the point symbol before '.dts', as a result we have:
> > 
> >   #define ar9344_tl_wdr4300_v1.7_dts 1
> > 
> > e.g.
> > 
> >   barebox$ grep -RHn -o "define ar9344_tl_wdr4300_v1.* 1" . 2>/dev/null
> >   ./arch/mips/dts/.ar9344-tl-wdr4300-v1.7.dtb.cmd:1:define ar9344_tl_wdr4300_v1.7_dts 1
> > 
> > cmd_dtc in scripts/Makefile.lib substitutes the '-' symbols with the '_' symbols but do nothing with other unwanted C preprocessor macro name symbols.
> > 
> > It looks like the linux kernel has no problems with extra point symbols in dts file names, there are several files with extra dot in dts:
> > 
> >   barebox$ find dts/ -iname '*.*.dts' | wc -l
> >   33
> > 
> > So we have to fix Makefile.lib.
> > 
> > This simple patch fixes the warning problem:
> > 
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index 16308497b84..2f79656c1e9 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -363,7 +363,7 @@ $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
> >  dts-frags = $(subst $(quote),,$(CONFIG_EXTERNAL_DTS_FRAGMENTS))
> >  quiet_cmd_dtc = DTC     $@
> >  # For compatibility between make 4.2 and 4.3
> > -cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
> > +cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(subst .,_,$(*F)))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \
> >  	$(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) - ; \
> >  	$(objtree)/scripts/dtc/dtc -O dtb -o $@ -b 0 \
> >  		-i $(srctree)/arch/$(SRCARCH)/dts $(DTC_FLAGS) \
> > 
> > I suppose that this simple patch may lead to some undesirable side effects.
> 
> One side effect is that this gets even less readable.

Yes, you are right :)

> Another one would be that two dts filenames which only differ in the
> usage of '.' and '_' would result in the same define, but I think that
> case is negligible as this define is unused in barebox itself. It could
                                       ^^^^^^^^^^^^^^^^^^^^^^^^
> be used by external dts fragments passed in via CONFIG_EXTERNAL_DTS_FRAGMENTS.
> 
> Other than that, what side effects are you afraid of?

I have an idea that someone relies on these *_dts macros.

Now I see (as you have noted above) that there is no macro users in mainline source tree:

  $ git grep -w .*_dts
  common/Kconfig:   #ifdef foo_board_dts
  scripts/Makefile.lib:cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 1\n'$(foreach f,$< $(dts-frags),'$(pound)include "$(f)"\n') | \

-- 
Best regards,
  Antony Pavlov



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Makefile.lib: cmd_dtc: warning: missing whitespace after the macro name
  2022-09-12  9:07   ` Antony Pavlov
@ 2022-09-19  9:14     ` Sascha Hauer
  2022-09-22 16:24       ` Antony Pavlov
  0 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2022-09-19  9:14 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: Trent Piepho, Oleksij Rempel, Ahmad Fatoum, Barebox List

Hi Antony,

On Mon, Sep 12, 2022 at 12:07:34PM +0300, Antony Pavlov wrote:
> On Mon, 12 Sep 2022 10:01:52 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > One side effect is that this gets even less readable.
> 
> Yes, you are right :)
> 
> > Another one would be that two dts filenames which only differ in the
> > usage of '.' and '_' would result in the same define, but I think that
> > case is negligible as this define is unused in barebox itself. It could
>                                        ^^^^^^^^^^^^^^^^^^^^^^^^
> > be used by external dts fragments passed in via CONFIG_EXTERNAL_DTS_FRAGMENTS.
> > 
> > Other than that, what side effects are you afraid of?
> 
> I have an idea that someone relies on these *_dts macros.
> 
> Now I see (as you have noted above) that there is no macro users in mainline source tree:

Could you create a formal patch from this?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Makefile.lib: cmd_dtc: warning: missing whitespace after the macro name
  2022-09-19  9:14     ` Sascha Hauer
@ 2022-09-22 16:24       ` Antony Pavlov
  0 siblings, 0 replies; 5+ messages in thread
From: Antony Pavlov @ 2022-09-22 16:24 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Trent Piepho, Oleksij Rempel, Ahmad Fatoum, Barebox List

On Mon, 19 Sep 2022 11:14:10 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

Hi Sascha!

> Hi Antony,
> 
> On Mon, Sep 12, 2022 at 12:07:34PM +0300, Antony Pavlov wrote:
> > On Mon, 12 Sep 2022 10:01:52 +0200
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > > One side effect is that this gets even less readable.
> > 
> > Yes, you are right :)
> > 
> > > Another one would be that two dts filenames which only differ in the
> > > usage of '.' and '_' would result in the same define, but I think that
> > > case is negligible as this define is unused in barebox itself. It could
> >                                        ^^^^^^^^^^^^^^^^^^^^^^^^
> > > be used by external dts fragments passed in via CONFIG_EXTERNAL_DTS_FRAGMENTS.
> > > 
> > > Other than that, what side effects are you afraid of?
> > 
> > I have an idea that someone relies on these *_dts macros.
> > 
> > Now I see (as you have noted above) that there is no macro users in mainline source tree:
> 
> Could you create a formal patch from this?

I'll a formal patch in a few days.

-- 
Best regards,
  Antony Pavlov



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-09-22 16:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-07  8:21 Makefile.lib: cmd_dtc: warning: missing whitespace after the macro name Antony Pavlov
2022-09-12  8:01 ` Sascha Hauer
2022-09-12  9:07   ` Antony Pavlov
2022-09-19  9:14     ` Sascha Hauer
2022-09-22 16:24       ` Antony Pavlov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox