linux4sam/at91bootstrap

Unselecting an LED causes compile errors

Closed this issue · 7 comments

For our SAMA5D27 project that is based on SAMA5D27SOM1EK I wanted to unselct the use of the blue LED as we only have 2 LEDs connected but that causes compile errors in driver/led.c

CC driver/led.c
driver/led.c: In function ‘at91_leds_init’:
driver/led.c:53:22: error: ‘LED_B_PIO’ undeclared (first use in this function); did you mean ‘LED_G_PIO’?
53 | pio_set_gpio_output(LED_B_PIO, CONFIG_LED_B_VALUE);
| ^~~~~~~~~
| LED_G_PIO
driver/led.c:53:22: note: each undeclared identifier is reported only once for each function it appears in
driver/led.c:53:33: error: ‘CONFIG_LED_B_VALUE’ undeclared (first use in this function); did you mean ‘CONFIG_LED_G_VALUE’?
53 | pio_set_gpio_output(LED_B_PIO, CONFIG_LED_B_VALUE);
| ^~~~~~~~~~~~~~~~~~
| CONFIG_LED_G_VALUE

I found out that #ifndef for the macros does not work as selecting "No LED on board" defines LED_x_NONE with 'y' which means the macros are always true.

I fixed in on my end by doing a different check. Patch file attached.
at91bootstrap3-v4.0.0-fix-skipping-unselected-leds.patch.txt

Hi,

You are correct, there is a bug. But I think your fix is not correct. Here is what I think it's better:

diff --git a/driver/led.c b/driver/led.c
index 59dffc23..767614ad 100644
--- a/driver/led.c
+++ b/driver/led.c
@@ -43,13 +43,13 @@
 
 void at91_leds_init(void)
 {
-#ifndef LED_R_NONE
+#ifndef CONFIG_LED_R_NONE
        pio_set_gpio_output(LED_R_PIO, CONFIG_LED_R_VALUE);
 #endif
-#ifndef LED_G_NONE
+#ifndef CONFIG_LED_G_NONE
        pio_set_gpio_output(LED_G_PIO, CONFIG_LED_G_VALUE);
 #endif
-#ifndef LED_B_NONE
+#ifndef CONFIG_LED_B_NONE
        pio_set_gpio_output(LED_B_PIO, CONFIG_LED_B_VALUE);
 #endif
 }

Does this solve your problem ?

If you wish I can add you to the commit with your sign-off , if you provide me your email/name, in the form :

Reported-by: First name last name <email@..>

checking for CONFIG_LED_x_NONE seems to have the same issues than LED_B_NONE. That is also defined when the LED is set to "No LED on board".

Checking for the existance of LED_x_PIO still feels right as those are only defined if any of the CONFIG_LED_x_ON_PIOn is selected.

You do make sense. I thought more about it, and if we are using PIO calls, makes sense to check for PIO config.
Do you wish to add your name to your patch ?

Yes, you can add my name for the patch.
Do I need to do something else?

Yes, you can add my name for the patch.
Do I need to do something else?

Then give me your tag :
Signed-off-by: Name <email>

I tried to create a pull-request but have trouble to get it working.

My tag is
Signed-off-by: Matthias Wieloch matthias.wieloch@few-bauer.de

Patched applied and pushed out. Thanks !