Re: [PATCH 1/2] x86: Remove led/gpio setup from pcengines platform driver

[Hans de Goede]

Hi,

On 10/13/20 11:40 PM, Ed W wrote:

<snip>

> Hans, can I ask you to look again at the history of this please. Bearing in mind the speed kernel
> stuff takes to get to end users, we are talking about a very small window of userland changes here.
> I would vote for simplifying this module and trying to reduce some baggage. However, my main goal is
> to get support in for APU5. Second goal is to reduce the duplicate LED devices. Beyond that I'm not
> so fussed?

Honestly I would prefer for you and Enrico to come to some sort of
consensus here, since you both know this code a lot better then I do.

If you cannot come to a consensus then I guess I will have to make
a decision here, but I would really prefer not to have to
arbitrate here.

Also note that I did already take a peek at the backlog before
Enrico's email and I was already wondering about the userspace
breakage _myself_ before Enrico chimed in. I did not reply then
because I only took a quick peek and decided to deal with the
backlog later.

As for the history of this all. Just because the userspace API
was broken once and we got away with it (IOW no body complained I
guess), does not mean that we should do this again.

Generally speaking there is a very hard rule that once shipped we
never break the userspace API and if I don't enforce that rule
then Torvalds will and in the process get angry at me
(been there done that). So sorry, but breaking userspace is
really not an option.

Also you mention in the commit messages for this patch that the
code is being removed because a new BIOS now enumerates them
through the new device-tree embedded in ACPI tables mechanism,
correct ?

That means that if people stick with the old BIOS and get a new
kernel they will loose all access to the LED functionality that
seems quite bad?  Note that also as a general, but certainly
as a pdx86 rule we try very hard to not rely on people installing
BIOS updates because whole troves of users do not install BIOS updates
(I understand that these boards are all kinds of special, so this may
apply here even more (or less so)).

So I have a suggested compromise:

Keep the current LED/gpio setup code, but make executing it conditional
on the BIOS version and skip the LED/gpio setup when the new BIOS is
present to avoid having duplicate LED entries, etc. in that case.

I guess this would still break userspace because if I understand things
correctly the new ACPI based setup uses different LED names ? That
seems unfortunate, but I guess that from the kernel pov we can just
blame the BIOS for this, and since we definitely do not want duplicate
LED entries for the same LED, this seems the least bad choice.

Enrico, would that work for you ?

Regards,

Hans