Hi, On 2/17/23 14:50, Enrico Weigelt, metux IT consult wrote: > On 13.02.23 15:25, Hans de Goede wrote: > >> It would be good to know the ACPI names. Also what happens with the ACPI >> registered LED devices when the pcengines-apu driver loads, does it >> somehow unregister those ? > > It doesn't (hasn't any code for that). But the even more interesting > question is: does the acpi driver lock the IO space so my gpio driver, > and so the apu driver itself, can't initialize at all ? Or are we in a > situation where two different drivers meddle with the same chip ? Since it seems that people are using a patched version of the pcengines-apu driver with openwrt on newer models I would assume that the answer here is: "No the ACPI code does not lock the IO space" although it is unclear to me how the ACPI code exposes leds at all ? Later in the thread you write that they do some hacks to expose all io-space of the FCH as gpios and then presumably the embed devicetree-bits pointing to those new GPIOs into ACPI to make the leds-gpio.c code handle the LEDs ? > >> If the ACPI LEDs are not unregistered and keep working, then I guess there is no userspace >> API breakage when using pcengines-apu on newer APU models, "just" duplicate LED devices ? > > Supporting both LED name schemes on the newer boards is an interesting > thought. Do we have some way for aliasing LED names ? > > OTOH, I wonder whether we need the model specific LED naming at all. > In the old apuv1 driver, there used to be some (unsupported) LED-only > support for apuv2, which used model-specific naming. When adding full > apuv2/v3 support, I specifically chose not to do this, since I don't want userland having to care about the specific model version. And the > naming is a bit more clear on the actual meaning of these LEDs. > >>> - Additionally, we already broke this in the (distant) past because there was a previous APU driver >>> which used different names still... >> >> Just because we have gotten away with it once, does not mean we should do it again :) > > Back then the situation was different. Haven't even found anybody who's > was actually using this in the field. This ancient driver (actually made > for acpuv1, which is totally different HW) was only serving the three > front LEDs, nothing else, and blocked using the other GPIOs (eg. button) > Some people out there did weird hacks by directly writing registers from > userland (obviously w/o loading the ancient driver) - and even worse: > pcengines publically adviced to so. > >> For the new models I'm fine with whatever LED naming is preferred. > > NAK. The problem here is: userland now has to differenciate between > various models again. Applications suddenly need to be rewritten in > order to work with the next higher model, or it fails. That might be not > a problem for home users, but in the industrial field it is a huge > problem: you suddenly end up with two product configurations or need > extra SW complexity to cope with several models at runtime - and this > even grows with the next one. > > Exactly what I wanted to prevent once and for all. Note that making applications work OOTB on newer platforms is nice to have, but is not specifically a blocker from the upstream kernel review pov. Userspace needing to adjust to e.g. /dev/sda becoming /dev/nvme is not considerer userspace API breakage and this is more or less the same. Still I agree that preserving the userspace API across different board models is something which we want to do if possible. > Since the meaning of these LEDs doesn't change, there's just no need to > change their naming. I agree that if the LEDs have the same function as before the name should be preserved to not needlessly make life harder for userspace consumers of these LEDs. Regards, Hans
