[PATCH 1/2] board: sunxi: enable status LED early
Arnaud Ferraris
arnaud.ferraris at collabora.com
Tue Sep 7 13:41:11 CEST 2021
Hi André,
Thanks for your feedback!
Le 07/09/2021 à 01:46, Andre Przywara a écrit :
> On Mon, 6 Sep 2021 22:57:52 +0200
> Arnaud Ferraris <arnaud.ferraris at collabora.com> wrote:
>
> Hi Arnaud,
>
>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> index 1a46100e40..6e0bf5fbf9 100644
>> --- a/board/sunxi/board.c
>> +++ b/board/sunxi/board.c
>> @@ -46,6 +46,9 @@
>> #include <spl.h>
>> #include <sy8106a.h>
>> #include <asm/setup.h>
>> +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_SPL_DRIVERS_MISC)
>> +#include <status_led.h>
>
> status_led.h already guards for CONFIG_LED_STATUS, so you can just
> unconditionally include this here, no #ifdefs needed.
Understood, will do.
>
>> +#endif
>>
>> #if defined CONFIG_VIDEO_LCD_PANEL_I2C && !(defined CONFIG_SPL_BUILD)
>> /* So that we can use pin names in Kconfig and sunxi_name_to_gpio() */
>> @@ -672,6 +675,10 @@ void sunxi_board_init(void)
>> {
>> int power_failed = 0;
>>
>> +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_SPL_DRIVERS_MISC)
>
> Unfortunately status_led.h doesn't define a dummy prototype for this,
> so we need the #ifdef CONFIG_LED_STATUS. But I don't think you need
> CONFIG_SPL_DRIVERS_MISC here. If that should only trigger in the SPL, use:
>
> if (IS_ENABLED(CONFIG_SPL_BUILD))
> status_led_init();
>
Actually the status_led driver isn't compiled into SPL unless
CONFIG_SPL_DRIVERS_MISC is set, resulting in a link error if that's not
the case. We should therefore check for both CONFIG_LED_STATUS and
CONFIG_SPL_DRIVERS_MISC to prevent build errors both at compile time and
link time.
The alternative would be:
#ifdef CONFIG_LED_STATUS
if (IS_ENABLED(CONFIG_SPL_DRIVERS_MISC))
status_led_init();
#endif
Which is more or less the same, except that it relies on the compiler
optimizing out this function call if CONFIG_SPL_DRIVERS_MISC isn't
defined. That assumption feels less safe to me, but overall I'm fine
with both implementations.
Please also note the whole sunxi_board_init() function itself is already
inside a #ifdef CONFIG_SPL_BUILD block.
Cheers,
Arnaud
> Cheers,
> Andre
>
>> + status_led_init();
>> +#endif
>> +
>> #ifdef CONFIG_SY8106A_POWER
>> power_failed = sy8106a_set_vout1(CONFIG_SY8106A_VOUT1_VOLT);
>> #endif
More information about the U-Boot
mailing list