devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels
@ 2023-11-22 22:12 Rob Herring
  2023-11-22 22:12 ` [PATCH v2 1/4] kbuild: Move dtc graph_child_address warning to W=2 Rob Herring
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Rob Herring @ 2023-11-22 22:12 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Krzysztof Kozlowski, Alim Akhtar,
	Conor Dooley
  Cc: linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

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@kernel.org/

Signed-off-by: Rob Herring <robh@kernel.org>
---
Rob Herring (4):
      kbuild: Move dtc graph_child_address warning to W=2
      kbuild: Allow arch/platform override of dtc warning level
      riscv: dts: Always enable extra W=1 warnings
      arm/arm64: dts: samsung: Always enable extra W=1 warnings

 arch/arm/boot/dts/samsung/Makefile  |  3 +++
 arch/arm64/boot/dts/exynos/Makefile |  3 +++
 arch/riscv/boot/dts/Makefile        |  3 +++
 scripts/Makefile.lib                | 16 +++++++++-------
 4 files changed, 18 insertions(+), 7 deletions(-)
---
base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
change-id: 20231122-dtc-warnings-968ff83a86ed

Best regards,
-- 
Rob Herring <robh@kernel.org>


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

* [PATCH v2 1/4] kbuild: Move dtc graph_child_address warning to W=2
  2023-11-22 22:12 [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Rob Herring
@ 2023-11-22 22:12 ` Rob Herring
  2023-11-22 22:12 ` [PATCH v2 2/4] kbuild: Allow arch/platform override of dtc warning level Rob Herring
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2023-11-22 22:12 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Krzysztof Kozlowski, Alim Akhtar,
	Conor Dooley
  Cc: linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

There are cases where having a single child node with a unit-address is
valid. For example, other child nodes may be optional or added via a
DT overlay. This hasn't mattered until now, but as platforms become
free of W=1 warnings, we want to enable W=1 by default on a per
platform basis. So move the warning to W=2.

In the process, the logic can be reworked a bit to avoid specifying a
given warning in multiple places.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 scripts/Makefile.lib | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 68d0134bdbf9..a67f781ae8ee 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -340,8 +340,6 @@ quiet_cmd_gzip = GZIP    $@
 # DTC
 # ---------------------------------------------------------------------------
 DTC ?= $(objtree)/scripts/dtc/dtc
-DTC_FLAGS += -Wno-interrupt_provider \
-	-Wno-unique_unit_address
 
 # Disable noisy checks by default
 ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)
@@ -357,9 +355,11 @@ endif
 
 ifneq ($(findstring 2,$(KBUILD_EXTRA_WARN)),)
 DTC_FLAGS += -Wnode_name_chars_strict \
-	-Wproperty_name_chars_strict \
-	-Winterrupt_provider \
-	-Wunique_unit_address
+	-Wproperty_name_chars_strict
+else
+DTC_FLAGS += -Wno-interrupt_provider \
+	-Wno-unique_unit_address \
+	-Wno-graph_child_address
 endif
 
 DTC_FLAGS += $(DTC_FLAGS_$(basetarget))

-- 
2.42.0


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

* [PATCH v2 2/4] kbuild: Allow arch/platform override of dtc warning level
  2023-11-22 22:12 [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Rob Herring
  2023-11-22 22:12 ` [PATCH v2 1/4] kbuild: Move dtc graph_child_address warning to W=2 Rob Herring
@ 2023-11-22 22:12 ` Rob Herring
  2023-11-22 22:12 ` [PATCH v2 3/4] riscv: dts: Always enable extra W=1 warnings Rob Herring
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2023-11-22 22:12 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Krzysztof Kozlowski, Alim Akhtar,
	Conor Dooley
  Cc: linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

In order to allow enabling extra dtc warnings on a per arch or platform
basis, define a new variable, KBUILD_EXTRA_WARN_DTC, which contains the
warning level and can be set by an arch or platform independently of
KBUILD_EXTRA_WARN. The default value is KBUILD_EXTRA_WARN as before.

The platform setting and command line setting are merged. For example,
with a platform that defaults to W=1, one can run 'make W=2 dtbs' which
will enable both level 1 and 2 warnings.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 scripts/Makefile.lib | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index a67f781ae8ee..53a74e53e0ca 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -341,8 +341,10 @@ quiet_cmd_gzip = GZIP    $@
 # ---------------------------------------------------------------------------
 DTC ?= $(objtree)/scripts/dtc/dtc
 
+KBUILD_EXTRA_WARN_DTC += $(KBUILD_EXTRA_WARN)
+
 # Disable noisy checks by default
-ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN)),)
+ifeq ($(findstring 1,$(KBUILD_EXTRA_WARN_DTC)),)
 DTC_FLAGS += -Wno-unit_address_vs_reg \
 	-Wno-avoid_unnecessary_addr_size \
 	-Wno-alias_paths \
@@ -353,7 +355,7 @@ DTC_FLAGS += \
         -Wunique_unit_address_if_enabled
 endif
 
-ifneq ($(findstring 2,$(KBUILD_EXTRA_WARN)),)
+ifneq ($(findstring 2,$(KBUILD_EXTRA_WARN_DTC)),)
 DTC_FLAGS += -Wnode_name_chars_strict \
 	-Wproperty_name_chars_strict
 else

-- 
2.42.0


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

* [PATCH v2 3/4] riscv: dts: Always enable extra W=1 warnings
  2023-11-22 22:12 [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Rob Herring
  2023-11-22 22:12 ` [PATCH v2 1/4] kbuild: Move dtc graph_child_address warning to W=2 Rob Herring
  2023-11-22 22:12 ` [PATCH v2 2/4] kbuild: Allow arch/platform override of dtc warning level Rob Herring
@ 2023-11-22 22:12 ` Rob Herring
  2023-11-22 22:12 ` [PATCH v2 4/4] arm/arm64: dts: samsung: " Rob Herring
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2023-11-22 22:12 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Krzysztof Kozlowski, Alim Akhtar,
	Conor Dooley
  Cc: linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

As all RISC-V .dts files are W=1 clean, enable the W=1 warnings all the
time.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 arch/riscv/boot/dts/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/riscv/boot/dts/Makefile b/arch/riscv/boot/dts/Makefile
index 72030fd727af..a7b17e0ac8c8 100644
--- a/arch/riscv/boot/dts/Makefile
+++ b/arch/riscv/boot/dts/Makefile
@@ -1,4 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
+
+export KBUILD_EXTRA_WARN_DTC=1
+
 subdir-y += allwinner
 subdir-y += canaan
 subdir-y += microchip

-- 
2.42.0


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

* [PATCH v2 4/4] arm/arm64: dts: samsung: Always enable extra W=1 warnings
  2023-11-22 22:12 [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Rob Herring
                   ` (2 preceding siblings ...)
  2023-11-22 22:12 ` [PATCH v2 3/4] riscv: dts: Always enable extra W=1 warnings Rob Herring
@ 2023-11-22 22:12 ` Rob Herring
  2023-11-23  7:18   ` Krzysztof Kozlowski
  2023-11-23  7:38 ` [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Masahiro Yamada
  2023-11-23  9:47 ` Conor Dooley
  5 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2023-11-22 22:12 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Krzysztof Kozlowski, Alim Akhtar,
	Conor Dooley
  Cc: linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

Samsung platforms are clean of W=1 dtc warnings, so enable the warnings
by default. This way submitters don't have to remember to run a W=1
build of the .dts files and the grumpiness of the maintainers can be
reduced.

Signed-off-by: Rob Herring <robh@kernel.org>
---
v2:
 - Use new variable name
---
 arch/arm/boot/dts/samsung/Makefile  | 3 +++
 arch/arm64/boot/dts/exynos/Makefile | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/samsung/Makefile b/arch/arm/boot/dts/samsung/Makefile
index 7becf36656b1..fffc5c45d441 100644
--- a/arch/arm/boot/dts/samsung/Makefile
+++ b/arch/arm/boot/dts/samsung/Makefile
@@ -1,4 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
+
+KBUILD_EXTRA_WARN_DTC=1
+
 dtb-$(CONFIG_ARCH_EXYNOS3) += \
 	exynos3250-artik5-eval.dtb \
 	exynos3250-monk.dtb \
diff --git a/arch/arm64/boot/dts/exynos/Makefile b/arch/arm64/boot/dts/exynos/Makefile
index 6e4ba69268e5..9cb188ef470d 100644
--- a/arch/arm64/boot/dts/exynos/Makefile
+++ b/arch/arm64/boot/dts/exynos/Makefile
@@ -1,4 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
+
+KBUILD_EXTRA_WARN_DTC=1
+
 dtb-$(CONFIG_ARCH_EXYNOS) += \
 	exynos5433-tm2.dtb		\
 	exynos5433-tm2e.dtb		\

-- 
2.42.0


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

* Re: [PATCH v2 4/4] arm/arm64: dts: samsung: Always enable extra W=1 warnings
  2023-11-22 22:12 ` [PATCH v2 4/4] arm/arm64: dts: samsung: " Rob Herring
@ 2023-11-23  7:18   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-23  7:18 UTC (permalink / raw)
  To: Rob Herring, Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Alim Akhtar, Conor Dooley
  Cc: linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

On 22/11/2023 23:12, Rob Herring wrote:
> Samsung platforms are clean of W=1 dtc warnings, so enable the warnings
> by default. This way submitters don't have to remember to run a W=1
> build of the .dts files and the grumpiness of the maintainers can be
> reduced.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


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

* Re: [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels
  2023-11-22 22:12 [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Rob Herring
                   ` (3 preceding siblings ...)
  2023-11-22 22:12 ` [PATCH v2 4/4] arm/arm64: dts: samsung: " Rob Herring
@ 2023-11-23  7:38 ` Masahiro Yamada
  2023-11-23  9:43   ` Conor Dooley
  2023-11-27 14:03   ` Rob Herring
  2023-11-23  9:47 ` Conor Dooley
  5 siblings, 2 replies; 14+ messages in thread
From: Masahiro Yamada @ 2023-11-23  7:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Krzysztof Kozlowski, Alim Akhtar, Conor Dooley,
	linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

On Thu, Nov 23, 2023 at 7:12 AM Rob Herring <robh@kernel.org> 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@kernel.org/
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---


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.

Linus requires a clean build (i.e. zero warning) when W= option is not given.






> Rob Herring (4):
>       kbuild: Move dtc graph_child_address warning to W=2
>       kbuild: Allow arch/platform override of dtc warning level
>       riscv: dts: Always enable extra W=1 warnings
>       arm/arm64: dts: samsung: Always enable extra W=1 warnings
>
>  arch/arm/boot/dts/samsung/Makefile  |  3 +++
>  arch/arm64/boot/dts/exynos/Makefile |  3 +++
>  arch/riscv/boot/dts/Makefile        |  3 +++
>  scripts/Makefile.lib                | 16 +++++++++-------
>  4 files changed, 18 insertions(+), 7 deletions(-)
> ---
> base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
> change-id: 20231122-dtc-warnings-968ff83a86ed
>
> Best regards,
> --
> Rob Herring <robh@kernel.org>
>


-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels
  2023-11-23  7:38 ` [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Masahiro Yamada
@ 2023-11-23  9:43   ` Conor Dooley
  2023-11-23 12:00     ` Masahiro Yamada
  2023-11-27 14:03   ` Rob Herring
  1 sibling, 1 reply; 14+ messages in thread
From: Conor Dooley @ 2023-11-23  9:43 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Rob Herring, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Krzysztof Kozlowski, Alim Akhtar, Conor Dooley,
	linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

[-- Attachment #1: Type: text/plain, Size: 1727 bytes --]

Yo,

On Thu, Nov 23, 2023 at 04:38:37PM +0900, Masahiro Yamada wrote:
> On Thu, Nov 23, 2023 at 7:12 AM Rob Herring <robh@kernel.org> 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@kernel.org/
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> 
> 
> 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.

We carry a copy of the dtc in scripts/dtc, so I would expect that before
an upgrade is done that would introduce new warnings we can fix them in
the relevant platforms. Perhaps I am misunderstanding something there,
but I'm sure Rob can clear it up if I made a mistake.

> Linus requires a clean build (i.e. zero warning) when W= option is not given.

For RISC-V at least, there are currently no W=1 warnings while building
the dtbs (because I put effort into fixing them all) and I would like to
keep it that way, so that requirement is not a concern.

Cheers,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels
  2023-11-22 22:12 [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Rob Herring
                   ` (4 preceding siblings ...)
  2023-11-23  7:38 ` [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Masahiro Yamada
@ 2023-11-23  9:47 ` Conor Dooley
  5 siblings, 0 replies; 14+ messages in thread
From: Conor Dooley @ 2023-11-23  9:47 UTC (permalink / raw)
  To: Rob Herring
  Cc: Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Krzysztof Kozlowski, Alim Akhtar,
	Conor Dooley, linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On Wed, Nov 22, 2023 at 03:12:31PM -0700, Rob Herring 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.

IMO, you should take the lot.

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels
  2023-11-23  9:43   ` Conor Dooley
@ 2023-11-23 12:00     ` Masahiro Yamada
  0 siblings, 0 replies; 14+ messages in thread
From: Masahiro Yamada @ 2023-11-23 12:00 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Rob Herring, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Krzysztof Kozlowski, Alim Akhtar, Conor Dooley,
	linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

On Thu, Nov 23, 2023 at 6:44 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> Yo,
>
> On Thu, Nov 23, 2023 at 04:38:37PM +0900, Masahiro Yamada wrote:
> > On Thu, Nov 23, 2023 at 7:12 AM Rob Herring <robh@kernel.org> 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@kernel.org/
> > >
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > ---
> >
> >
> > 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.
>
> We carry a copy of the dtc in scripts/dtc, so I would expect that before
> an upgrade is done that would introduce new warnings we can fix them in
> the relevant platforms.


It might be easy to fix up only riscv and samsung platforms.

If more and more platforms add KBUILD_EXTRA_WARN=1 in their Makefiles,
it will become harder to sort them out before adding a new warning flag.


The sync of scripts/dtc will be delayed until then, or
you will end up with demoting them (i.e. delete KBUILD_EXTRA_WARN=1).



> Perhaps I am misunderstanding something there,
> but I'm sure Rob can clear it up if I made a mistake.
>
> > Linus requires a clean build (i.e. zero warning) when W= option is not given.
>
> For RISC-V at least, there are currently no W=1 warnings while building
> the dtbs (because I put effort into fixing them all) and I would like to
> keep it that way, so that requirement is not a concern.
>
> Cheers,
> Conor.
>


-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels
  2023-11-23  7:38 ` [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Masahiro Yamada
  2023-11-23  9:43   ` Conor Dooley
@ 2023-11-27 14:03   ` Rob Herring
  2023-11-28 12:02     ` Masahiro Yamada
  1 sibling, 1 reply; 14+ messages in thread
From: Rob Herring @ 2023-11-27 14:03 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Krzysztof Kozlowski, Alim Akhtar, Conor Dooley,
	linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

On Thu, Nov 23, 2023 at 1:39 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Thu, Nov 23, 2023 at 7:12 AM Rob Herring <robh@kernel.org> 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@kernel.org/
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
>
>
> 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.

Rob

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

* Re: [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels
  2023-11-27 14:03   ` Rob Herring
@ 2023-11-28 12:02     ` Masahiro Yamada
  2023-11-28 22:25       ` Rob Herring
  0 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2023-11-28 12:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Krzysztof Kozlowski, Alim Akhtar, Conor Dooley,
	linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

On Mon, Nov 27, 2023 at 11:03 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Nov 23, 2023 at 1:39 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Thu, Nov 23, 2023 at 7:12 AM Rob Herring <robh@kernel.org> 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@kernel.org/
> > >
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > ---
> >
> >
> > 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?






>
> Rob
>


-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels
  2023-11-28 12:02     ` Masahiro Yamada
@ 2023-11-28 22:25       ` Rob Herring
  2023-11-29 17:49         ` Masahiro Yamada
  0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2023-11-28 22:25 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Krzysztof Kozlowski, Alim Akhtar, Conor Dooley,
	linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

On Tue, Nov 28, 2023 at 6:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Mon, Nov 27, 2023 at 11:03 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Nov 23, 2023 at 1:39 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Thu, Nov 23, 2023 at 7:12 AM Rob Herring <robh@kernel.org> 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@kernel.org/
> > > >
> > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > ---
> > >
> > >
> > > 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?

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
+
 KBUILD_EXTRA_WARN_DTC += $(KBUILD_EXTRA_WARN)

 # Disable noisy checks by default

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

* Re: [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels
  2023-11-28 22:25       ` Rob Herring
@ 2023-11-29 17:49         ` Masahiro Yamada
  0 siblings, 0 replies; 14+ messages in thread
From: Masahiro Yamada @ 2023-11-29 17:49 UTC (permalink / raw)
  To: Rob Herring
  Cc: Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Krzysztof Kozlowski, Alim Akhtar, Conor Dooley,
	linux-kbuild, linux-kernel, devicetree, linux-riscv,
	linux-arm-kernel, linux-samsung-soc

On Wed, Nov 29, 2023 at 7:25 AM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Nov 28, 2023 at 6:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Mon, Nov 27, 2023 at 11:03 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Thu, Nov 23, 2023 at 1:39 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > >
> > > > On Thu, Nov 23, 2023 at 7:12 AM Rob Herring <robh@kernel.org> 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@kernel.org/
> > > > >
> > > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > > ---
> > > >
> > > >
> > > > 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

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

end of thread, other threads:[~2023-11-29 17:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-22 22:12 [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Rob Herring
2023-11-22 22:12 ` [PATCH v2 1/4] kbuild: Move dtc graph_child_address warning to W=2 Rob Herring
2023-11-22 22:12 ` [PATCH v2 2/4] kbuild: Allow arch/platform override of dtc warning level Rob Herring
2023-11-22 22:12 ` [PATCH v2 3/4] riscv: dts: Always enable extra W=1 warnings Rob Herring
2023-11-22 22:12 ` [PATCH v2 4/4] arm/arm64: dts: samsung: " Rob Herring
2023-11-23  7:18   ` Krzysztof Kozlowski
2023-11-23  7:38 ` [PATCH v2 0/4] kbuild: Per arch/platform dtc warning levels Masahiro Yamada
2023-11-23  9:43   ` Conor Dooley
2023-11-23 12:00     ` Masahiro Yamada
2023-11-27 14:03   ` Rob Herring
2023-11-28 12:02     ` Masahiro Yamada
2023-11-28 22:25       ` Rob Herring
2023-11-29 17:49         ` Masahiro Yamada
2023-11-23  9:47 ` Conor Dooley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).