leds: fix wrong dmi_match on PC Engines APU LEDs

PATCH In review Hans Ulli Kroll 2 participants 4 messages Mar 5, 2018 – Mar 20, 2018

Subsystems (2)

x86 platform drivers LED

lore

Share & tools

lore

Hans Ulli Kroll

author

2018-03-05 17:09 UTC #0 lore raw

PATCH 1/1 leds: fix wrong dmi_match on PC Engines APU LEDs

APU has compared to APU2 no DMI_BOARD_NAME.
Use DMI_PRODUCT_NAME instead.

Signed-off-by Hans Ulli Kroll <[email protected]>

---
 drivers/leds/leds-apu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/leds/leds-apu.c +1 -1

index 74820aab9497..5bbf5c31413e 100644
@@ -206,7 +206,7 @@ static int __init apu_led_probe(struct platform_device *pdev)
 
 	apu_led->pdev = pdev;
 
-	if (dmi_match(DMI_BOARD_NAME, "APU")) {
+	if (dmi_match(DMI_PRODUCT_NAME, "APU")) {
 		apu_led->profile = apu1_led_profile;
 		apu_led->platform = APU1_LED_PLATFORM;
 		apu_led->num_led_instances = ARRAY_SIZE(apu1_led_profile);

Signature · 1 line

2.16.2

Jacek Anaszewski

2018-03-10 21:13 UTC #1 lore raw

Hi Hans,

Thank you for the patch.

Quoted 2 lines · Hans Ulli Kroll

APU has compared to APU2 no DMI_BOARD_NAME.
Use DMI_PRODUCT_NAME instead.

Could we have the commit message more expressive?

Is it that now this driver doesn't work for APU board?

Quoted 11 lines

Signed-off-by Hans Ulli Kroll <[email protected]>
---
 drivers/leds/leds-apu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/leds/leds-apu.c +0 -0
index 74820aab9497..5bbf5c31413e 100644

drivers/leds/leds-apu.c static int __init apu_led_probe(struct platform_device *pdev) · line 212

Quoted 8 lines

drivers/leds/leds-apu.c +1 -1

@@ -206,7 +206,7 @@ static int __init apu_led_probe(struct platform_device *pdev)
 
 	apu_led->pdev = pdev;
 
-	if (dmi_match(DMI_BOARD_NAME, "APU")) {
+	if (dmi_match(DMI_PRODUCT_NAME, "APU")) {
 		apu_led->profile = apu1_led_profile;
 		apu_led->platform = APU1_LED_PLATFORM;
 		apu_led->num_led_instances = ARRAY_SIZE(apu1_led_profile);

If it fails here, then how it is possible that it succeeds in the apu_led_init() ?

Signature · 2 lines

Best regards,
Jacek Anaszewski

Hans Ulli Kroll

author

2018-03-18 14:24 UTC #2 lore raw

Hi Jacek

Quoted 11 lines · Jacek Anaszewski

Hi Hans,
Thank you for the patch.
Quoted 2 lines · Hans Ulli Kroll
APU has compared to APU2 no DMI_BOARD_NAME.
Use DMI_PRODUCT_NAME instead.
Could we have the commit message more expressive?
Is it that now this driver doesn't work for APU board?

Yes it doesn't work for my APU board.
I've already checked the updated driver in -next

As you can see here

# grep . /sys/class/dmi/id/*
/sys/class/dmi/id/bios_date:04/05/2014
/sys/class/dmi/id/bios_vendor:coreboot
/sys/class/dmi/id/bios_version:SageBios_PCEngines_APU-45
/sys/class/dmi/id/chassis_type:3
/sys/class/dmi/id/chassis_vendor:PC Engines
/sys/class/dmi/id/modalias:dmi:bvncoreboot:bvrSageBios_PCEngines_APU-45:bd04/05/2014:svnPCEngines:pnAPU:pvr1.0:cvnPCEngines:ct3:cvr:
/sys/class/dmi/id/product_family:None Provided
/sys/class/dmi/id/product_name:APU
/sys/class/dmi/id/product_serial:XXXXXXX
/sys/class/dmi/id/product_version:1.0
/sys/class/dmi/id/sys_vendor:PC Engines
/sys/class/dmi/id/uevent:MODALIAS=dmi:bvncoreboot:bvrSageBios_PCEngines_APU-45:bd04/05/2014:svnPCEngines:pnAPU:pvr1.0:cvnPCEngines:ct3:cvr:

there is no board name on APU.

here is the output from my APU2 board

# grep . /sys/class/dmi/id/*
/sys/class/dmi/id/bios_date:02/28/2017
/sys/class/dmi/id/bios_vendor:coreboot
/sys/class/dmi/id/bios_version:4.0.7
/sys/class/dmi/id/board_name:APU2
/sys/class/dmi/id/board_serial:XXXXXXX
/sys/class/dmi/id/board_vendor:PC Engines
/sys/class/dmi/id/board_version:1.0
/sys/class/dmi/id/chassis_type:3
/sys/class/dmi/id/chassis_vendor:PC Engines
/sys/class/dmi/id/modalias:dmi:bvncoreboot:bvr4.0.7:bd02/28/2017:svnPCEngines:pnAPU2:pvr1.0:rvnPCEngines:rnAPU2:rvr1.0:cvnPCEngines:ct3:cvr:
/sys/class/dmi/id/product_name:APU2
/sys/class/dmi/id/product_serial:XXXXXX
/sys/class/dmi/id/product_version:1.0
/sys/class/dmi/id/sys_vendor:PC Engines
/sys/class/dmi/id/uevent:MODALIAS=dmi:bvncoreboot:bvr4.0.7:bd02/28/2017:svnPCEngines:pnAPU2:pvr1.0:rvnPCEngines:rnAPU2:rvr1.0:cvnPCEngines:ct3:cvr:

As you can see here for APU2 the board_name is set.

Quoted 4 lines

If it fails here, then how it is possible that it succeeds in the apu_led_init() ?

in apu_led_init() is a check for product_name for both both boards. Ans this succeeds

Thus the fix here

drivers/leds/leds-apu.c static int __init apu_led_probe(struct platform_device *pdev) · line 209

Quoted 2 lines

drivers/leds/leds-apu.c +1 -1

-     if (dmi_match(DMI_BOARD_NAME, "APU")) {
+     if (dmi_match(DMI_PRODUCT_NAME, "APU")) {

I can rewrite the patch to catch both board_name and product_name for the APU board, and add a better commit log.

They are more recent bios updates for this board, but the are specified as "beta" so I'm afraid of an update. I have no hardware to recover a "damaged" SPI flash chip.

Greetings
Hans Ulli Kroll

Jacek Anaszewski

2018-03-20 19:40 UTC #3 lore raw

Hi Hans,

Quoted 54 lines · Hans Ulli Kroll

Hi Jacek

Quoted 11 lines · Jacek Anaszewski

Hi Hans,
Thank you for the patch.
Quoted 2 lines · Hans Ulli Kroll
APU has compared to APU2 no DMI_BOARD_NAME.
Use DMI_PRODUCT_NAME instead.
Could we have the commit message more expressive?
Is it that now this driver doesn't work for APU board?

Yes it doesn't work for my APU board.
I've already checked the updated driver in -next

As you can see here

# grep . /sys/class/dmi/id/*
/sys/class/dmi/id/bios_date:04/05/2014
/sys/class/dmi/id/bios_vendor:coreboot
/sys/class/dmi/id/bios_version:SageBios_PCEngines_APU-45
/sys/class/dmi/id/chassis_type:3
/sys/class/dmi/id/chassis_vendor:PC Engines
/sys/class/dmi/id/modalias:dmi:bvncoreboot:bvrSageBios_PCEngines_APU-45:bd04/05/2014:svnPCEngines:pnAPU:pvr1.0:cvnPCEngines:ct3:cvr:
/sys/class/dmi/id/product_family:None Provided
/sys/class/dmi/id/product_name:APU
/sys/class/dmi/id/product_serial:XXXXXXX
/sys/class/dmi/id/product_version:1.0
/sys/class/dmi/id/sys_vendor:PC Engines
/sys/class/dmi/id/uevent:MODALIAS=dmi:bvncoreboot:bvrSageBios_PCEngines_APU-45:bd04/05/2014:svnPCEngines:pnAPU:pvr1.0:cvnPCEngines:ct3:cvr:

there is no board name on APU.

here is the output from my APU2 board

# grep . /sys/class/dmi/id/*
/sys/class/dmi/id/bios_date:02/28/2017
/sys/class/dmi/id/bios_vendor:coreboot
/sys/class/dmi/id/bios_version:4.0.7
/sys/class/dmi/id/board_name:APU2
/sys/class/dmi/id/board_serial:XXXXXXX
/sys/class/dmi/id/board_vendor:PC Engines
/sys/class/dmi/id/board_version:1.0
/sys/class/dmi/id/chassis_type:3
/sys/class/dmi/id/chassis_vendor:PC Engines
/sys/class/dmi/id/modalias:dmi:bvncoreboot:bvr4.0.7:bd02/28/2017:svnPCEngines:pnAPU2:pvr1.0:rvnPCEngines:rnAPU2:rvr1.0:cvnPCEngines:ct3:cvr:
/sys/class/dmi/id/product_name:APU2
/sys/class/dmi/id/product_serial:XXXXXX
/sys/class/dmi/id/product_version:1.0
/sys/class/dmi/id/sys_vendor:PC Engines
/sys/class/dmi/id/uevent:MODALIAS=dmi:bvncoreboot:bvr4.0.7:bd02/28/2017:svnPCEngines:pnAPU2:pvr1.0:rvnPCEngines:rnAPU2:rvr1.0:cvnPCEngines:ct3:cvr:

Quoted 1 line

As you can see here for APU2 the board_name is set.

Thanks for this explanation. I modified the commit message accordingly and applied the patch to the for-next branch in the following form:

commit 92d7ec1d71e351f11ba503369eb78225510cfcc7

Author: Hans Ulli Kroll <[email protected]>
Date:   Mon Mar 5 18:09:10 2018 +0100

leds: Fix wrong dmi_match on PC Engines APU LEDs

BIOS on APU board doesn't expose board_name property, and thus we have to rely on the product_name instead.

Signed-off-by Hans Ulli Kroll <[email protected]>

Signed-off-by Jacek Anaszewski <[email protected]>

Please let me know if you have any comments.

Best regards,
Jacek Anaszewski

Quoted 18 lines

Quoted 4 lines
If it fails here, then how it is possible that it succeeds in the apu_led_init() ?
in apu_led_init() is a check for product_name for both both boards. Ans this succeeds
Thus the fix here
drivers/leds/leds-apu.c static int __init apu_led_probe(struct platform_device *pdev) · line 209
Quoted 2 lines
drivers/leds/leds-apu.c +1 -1
-     if (dmi_match(DMI_BOARD_NAME, "APU")) {
+     if (dmi_match(DMI_PRODUCT_NAME, "APU")) {
I can rewrite the patch to catch both board_name and product_name for the APU board, and add a better commit log.
They are more recent bios updates for this board, but the are specified as "beta" so I'm afraid of an update. I have no hardware to recover a "damaged" SPI flash chip.