Re: [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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ç



[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux