Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
MIME-Version: 1.0
References: <20231122-dtc-warnings-v2-0-bd4087325392@kernel.org>
 <CAK7LNASVMjVg4dr=KdSDHwGww_47H78H7rMXA=wf+ncugesDSA@mail.gmail.com>
 <CAL_Jsq+N0GxwZ2YmspEzfiuGOw7M+DmYkyhLgaYtk+Ov2ycY_A@mail.gmail.com>
 <CAK7LNAT6-pBjUbB+Fcik27QWniK7BizvoUG+EiFvFtJ+MTdmJA@mail.gmail.com> <CAL_JsqJSFnVG6+CfcbgVFGo3EyiSTt-et0NSW2qWjei+zXURcg@mail.gmail.com>
In-Reply-To: <CAL_JsqJSFnVG6+CfcbgVFGo3EyiSTt-et0NSW2qWjei+zXURcg@mail.gmail.com>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Thu, 30 Nov 2023 02:49:21 +0900
Message-ID: <CAK7LNARrRwzrU0m0SVOfnJj87hw4kL2YpDK0xRCwuW6fYT9OGg@mail.gmail.com>
Subject: Re: [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels
To:     Rob Herring <robh@kernel.org>
Cc:     Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Nicolas Schier <nicolas@fjasle.eu>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Alim Akhtar <alim.akhtar@samsung.com>,
        Conor Dooley <conor@kernel.org>, linux-kbuild@vger.kernel.org,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        linux-riscv@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Nov 29, 2023 at 7:25=E2=80=AFAM Rob Herring <robh@kernel.org> wrote=
:
>
> On Tue, Nov 28, 2023 at 6:03=E2=80=AFAM Masahiro Yamada <masahiroy@kernel=
.org> wrote:
> >
> > On Mon, Nov 27, 2023 at 11:03=E2=80=AFPM Rob Herring <robh@kernel.org> =
wrote:
> > >
> > > On Thu, Nov 23, 2023 at 1:39=E2=80=AFAM Masahiro Yamada <masahiroy@ke=
rnel.org> wrote:
> > > >
> > > > On Thu, Nov 23, 2023 at 7:12=E2=80=AFAM Rob Herring <robh@kernel.or=
g> 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 overrid=
e 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 s=
amsung
> > > > > patches can go via their normal trees. The added variable will ha=
ve no
> > > > > effect until merged with patch 2.
> > > > >
> > > > > v1:
> > > > >  - https://lore.kernel.org/all/20231116211739.3228239-1-robh@kern=
el.org/
> > > > >
> > > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > > ---
> > > >
> > > >
> > > > There were some attempts in the past to enable W=3D1 in particular =
subsystems,
> > > > so here is a similar comment.
> > > >
> > > > Adding a new warning flag to W=3D1 is always safe without doing any=
 compile test.
> > > >
> > > > With this series, it would not be true any more because a new warni=
ng in W=3D1
> > > > would potentially break riscv/samsung platforms.
> > >
> > > The difference here is the people potentially adding warnings are als=
o
> > > the ones ensuring no warnings.
> > >
> > > > Linus requires a clean build (i.e. zero warning) when W=3D option i=
s not given.
> > >
> > > Linus doesn't build any of this AFAICT. We are not always warning fre=
e
> > > for W=3D0 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=3D1 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 :=3D \
+       -Wavoid_unnecessary_addr_size \
+       -Walias_paths \
+       -Wgraph_child_address
+
 dtb-$(CONFIG_ARCH_EXYNOS3) +=3D \
        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 +=3D -Wnode_name_chars_strict \
        -Wunique_unit_address
 endif

+# per-directory flags
+DTC_FLAGS +=3D $(dtcflags)
+# per-file flags
 DTC_FLAGS +=3D $(DTC_FLAGS_$(basetarget))

 # Set -@ if the target is a base DTB that overlay is applied onto


>
> I understand your point on W=3D1 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=3D1. 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=3D1
> 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 =3D GZIP    $@
>  # ----------------------------------------------------------------------=
-----
>  DTC ?=3D $(objtree)/scripts/dtc/dtc
>
> +ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)
> +DTC_FLAGS +=3D -Wno-some_new_warning_we_need_off_globally
> +endif
> +

Hmm. Tricky, but works.

KBUILD_EXTRA_WARN_DTC=3D1 is weaker than KBUILD_EXTRA_WARN=3D1


>  KBUILD_EXTRA_WARN_DTC +=3D $(KBUILD_EXTRA_WARN)
>
>  # Disable noisy checks by default
>
--
Best Regards
Masahiro Yamada