[PATCH v3] backlight: lms283gf05: Convert to GPIO descriptors

[Daniel Thompson]

On Wed, Apr 29, 2020 at 02:43:54PM +0200, Linus Walleij wrote:
> This converts the lms283gf05 backlight driver to use GPIO
> descriptors and switches the single PXA Palm Z2 device
> over to defining these.
> 
> Since the platform data was only used to convey GPIO
> information we can delete the platform data header.
> 
> Notice that we define the proper active low semantics in
> the board file GPIO descriptor table (active low) and
> assert the reset line by bringing it to "1" (asserted).
> 
> Cc: Marek Vasut <marex at denx.de>
> Cc: Daniel Mack <daniel at zonque.org>
> Cc: Haojian Zhuang <haojian.zhuang at gmail.com>
> Cc: Robert Jarzmik <robert.jarzmik at free.fr>
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
> ---
> ChangeLog v2->v3:
> - Fix a use-before-allocated bug discovered by compile tests.
> - Remove unused ret variable as autobuilders complained.
> ChangeLog v1->v2:
> - Bring up the GPIO de-asserted in probe()
> 
> Marek: I saw this was written by you, are you regularly
> testing the Z2 device?
> ---
>  arch/arm/mach-pxa/z2.c               | 12 +++++---
>  drivers/video/backlight/lms283gf05.c | 42 +++++++++++-----------------
>  include/linux/spi/lms283gf05.h       | 16 -----------
>  3 files changed, 24 insertions(+), 46 deletions(-)
>  delete mode 100644 include/linux/spi/lms283gf05.h
> 
> diff --git a/drivers/video/backlight/lms283gf05.c b/drivers/video/backlight/lms283gf05.c
> index 0e45685bcc1c..529c415eb03b 100644
> --- a/drivers/video/backlight/lms283gf05.c
> +++ b/drivers/video/backlight/lms283gf05.c
> @@ -150,24 +147,17 @@ static struct lcd_ops lms_ops = {
>  static int lms283gf05_probe(struct spi_device *spi)
>  {
>  	struct lms283gf05_state *st;
> -	struct lms283gf05_pdata *pdata = dev_get_platdata(&spi->dev);
>  	struct lcd_device *ld;
> -	int ret = 0;
> -
> -	if (pdata != NULL) {
> -		ret = devm_gpio_request_one(&spi->dev, pdata->reset_gpio,
> -				GPIOF_DIR_OUT | (!pdata->reset_inverted ?
> -				GPIOF_INIT_HIGH : GPIOF_INIT_LOW),
> -				"LMS283GF05 RESET");
> -		if (ret)
> -			return ret;
> -	}
>  
>  	st = devm_kzalloc(&spi->dev, sizeof(struct lms283gf05_state),
>  				GFP_KERNEL);
>  	if (st == NULL)
>  		return -ENOMEM;
>  
> +	st->reset = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
> +	if (st->reset)
> +		gpiod_set_consumer_name(st->reset, "LMS283GF05 RESET");
> +


Sorry, I should have picked this up before but shouldn't st->reset have
an IS_ERR() check. There are certainly no other examples of this API
within the kernel that are not followed by an IS_ERR() check!

In fact I understood that the gpiod API is intended to tolerate NULLs
so I would have expected this code to be:

	st->reset = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
	if (IS_ERR(st->reset))
		return PTR_ERR(st->reset);
	gpiod_set_consumer_name(st->reset, "LMS283GF05 RESET");


>  	ld = devm_lcd_device_register(&spi->dev, "lms283gf05", &spi->dev, st,
>  					&lms_ops);
>  	if (IS_ERR(ld))


Daniel.
>