2022-11-25 19:59:14

by Samuel Holland

Subject: [PATCH 1/2] ARM: dts: sunxi: Fix GPIO LED node names

These board devicetrees fail to validate because the gpio-leds schema
requires its child nodes to have "led" in the node name.

Signed-off-by: Samuel Holland <[email protected]>
---

arch/arm/boot/dts/sun5i-gr8-chip-pro.dts | 2 +-
arch/arm/boot/dts/sun5i-r8-chip.dts | 2 +-
arch/arm/boot/dts/sun6i-a31s-sina31s.dts | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts b/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
index a32cde3e32eb..3222f1490716 100644
--- a/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
+++ b/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
@@ -70,7 +70,7 @@ chosen {
leds {
compatible = "gpio-leds";

- status {
+ led-status {
label = "chip-pro:white:status";
gpios = <&axp_gpio 2 GPIO_ACTIVE_HIGH>;
default-state = "on";
diff --git a/arch/arm/boot/dts/sun5i-r8-chip.dts b/arch/arm/boot/dts/sun5i-r8-chip.dts
index 4bf4943d4eb7..303191c926c2 100644
--- a/arch/arm/boot/dts/sun5i-r8-chip.dts
+++ b/arch/arm/boot/dts/sun5i-r8-chip.dts
@@ -70,7 +70,7 @@ chosen {
leds {
compatible = "gpio-leds";

- status {
+ led-status {
label = "chip:white:status";
gpios = <&axp_gpio 2 GPIO_ACTIVE_HIGH>;
default-state = "on";
diff --git a/arch/arm/boot/dts/sun6i-a31s-sina31s.dts b/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
index 0af48e143b66..b84822453381 100644
--- a/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
+++ b/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
@@ -67,7 +67,7 @@ hdmi_con_in: endpoint {
leds {
compatible = "gpio-leds";

- status {
+ led-status {
label = "sina31s:status:usr";
gpios = <&pio 7 13 GPIO_ACTIVE_HIGH>; /* PH13 */
};
--
2.37.4

2022-11-25 20:19:10

by Samuel Holland

Subject: [PATCH 2/2] ARM: dts: sun8i: nanopi-duo2: Fix regulator GPIO reference

The property named in the schema is 'enable-gpios', not 'enable-gpio'.
This makes no difference at runtime, because the regulator is marked as
always-on, but it breaks validation.

Fixes: 4701fc6e5dd9 ("ARM: dts: sun8i: add FriendlyARM NanoPi Duo2")
Signed-off-by: Samuel Holland <[email protected]>
---

arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dts b/arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dts
index 43641cb82398..343b02b97155 100644
--- a/arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dts
@@ -57,7 +57,7 @@ reg_vdd_cpux: vdd-cpux-regulator {
regulator-ramp-delay = <50>; /* 4ms */

enable-active-high;
- enable-gpio = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
+ enable-gpios = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
gpios-states = <0x1>;
states = <1100000 0>, <1300000 1>;
--
2.37.4

2022-11-25 22:05:01

by Andre Przywara

Subject: Re: [PATCH 2/2] ARM: dts: sun8i: nanopi-duo2: Fix regulator GPIO reference

On Fri, 25 Nov 2022 13:54:01 -0600
Samuel Holland <[email protected]> wrote:

> The property named in the schema is 'enable-gpios', not 'enable-gpio'.
> This makes no difference at runtime, because the regulator is marked as
> always-on, but it breaks validation.
>
> Fixes: 4701fc6e5dd9 ("ARM: dts: sun8i: add FriendlyARM NanoPi Duo2")
> Signed-off-by: Samuel Holland <[email protected]>

Reviewed-by: Andre Przywara <[email protected]>

Cheers,
Andre

> ---
>
> arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dts | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dts b/arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dts
> index 43641cb82398..343b02b97155 100644
> --- a/arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dts
> +++ b/arch/arm/boot/dts/sun8i-h3-nanopi-duo2.dts
> @@ -57,7 +57,7 @@ reg_vdd_cpux: vdd-cpux-regulator {
> regulator-ramp-delay = <50>; /* 4ms */
>
> enable-active-high;
> - enable-gpio = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
> + enable-gpios = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
> gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
> gpios-states = <0x1>;
> states = <1100000 0>, <1300000 1>;

2022-11-25 22:07:38

by Samuel Holland

Subject: Re: [PATCH 1/2] ARM: dts: sunxi: Fix GPIO LED node names

Hi Andre,

On 11/25/22 15:40, Andre Przywara wrote:
> On Fri, 25 Nov 2022 13:54:00 -0600
> Samuel Holland <[email protected]> wrote:
>
> Hi Samuel,
>
>> These board devicetrees fail to validate because the gpio-leds schema
>> requires its child nodes to have "led" in the node name.
>>
>> Signed-off-by: Samuel Holland <[email protected]>
>
> That looks alright, though the comment in the binding says that we
> should just have led-0, led-1 instead, so just (hex) numbers. The
> "status" name is also in the label, so we wouldn't lose information.

I am not a fan of giving the LEDs meaningless enumerators, but I can do
that if the maintainers insist.

> Actually, also "label" is deprecated, in favour of "color" and
> "function", shall this be fixed on the way? Or is there anything that
> breaks (older kernels) when removing the label property?

The label is exposed to userspace as the path in sysfs, so we cannot
change it. There is no way to construct that exact label using function
and color -- see led_compose_name().

Regards,
Samuel

>> ---
>>
>> arch/arm/boot/dts/sun5i-gr8-chip-pro.dts | 2 +-
>> arch/arm/boot/dts/sun5i-r8-chip.dts | 2 +-
>> arch/arm/boot/dts/sun6i-a31s-sina31s.dts | 2 +-
>> 3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts b/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
>> index a32cde3e32eb..3222f1490716 100644
>> --- a/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
>> +++ b/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
>> @@ -70,7 +70,7 @@ chosen {
>> leds {
>> compatible = "gpio-leds";
>>
>> - status {
>> + led-status {
>> label = "chip-pro:white:status";
>> gpios = <&axp_gpio 2 GPIO_ACTIVE_HIGH>;
>> default-state = "on";
>> diff --git a/arch/arm/boot/dts/sun5i-r8-chip.dts b/arch/arm/boot/dts/sun5i-r8-chip.dts
>> index 4bf4943d4eb7..303191c926c2 100644
>> --- a/arch/arm/boot/dts/sun5i-r8-chip.dts
>> +++ b/arch/arm/boot/dts/sun5i-r8-chip.dts
>> @@ -70,7 +70,7 @@ chosen {
>> leds {
>> compatible = "gpio-leds";
>>
>> - status {
>> + led-status {
>> label = "chip:white:status";
>> gpios = <&axp_gpio 2 GPIO_ACTIVE_HIGH>;
>> default-state = "on";
>> diff --git a/arch/arm/boot/dts/sun6i-a31s-sina31s.dts b/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
>> index 0af48e143b66..b84822453381 100644
>> --- a/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
>> +++ b/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
>> @@ -67,7 +67,7 @@ hdmi_con_in: endpoint {
>> leds {
>> compatible = "gpio-leds";
>>
>> - status {
>> + led-status {
>> label = "sina31s:status:usr";
>> gpios = <&pio 7 13 GPIO_ACTIVE_HIGH>; /* PH13 */
>> };
>

2022-11-25 22:13:22

by Andre Przywara

Subject: Re: [PATCH 1/2] ARM: dts: sunxi: Fix GPIO LED node names

On Fri, 25 Nov 2022 13:54:00 -0600
Samuel Holland <[email protected]> wrote:

Hi Samuel,

> These board devicetrees fail to validate because the gpio-leds schema
> requires its child nodes to have "led" in the node name.
>
> Signed-off-by: Samuel Holland <[email protected]>

That looks alright, though the comment in the binding says that we
should just have led-0, led-1 instead, so just (hex) numbers. The
"status" name is also in the label, so we wouldn't lose information.

Actually, also "label" is deprecated, in favour of "color" and
"function", shall this be fixed on the way? Or is there anything that
breaks (older kernels) when removing the label property?

Cheers,
Andre

> ---
>
> arch/arm/boot/dts/sun5i-gr8-chip-pro.dts | 2 +-
> arch/arm/boot/dts/sun5i-r8-chip.dts | 2 +-
> arch/arm/boot/dts/sun6i-a31s-sina31s.dts | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts b/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
> index a32cde3e32eb..3222f1490716 100644
> --- a/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
> +++ b/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
> @@ -70,7 +70,7 @@ chosen {
> leds {
> compatible = "gpio-leds";
>
> - status {
> + led-status {
> label = "chip-pro:white:status";
> gpios = <&axp_gpio 2 GPIO_ACTIVE_HIGH>;
> default-state = "on";
> diff --git a/arch/arm/boot/dts/sun5i-r8-chip.dts b/arch/arm/boot/dts/sun5i-r8-chip.dts
> index 4bf4943d4eb7..303191c926c2 100644
> --- a/arch/arm/boot/dts/sun5i-r8-chip.dts
> +++ b/arch/arm/boot/dts/sun5i-r8-chip.dts
> @@ -70,7 +70,7 @@ chosen {
> leds {
> compatible = "gpio-leds";
>
> - status {
> + led-status {
> label = "chip:white:status";
> gpios = <&axp_gpio 2 GPIO_ACTIVE_HIGH>;
> default-state = "on";
> diff --git a/arch/arm/boot/dts/sun6i-a31s-sina31s.dts b/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
> index 0af48e143b66..b84822453381 100644
> --- a/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
> +++ b/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
> @@ -67,7 +67,7 @@ hdmi_con_in: endpoint {
> leds {
> compatible = "gpio-leds";
>
> - status {
> + led-status {
> label = "sina31s:status:usr";
> gpios = <&pio 7 13 GPIO_ACTIVE_HIGH>; /* PH13 */
> };

2022-12-05 21:39:36

by Jernej Škrabec

Subject: Re: [PATCH 2/2] ARM: dts: sun8i: nanopi-duo2: Fix regulator GPIO reference

Dne petek, 25. november 2022 ob 20:54:01 CET je Samuel Holland napisal(a):
> The property named in the schema is 'enable-gpios', not 'enable-gpio'.
> This makes no difference at runtime, because the regulator is marked as
> always-on, but it breaks validation.
>
> Fixes: 4701fc6e5dd9 ("ARM: dts: sun8i: add FriendlyARM NanoPi Duo2")
> Signed-off-by: Samuel Holland <[email protected]>

Acked-by: Jernej Skrabec <[email protected]>

Best regards,
Jernej

2022-12-05 22:12:12

by Jernej Škrabec

Subject: Re: Re: [PATCH 1/2] ARM: dts: sunxi: Fix GPIO LED node names

Hi Samuel,

Dne petek, 25. november 2022 ob 22:50:07 CET je Samuel Holland napisal(a):
> Hi Andre,
>
> On 11/25/22 15:40, Andre Przywara wrote:
> > On Fri, 25 Nov 2022 13:54:00 -0600
> > Samuel Holland <[email protected]> wrote:
> >
> > Hi Samuel,
> >
> >> These board devicetrees fail to validate because the gpio-leds schema
> >> requires its child nodes to have "led" in the node name.
> >>
> >> Signed-off-by: Samuel Holland <[email protected]>
> >
> > That looks alright, though the comment in the binding says that we
> > should just have led-0, led-1 instead, so just (hex) numbers. The
> > "status" name is also in the label, so we wouldn't lose information.
>
> I am not a fan of giving the LEDs meaningless enumerators, but I can do
> that if the maintainers insist.

I'm not a fan of that either, but binding really wants enumerator. So let's
conform to that.

Best regards,
Jernej

>
> > Actually, also "label" is deprecated, in favour of "color" and
> > "function", shall this be fixed on the way? Or is there anything that
> > breaks (older kernels) when removing the label property?
>
> The label is exposed to userspace as the path in sysfs, so we cannot
> change it. There is no way to construct that exact label using function
> and color -- see led_compose_name().
>
> Regards,
> Samuel
>
> >> ---
> >>
> >> arch/arm/boot/dts/sun5i-gr8-chip-pro.dts | 2 +-
> >> arch/arm/boot/dts/sun5i-r8-chip.dts | 2 +-
> >> arch/arm/boot/dts/sun6i-a31s-sina31s.dts | 2 +-
> >> 3 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
> >> b/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts index
> >> a32cde3e32eb..3222f1490716 100644
> >> --- a/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
> >> +++ b/arch/arm/boot/dts/sun5i-gr8-chip-pro.dts
> >> @@ -70,7 +70,7 @@ chosen {
> >>
> >> leds {
> >>
> >> compatible = "gpio-leds";
> >>
> >> - status {
> >> + led-status {
> >>
> >> label = "chip-pro:white:status";
> >> gpios = <&axp_gpio 2 GPIO_ACTIVE_HIGH>;
> >> default-state = "on";
> >>
> >> diff --git a/arch/arm/boot/dts/sun5i-r8-chip.dts
> >> b/arch/arm/boot/dts/sun5i-r8-chip.dts index 4bf4943d4eb7..303191c926c2
> >> 100644
> >> --- a/arch/arm/boot/dts/sun5i-r8-chip.dts
> >> +++ b/arch/arm/boot/dts/sun5i-r8-chip.dts
> >> @@ -70,7 +70,7 @@ chosen {
> >>
> >> leds {
> >>
> >> compatible = "gpio-leds";
> >>
> >> - status {
> >> + led-status {
> >>
> >> label = "chip:white:status";
> >> gpios = <&axp_gpio 2 GPIO_ACTIVE_HIGH>;
> >> default-state = "on";
> >>
> >> diff --git a/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
> >> b/arch/arm/boot/dts/sun6i-a31s-sina31s.dts index
> >> 0af48e143b66..b84822453381 100644
> >> --- a/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
> >> +++ b/arch/arm/boot/dts/sun6i-a31s-sina31s.dts
> >> @@ -67,7 +67,7 @@ hdmi_con_in: endpoint {
> >>
> >> leds {
> >>
> >> compatible = "gpio-leds";
> >>
> >> - status {
> >> + led-status {
> >>
> >> label = "sina31s:status:usr";
> >> gpios = <&pio 7 13 GPIO_ACTIVE_HIGH>; /*
PH13 */
> >>
> >> };