On 16.02.21 14:30, Andreas Eberlein wrote:
Hi,
> The DMI_PRODUCT_NAME entry on current firmware of PC Engines APU1 changed
> from "APU" to "apu1"
>
> This modification adds the missing DMI data and thereby the LED support for
> the PC Engines APU1 with firmware versions >= 4.6.8.
Do you have a device for more intensive testing ?
In that case I'd like to suggest splitting the driver into gpio and
gpio-based LED (using leds-gpio) - just like already I did for apu2/3/4.
Maybe this even could also be moveed into the apu2 driver. This probably
just makes sense if there're more gpio-connected devices than just LED)
Personally, I don't have access to the old apu1 board (IIRC not even
produced anymore for several years), so I didn't dare to touch anything
here.
Note that apu1 vs. apu2/3/4 have completely different SOC with different
gpio logic - that was one of the reasons for writing a completely new
driver for apu2+ from scrath, rather than extending the old one.
> --- a/drivers/leds/leds-apu.c
> +++ b/drivers/leds/leds-apu.c
> @@ -83,6 +83,7 @@ static const struct apu_led_profile apu1_led_profile[] = {
> };
>
> static const struct dmi_system_id apu_led_dmi_table[] __initconst = {
> + /* PC Engines APU with factory bios "SageBios_PCEngines_APU-45" */
> {
> .ident = "apu",
> .matches = {
> @@ -90,6 +91,14 @@ static const struct dmi_system_id apu_led_dmi_table[] __initconst = {
> DMI_MATCH(DMI_PRODUCT_NAME, "APU")
> }
> },
> + /* PC Engines APU with "Mainline" bios >= 4.6.8 */
> + {
> + .ident = "apu",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "apu1")
> + }
> + },
> {}
> };
> MODULE_DEVICE_TABLE(dmi, apu_led_dmi_table);
> @@ -173,7 +182,7 @@ static int __init apu_led_init(void)
> int err;
>
> if (!(dmi_match(DMI_SYS_VENDOR, "PC Engines") &&
> - dmi_match(DMI_PRODUCT_NAME, "APU"))) {
> + (dmi_match(DMI_PRODUCT_NAME, "APU") || dmi_match(DMI_PRODUCT_NAME, "apu1")))) {
> pr_err("No PC Engines APUv1 board detected. For APUv2,3 support, enable CONFIG_PCENGINES_APU2\n");
> return -ENODEV;
> }
>
Looks good to me. But don't dare giving official ack, since I don't
have an apu1 board for testing.
Is Alan Mizrahi (original author) still here ?
--mtx
--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
> > MODULE_DEVICE_TABLE(dmi, apu_led_dmi_table);
> > @@ -173,7 +182,7 @@ static int __init apu_led_init(void)
> > int err;
> > if (!(dmi_match(DMI_SYS_VENDOR, "PC Engines") &&
> > - dmi_match(DMI_PRODUCT_NAME, "APU"))) {
> > + (dmi_match(DMI_PRODUCT_NAME, "APU") || dmi_match(DMI_PRODUCT_NAME, "apu1")))) {
> > pr_err("No PC Engines APUv1 board detected. For APUv2,3 support, enable CONFIG_PCENGINES_APU2\n");
> > return -ENODEV;
> > }
> >
>
> Looks good to me. But don't dare giving official ack, since I don't
> have an apu1 board for testing.
>
> Is Alan Mizrahi (original author) still here ?
People are usually not that careful with thier acks.
Applied, thanks.
BR,
Pavel
--
http://www.livejournal.com/~pavelmachek
Enrico Weigelt, metux IT consult <[email protected]> [2021-02-17 12:02:47]:
Hi,
(adding Zbynek to Cc: who seems to own a complete range of APU boards and has
done the runtime testing already)
> Looks good to me. But don't dare giving official ack, since I don't
> have an apu1 board for testing.
@Zbynek can you perhaps provide your `Tested-by: Your Name <your@email>` tag
as it seems, that you've tested[1] this extensively on several boards/versions
already? I think, that you deserve credits as well. Thanks!
1. https://github.com/openwrt/openwrt/pull/3135#issuecomment-748953745
Cheers,
Petr