diff mbox series

[v2,1/3] sunxi: spl: fix SPL_SUNXI_LED active low configuration

Message ID 20260429092828.3076719-2-andre.przywara@arm.com (mailing list archive)
State New
Headers
Series Funxi: Fix and extend SPL power LED support |

Commit Message

Andre Przywara April 29, 2026, 9:28 a.m. UTC 
The newly introduced Allwinner SPL LED "framework" defined a
SPL_SUNXI_LED_STATUS_STATE Kconfig symbol, that was supposed to denote
the active-low vs. active-high polarity of the LED. However this is
a bool symbol, so it will simply vanish if not defined, and we cannot use
it directly inside a C statement.

Filter the symbol through the IS_ENABLED() macro, which will return 0 if
the symbol is not defined, which is the intended value here.

Since the STATUS_STATE name is a bit confusing, rename it to ACTIVE_HIGH
on the way, because that is its real meaning. Also the LED_STATUS_BIT
name for the GPIO number is similarly a remnant of the old status LED
code, so rename it to LED_STATUS_GPIO as well.

This fixes configuring LEDs with active-low polarity.

Fixes: 256557dd9aae ("sunxi: remove usage of legacy LED API")
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/mach-sunxi/Kconfig | 10 ++++++----
 board/sunxi/board.c         |  4 ++--
 configs/pinephone_defconfig |  3 +--
 3 files changed, 9 insertions(+), 8 deletions(-)

Comments

Quentin Schulz April 29, 2026, 9:49 a.m. UTC | #1 
Hi Andre,

On 4/29/26 11:28 AM, Andre Przywara wrote:
> The newly introduced Allwinner SPL LED "framework" defined a
> SPL_SUNXI_LED_STATUS_STATE Kconfig symbol, that was supposed to denote
> the active-low vs. active-high polarity of the LED. However this is
> a bool symbol, so it will simply vanish if not defined, and we cannot use
> it directly inside a C statement.
> 
> Filter the symbol through the IS_ENABLED() macro, which will return 0 if
> the symbol is not defined, which is the intended value here.
> 
> Since the STATUS_STATE name is a bit confusing, rename it to ACTIVE_HIGH
> on the way, because that is its real meaning. Also the LED_STATUS_BIT
> name for the GPIO number is similarly a remnant of the old status LED
> code, so rename it to LED_STATUS_GPIO as well.
> 
> This fixes configuring LEDs with active-low polarity.
> 

Individual patches would have been better.

> Fixes: 256557dd9aae ("sunxi: remove usage of legacy LED API")

Reported-by: Paul Kocialkowski <paulk@sys-base.io>
Closes: https://lore.kernel.org/u-boot/adfMQBPdntWy1KIq@shepard/

Acked-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin
Paul Kocialkowski April 29, 2026, 1:07 p.m. UTC | #2 
Hi Andre,

On Wed 29 Apr 26, 11:28, Andre Przywara wrote:
> The newly introduced Allwinner SPL LED "framework" defined a
> SPL_SUNXI_LED_STATUS_STATE Kconfig symbol, that was supposed to denote
> the active-low vs. active-high polarity of the LED. However this is
> a bool symbol, so it will simply vanish if not defined, and we cannot use
> it directly inside a C statement.
> 
> Filter the symbol through the IS_ENABLED() macro, which will return 0 if
> the symbol is not defined, which is the intended value here.
> 
> Since the STATUS_STATE name is a bit confusing, rename it to ACTIVE_HIGH
> on the way, because that is its real meaning. Also the LED_STATUS_BIT
> name for the GPIO number is similarly a remnant of the old status LED
> code, so rename it to LED_STATUS_GPIO as well.
> 
> This fixes configuring LEDs with active-low polarity.
>
> Fixes: 256557dd9aae ("sunxi: remove usage of legacy LED API")
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Thanks for your work!

Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>

All the best,

Paul

> ---
>  arch/arm/mach-sunxi/Kconfig | 10 ++++++----
>  board/sunxi/board.c         |  4 ++--
>  configs/pinephone_defconfig |  3 +--
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 89f0e77bcdb..07d7ea2b7ad 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -1245,15 +1245,17 @@ config SPL_SUNXI_LED_STATUS
>  
>  if SPL_SUNXI_LED_STATUS
>  
> -config SPL_SUNXI_LED_STATUS_BIT
> +config SPL_SUNXI_LED_STATUS_GPIO
>  	int "GPIO number for GPIO status LED"
>  	help
>  	  GPIO number for the GPIO controlling the GPIO status LED in SPL.
>  
> -config SPL_SUNXI_LED_STATUS_STATE
> -	bool "GPIO status LED initial state is on"
> +config SPL_SUNXI_LED_STATUS_ACTIVE_HIGH
> +	bool "GPIO status LED is active high"
> +	default y
>  	help
> -	  Whether the initial state of the status LED in SPL must be on or off.
> +	  Whether the GPIO of the status LED must be set high or low to turn
> +	  the LED on.
>  
>  endif # SPL_SUNXI_LED_STATUS
>  
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index d7722d1858a..3d1afec7c66 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -563,8 +563,8 @@ static void sunxi_spl_store_dram_size(phys_addr_t dram_size)
>  static void status_led_init(void)
>  {
>  #if CONFIG_IS_ENABLED(SUNXI_LED_STATUS)
> -	unsigned int state = CONFIG_SPL_SUNXI_LED_STATUS_STATE;
> -	unsigned int gpio = CONFIG_SPL_SUNXI_LED_STATUS_BIT;
> +	unsigned int state = IS_ENABLED(CONFIG_SPL_SUNXI_LED_STATUS_ACTIVE_HIGH);
> +	unsigned int gpio = CONFIG_SPL_SUNXI_LED_STATUS_GPIO;
>  
>  	gpio_request(gpio, "gpio_led");
>  	gpio_direction_output(gpio, state);
> diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
> index ed7dbc1852b..3a4f4ec51cf 100644
> --- a/configs/pinephone_defconfig
> +++ b/configs/pinephone_defconfig
> @@ -9,8 +9,7 @@ CONFIG_DRAM_ZQ=3881949
>  CONFIG_MMC_SUNXI_SLOT_EXTRA=2
>  CONFIG_PINEPHONE_DT_SELECTION=y
>  CONFIG_SPL_SUNXI_LED_STATUS=y
> -CONFIG_SPL_SUNXI_LED_STATUS_BIT=114
> -CONFIG_SPL_SUNXI_LED_STATUS_STATE=y
> +CONFIG_SPL_SUNXI_LED_STATUS_GPIO=114
>  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
>  CONFIG_CMD_PSTORE=y
>  CONFIG_CMD_PSTORE_MEM_ADDR=0x61000000
> -- 
> 2.43.0
>
diff mbox series

Patch

diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 89f0e77bcdb..07d7ea2b7ad 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -1245,15 +1245,17 @@  config SPL_SUNXI_LED_STATUS
 
 if SPL_SUNXI_LED_STATUS
 
-config SPL_SUNXI_LED_STATUS_BIT
+config SPL_SUNXI_LED_STATUS_GPIO
 	int "GPIO number for GPIO status LED"
 	help
 	  GPIO number for the GPIO controlling the GPIO status LED in SPL.
 
-config SPL_SUNXI_LED_STATUS_STATE
-	bool "GPIO status LED initial state is on"
+config SPL_SUNXI_LED_STATUS_ACTIVE_HIGH
+	bool "GPIO status LED is active high"
+	default y
 	help
-	  Whether the initial state of the status LED in SPL must be on or off.
+	  Whether the GPIO of the status LED must be set high or low to turn
+	  the LED on.
 
 endif # SPL_SUNXI_LED_STATUS
 
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index d7722d1858a..3d1afec7c66 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -563,8 +563,8 @@  static void sunxi_spl_store_dram_size(phys_addr_t dram_size)
 static void status_led_init(void)
 {
 #if CONFIG_IS_ENABLED(SUNXI_LED_STATUS)
-	unsigned int state = CONFIG_SPL_SUNXI_LED_STATUS_STATE;
-	unsigned int gpio = CONFIG_SPL_SUNXI_LED_STATUS_BIT;
+	unsigned int state = IS_ENABLED(CONFIG_SPL_SUNXI_LED_STATUS_ACTIVE_HIGH);
+	unsigned int gpio = CONFIG_SPL_SUNXI_LED_STATUS_GPIO;
 
 	gpio_request(gpio, "gpio_led");
 	gpio_direction_output(gpio, state);
diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
index ed7dbc1852b..3a4f4ec51cf 100644
--- a/configs/pinephone_defconfig
+++ b/configs/pinephone_defconfig
@@ -9,8 +9,7 @@  CONFIG_DRAM_ZQ=3881949
 CONFIG_MMC_SUNXI_SLOT_EXTRA=2
 CONFIG_PINEPHONE_DT_SELECTION=y
 CONFIG_SPL_SUNXI_LED_STATUS=y
-CONFIG_SPL_SUNXI_LED_STATUS_BIT=114
-CONFIG_SPL_SUNXI_LED_STATUS_STATE=y
+CONFIG_SPL_SUNXI_LED_STATUS_GPIO=114
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
 CONFIG_CMD_PSTORE=y
 CONFIG_CMD_PSTORE_MEM_ADDR=0x61000000