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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <EXU1ER.FH53VZXY9EYP3@crapouillou.net>
Date:   Sat, 25 Jun 2022 21:15:14 +0100
From:   Paul Cercueil <paul@...pouillou.net>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Rahul Bedarkar <rahulbedarkar89@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Arınç ÜNAL <arinc.unal@...nc9.com>,
        Sergio Paracuellos <sergio.paracuellos@...il.com>,
        linux-mips@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 1/2] MIPS: dts: correct gpio-keys names and properties

Hi Krzysztof,

Le sam., juin 25 2022 at 21:58:08 +0200, Krzysztof Kozlowski 
<krzysztof.kozlowski@...aro.org> 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@...aro.org> a écrit :
>>>  gpio-keys children do not use unit addresses.
>>> 
>>>  Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>>> 
>>>  ---
>>> 
>>>  See:
>>>  
>>> https://lore.kernel.org/all/20220616005224.18391-1-krzysztof.kozlowski@linaro.org/
>>>  ---
>>>   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."

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.

Cheers,
-Paul
>