* [PATCH] PinePhone: enable LED on boot for improved visual feedback @ 2021-09-06 20:57 Arnaud Ferraris 2021-09-06 20:57 ` [PATCH 1/2] board: sunxi: enable status LED early Arnaud Ferraris 2021-09-06 20:57 ` [PATCH 2/2] pinephone_defconfig: add support for early-boot status LED Arnaud Ferraris 0 siblings, 2 replies; 7+ messages in thread From: Arnaud Ferraris @ 2021-09-06 20:57 UTC (permalink / raw) To: u-boot; +Cc: Jagan Teki, Andre Przywara, Samuel Holland, Maxime Ripard Hi, As the PinePhone doesn't provide any visual feedback when booting, these patches take advantage of the built-in RGB LED to indicate the device is indeed powered on. I've been carrying those downstream in Mobian for some time now, allowing users to see their phone is booting (otherwise it can be several seconds before the kernel is loaded and able to show a sign of life). Best regards, Arnaud ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] board: sunxi: enable status LED early 2021-09-06 20:57 [PATCH] PinePhone: enable LED on boot for improved visual feedback Arnaud Ferraris @ 2021-09-06 20:57 ` Arnaud Ferraris 2021-09-06 23:46 ` Andre Przywara 2021-09-06 20:57 ` [PATCH 2/2] pinephone_defconfig: add support for early-boot status LED Arnaud Ferraris 1 sibling, 1 reply; 7+ messages in thread From: Arnaud Ferraris @ 2021-09-06 20:57 UTC (permalink / raw) To: u-boot Cc: Jagan Teki, Andre Przywara, Samuel Holland, Maxime Ripard, Arnaud Ferraris For some systems, such as the PinePhone, there is no way for the end user to make sure the system is indeed booting before the boot script is executed, which takes several seconds. Therefore, it can be useful to provide early visual feedback as soon as possible. In order achieve this goal, this patch initializes the status LED (if configured) in the SPL. Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com> --- board/sunxi/board.c | 7 +++++++ 1 file changed, 7 insertions(+) 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> +#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) + status_led_init(); +#endif + #ifdef CONFIG_SY8106A_POWER power_failed = sy8106a_set_vout1(CONFIG_SY8106A_VOUT1_VOLT); #endif -- 2.33.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] board: sunxi: enable status LED early 2021-09-06 20:57 ` [PATCH 1/2] board: sunxi: enable status LED early Arnaud Ferraris @ 2021-09-06 23:46 ` Andre Przywara 2021-09-07 11:41 ` Arnaud Ferraris 0 siblings, 1 reply; 7+ messages in thread From: Andre Przywara @ 2021-09-06 23:46 UTC (permalink / raw) To: Arnaud Ferraris; +Cc: u-boot, Jagan Teki, Samuel Holland, Maxime Ripard On Mon, 6 Sep 2021 22:57:52 +0200 Arnaud Ferraris <arnaud.ferraris@collabora.com> wrote: Hi Arnaud, > For some systems, such as the PinePhone, there is no way for the end > user to make sure the system is indeed booting before the boot script is > executed, which takes several seconds. Therefore, it can be useful to > provide early visual feedback as soon as possible. > > In order achieve this goal, this patch initializes the status LED (if > configured) in the SPL. Thanks for sending this, it looks much better than the previous attempt on the list from earlier this year. > Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com> > --- > board/sunxi/board.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > 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. > +#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(); Cheers, Andre > + status_led_init(); > +#endif > + > #ifdef CONFIG_SY8106A_POWER > power_failed = sy8106a_set_vout1(CONFIG_SY8106A_VOUT1_VOLT); > #endif ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] board: sunxi: enable status LED early 2021-09-06 23:46 ` Andre Przywara @ 2021-09-07 11:41 ` Arnaud Ferraris 2021-09-07 22:42 ` Andre Przywara 0 siblings, 1 reply; 7+ messages in thread From: Arnaud Ferraris @ 2021-09-07 11:41 UTC (permalink / raw) To: Andre Przywara; +Cc: u-boot, Jagan Teki, Samuel Holland, Maxime Ripard 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@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] board: sunxi: enable status LED early 2021-09-07 11:41 ` Arnaud Ferraris @ 2021-09-07 22:42 ` Andre Przywara 0 siblings, 0 replies; 7+ messages in thread From: Andre Przywara @ 2021-09-07 22:42 UTC (permalink / raw) To: Arnaud Ferraris; +Cc: u-boot, Jagan Teki, Samuel Holland, Maxime Ripard On Tue, 7 Sep 2021 13:41:11 +0200 Arnaud Ferraris <arnaud.ferraris@collabora.com> wrote: Hi Arnaud, > 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@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 Right, the nasty bit here is that you could just have LED_STATUS for U-Boot proper, but there is no separate CONFIG_SPL_LED_STATUS, so we need both symbols, although it looks weird. > 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. Yes, but I still prefer to have as little #ifdef as possible, because ... > Please also note the whole sunxi_board_init() function itself is already > inside a #ifdef CONFIG_SPL_BUILD block. You are right, I missed that. That whole file is an #ifdef nightmare, and especially nested #ifdefs are the worst. So I would like to not *add* more of them, hence asking for IS_ENABLED() wherever possible. Thanks, Andre > Cheers, > Arnaud > > > Cheers, > > Andre > > > >> + status_led_init(); > >> +#endif > >> + > >> #ifdef CONFIG_SY8106A_POWER > >> power_failed = sy8106a_set_vout1(CONFIG_SY8106A_VOUT1_VOLT); > >> #endif ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] pinephone_defconfig: add support for early-boot status LED 2021-09-06 20:57 [PATCH] PinePhone: enable LED on boot for improved visual feedback Arnaud Ferraris 2021-09-06 20:57 ` [PATCH 1/2] board: sunxi: enable status LED early Arnaud Ferraris @ 2021-09-06 20:57 ` Arnaud Ferraris 2021-09-06 23:46 ` Andre Przywara 1 sibling, 1 reply; 7+ messages in thread From: Arnaud Ferraris @ 2021-09-06 20:57 UTC (permalink / raw) To: u-boot Cc: Jagan Teki, Andre Przywara, Samuel Holland, Maxime Ripard, Arnaud Ferraris, Arnaud Ferraris From: Arnaud Ferraris <arnaud.ferraris@gmail.com> This commit enables the green status LED (PD18/GPIO 114) on boot in the SPL, in order to provide visual feedback that the PinePhone is booting. Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com> --- configs/pinephone_defconfig | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig index 64e13d3132..9d39204a43 100644 --- a/configs/pinephone_defconfig +++ b/configs/pinephone_defconfig @@ -1,6 +1,7 @@ CONFIG_ARM=y CONFIG_ARCH_SUNXI=y CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pinephone-1.2" +CONFIG_SPL_DRIVERS_MISC=y CONFIG_SPL=y CONFIG_MACH_SUN50I=y CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y @@ -10,3 +11,8 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2 CONFIG_PINEPHONE_DT_SELECTION=y # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.2" +CONFIG_LED_STATUS=y +CONFIG_LED_STATUS_GPIO=y +CONFIG_LED_STATUS0=y +CONFIG_LED_STATUS_BIT=114 +CONFIG_LED_STATUS_STATE=2 -- 2.33.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] pinephone_defconfig: add support for early-boot status LED 2021-09-06 20:57 ` [PATCH 2/2] pinephone_defconfig: add support for early-boot status LED Arnaud Ferraris @ 2021-09-06 23:46 ` Andre Przywara 0 siblings, 0 replies; 7+ messages in thread From: Andre Przywara @ 2021-09-06 23:46 UTC (permalink / raw) To: Arnaud Ferraris Cc: u-boot, Jagan Teki, Samuel Holland, Maxime Ripard, Arnaud Ferraris On Mon, 6 Sep 2021 22:57:53 +0200 Arnaud Ferraris <arnaud.ferraris@collabora.com> wrote: > From: Arnaud Ferraris <arnaud.ferraris@gmail.com> > > This commit enables the green status LED (PD18/GPIO 114) on boot in the > SPL, in order to provide visual feedback that the PinePhone is booting. Looks alright, and while I don't have a Pinephone to test this, I tried the similar setting on a Pine64-LTS. It should be noted that this increases the SPL by 364 bytes (in my setup), which kills the Pine H64, for instance. But it looks fine for A64 boards. > Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com> Reviewed-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre > --- > configs/pinephone_defconfig | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig > index 64e13d3132..9d39204a43 100644 > --- a/configs/pinephone_defconfig > +++ b/configs/pinephone_defconfig > @@ -1,6 +1,7 @@ > CONFIG_ARM=y > CONFIG_ARCH_SUNXI=y > CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pinephone-1.2" > +CONFIG_SPL_DRIVERS_MISC=y > CONFIG_SPL=y > CONFIG_MACH_SUN50I=y > CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y > @@ -10,3 +11,8 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2 > CONFIG_PINEPHONE_DT_SELECTION=y > # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set > CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.2" > +CONFIG_LED_STATUS=y > +CONFIG_LED_STATUS_GPIO=y > +CONFIG_LED_STATUS0=y > +CONFIG_LED_STATUS_BIT=114 > +CONFIG_LED_STATUS_STATE=2 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-07 22:42 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-06 20:57 [PATCH] PinePhone: enable LED on boot for improved visual feedback Arnaud Ferraris 2021-09-06 20:57 ` [PATCH 1/2] board: sunxi: enable status LED early Arnaud Ferraris 2021-09-06 23:46 ` Andre Przywara 2021-09-07 11:41 ` Arnaud Ferraris 2021-09-07 22:42 ` Andre Przywara 2021-09-06 20:57 ` [PATCH 2/2] pinephone_defconfig: add support for early-boot status LED Arnaud Ferraris 2021-09-06 23:46 ` Andre Przywara
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.