2019-07-04 09:11:29

by Florian Eckert

[permalink] [raw]
Subject: [PATCH 2/3] platform/x86/pcengines-apuv2: add legacy leds gpio definitions

Extend the apu2_leds definition to make the leds exportable via the
legacy gpio subsystem. Without this change the leds are not visible
under "/sys/class/leds" and could not be configured.

Signed-off-by: Florian Eckert <[email protected]>
---
drivers/platform/x86/pcengines-apuv2.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/pcengines-apuv2.c b/drivers/platform/x86/pcengines-apuv2.c
index f6d8ed100cab..d50a50e9d34c 100644
--- a/drivers/platform/x86/pcengines-apuv2.c
+++ b/drivers/platform/x86/pcengines-apuv2.c
@@ -75,9 +75,24 @@ static const struct amd_fch_gpio_pdata board_apu2 = {
/* gpio leds device */

static const struct gpio_led apu2_leds[] = {
- { .name = "apu:green:1" },
- { .name = "apu:green:2" },
- { .name = "apu:green:3" }
+ {
+ .name = "apu:green:1",
+ .gpio = 505,
+ .default_trigger = "default-off",
+ .active_low = 1,
+ },
+ {
+ .name = "apu:green:2",
+ .gpio = 506,
+ .default_trigger = "default-off",
+ .active_low = 1,
+ },
+ {
+ .name = "apu:green:3",
+ .gpio = 507,
+ .default_trigger = "default-off",
+ .active_low = 1,
+ }
};

static const struct gpio_led_platform_data apu2_leds_pdata = {
--
2.11.0


Subject: Re: [PATCH 2/3] platform/x86/pcengines-apuv2: add legacy leds gpio definitions

On 04.07.19 11:02, Florian Eckert wrote:
> Extend the apu2_leds definition to make the leds exportable via the
> legacy gpio subsystem.

What for ? The gpios are bound to LED devices as that's exactly what
they are: LEDs.

> Without this change the leds are not visible
> under "/sys/class/leds" and could not be configured.

How so ? (I'm using exactly that all the day ...)


--mtx

--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2019-07-10 12:05:43

by Florian Eckert

[permalink] [raw]
Subject: Re: [PATCH 2/3] platform/x86/pcengines-apuv2: add legacy leds gpio definitions

On 2019-07-08 21:42, Enrico Weigelt, metux IT consult wrote:
> On 04.07.19 11:02, Florian Eckert wrote:
>> Extend the apu2_leds definition to make the leds exportable via the
>> legacy gpio subsystem.
>
> What for ? The gpios are bound to LED devices as that's exactly what
> they are: LEDs.

I have back ported your pcengines-apuv2 device and gpio-amd-fch GPIO
driver to the kernel version 4.19 on OpenWrt.
If I compile and load this without the change no LEDs are visible in
"/sys/class/leds"!

From my point of view the connection between the GPIO and the LEDs
subsystem is missing.
How should the LED subsystem know which GPIO to use?
If I add the change to the pcengines-apuv2 device then the LEDs will be
visilbe under "/sys/class/leds"
and could be used, by OpenWrt userland.

Mybe I miss something.

Subject: Re: [PATCH 2/3] platform/x86/pcengines-apuv2: add legacy leds gpio definitions

On 10.07.19 14:03, Florian Eckert wrote:

> I have back ported your pcengines-apuv2 device and gpio-amd-fch GPIO
> driver to the kernel version 4.19 on OpenWrt.
> If I compile and load this without the change no LEDs are visible in
> "/sys/class/leds"!

Maybe this old kernel just ignored all entries w/o gpio ID. Note that
these IDs are deprecated for quite a while and shouldn't be used in new
code anymore.

> From my point of view the connection between the GPIO and the LEDs
> subsystem is missing.
> How should the LED subsystem know which GPIO to use?

See gpiod_lookup_table.

> If I add the change to the pcengines-apuv2 device then the LEDs will be
> visilbe under "/sys/class/leds"
> and could be used, by OpenWrt userland.
>
> Mybe I miss something.

Your patch is only valid for your backport onto this old kernel, not for
recent mainline.


--mtx

--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287