diff mbox

[RFC] kbuild: turn of dtc unit address warnings by default

Message ID 1488176685-23740-1-git-send-email-yamada.masahiro@socionext.com
State Accepted
Commit 4b83f0d98a17aef896a9d772275bd09a4dfab07c
Headers show

Commit Message

Masahiro Yamada Feb. 27, 2017, 6:24 a.m. UTC
DTC 1.4.2 or later checks DT unit-address without reg property and
vice-versa, and generates lots of warnings.  Fixing DT files will
take for a while.  Until then, let's turn off the check unless
building with W=*.

Introduce a new helper dtc-option to check if the option is supported
in order to suppress warnings on older versions.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

This is one possible answer to Tom's:
https://www.mail-archive.com/u-boot@lists.denx.de/msg240328.html

He fixed the problem on travis-ci by commit a0f3e3d, but it is still
annoying during the regular development.
Perhaps we may want to hide the warnings (at least hidden in Linux
Kernel by default).
Or, is it better to keep it noisy
to motivate people to fix their DT files?
I am not quite sure...

Now I am sending this as RFC patch in case people may want to start discussion.

BTW, this is a counter-part of the patch I sent to the Kbuild sub-system
(https://patchwork.kernel.org/patch/9592747/)
because I want the U-Boot build system with Linux as much as possible.
Let's see if I will get possible opinions in the Kbuild review.


 Makefile                   | 2 +-
 scripts/Kbuild.include     | 5 +++++
 scripts/Makefile.extrawarn | 6 ++++++
 3 files changed, 12 insertions(+), 1 deletion(-)

-- 
2.7.4

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Simon Glass March 3, 2017, 4:52 a.m. UTC | #1
On 26 February 2017 at 23:24, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> DTC 1.4.2 or later checks DT unit-address without reg property and
> vice-versa, and generates lots of warnings.  Fixing DT files will
> take for a while.  Until then, let's turn off the check unless
> building with W=*.
>
> Introduce a new helper dtc-option to check if the option is supported
> in order to suppress warnings on older versions.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> This is one possible answer to Tom's:
> https://www.mail-archive.com/u-boot@lists.denx.de/msg240328.html
>
> He fixed the problem on travis-ci by commit a0f3e3d, but it is still
> annoying during the regular development.
> Perhaps we may want to hide the warnings (at least hidden in Linux
> Kernel by default).
> Or, is it better to keep it noisy
> to motivate people to fix their DT files?
> I am not quite sure...
>
> Now I am sending this as RFC patch in case people may want to start discussion.
>
> BTW, this is a counter-part of the patch I sent to the Kbuild sub-system
> (https://patchwork.kernel.org/patch/9592747/)
> because I want the U-Boot build system with Linux as much as possible.
> Let's see if I will get possible opinions in the Kbuild review.
>
>
>  Makefile                   | 2 +-
>  scripts/Kbuild.include     | 5 +++++
>  scripts/Makefile.extrawarn | 6 ++++++
>  3 files changed, 12 insertions(+), 1 deletion(-)

Seems like a good idea to me, at least until more are converted.

Reviewed-by: Simon Glass <sjg@chromium.org>
Bin Meng March 6, 2017, 9:58 a.m. UTC | #2
On Mon, Feb 27, 2017 at 2:24 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> DTC 1.4.2 or later checks DT unit-address without reg property and
> vice-versa, and generates lots of warnings.  Fixing DT files will
> take for a while.  Until then, let's turn off the check unless
> building with W=*.
>
> Introduce a new helper dtc-option to check if the option is supported
> in order to suppress warnings on older versions.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> This is one possible answer to Tom's:
> https://www.mail-archive.com/u-boot@lists.denx.de/msg240328.html
>
> He fixed the problem on travis-ci by commit a0f3e3d, but it is still
> annoying during the regular development.
> Perhaps we may want to hide the warnings (at least hidden in Linux
> Kernel by default).
> Or, is it better to keep it noisy
> to motivate people to fix their DT files?
> I am not quite sure...
>
> Now I am sending this as RFC patch in case people may want to start discussion.
>
> BTW, this is a counter-part of the patch I sent to the Kbuild sub-system
> (https://patchwork.kernel.org/patch/9592747/)
> because I want the U-Boot build system with Linux as much as possible.
> Let's see if I will get possible opinions in the Kbuild review.
>
>
>  Makefile                   | 2 +-
>  scripts/Kbuild.include     | 5 +++++
>  scripts/Makefile.extrawarn | 6 ++++++
>  3 files changed, 12 insertions(+), 1 deletion(-)
>

Tested-by: Bin Meng <bmeng.cn@gmail.com>
Masahiro Yamada March 7, 2017, 5:09 p.m. UTC | #3
2017-02-27 15:24 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> DTC 1.4.2 or later checks DT unit-address without reg property and
> vice-versa, and generates lots of warnings.  Fixing DT files will
> take for a while.  Until then, let's turn off the check unless
> building with W=*.
>
> Introduce a new helper dtc-option to check if the option is supported
> in order to suppress warnings on older versions.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> This is one possible answer to Tom's:
> https://www.mail-archive.com/u-boot@lists.denx.de/msg240328.html
>
> He fixed the problem on travis-ci by commit a0f3e3d, but it is still
> annoying during the regular development.
> Perhaps we may want to hide the warnings (at least hidden in Linux
> Kernel by default).
> Or, is it better to keep it noisy
> to motivate people to fix their DT files?
> I am not quite sure...
>
> Now I am sending this as RFC patch in case people may want to start discussion.
>
> BTW, this is a counter-part of the patch I sent to the Kbuild sub-system
> (https://patchwork.kernel.org/patch/9592747/)
> because I want the U-Boot build system with Linux as much as possible.
> Let's see if I will get possible opinions in the Kbuild review.

I sent a similar patch for Linux, but
the response was negative in the Kbuild subsystem.

So, this patch will be our own, if applied.


I think the necessity for this patch comes down to
how long we expect for fixing DT files.


The only difficulty is in our basic rule:
Send patches to Linux first, then import DT to U-Boot.

Without this, fixing the warnings would be pretty easy...
Tom Rini March 7, 2017, 6:10 p.m. UTC | #4
On Wed, Mar 08, 2017 at 02:09:46AM +0900, Masahiro Yamada wrote:
> 2017-02-27 15:24 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:

> > DTC 1.4.2 or later checks DT unit-address without reg property and

> > vice-versa, and generates lots of warnings.  Fixing DT files will

> > take for a while.  Until then, let's turn off the check unless

> > building with W=*.

> >

> > Introduce a new helper dtc-option to check if the option is supported

> > in order to suppress warnings on older versions.

> >

> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> > ---

> >

> > This is one possible answer to Tom's:

> > https://www.mail-archive.com/u-boot@lists.denx.de/msg240328.html

> >

> > He fixed the problem on travis-ci by commit a0f3e3d, but it is still

> > annoying during the regular development.

> > Perhaps we may want to hide the warnings (at least hidden in Linux

> > Kernel by default).

> > Or, is it better to keep it noisy

> > to motivate people to fix their DT files?

> > I am not quite sure...

> >

> > Now I am sending this as RFC patch in case people may want to start discussion.

> >

> > BTW, this is a counter-part of the patch I sent to the Kbuild sub-system

> > (https://patchwork.kernel.org/patch/9592747/)

> > because I want the U-Boot build system with Linux as much as possible.

> > Let's see if I will get possible opinions in the Kbuild review.

> 

> I sent a similar patch for Linux, but

> the response was negative in the Kbuild subsystem.

> 

> So, this patch will be our own, if applied.


I think we should be proactive here and take this patch.  Newer dtc will
be seen more widely I believe as various overlay related changes go in
upstream.  Do you have anything you would change in a v2 non-RFC?

> I think the necessity for this patch comes down to

> how long we expect for fixing DT files.

> 

> 

> The only difficulty is in our basic rule:

> Send patches to Linux first, then import DT to U-Boot.

> 

> Without this, fixing the warnings would be pretty easy...


I guess I would suggest that for the DT files we really own, we should
update them.  Otherwise, just make the warnings happen in the kernel and
correct them and upstream them there.  Thanks!

-- 
Tom
Masahiro Yamada March 8, 2017, 2:20 a.m. UTC | #5
Hi Tom,


2017-03-08 3:10 GMT+09:00 Tom Rini <trini@konsulko.com>:
> On Wed, Mar 08, 2017 at 02:09:46AM +0900, Masahiro Yamada wrote:
>> 2017-02-27 15:24 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
>> > DTC 1.4.2 or later checks DT unit-address without reg property and
>> > vice-versa, and generates lots of warnings.  Fixing DT files will
>> > take for a while.  Until then, let's turn off the check unless
>> > building with W=*.
>> >
>> > Introduce a new helper dtc-option to check if the option is supported
>> > in order to suppress warnings on older versions.
>> >
>> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> > ---
>> >
>> > This is one possible answer to Tom's:
>> > https://www.mail-archive.com/u-boot@lists.denx.de/msg240328.html
>> >
>> > He fixed the problem on travis-ci by commit a0f3e3d, but it is still
>> > annoying during the regular development.
>> > Perhaps we may want to hide the warnings (at least hidden in Linux
>> > Kernel by default).
>> > Or, is it better to keep it noisy
>> > to motivate people to fix their DT files?
>> > I am not quite sure...
>> >
>> > Now I am sending this as RFC patch in case people may want to start discussion.
>> >
>> > BTW, this is a counter-part of the patch I sent to the Kbuild sub-system
>> > (https://patchwork.kernel.org/patch/9592747/)
>> > because I want the U-Boot build system with Linux as much as possible.
>> > Let's see if I will get possible opinions in the Kbuild review.
>>
>> I sent a similar patch for Linux, but
>> the response was negative in the Kbuild subsystem.
>>
>> So, this patch will be our own, if applied.
>
> I think we should be proactive here and take this patch.  Newer dtc will
> be seen more widely I believe as various overlay related changes go in
> upstream.  Do you have anything you would change in a v2 non-RFC?

No more update.
If you like this patch, please feel free to take it.
Peter Robinson March 8, 2017, 7:56 a.m. UTC | #6
On Tue, Mar 7, 2017 at 6:10 PM, Tom Rini <trini@konsulko.com> wrote:
> On Wed, Mar 08, 2017 at 02:09:46AM +0900, Masahiro Yamada wrote:
>> 2017-02-27 15:24 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
>> > DTC 1.4.2 or later checks DT unit-address without reg property and
>> > vice-versa, and generates lots of warnings.  Fixing DT files will
>> > take for a while.  Until then, let's turn off the check unless
>> > building with W=*.
>> >
>> > Introduce a new helper dtc-option to check if the option is supported
>> > in order to suppress warnings on older versions.
>> >
>> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> > ---
>> >
>> > This is one possible answer to Tom's:
>> > https://www.mail-archive.com/u-boot@lists.denx.de/msg240328.html
>> >
>> > He fixed the problem on travis-ci by commit a0f3e3d, but it is still
>> > annoying during the regular development.
>> > Perhaps we may want to hide the warnings (at least hidden in Linux
>> > Kernel by default).
>> > Or, is it better to keep it noisy
>> > to motivate people to fix their DT files?
>> > I am not quite sure...
>> >
>> > Now I am sending this as RFC patch in case people may want to start discussion.
>> >
>> > BTW, this is a counter-part of the patch I sent to the Kbuild sub-system
>> > (https://patchwork.kernel.org/patch/9592747/)
>> > because I want the U-Boot build system with Linux as much as possible.
>> > Let's see if I will get possible opinions in the Kbuild review.
>>
>> I sent a similar patch for Linux, but
>> the response was negative in the Kbuild subsystem.
>>
>> So, this patch will be our own, if applied.
>
> I think we should be proactive here and take this patch.  Newer dtc will
> be seen more widely I believe as various overlay related changes go in
> upstream.  Do you have anything you would change in a v2 non-RFC?

For reference they updated the kernel included dtc to upstream commit
0931cea3ba20 in 4.11 for overlay support [1] so I similarly updated
the Fedora dtc to the same revision for consistency so I think this is
a pragmatic approach.

Peter

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6f05afcbb031722ec1eff77dde188ff2edf8940e
Tom Rini March 10, 2017, 3:10 p.m. UTC | #7
On Mon, Feb 27, 2017 at 03:24:45PM +0900, Masahiro Yamada wrote:

> DTC 1.4.2 or later checks DT unit-address without reg property and

> vice-versa, and generates lots of warnings.  Fixing DT files will

> take for a while.  Until then, let's turn off the check unless

> building with W=*.

> 

> Introduce a new helper dtc-option to check if the option is supported

> in order to suppress warnings on older versions.

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> Reviewed-by: Simon Glass <sjg@chromium.org>

> Tested-by: Bin Meng <bmeng.cn@gmail.com>


Applied to u-boot/master, thanks!

-- 
Tom
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 38b42da..b258de8 100644
--- a/Makefile
+++ b/Makefile
@@ -371,7 +371,7 @@  export ARCH CPU BOARD VENDOR SOC CPUDIR BOARDDIR
 export CONFIG_SHELL HOSTCC HOSTCFLAGS HOSTLDFLAGS CROSS_COMPILE AS LD CC
 export CPP AR NM LDR STRIP OBJCOPY OBJDUMP
 export MAKE AWK PERL PYTHON
-export HOSTCXX HOSTCXXFLAGS DTC CHECK CHECKFLAGS
+export HOSTCXX HOSTCXXFLAGS CHECK CHECKFLAGS DTC DTC_FLAGS
 
 export KBUILD_CPPFLAGS NOSTDINC_FLAGS UBOOTINCLUDE OBJCOPYFLAGS LDFLAGS
 export KBUILD_CFLAGS KBUILD_AFLAGS
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 30e6e31..1b62aed 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -172,6 +172,11 @@  ld-version = $(shell $(LD) --version | $(srctree)/scripts/ld-version.sh)
 # Usage:  $(call ld-ifversion, -ge, 22252, y)
 ld-ifversion = $(shell [ $(ld-version) $(1) $(2) ] && echo $(3) || echo $(4))
 
+# dtc-option
+# Usage:  DTC_FLAGS += $(call dtc-option,-Wno-unit_address_vs_reg)
+dtc-option = $(call try-run,\
+	echo '/dts-v1/; / {};' | $(DTC) $(1),$(1),$(2))
+
 ######
 
 ###
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 6547e57..7b2cffc 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -57,4 +57,10 @@  ifeq ("$(strip $(warning))","")
 endif
 
 KBUILD_CFLAGS += $(warning)
+
+else
+
+# Disable noisy checks by default
+DTC_FLAGS += $(call dtc-option,-Wno-unit_address_vs_reg)
+
 endif