On Wed, Nov 29, 2023 at 7:25 AM Rob Herring <robh@xxxxxxxxxx> wrote: > > On Tue, Nov 28, 2023 at 6:03 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote: > > > > On Mon, Nov 27, 2023 at 11:03 PM Rob Herring <robh@xxxxxxxxxx> wrote: > > > > > > On Thu, Nov 23, 2023 at 1:39 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote: > > > > > > > > On Thu, Nov 23, 2023 at 7:12 AM Rob Herring <robh@xxxxxxxxxx> wrote: > > > > > > > > > > This series adds support to set the dtc extra warning level on a per > > > > > arch or per platform (directory really) basis. > > > > > > > > > > The first version of this was just a simple per directory override for > > > > > Samsung platforms, but Conor asked to be able to do this for all of > > > > > riscv. > > > > > > > > > > For merging, either I can take the whole thing or the riscv and samsung > > > > > patches can go via their normal trees. The added variable will have no > > > > > effect until merged with patch 2. > > > > > > > > > > v1: > > > > > - https://lore.kernel.org/all/20231116211739.3228239-1-robh@xxxxxxxxxx/ > > > > > > > > > > Signed-off-by: Rob Herring <robh@xxxxxxxxxx> > > > > > --- > > > > > > > > > > > > There were some attempts in the past to enable W=1 in particular subsystems, > > > > so here is a similar comment. > > > > > > > > Adding a new warning flag to W=1 is always safe without doing any compile test. > > > > > > > > With this series, it would not be true any more because a new warning in W=1 > > > > would potentially break riscv/samsung platforms. > > > > > > The difference here is the people potentially adding warnings are also > > > the ones ensuring no warnings. > > > > > > > Linus requires a clean build (i.e. zero warning) when W= option is not given. > > > > > > Linus doesn't build any of this AFAICT. We are not always warning free > > > for W=0 with dtbs. > > > > > > > > Does it mean, you can enable all warnings by default? > > No, Linus might not care, but others (me) do. The whole point of not > allowing warnings is the same. Get to zero warnings so any new > warnings stand out. We now have some subset of platforms which are > warning free and want warnings enabled by default to keep them that > way. How do you suggest we do that? You may not like it, but an alternative solution could be, hard-code extra warning flags. In my compile-tests, Samsung platform is not W=1 clean yet. I see -Wunit_address_vs_reg, -Wsimple_bus_reg, -Wunique_unit_address_if_enabled warnings. I do not see anything else, so you can add the following three flags to keep it warning-free. diff --git a/arch/arm/boot/dts/samsung/Makefile b/arch/arm/boot/dts/samsung/Makefile index 7becf36656b1..1e15784ec51f 100644 --- a/arch/arm/boot/dts/samsung/Makefile +++ b/arch/arm/boot/dts/samsung/Makefile @@ -1,4 +1,10 @@ # SPDX-License-Identifier: GPL-2.0 + +dtcflags := \ + -Wavoid_unnecessary_addr_size \ + -Walias_paths \ + -Wgraph_child_address + dtb-$(CONFIG_ARCH_EXYNOS3) += \ exynos3250-artik5-eval.dtb \ exynos3250-monk.dtb \ diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 1a965fe68e01..aa5a5fc39cec 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -362,6 +362,9 @@ DTC_FLAGS += -Wnode_name_chars_strict \ -Wunique_unit_address endif +# per-directory flags +DTC_FLAGS += $(dtcflags) +# per-file flags DTC_FLAGS += $(DTC_FLAGS_$(basetarget)) # Set -@ if the target is a base DTB that overlay is applied onto > > I understand your point on W=1 in general, but I think it just doesn't > apply in this case. In general, > someone may be testing a new compiler and there's some new warning to > enable, so they add it to W=1. They are working independently of any > subsystem (and Linus) and introducing new warnings would be a burden > to fix and a problem to leave. For DT, it is a bit different as adding > new warnings, updating dtc version, and selecting warnings to enable > are pretty much all done together. > Plus, schema warnings have pretty > much superseded dtc warnings. If we do add new warnings which can't be > fixed up front, then we could still only enable the warning for W=1 > from the command line. Something like this on top of this series: > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 53a74e53e0ca..41307c6e1fee 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -341,6 +341,10 @@ quiet_cmd_gzip = GZIP $@ > # --------------------------------------------------------------------------- > DTC ?= $(objtree)/scripts/dtc/dtc > > +ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),) > +DTC_FLAGS += -Wno-some_new_warning_we_need_off_globally > +endif > + Hmm. Tricky, but works. KBUILD_EXTRA_WARN_DTC=1 is weaker than KBUILD_EXTRA_WARN=1 > KBUILD_EXTRA_WARN_DTC += $(KBUILD_EXTRA_WARN) > > # Disable noisy checks by default > -- Best Regards Masahiro Yamada
