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

From: Ed W
Date: Thu Oct 22 2020 - 11:10:36 EST


Hi


>> This is followed up by the patch I really want to try and get in, which is to add support for APU5
>> and APU6. Particularly APU5 is quite interesting to me and significantly different to previous
>> boards in that it has a lot more mpcie slots that can be used for LTE modules or wifi cards.
> Okay, why not just treating them differently ?
>
> Personally, I don't have an apu5, so can't tell much of them yet. But if
> they're so different, I can live with those having different names /
> keycode, assuming all this stuff is already supported by the lowest BIOS
> version they've ever been shipped with (remember: requirement of BIOS
> upgrade in the field is also not an option). Can you please check
> whether that's the case ?
>
> In that case we don't need any per bios-version special magic, just
> another entry in the DMI match table and separate board structs.


You are reaching the wrong conclusion I feel?

a) your proposal doesn't address that ACPI and the upstream board configure all this stuff already.
We are creating duplicate devices right now. Patch 1 removes the duplicate device. I don't see that
your proposal addresses the duplicate devices?

b) there is no reason for naming to be different between APU2-4 and 5. Lets standardise. I use the
boards somewhat interchangeably

c) If you check my patch, there is already a DMI match for APU5, so this can do whatever you want it
to, however, since it's green fields, I've set it up to match the way APU2-4 is working (in my patch)

d) Your driver already broke my userspace a year back when you changed these names. I've just
supported all/both versions of the LED naming conventions since then (it isn't hard). I do believe
we are overthinking this? It's really trivial to incorporate?



> For the mpcie reset lines we really should consider using the reset
> controller framework or add it to pci core (so pci_bus drivers also
> expose it to sysfs, in order to let userland trigger it). Tricky part
> is letting the board driver attach the reset functionality to the
> generic pci_bus driver. (grmpf, it wasn't ugly acpi, but dt instead,
> this would be pretty trivial via dt-overlay :o).


Knock yourself out. However, make sure that you read Pascal's email to you where he states that
these lines aren't actually wired up on many of the board revisions?


>> This
>> creates the realisation that the reset and sim-swap lines are always wired to the LTE slots, not to
>> the mpcie slots (although often they overlap in functionality), so naming is corrected here.
> Sorry, but i'm confused. What's the actual difference between mpcie and
> LTE slots ? What is an LTE slot anyways ? Are you talking about the USB
> lanes on the M2 slot ?


The APU2-4 have

- 3x mpcie *shaped* slots

- On some boards there are two of these wired with the pcie signals

- On some boards there are two or three of these wired with USB signals

- On some boards there are mSATA signals to one of these slots

- On some boards there are either 1x or 2x SIM cards wired to some of the slots


What is common is that the reset lines (and rfkill) go to the same slots as the SIM lines.

So my suggestion is to name these lines based on "purpose", ie modem1 and modem2, which is 100%
consistent, rather than slot number, since that varies based on which board you pickup off the pile


The schematics are all publicly available. Although I think you may have to ask Pascal for the
APU5/6 ones. The rest are on the website


I'm confused about what you mean by M2 slot? There are no m.2 slots on the board? only mpcie, some
with USB signals, some with SIM?


> According to the pinouts i've found, the reset line is dedicated to
> PCIE, but perhaps we should treat it as "card reset" - IOW the whole
> card resets, no matter which channels (sata, pcie, usb, ...) it is
> actually using.


Please read Pascal's email to you. He disconnected these lines on many board revisions.


>> I don't know if it's useful, but I uploaded a couple of scripts for beeping and flashing the leds.
>> Here I just used globs to handle the different naming on the different boards (since I need to
>> handle the older Alix boards as well). Enrico, is this useful to you?
>>
>>     https://github.com/nippynetworks/gpio-utils
> The swap_sim script illustrates a good point, that I resisted to think
> about: sim swapping needs extra, baseband specific logic, to make it
> actually work :(


We just need the GPIOs exposing in some way. Nothing else. Everything else is done in userspace


> So, I'm still unsure whether this stuff should go into a separate
> portmux subsystem or an extended rfkill (where the basebands have to
> be put into) ... both options still a lot of work to do.


Lets please focus on things we can achieve in the near future in this thread


>> As an aside, these boards are super easy to flash as they support flashrom.
> Easy in the lab. But not in the field - if *anything* goes wrong,
> technician needs to fly out - several k$ plus days of downtime,
> until the technician reaches the place.


Not disagreeing, but it seems about as reliable as doing a software upgrade. If you do those then
you are likely safe doing this



>> The generic bios is quite slow to startup and I would like to prepare
>> a customised version with shorter timeouts. Happy to work with
>> you on something separately if this is interesting?
> Actually, I already planned an actual factory setup for these boxes,
> which includes the whole process, hw testing, bios deployment,
> OS/application deployment, etc, etc. Unfortunately, this is lots of
> work to do (has to be someting that arbitrary field technicians,
> who aren't sw engineers, can actually do on their own).


I've already got one

Hint: use netboot for simplest setup


> Tried to get some sponsoring from pcengines, but they don't even seems
> to be interested in an arranging simple things like LED names.
> Damn, I'm really unhappy with these folks - we could have prevented much
> of this whole mess if they would talked to me (they know that I'm the
> apu-board kernel maintainer) :(


I'm hiring. Contact me offlist if you want to talk further?



> Let's try summarize the current state of knowledge:
>
> * apu2..4:
> * BIOS support highly depends on BIOS version, we need to
> support older versions.
> * Field depends on on driver's naming, keycodes, ...


Realistically that's only partially true:

- older kernels and older bios has "old names"

- kernels from a year back with your driver have a mix of "new" or "new + old" names depending on
whether the bios is newer than 4.10.

- After the patch I proposed: newer bios and newer kernel use ACPI, older bios match your driver,
people upgrading from kernels more than about a year old don't notice since they move from "old"
names to "acpi names" which are identical.



> * simsw:
> * similar to mpcie-reset (still experimental, ...) ...
> * seems to be really bound to M2 ports


Not experimental. Works reliably. Just don't toggle them during bootup (as your driver was doing in
the past - current driver doesn't do this anymore - problem resolved)

The GPIO lines are wired to a SIM swap chip which does all the work. The LTE cards don't know or see
what is happening


Regards

Ed W