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

[Paul Kocialkowski <paulk@sys-base.io>]

[-- Attachment #1: Type: text/plain, Size: 3665 bytes --]

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
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]