linux-kernel - Re: [PATCH] mfd: dm355evm_msp: Convert LEDs to GPIO descriptor table

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200728070618.GD1850026@dell>
Date:   Tue, 28 Jul 2020 08:06:18 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     linux-kernel@...r.kernel.org, Sekhar Nori <nsekhar@...com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>
Subject: Re: [PATCH] mfd: dm355evm_msp: Convert LEDs to GPIO descriptor table

On Mon, 27 Jul 2020, Linus Walleij wrote:

> This converts the DaVinci DM355EVM LEDs to use GPIO
> descriptor look-ups.
> 
> Cc: Sekhar Nori <nsekhar@...com>
> Cc: Bartosz Golaszewski <bgolaszewski@...libre.com>
> Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> ---
>  drivers/mfd/dm355evm_msp.c | 49 ++++++++++++++++++++++++++++----------
>  1 file changed, 37 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mfd/dm355evm_msp.c b/drivers/mfd/dm355evm_msp.c
> index 151c36ce7343..af24712d605e 100644
> --- a/drivers/mfd/dm355evm_msp.c
> +++ b/drivers/mfd/dm355evm_msp.c
> @@ -12,6 +12,7 @@
>  #include <linux/module.h>
>  #include <linux/err.h>
>  #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
>  #include <linux/leds.h>
>  #include <linux/i2c.h>
>  #include <linux/mfd/dm355evm_msp.h>
> @@ -260,31 +261,55 @@ static int add_children(struct i2c_client *client)
>  
>  	/* LED output */
>  	if (msp_has_leds()) {
> -#define GPIO_LED(l)	.name = l, .active_low = true
>  		static struct gpio_led evm_leds[] = {
> -			{ GPIO_LED("dm355evm::ds14"),
> +			{ .name = "dm355evm::ds14",
>  				.default_trigger = "heartbeat", },
> -			{ GPIO_LED("dm355evm::ds15"),
> +			{ .name = "dm355evm::ds15",
>  				.default_trigger = "mmc0", },
> -			{ GPIO_LED("dm355evm::ds16"),
> +			{ .name = "dm355evm::ds16",
>  				/* could also be a CE-ATA drive */
>  				.default_trigger = "mmc1", },
> -			{ GPIO_LED("dm355evm::ds17"),
> +			{ .name = "dm355evm::ds17",
>  				.default_trigger = "nand-disk", },
> -			{ GPIO_LED("dm355evm::ds18"), },
> -			{ GPIO_LED("dm355evm::ds19"), },
> -			{ GPIO_LED("dm355evm::ds20"), },
> -			{ GPIO_LED("dm355evm::ds21"), },
> +			{ .name = "dm355evm::ds18", },
> +			{ .name = "dm355evm::ds19", },
> +			{ .name = "dm355evm::ds20", },
> +			{ .name = "dm355evm::ds21", },
>  		};
> -#undef GPIO_LED
>  
>  		struct gpio_led_platform_data evm_led_data = {
>  			.num_leds	= ARRAY_SIZE(evm_leds),
>  			.leds		= evm_leds,
>  		};
>  
> -		for (i = 0; i < ARRAY_SIZE(evm_leds); i++)
> -			evm_leds[i].gpio = i + dm355evm_msp_gpio.base;
> +		static struct gpiod_lookup_table evm_leds_gpio_table = {
> +			.dev_id = "leds-gpio",
> +			.table = {
> +				/*
> +				 * These GPIOs are on the dm355evm_msp
> +				 * GPIO chip at index 0..7
> +				 */
> +				GPIO_LOOKUP_IDX("dm355evm_msp", 0, NULL,
> +						0, GPIO_ACTIVE_LOW),
> +				GPIO_LOOKUP_IDX("dm355evm_msp", 1, NULL,
> +						1, GPIO_ACTIVE_LOW),
> +				GPIO_LOOKUP_IDX("dm355evm_msp", 2, NULL,
> +						2, GPIO_ACTIVE_LOW),
> +				GPIO_LOOKUP_IDX("dm355evm_msp", 3, NULL,
> +						3, GPIO_ACTIVE_LOW),
> +				GPIO_LOOKUP_IDX("dm355evm_msp", 4, NULL,
> +						4, GPIO_ACTIVE_LOW),
> +				GPIO_LOOKUP_IDX("dm355evm_msp", 5, NULL,
> +						5, GPIO_ACTIVE_LOW),
> +				GPIO_LOOKUP_IDX("dm355evm_msp", 6, NULL,
> +						6, GPIO_ACTIVE_LOW),
> +				GPIO_LOOKUP_IDX("dm355evm_msp", 7, NULL,
> +						7, GPIO_ACTIVE_LOW),
> +				{ },
> +			},
> +		};

Horrible.

Can this (and the table above) be placed somewhere else and simply
referenced from here?  Tables are usually placed globally at the top
of the file to prevent this kind of formatting craziness from
appearing inside functions.

> +		gpiod_add_lookup_table(&evm_leds_gpio_table);
>  
>  		/* NOTE:  these are the only fully programmable LEDs
>  		 * on the board, since GPIO-61/ds22 (and many signals

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog