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

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

[Andreas Eberlein]

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.

Signed-off-by: Andreas Eberlein <foodeas@aeberlein.de>
---
Changes in v2:
  - Add missing brace
---
 drivers/leds/leds-apu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-apu.c b/drivers/leds/leds-apu.c
index 7fd557ace..c409b80c2 100644
--- 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;
 	}
-- 
2.25.1

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

[Enrico Weigelt, metux IT consult]

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

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

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 790 bytes --]

> >   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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

[Petr Štetiar]

Enrico Weigelt, metux IT consult <lkml@metux.net> [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

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

[Zbyněk Kocur]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

Petr Štetiar <ynezz@true.cz>


Hi Peter,

Thanks for adding to the discussion. I tested the proposed modification on APU1 with different versions of bios. 
The LED subsystem now behaves the same as the APU2 and higher. If it needs more tests on various boards
 from PCengines, I'm available.


Tested-by: Zbyněk Kocur <zbynek.kocur@fel.cvut.cz>


Zbyněk

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4233 bytes --]

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

[Zbyněk Kocur]

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

Petr Štetiar <ynezz@true.cz>


Hi Peter,

Thanks for adding to the discussion. I tested the proposed modification on APU1 with different versions of bios. 
The LED subsystem now behaves the same as the APU2 and higher. If it needs more tests on various boards
from PCengines, I'm available.


Tested-by: Zbyněk Kocur <zbynek.kocur@fel.cvut.cz>


Zbyněk


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4233 bytes --]

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

[Enrico Weigelt, metux IT consult]

On 19.02.21 21:51, Zbyněk Kocur wrote:

Hi Zbyněk,

> Thanks for adding to the discussion. I tested the proposed modification on APU1 with different versions of bios.
> The LED subsystem now behaves the same as the APU2 and higher. If it needs more tests on various boards
>   from PCengines, I'm available.

Do you also happen to have different apu2/3/4 boards (various hw revs
and bios versions) for testing ? I've still got some open issues, eg.
regarding pcie reset lines, etc.


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

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

[Zbyněk Kocur]

Hello,

I checked my stock and I have following models:

- apu2e2
- apu2e4
- apu3d4
- apu4c4

The bios in all models can be upgraded from version 4.0.6 to higher. The latest is 4.13.0.3.

Zbyněk
---
email: zbynek.kocur@fel.cvut.cz
mobile: +420 728 946 803
phone: +420 224 354 054
web: https://comtel.fel.cvut.cz
Department of Telecommunications Engineering
Faculty of Electrical Engineering

> On 23. 2. 2021, at 21:02, Enrico Weigelt, metux IT consult <info@metux.net> wrote:
> 
> On 19.02.21 21:51, Zbyněk Kocur wrote:
> 
> Hi Zbyněk,
> 
>> Thanks for adding to the discussion. I tested the proposed modification on APU1 with different versions of bios.
>> The LED subsystem now behaves the same as the APU2 and higher. If it needs more tests on various boards
>>  from PCengines, I'm available.
> 
> Do you also happen to have different apu2/3/4 boards (various hw revs
> and bios versions) for testing ? I've still got some open issues, eg.
> regarding pcie reset lines, etc.
> 
> 
> --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@metux.net -- +49-151-27565287

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

[Alexander Dahl]

[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]

Hello,

On Wed, Feb 17, 2021 at 12:02:47PM +0100, Enrico Weigelt, metux IT consult wrote:
> On 16.02.21 14:30, Andreas Eberlein wrote:
> > 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 ?

I have an apu1d4 on my desk, which is supposed to be used soon, but
available for testing at the moment.  I put the latest coreboot bios
version 4.13.0.3 on it yesterday.

> 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.

If you give me a hint, which tree or patchset should be tested, and
some hints what should be tested, I can try.

> 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.

Thanks for that work.  I have to admit someone from the fli4l linux
router distribution team also wrote LED and button drivers for the APU
boards, but never managed to upstream those. :-/

If someone is interested, those are spread in our Subversion
repository, but the apu drivers are here:

https://repo.nettworks.org/svn/fli4l/branches/4.0/trunk/src/packages/src/src/fli4l/hwsupp/pcengines-apu/

Personally, I'd rather have mainline drivers for all that boards.
Don't know if it still makes sense for the older wrap or alix boards,
though.  I also have those lying around. ;-)

Greets
Alex

-- 
/"\ ASCII RIBBON | »With the first link, the chain is forged. The first
\ / CAMPAIGN     | speech censured, the first thought forbidden, the
 X  AGAINST      | first freedom denied, chains us all irrevocably.«
/ \ HTML MAIL    | (Jean-Luc Picard, quoting Judge Aaron Satie)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Enrico Weigelt, metux IT consult]

On 06.03.21 17:51, Alexander Dahl wrote:

Hi,

> If you give me a hint, which tree or patchset should be tested, and
> some hints what should be tested, I can try.

haven't written anything for apu1 yet (as I dont have one), but I wrote
the drivers for apu2/3/4.

My idea (which I never actually started on) was writing a separate gpio
driver (not LED) for the old Soc in apu1 and add instantiation w/ 
leds-gpio, keys, etc, into the pcengines-apu2 driver.

> Thanks for that work.  I have to admit someone from the fli4l linux
> router distribution team also wrote LED and button drivers for the APU
> boards, but never managed to upstream those. :-/
> 
> If someone is interested, those are spread in our Subversion
> repository, but the apu drivers are here:
> 
> https://repo.nettworks.org/svn/fli4l/branches/4.0/trunk/src/packages/src/src/fli4l/hwsupp/pcengines-apu/

hmm, maybe I could pick up pieces for the FCH functionality that's
not supported yet (eg. wdt) ... not sure how much they differ between
different SoC versions.

> Personally, I'd rather have mainline drivers for all that boards.
> Don't know if it still makes sense for the older wrap or alix boards,
> though.  I also have those lying around. ;-)

Well, somebody has to make his hands dirty, write those drivers, bring
them to mainline, and maintain them.


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