RE: [PATCH] gpio: adp5588: Remove support for platform setup and teardown callbacks

["Hennerich, Michael" <Michael.Hennerich@xxxxxxxxxx>]

> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> Sent: Montag, 23. Mai 2022 10:40
> To: Hennerich, Michael <Michael.Hennerich@xxxxxxxxxx>; Linus Walleij
> <linus.walleij@xxxxxxxxxx>; Bartosz Golaszewski <brgl@xxxxxxxx>
> Cc: linux-gpio@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx
> Subject: [PATCH] gpio: adp5588: Remove support for platform setup and
> teardown callbacks
> 
> 
> If the teardown callback failed in the gpio driver, it fails to free the irq (if there is
> one). The device is removed anyhow. If later on the irq triggers, all sorts of
> unpleasant things might happen (e.g. accessing the struct adp5588_gpio which
> is already freed in the meantime or starting i2c bus transfers for an unregistered
> device). Even before irq support was added to this driver, exiting early was
> wrong; back then it failed to unregister the gpiochip.
> 
> Fortunately these callbacks aren't used any more since at least blackfin was
> removed in 2018. So just drop them.
> 
> Note that they are not removed from struct adp5588_gpio_platform_data
> because the keyboard driver adp5588-keys.c also makes use of them.
> (I didn't check if the callbacks might have been called twice, maybe there is
> another reason hidden to better not call these functions.)
> 
> This patch is a preparation for making i2c remove callbacks return void.
> 
> Fixes: 80884094e344 ("gpio: adp5588-gpio: new driver for ADP5588 GPIO
> expanders")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>

Acked-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx>

> ---
>  drivers/gpio/gpio-adp5588.c | 19 -------------------
>  1 file changed, 19 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-adp5588.c b/drivers/gpio/gpio-adp5588.c index
> f1e4ac90e7d3..e388e75103f4 100644
> --- a/drivers/gpio/gpio-adp5588.c
> +++ b/drivers/gpio/gpio-adp5588.c
> @@ -406,12 +406,6 @@ static int adp5588_gpio_probe(struct i2c_client
> *client)
>  	if (ret)
>  		return ret;
> 
> -	if (pdata && pdata->setup) {
> -		ret = pdata->setup(client, gc->base, gc->ngpio, pdata-
> >context);
> -		if (ret < 0)
> -			dev_warn(&client->dev, "setup failed, %d\n", ret);
> -	}
> -
>  	i2c_set_clientdata(client, dev);
> 
>  	return 0;
> @@ -419,20 +413,7 @@ static int adp5588_gpio_probe(struct i2c_client
> *client)
> 
>  static int adp5588_gpio_remove(struct i2c_client *client)  {
> -	struct adp5588_gpio_platform_data *pdata =
> -			dev_get_platdata(&client->dev);
>  	struct adp5588_gpio *dev = i2c_get_clientdata(client);
> -	int ret;
> -
> -	if (pdata && pdata->teardown) {
> -		ret = pdata->teardown(client,
> -				      dev->gpio_chip.base, dev-
> >gpio_chip.ngpio,
> -				      pdata->context);
> -		if (ret < 0) {
> -			dev_err(&client->dev, "teardown failed %d\n", ret);
> -			return ret;
> -		}
> -	}
> 
>  	if (dev->client->irq)
>  		free_irq(dev->client->irq, dev);
> 
> base-commit: 4b0986a3613c92f4ec1bdc7f60ec66fea135991f
> --
> 2.35.1