Re: [PATCH] scripts/dtc: Update to upstream version 53bf130b1cdd

[Rob Herring]

On Tue, Mar 8, 2016 at 10:22 AM, Olof Johansson <olof@xxxxxxxxx> wrote:
> Hi,
>
> On Mon, Mar 7, 2016 at 11:37 PM, Rob Herring <robh@xxxxxxxxxx> wrote:
>> On Mon, Mar 7, 2016 at 5:10 PM, Olof Johansson <olof@xxxxxxxxx> wrote:
>>> Hi,
>>>
>>> On Fri, Mar 4, 2016 at 7:13 AM, Rob Herring <robh@xxxxxxxxxx> wrote:
>>>> Sync to upstream dtc commit 53bf130b1cdd ("libfdt: simplify
>>>> fdt_node_check_compatible()"). This adds the following commits from
>>>> upstream:
>>>>
>>>> 53bf130 libfdt: simplify fdt_node_check_compatible()
>>>> c9d9121 Warn on node name unit-address presence/absence mismatch
>>>> 2e53f9d Catch unsigned 32bit overflow when parsing flattened device tree offsets
>>>>
>>>> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
>>>> ---
>>>> As usual, this is just an automated copy of upstream dtc into the kernel
>>>> tree. The changeset is small enough that I have left it here.
>>>>
>>>> The main reason for this sync is to pick-up the new unit-address
>>>> warnings.
>>>
>>> This spews a crazy amount of warnings on a multi_v7_defconfig build.
>>
>> Shocking, huh? And I've got more checks in the works. :)
>>
>>> I'd prefer to see most of those warnings fixed _before_ we introduce
>>> it by default. Otherwise we just add a huge amount of noise that will
>>> hide any real valid warnings that are now brought up.
>>
>> How do you propose to do that? If it is not enabled, then no one will
>> see them nor care. I don't intend to fix everyone's stuff myself.
>
> Right, but pushing a change on everyone that makes the build
> near-unusable as a tool to see if you're introducing a new error is
> also not a good way to do it. However:
>
>> We could hide the check behind COMPILE_TEST perhaps.
>
> Putting it behind an option sounds like a suitable approach. Would it
> be possible to plumb in under "make C=1" / "make C=2" instead? That's
> closer in meaning to what you're doing here than COMPILE_TEST, which
> is more about turning on drivers that might not make sense on your
> platform to get compile coverage.

C=1/2 is only designed to run on C files, so it would be a bit of work
to extend it. However, "W=1" seems like a better match and easy to
add:

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index ad50d58..417d8c2 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -271,6 +271,10 @@ cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -n
-f -9 > $@) || \
 # ---------------------------------------------------------------------------
 DTC ?= $(objtree)/scripts/dtc/dtc

+ifeq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),)
+DTC_FLAGS += -Wno-unit_address_vs_reg
+endif
+
 # Generate an assembly file to wrap the output of the device tree compiler
 quiet_cmd_dt_S_dtb= DTB     $@
 cmd_dt_S_dtb=                                          \


> This would make people aware of the tool, give them an easy way to run
> it to do the cleanups. Nag maintainers when they post new code without
> having paid attention to the old sources/cleanup, and with some amount
> of time most of it might be fixed.

I added the ability to do "make DTC=my/dtc", so there already is a way
for people to run it if they want to. Of course, they'd have to fetch
and build dtc compared to the above. Still, with either option it is
dependent on people running something they don't likely run normally.

Rob