linux-kernel - Re: [PATCH v2] leds: apu: extend support for PC Engines APU1 with newer firmware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c7eebbb6-df0c-51df-7701-ecb8f6543466@metux.net>
Date:   Wed, 17 Feb 2021 12:02:47 +0100
From:   "Enrico Weigelt, metux IT consult" <lkml@...ux.net>
To:     Andreas Eberlein <foodeas@...erlein.de>,
        Pavel Machek <pavel@....cz>
Cc:     linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] leds: apu: extend support for PC Engines APU1 with
 newer firmware

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
info@...ux.net -- +49-151-27565287