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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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@linaro.org> a écrit :
>> gpio-keys children do not use unit addresses.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.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.

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)?


Best regards,
Krzysztof