Re: [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties
From: Arınç ÜNAL
Date: Wed Jun 29 2022 - 08:23:55 EST
On 25.06.2022 23:25, Krzysztof Kozlowski wrote:
On 25/06/2022 22:15, Paul Cercueil wrote:
Hi Krzysztof,
Le sam., juin 25 2022 at 21:58:08 +0200, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> a écrit :
On 24/06/2022 20:40, Paul Cercueil wrote:
Hi Krzysztof,
Le ven., juin 24 2022 at 19:07:39 +0200, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> a écrit :
gpio-keys children do not use unit addresses.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
---
See:
https://lore.kernel.org/all/20220616005224.18391-1-krzysztof.kozlowski@xxxxxxxxxx/
---
arch/mips/boot/dts/img/pistachio_marduk.dts | 4 +--
arch/mips/boot/dts/ingenic/gcw0.dts | 31
+++++++++----------
arch/mips/boot/dts/ingenic/rs90.dts | 18 +++++------
arch/mips/boot/dts/pic32/pic32mzda_sk.dts | 9 ++----
.../boot/dts/qca/ar9132_tl_wr1043nd_v1.dts | 6 ++--
arch/mips/boot/dts/qca/ar9331_dpt_module.dts | 4 +--
.../mips/boot/dts/qca/ar9331_dragino_ms14.dts | 6 ++--
arch/mips/boot/dts/qca/ar9331_omega.dts | 4 +--
.../qca/ar9331_openembed_som9331_board.dts | 4 +--
arch/mips/boot/dts/qca/ar9331_tl_mr3020.dts | 8 ++---
10 files changed, 37 insertions(+), 57 deletions(-)
diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts
b/arch/mips/boot/dts/img/pistachio_marduk.dts
index a8708783f04b..a8da2f992b1a 100644
--- a/arch/mips/boot/dts/img/pistachio_marduk.dts
+++ b/arch/mips/boot/dts/img/pistachio_marduk.dts
@@ -59,12 +59,12 @@ led-1 {
keys {
compatible = "gpio-keys";
- button@1 {
+ button-1 {
label = "Button 1";
linux,code = <0x101>; /* BTN_1 */
gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
};
- button@2 {
+ button-2 {
label = "Button 2";
linux,code = <0x102>; /* BTN_2 */
gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
diff --git a/arch/mips/boot/dts/ingenic/gcw0.dts
b/arch/mips/boot/dts/ingenic/gcw0.dts
index 4abb0318416c..5d33f26fd28c 100644
--- a/arch/mips/boot/dts/ingenic/gcw0.dts
+++ b/arch/mips/boot/dts/ingenic/gcw0.dts
@@ -130,89 +130,86 @@ backlight: backlight {
gpio-keys {
compatible = "gpio-keys";
- #address-cells = <1>;
- #size-cells = <0>;
Are you sure you can remove these?
Yes, from DT spec point of view, DT bindings and Linux implementation.
However this particular change was not tested, except building.
Looking at paragraph 2.3.5 of the DT spec, I would think they have
to
stay (although with #address-cells = <0>).
The paragraph 2.3.5 says nothing about regular properties (which can
be
also child nodes). It says about children of a bus, right? It's not
related here, it's not a bus.
I quote:
"A DTSpec-compliant boot program shall supply #address-cells and
#size-cells on all nodes that have children."
And paragraph 2.2.3 says:
"A unit address may be omitted if the full path to the node is unambiguous."
You have address/size cells for nodes with children having unit
addresses. If they don't unit addresses, you don't add address/size
cells (with some exceptions).
The paragraph 2.3.5 mentions "child device nodes" and these properties
are not devices, although I agree that DT spec here is actually confusing.
The gpio-keys node has children nodes, therefore it should have
#address-cells and #size-cells, there's no room for interpretation here.
Second, why exactly this one gpio-keys node is different than all
other
gpio-keys everywhere and than bindings? Why this one has to be
incompatible/wrong according to bindings (which do not allow
address-cells and nodes with unit addresses)?
Nothing is different. I'm just stating that your proposed fix is
invalid if we want to enforce compliance with the DT spec.
In such case, we rather enforce the compliance with the bindings.
Best regards,
Krzysztof
I recall them to be unnecessary as well. I have a patch of mine applied
identical to this:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8c9f00d4b05134164e462f27b21c8295255ffa64
Also, I don't see any warnings with this patch applied:
$ ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- make clean dtbs -j$(nproc)
SYNC include/config/auto.conf.cmd
HOSTCC scripts/basic/fixdep
HOSTCC scripts/kconfig/conf.o
HOSTCC scripts/kconfig/confdata.o
HOSTCC scripts/kconfig/expr.o
LEX scripts/kconfig/lexer.lex.c
YACC scripts/kconfig/parser.tab.[ch]
HOSTCC scripts/kconfig/menu.o
HOSTCC scripts/kconfig/preprocess.o
HOSTCC scripts/kconfig/symbol.o
HOSTCC scripts/kconfig/util.o
HOSTCC scripts/kconfig/lexer.lex.o
HOSTCC scripts/kconfig/parser.tab.o
HOSTLD scripts/kconfig/conf
HOSTCC scripts/dtc/dtc.o
HOSTCC scripts/dtc/flattree.o
HOSTCC scripts/dtc/fstree.o
HOSTCC scripts/dtc/data.o
HOSTCC scripts/dtc/livetree.o
HOSTCC scripts/dtc/treesource.o
HOSTCC scripts/dtc/srcpos.o
HOSTCC scripts/dtc/checks.o
HOSTCC scripts/dtc/util.o
LEX scripts/dtc/dtc-lexer.lex.c
YACC scripts/dtc/dtc-parser.tab.[ch]
HOSTCC scripts/dtc/libfdt/fdt.o
HOSTCC scripts/dtc/libfdt/fdt_ro.o
HOSTCC scripts/dtc/libfdt/fdt_wip.o
HOSTCC scripts/dtc/libfdt/fdt_sw.o
HOSTCC scripts/dtc/libfdt/fdt_rw.o
HOSTCC scripts/dtc/libfdt/fdt_strerror.o
HOSTCC scripts/dtc/libfdt/fdt_empty_tree.o
HOSTCC scripts/dtc/libfdt/fdt_addresses.o
HOSTCC scripts/dtc/libfdt/fdt_overlay.o
HOSTCC scripts/dtc/fdtoverlay.o
HOSTCC scripts/dtc/dtc-lexer.lex.o
HOSTCC scripts/dtc/dtc-parser.tab.o
HOSTLD scripts/dtc/fdtoverlay
HOSTLD scripts/dtc/dtc
DTC arch/mips/boot/dts/ingenic/gcw0.dtb
Have my acked-by.
Acked-by: Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
Arınç