Re: [PATCH 1/2] x86: Remove led/gpio setup from pcengines platform driver
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
Re: [PATCH 1/2] x86: Conditional init of pcengines leds/keys gpios
On 22/10/2020 10:22, Enrico Weigelt, metux IT consult wrote: > On 21.10.20 23:41, Ed Wildgoose wrote: > > Hi, > >> The pcengines bios/firmware includes ACPI tables (since 4.10.0.1) which >> will cause the kernel to automatically create led + gpio_key devices for >> the platform. This means that the platform setup now creates duplicates >> of all these led/key devices. >> >> Driver conditionally initialises leds/keys only for older bios. > still breaks existing userlands as device names change - LEDs as well as > keys, and keycodes also change. > > IOW: userland needs to be depending on specific BIOS versions. > > > --mtx As a compromise can you change your userland to cope with dynamic names? I see two simple ways: 1) udev rule to set name as you wish 2) I uploaded a little script which uses simple globs to just figure out the base name depending on the board (also handles different board types entirely as well) I realise you have some stuff running with this, I don't know your situation specifically, but this feels like a really, really, small change to work around? Can you elaborate a little on why your users will be updating kernels without updating user code? Is there really no way to stick a compatibility udev rule in for your situation? To recap though, the situation for many years was that the naming convention was board specific. There is then just a small window (less than a year I think?) where users saw the name change to be non board specific (ie your new module). I would hazard a guess that given the speed of mainstream releases, few end users actually saw this change yet, or would notice? Those who did already accommodated the name change, so I would hazard a guess they can cope with the revert? (or not even notice?) Look, just propose a solution, I don't really mind. To me this is SUCH a TRIVIAL rename that I've already incorporated support for both naming conventions into my apps. I just want to get APU5/6 support in, which is affecting some real boards I want to use in volume. I don't feel the current situation of duplicate devices should stay though. My opinion is that we punt "this breaks userland" to being the board vendors problem now. The board is configured largely through ACPI, so if the upstream changes the ACPI, then it's on them, not us. This seems to be the direction the kernel is heading, with ACPI and device trees being used to configure the boards, in preference to heavy platform drivers? Hans, can you arbitrate here please? Your previous suggestion was to implement a compatibility shim for older BIOS, that's done here. Happy to implement something else, just need a guide? Thanks all Ed W
Re: [PATCH 1/2] x86: Remove led/gpio setup from pcengines platform driver
On 14/10/2020 12:29, Hans de Goede wrote: > Hi, > > On 10/14/20 1:21 PM, Ed W wrote: >> On 14/10/2020 09:41, Hans de Goede wrote: >>> >>> 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 ? >> >> >> I'm cool with this. Enrico? >> >> I may have some time imminently to have a stab at a new patch. Obviously any >> help structuring this >> would be appreciated - it feels clumsy using the existing detection >> mechanism, I think whatever I >> come up with you should kick back and recommend a new board detection >> structure, but perhaps we can >> shortcut that step with a few comments up front? > > I'm afraid I do not have any wisdom to share here. I would use the DMI > bios-version > or bios-date strings for the detection, but I guess that is obvious. Hi Hans & Enrico OK, I've just sent a new patch which conditionally configures GPIOs for boards with older firmware's (older than 4.10.0). 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. 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. That said, I don't think the reset lines function on most iterations of boards, so possibly supporting those lines with GPIOs is redundant anyway... APU6 is also a special order and is essentially the same as an APU4, so I have added detection for this also. 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 As an aside, these boards are super easy to flash as they support flashrom. So I'm personally giving some thought to bundling an updater into our software build. 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? Hans, thanks if you can look this over. Regards Ed W
Re: [PATCH 1/2] x86: Remove led/gpio setup from pcengines platform driver
On 14/10/2020 09:41, Hans de Goede wrote: > > 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 ? I'm cool with this. Enrico? I may have some time imminently to have a stab at a new patch. Obviously any help structuring this would be appreciated - it feels clumsy using the existing detection mechanism, I think whatever I come up with you should kick back and recommend a new board detection structure, but perhaps we can shortcut that step with a few comments up front? Cheers Ed W
Re: [PATCH 1/2] x86: Remove led/gpio setup from pcengines platform driver
On 13/10/2020 09:48, Hans de Goede wrote: > On 10/12/20 9:39 PM, Enrico Weigelt, metux IT consult wrote: >> On 22.09.20 00:17, Ed W wrote: >>> Hi, I've been adding support for the PC Engines APU5 board, which is a >>> variant of the APU 2-4 >>> boards >>> with some nice features. The current platform driver for pcengines boards >>> has some redundant >>> features with regards to recent bios/firmware packages for the board as >>> they now set the ACPI >>> tables >>> to indicate GPIOs for keys and leds. >> >> NAK. Breaks existing userlands in the field (literally field), forcing >> users to fw upgrade is not an option (field roll would be realy expensive). > > Thank you Enrico, I was wondering the same (what about userspace breakage) > when I was looking at this patch. It is good to have confirmation that > userspace breakage is a real issue here. This isn't the whole story. The original naming was board specific. Then Enrico (not unreasonably - I actually prefer his naming) changed the naming to be non board specific. Then within 2 months PC Engines introduced ACPI based config using the old names. So if we are holding "userspace breakage" as the gold standard, then the original (also the current) names have actually been around longest and likely cause the least userspace breakage. Also, some other pieces of this module have already been removed (SIM Swap), so there is an existing precedent for "userspace breakage" and trimming down this platform driver. In big picture terms, changing the name of the LED device doesn't seem a huge concern to me... A udev rule can setup compatibility forwards/backwards quite trivially I think? Kind regards Ed W
Re: [PATCH 1/2] x86: Remove led/gpio setup from pcengines platform driver
On 12/10/2020 20:39, Enrico Weigelt, metux IT consult wrote: > On 22.09.20 00:17, Ed W wrote: >> Hi, I've been adding support for the PC Engines APU5 board, which is a >> variant of the APU 2-4 boards >> with some nice features. The current platform driver for pcengines boards >> has some redundant >> features with regards to recent bios/firmware packages for the board as they >> now set the ACPI tables >> to indicate GPIOs for keys and leds. > NAK. Breaks existing userlands in the field (literally field), forcing > users to fw upgrade is not an option (field roll would be realy expensive). But why are users "in the field" updating a kernel willy nilly without also updating the userland software that talks to it? Why is the kernel upgrade trivial, but the fw upgrade is not an option? Why not also update the app or setup a udev rule? I would understand if we were talking something fairly major, but it's the case of matching a filename that YOU changed from an old name to the current name and it's now changing back to the original name? >> So I've submitted a patch to eliminate this. It could be argued >> that it's useful to support older firmware versions, but there is also a >> 'leds-apu' driver which a) >> probably ought to be marked deprecated with a view to removing it and b) >> implements the leds even on >> antique firmware versions. > leds-apu is only for *OLD* apu1 - it does *not* work with v2/3/4, > completely different chipset. That's extremely disingenuous!! It USED to work for the APU2-4 except that YOU removed support for APU2-4 from that module!! >> In implementing the APU5 I changed some of the exported gpio names to make >> them more closely match >> functionality across all the boards. > For example APU2 vs APU4 both support >> 2x LTE options, but in >> different mpcie slots and this affects the numbering of options, but not the >> sense of them (so I >> renamed them based on the intention of the option). This is particularly >> true on APU5 which supports >> 3x LTE cards > Dont break existing userlands. But YOU already did that!! Look, the original situation was that: - up to July 2019 there was a kernel module that named the LEDs in the form apu2:green:1, etc. - In July 2019 you removed that and "broke all of userland by renaming the LEDs" (never break userland right?) - Then in Sept 2019, PCEngines released a new bios/firmware line which setup ACPI correctly to register these GPIOs with some default names along the lines of apu2:green:1 or similar. So now we are back to the original naming convention - This meant that from Sept 2019 your module created duplicate LEDs with a different set of names So the situationt boils down to: - LEDs have been named like "apu2:green:1" continuously, with a brief outage in Aug-Sep 2019. - You have introduced a new module which unnecessarily duplicates those LEDs for users of this board with a bios/firmware post Sep 2019. - Your new naming convention isn't the same as this historic naming convention Now don't get me wrong, I prefer your module naming, but you are very disingenuous to claim that I'm trying to break userspace when you already did it! Plus I see no future for the LED piece of your module given that it's done by ACPI in all modern bios? Do you really want a duplicate set of LEDs to exist forever in userspace? This doesn't seem to be workable? Lets be clear, the current situation is: - LED names change from "apu:green:1" to "apu2:green:1". - This can be trivially worked around with some symlinks and/or a udev rule - The historic name has been "apu2:green:1" in the original LED module and now in ACPI. I'm not wild about this naming convention, but it's been around longest. If one has to pick which userspace to break, then this seems to have precedence. - Your LED based SIM toggle HAS already gone. So you have another example of userspace being broken right there. (Seems that this rule isn't so concrete?). So you already need to (significantly?) adjust your userspace code - I'm not seeing how/why the LED change is such a blocker? >> Can I get some advice: It would be helpful if the kernel would export the >> GPIOs to user-space >> automatically since toggling SIM slots is fairly useful task in userspace. > This is planned to be moved to either an own subsystem or into rfkill > (which would have to be extended for such things). Can you send me a pointer to this planning? Is this something concrete or aspirational? I need something I can use right now for SIM swap. exporting GPIOs with known names seems no more evil than exporting LEDs with known names? Do
Re: [PATCH] gpio: gpio-amd-fch: Fix typo on define of AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
Hi Could I get a final opinion (or signoff) on this patch please? The significant typo is the reference to "59", when the GPIO is actually 55 According to the PCEngines schematic the names of two similar GPIOs are G59/DEVSLP1 G55/DEVSLP The original developer named the second GPIO with a trailing 0, which doesn't seem unreasonable, hence I just corrected the name to: AMD_FCH_GPIO_REG_GPIO55_DEVSLP0 However another acceptable name could be: AMD_FCH_GPIO_REG_GPIO55_DEVSLP If I could ask for some guidance and if necessary I will resubmit this patch? Enrico, do you have an opinion? However, perhaps it's already acceptable as is? Kind regards Ed W On 21/09/2020 09:40, Ed W wrote: > On 21/09/2020 08:55, Andy Shevchenko wrote: >> On Sun, Sep 20, 2020 at 11:33 PM Ed Wildgoose wrote: >>> Schematics show that the GPIO number is 55 (not 59). Trivial typo. >> Does it still DEVSLP0? Perhaps you need to drop that part as well. >> >> ... > > > In the PCEngines schematic it's labelled as "G55/DEVSLP" (no 0) > > (In contrast G59 is labelled "G59/DEVSLP1") > > What is the quorum opinion on name? > > Thanks > > Ed W > > >> >>> #define APU2_GPIO_REG_LED3 AMD_FCH_GPIO_REG_GPIO59_DEVSLP1 >>> #define APU2_GPIO_REG_MODESW AMD_FCH_GPIO_REG_GPIO32_GE1 >>> #define APU2_GPIO_REG_SIMSWAP AMD_FCH_GPIO_REG_GPIO33_GE2 >>> -#define APU2_GPIO_REG_MPCIE2 AMD_FCH_GPIO_REG_GPIO59_DEVSLP0 >>> +#define APU2_GPIO_REG_MPCIE2 AMD_FCH_GPIO_REG_GPIO55_DEVSLP0 >>> #define APU2_GPIO_REG_MPCIE3 AMD_FCH_GPIO_REG_GPIO51 >>> >>> /* Order in which the GPIO lines are defined in the register list */ >>> diff --git a/include/linux/platform_data/gpio/gpio-amd-fch.h >>> b/include/linux/platform_data/gpio/gpio-amd-fch.h >>> index 9e46678ed..255d51c9d 100644 >>> --- a/include/linux/platform_data/gpio/gpio-amd-fch.h >>> +++ b/include/linux/platform_data/gpio/gpio-amd-fch.h >>> @@ -19,7 +19,7 @@ >>> #define AMD_FCH_GPIO_REG_GPIO49 0x40 >>> #define AMD_FCH_GPIO_REG_GPIO50 0x41 >>> #define AMD_FCH_GPIO_REG_GPIO51 0x42 >>> -#define AMD_FCH_GPIO_REG_GPIO59_DEVSLP0 0x43 >>> +#define AMD_FCH_GPIO_REG_GPIO55_DEVSLP0 0x43 >>> #define AMD_FCH_GPIO_REG_GPIO57 0x44 >>> #define AMD_FCH_GPIO_REG_GPIO58 0x45 >>> #define AMD_FCH_GPIO_REG_GPIO59_DEVSLP1 0x46 >> >
Re: [PATCH 1/2] x86: Remove led/gpio setup from pcengines platform driver
Hi, I've been adding support for the PC Engines APU5 board, which is a variant of the APU 2-4 boards with some nice features. The current platform driver for pcengines boards has some redundant features with regards to recent bios/firmware packages for the board as they now set the ACPI tables to indicate GPIOs for keys and leds. So I've submitted a patch to eliminate this. It could be argued that it's useful to support older firmware versions, but there is also a 'leds-apu' driver which a) probably ought to be marked deprecated with a view to removing it and b) implements the leds even on antique firmware versions. In implementing the APU5 I changed some of the exported gpio names to make them more closely match functionality across all the boards. For example APU2 vs APU4 both support 2x LTE options, but in different mpcie slots and this affects the numbering of options, but not the sense of them (so I renamed them based on the intention of the option). This is particularly true on APU5 which supports 3x LTE cards Can I get some advice: It would be helpful if the kernel would export the GPIOs to user-space automatically since toggling SIM slots is fairly useful task in userspace. At least for me the gpio numbers seem to jump around depending on the order of module loading, so doing something involving /sys/class/gpio/export isn't obviously an easy process. Reviewing the fine documentation suggests that I need to use gpio_export() to achieve this, but I concede I'm really not clear how to implement this in the platform module as currently structured... Any tips please? Thanks Ed W
Re: [PATCH] gpio: gpio-amd-fch: Fix typo on define of AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
On 21/09/2020 08:55, Andy Shevchenko wrote: On Sun, Sep 20, 2020 at 11:33 PM Ed Wildgoose wrote: Schematics show that the GPIO number is 55 (not 59). Trivial typo. Does it still DEVSLP0? Perhaps you need to drop that part as well. ... In the PCEngines schematic it's labelled as "G55/DEVSLP" (no 0) (In contrast G59 is labelled "G59/DEVSLP1") What is the quorum opinion on name? Thanks Ed W #define APU2_GPIO_REG_LED3 AMD_FCH_GPIO_REG_GPIO59_DEVSLP1 #define APU2_GPIO_REG_MODESW AMD_FCH_GPIO_REG_GPIO32_GE1 #define APU2_GPIO_REG_SIMSWAP AMD_FCH_GPIO_REG_GPIO33_GE2 -#define APU2_GPIO_REG_MPCIE2 AMD_FCH_GPIO_REG_GPIO59_DEVSLP0 +#define APU2_GPIO_REG_MPCIE2 AMD_FCH_GPIO_REG_GPIO55_DEVSLP0 #define APU2_GPIO_REG_MPCIE3 AMD_FCH_GPIO_REG_GPIO51 /* Order in which the GPIO lines are defined in the register list */ diff --git a/include/linux/platform_data/gpio/gpio-amd-fch.h b/include/linux/platform_data/gpio/gpio-amd-fch.h index 9e46678ed..255d51c9d 100644 --- a/include/linux/platform_data/gpio/gpio-amd-fch.h +++ b/include/linux/platform_data/gpio/gpio-amd-fch.h @@ -19,7 +19,7 @@ #define AMD_FCH_GPIO_REG_GPIO490x40 #define AMD_FCH_GPIO_REG_GPIO500x41 #define AMD_FCH_GPIO_REG_GPIO510x42 -#define AMD_FCH_GPIO_REG_GPIO59_DEVSLP00x43 +#define AMD_FCH_GPIO_REG_GPIO55_DEVSLP00x43 #define AMD_FCH_GPIO_REG_GPIO570x44 #define AMD_FCH_GPIO_REG_GPIO580x45 #define AMD_FCH_GPIO_REG_GPIO59_DEVSLP10x46
Ethernet not functioning on Nvidia MCP51 in 2.6.21
Hi, I am renting a root server which uses an nvidia motherboard and currently has a functioning 2.6.16 kernel. Unfortunately taking the same .config file (make oldconfig) in 2.6.21.6 is apparently not spotting the ethernet controller correctly (for various reasons I need a newer kernel, eventually with some other patches also) As near as I can find it should be the forcedeth driver being used here. Comparing dmesg on the working kernel and the non-working kernel shows no obvious differences. The working kernel uses no special command line options. Can someone please help me to debug this further? Symptoms are that dmesg suggests that the driver is being asked to load, but ifconfig does not show any eth0 device? Relevant chunks of dmesg on the non working kernel are below, full dmesg and lspci and kernel .config are in the links at the end. Grateful for any help debugging this? How do I find out why the driver is finding my hardware, but no eth0 device is being created? Ed W ... loop: loaded (max 8 devices) Intel(R) PRO/1000 Network Driver - version 7.3.20-k2-NAPI Copyright (c) 1999-2006 Intel Corporation. e100: Intel(R) PRO/100 Network Driver, 3.5.17-k2-NAPI e100: Copyright(c) 1999-2006 Intel Corporation sk98lin: driver has been replaced by the skge driver and is scheduled for removal Marvell 88E1101: Registered new driver Marvell 88E: Registered new driver Marvell 88E1145: Registered new driver Davicom DM9161E: Registered new driver Davicom DM9131: Registered new driver Cicada Cis8204: Registered new driver Cicada Cis8201: Registered new driver LXT970: Registered new driver LXT971: Registered new driver QS6612: Registered new driver forcedeth.c: Reverse Engineered nForce ethernet driver. Version 0.60. ACPI: PCI Interrupt Link [LNKL] enabled at IRQ 23 ACPI: PCI Interrupt :00:14.0[A] -> Link [LNKL] -> GSI 23 (level, high) -> IRQ 16 PCI: Setting latency timer of device :00:14.0 to 64 forcedeth: using HIGHDMA eth0: forcedeth.c: subsystem: 01734:10c6 bound to :00:14.0 ... 00:14.0 Ethernet controller: nVidia Corporation MCP51 Ethernet Controller (rev a3) Subsystem: Fujitsu Siemens Computer GmbH Unknown device 10c6 Flags: bus master, 66MHz, fast devsel, latency 0, IRQ 16 Memory at f2202000 (32-bit, non-prefetchable) [size=4K] I/O ports at 8c38 [size=8] Capabilities: [44] Power Management version 2 http://www.wildgooses.com/downloads/config http://www.wildgooses.com/downloads/dmesg.txt - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
