[PATCH 1/6] gpio: ich: Use devm_gpiochip_add_data() to simplify remove path

[PATCH 1/6] gpio: ich: Use devm_gpiochip_add_data() to simplify remove path

[Andrew Davis]

Use devm version of gpiochip add function to handle removal for us.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/gpio/gpio-ich.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-ich.c b/drivers/gpio/gpio-ich.c
index 3b31f5e9bf40..0be9285efebc 100644
--- a/drivers/gpio/gpio-ich.c
+++ b/drivers/gpio/gpio-ich.c
@@ -457,7 +457,7 @@ static int ichx_gpio_probe(struct platform_device *pdev)
 
 init:
 	ichx_gpiolib_setup(&ichx_priv.chip);
-	err = gpiochip_add_data(&ichx_priv.chip, NULL);
+	err = devm_gpiochip_add_data(dev, &ichx_priv.chip, NULL);
 	if (err) {
 		dev_err(dev, "Failed to register GPIOs\n");
 		return err;
@@ -469,19 +469,11 @@ static int ichx_gpio_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int ichx_gpio_remove(struct platform_device *pdev)
-{
-	gpiochip_remove(&ichx_priv.chip);
-
-	return 0;
-}
-
 static struct platform_driver ichx_gpio_driver = {
 	.driver		= {
 		.name	= DRV_NAME,
 	},
 	.probe		= ichx_gpio_probe,
-	.remove		= ichx_gpio_remove,
 };
 
 module_platform_driver(ichx_gpio_driver);
-- 
2.39.2

[PATCH 2/6] gpio: twl4030: Use devm_gpiochip_add_data() to simplify remove path

[Andrew Davis]

Use devm version of gpiochip add function to handle removal for us.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/gpio/gpio-twl4030.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
index c1bb2c3ca6f2..23f58bf3a415 100644
--- a/drivers/gpio/gpio-twl4030.c
+++ b/drivers/gpio/gpio-twl4030.c
@@ -492,18 +492,6 @@ static struct twl4030_gpio_platform_data *of_gpio_twl4030(struct device *dev,
 	return omap_twl_info;
 }
 
-/* Cannot use as gpio_twl4030_probe() calls us */
-static int gpio_twl4030_remove(struct platform_device *pdev)
-{
-	struct gpio_twl4030_priv *priv = platform_get_drvdata(pdev);
-
-	gpiochip_remove(&priv->gpio_chip);
-
-	/* REVISIT no support yet for deregistering all the IRQs */
-	WARN_ON(!is_module());
-	return 0;
-}
-
 static int gpio_twl4030_probe(struct platform_device *pdev)
 {
 	struct twl4030_gpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -581,12 +569,9 @@ static int gpio_twl4030_probe(struct platform_device *pdev)
 	if (ret < 0) {
 		dev_err(&pdev->dev, "could not register gpiochip, %d\n", ret);
 		priv->gpio_chip.ngpio = 0;
-		gpio_twl4030_remove(pdev);
 		goto out;
 	}
 
-	platform_set_drvdata(pdev, priv);
-
 	if (pdata->setup) {
 		int status;
 
@@ -615,7 +600,6 @@ static struct platform_driver gpio_twl4030_driver = {
 		.of_match_table = twl_gpio_match,
 	},
 	.probe		= gpio_twl4030_probe,
-	.remove		= gpio_twl4030_remove,
 };
 
 static int __init gpio_twl4030_init(void)
-- 
2.39.2

Re: [PATCH 2/6] gpio: twl4030: Use devm_gpiochip_add_data() to simplify remove path

[Andy Shevchenko]

On Tue, Mar 07, 2023 at 10:54:28AM -0600, Andrew Davis wrote:
> Use devm version of gpiochip add function to handle removal for us.

I do not see this change in the below code.
Can you shed a light?

> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/gpio/gpio-twl4030.c | 16 ----------------
>  1 file changed, 16 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
> index c1bb2c3ca6f2..23f58bf3a415 100644
> --- a/drivers/gpio/gpio-twl4030.c
> +++ b/drivers/gpio/gpio-twl4030.c
> @@ -492,18 +492,6 @@ static struct twl4030_gpio_platform_data *of_gpio_twl4030(struct device *dev,
>  	return omap_twl_info;
>  }
>  
> -/* Cannot use as gpio_twl4030_probe() calls us */
> -static int gpio_twl4030_remove(struct platform_device *pdev)
> -{
> -	struct gpio_twl4030_priv *priv = platform_get_drvdata(pdev);
> -
> -	gpiochip_remove(&priv->gpio_chip);
> -
> -	/* REVISIT no support yet for deregistering all the IRQs */
> -	WARN_ON(!is_module());
> -	return 0;
> -}
> -
>  static int gpio_twl4030_probe(struct platform_device *pdev)
>  {
>  	struct twl4030_gpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
> @@ -581,12 +569,9 @@ static int gpio_twl4030_probe(struct platform_device *pdev)
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "could not register gpiochip, %d\n", ret);
>  		priv->gpio_chip.ngpio = 0;
> -		gpio_twl4030_remove(pdev);
>  		goto out;
>  	}
>  
> -	platform_set_drvdata(pdev, priv);
> -
>  	if (pdata->setup) {
>  		int status;
>  
> @@ -615,7 +600,6 @@ static struct platform_driver gpio_twl4030_driver = {
>  		.of_match_table = twl_gpio_match,
>  	},
>  	.probe		= gpio_twl4030_probe,
> -	.remove		= gpio_twl4030_remove,
>  };
>  
>  static int __init gpio_twl4030_init(void)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/6] gpio: twl4030: Use devm_gpiochip_add_data() to simplify remove path

[Andrew Davis]

On 3/7/23 11:45 AM, Andy Shevchenko wrote:
> On Tue, Mar 07, 2023 at 10:54:28AM -0600, Andrew Davis wrote:
>> Use devm version of gpiochip add function to handle removal for us.
> 
> I do not see this change in the below code.
> Can you shed a light?
> 

Odd.. must have been lost in a rebase, will respin a v2.

Thanks,
Andrew

>> Signed-off-by: Andrew Davis <afd@ti.com>
>> ---
>>   drivers/gpio/gpio-twl4030.c | 16 ----------------
>>   1 file changed, 16 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
>> index c1bb2c3ca6f2..23f58bf3a415 100644
>> --- a/drivers/gpio/gpio-twl4030.c
>> +++ b/drivers/gpio/gpio-twl4030.c
>> @@ -492,18 +492,6 @@ static struct twl4030_gpio_platform_data *of_gpio_twl4030(struct device *dev,
>>   	return omap_twl_info;
>>   }
>>   
>> -/* Cannot use as gpio_twl4030_probe() calls us */
>> -static int gpio_twl4030_remove(struct platform_device *pdev)
>> -{
>> -	struct gpio_twl4030_priv *priv = platform_get_drvdata(pdev);
>> -
>> -	gpiochip_remove(&priv->gpio_chip);
>> -
>> -	/* REVISIT no support yet for deregistering all the IRQs */
>> -	WARN_ON(!is_module());
>> -	return 0;
>> -}
>> -
>>   static int gpio_twl4030_probe(struct platform_device *pdev)
>>   {
>>   	struct twl4030_gpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
>> @@ -581,12 +569,9 @@ static int gpio_twl4030_probe(struct platform_device *pdev)
>>   	if (ret < 0) {
>>   		dev_err(&pdev->dev, "could not register gpiochip, %d\n", ret);
>>   		priv->gpio_chip.ngpio = 0;
>> -		gpio_twl4030_remove(pdev);
>>   		goto out;
>>   	}
>>   
>> -	platform_set_drvdata(pdev, priv);
>> -
>>   	if (pdata->setup) {
>>   		int status;
>>   
>> @@ -615,7 +600,6 @@ static struct platform_driver gpio_twl4030_driver = {
>>   		.of_match_table = twl_gpio_match,
>>   	},
>>   	.probe		= gpio_twl4030_probe,
>> -	.remove		= gpio_twl4030_remove,
>>   };
>>   
>>   static int __init gpio_twl4030_init(void)
> 

[PATCH 3/6] gpio: sch311x: Use devm_gpiochip_add_data() to simplify remove path

[Andrew Davis]

Use devm version of gpiochip add function to handle removal for us.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/gpio/gpio-sch311x.c | 25 ++-----------------------
 1 file changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/gpio/gpio-sch311x.c b/drivers/gpio/gpio-sch311x.c
index da01e1cad7cb..ba7c300511a5 100644
--- a/drivers/gpio/gpio-sch311x.c
+++ b/drivers/gpio/gpio-sch311x.c
@@ -281,8 +281,6 @@ static int sch311x_gpio_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	platform_set_drvdata(pdev, priv);
-
 	for (i = 0; i < ARRAY_SIZE(priv->blocks); i++) {
 		block = &priv->blocks[i];
 
@@ -305,36 +303,17 @@ static int sch311x_gpio_probe(struct platform_device *pdev)
 		block->data_reg = sch311x_gpio_blocks[i].data_reg;
 		block->runtime_reg = pdata->runtime_reg;
 
-		err = gpiochip_add_data(&block->chip, block);
+		err = devm_gpiochip_add_data(&pdev->dev, &block->chip, block);
 		if (err < 0) {
 			dev_err(&pdev->dev,
 				"Could not register gpiochip, %d\n", err);
-			goto exit_err;
+			return err;
 		}
 		dev_info(&pdev->dev,
 			 "SMSC SCH311x GPIO block %d registered.\n", i);
 	}
 
 	return 0;
-
-exit_err:
-	/* release already registered chips */
-	for (--i; i >= 0; i--)
-		gpiochip_remove(&priv->blocks[i].chip);
-	return err;
-}
-
-static int sch311x_gpio_remove(struct platform_device *pdev)
-{
-	struct sch311x_gpio_priv *priv = platform_get_drvdata(pdev);
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(priv->blocks); i++) {
-		gpiochip_remove(&priv->blocks[i].chip);
-		dev_info(&pdev->dev,
-			 "SMSC SCH311x GPIO block %d unregistered.\n", i);
-	}
-	return 0;
 }
 
 static struct platform_driver sch311x_gpio_driver = {
-- 
2.39.2

Re: [PATCH 3/6] gpio: sch311x: Use devm_gpiochip_add_data() to simplify remove path

[Bartosz Golaszewski]

On Tue, Mar 7, 2023 at 5:54 PM Andrew Davis <afd@ti.com> wrote:
>
> Use devm version of gpiochip add function to handle removal for us.
>
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/gpio/gpio-sch311x.c | 25 ++-----------------------
>  1 file changed, 2 insertions(+), 23 deletions(-)
>

Applied, thanks!

Bart

Re: [PATCH 3/6] gpio: sch311x: Use devm_gpiochip_add_data() to simplify remove path

[Bartosz Golaszewski]

On Wed, Mar 8, 2023 at 11:24 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Tue, Mar 7, 2023 at 5:54 PM Andrew Davis <afd@ti.com> wrote:
> >
> > Use devm version of gpiochip add function to handle removal for us.
> >
> > Signed-off-by: Andrew Davis <afd@ti.com>
> > ---
> >  drivers/gpio/gpio-sch311x.c | 25 ++-----------------------
> >  1 file changed, 2 insertions(+), 23 deletions(-)
> >
>
> Applied, thanks!
>
> Bart

I see there's v2 out, backing it out then.

Bart

Re: [PATCH 3/6] gpio: sch311x: Use devm_gpiochip_add_data() to simplify remove path

[Andrew Davis]

On 3/8/23 4:32 AM, Bartosz Golaszewski wrote:
> On Wed, Mar 8, 2023 at 11:24 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>
>> On Tue, Mar 7, 2023 at 5:54 PM Andrew Davis <afd@ti.com> wrote:
>>>
>>> Use devm version of gpiochip add function to handle removal for us.
>>>
>>> Signed-off-by: Andrew Davis <afd@ti.com>
>>> ---
>>>   drivers/gpio/gpio-sch311x.c | 25 ++-----------------------
>>>   1 file changed, 2 insertions(+), 23 deletions(-)
>>>
>>
>> Applied, thanks!
>>
>> Bart
> 
> I see there's v2 out, backing it out then.
> 

Looks like I missed something that kernel test robot found, so there
will be a v3.

Andrew

Re: [PATCH 3/6] gpio: sch311x: Use devm_gpiochip_add_data() to simplify remove path

[Andy Shevchenko]

On Wed, Mar 8, 2023 at 5:50 PM Andrew Davis <afd@ti.com> wrote:
> On 3/8/23 4:32 AM, Bartosz Golaszewski wrote:
> > On Wed, Mar 8, 2023 at 11:24 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

...

> > I see there's v2 out, backing it out then.
>
> Looks like I missed something that kernel test robot found, so there
> will be a v3.

Just split your series on a per driver basis. This will help with
understanding what's going on. Also use a cover letter to explain what
your series is for.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/6] gpio: sch311x: Use devm_gpiochip_add_data() to simplify remove path

[Andrew Davis]

On 3/8/23 9:53 AM, Andy Shevchenko wrote:
> On Wed, Mar 8, 2023 at 5:50 PM Andrew Davis <afd@ti.com> wrote:
>> On 3/8/23 4:32 AM, Bartosz Golaszewski wrote:
>>> On Wed, Mar 8, 2023 at 11:24 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 
> ...
> 
>>> I see there's v2 out, backing it out then.
>>
>> Looks like I missed something that kernel test robot found, so there
>> will be a v3.
> 
> Just split your series on a per driver basis. This will help with
> understanding what's going on. Also use a cover letter to explain what
> your series is for.
> 

There is one patch per driver, not sure what you mean by split per driver?

Will add a cover letter for v3 and drop the first patch that's in your tree already.

Andrew

Re: [PATCH 3/6] gpio: sch311x: Use devm_gpiochip_add_data() to simplify remove path

[Andy Shevchenko]

On Wed, Mar 08, 2023 at 10:20:13AM -0600, Andrew Davis wrote:
> On 3/8/23 9:53 AM, Andy Shevchenko wrote:
> > On Wed, Mar 8, 2023 at 5:50 PM Andrew Davis <afd@ti.com> wrote:
> > > On 3/8/23 4:32 AM, Bartosz Golaszewski wrote:
> > > > On Wed, Mar 8, 2023 at 11:24 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

...

> > > > I see there's v2 out, backing it out then.
> > > 
> > > Looks like I missed something that kernel test robot found, so there
> > > will be a v3.
> > 
> > Just split your series on a per driver basis. This will help with
> > understanding what's going on. Also use a cover letter to explain what
> > your series is for.
> 
> There is one patch per driver, not sure what you mean by split per driver?

In the future for similar cases it's better to split the series on the driver
basis, so each patch is sent separately and handled individually. That way
you won't need to resend the whole bunch over and over because of some subtle
mistake made in the middle.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 4/6] gpio: pisosr: Use devm_gpiochip_add_data() to simplify remove path

[Andrew Davis]

Use devm version of gpiochip add function to handle removal for us.

While here update copyright and module author.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/gpio/gpio-pisosr.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/gpio/gpio-pisosr.c b/drivers/gpio/gpio-pisosr.c
index 67071bea08c2..eaf65606035d 100644
--- a/drivers/gpio/gpio-pisosr.c
+++ b/drivers/gpio/gpio-pisosr.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
- *	Andrew F. Davis <afd@ti.com>
+ * Copyright (C) 2015-2023 Texas Instruments Incorporated - https://www.ti.com/
+ *	Andrew Davis <afd@ti.com>
  */
 
 #include <linux/bitmap.h>
@@ -126,8 +126,6 @@ static int pisosr_gpio_probe(struct spi_device *spi)
 	if (!gpio)
 		return -ENOMEM;
 
-	spi_set_drvdata(spi, gpio);
-
 	gpio->chip = template_chip;
 	gpio->chip.parent = dev;
 	of_property_read_u16(dev->of_node, "ngpios", &gpio->chip.ngpio);
@@ -146,7 +144,7 @@ static int pisosr_gpio_probe(struct spi_device *spi)
 
 	mutex_init(&gpio->lock);
 
-	ret = gpiochip_add_data(&gpio->chip, gpio);
+	ret = devm_gpiochip_add_data(dev, &gpio->chip, gpio);
 	if (ret < 0) {
 		dev_err(dev, "Unable to register gpiochip\n");
 		return ret;
@@ -155,15 +153,6 @@ static int pisosr_gpio_probe(struct spi_device *spi)
 	return 0;
 }
 
-static void pisosr_gpio_remove(struct spi_device *spi)
-{
-	struct pisosr_gpio *gpio = spi_get_drvdata(spi);
-
-	gpiochip_remove(&gpio->chip);
-
-	mutex_destroy(&gpio->lock);
-}
-
 static const struct spi_device_id pisosr_gpio_id_table[] = {
 	{ "pisosr-gpio", },
 	{ /* sentinel */ }
@@ -182,11 +171,10 @@ static struct spi_driver pisosr_gpio_driver = {
 		.of_match_table = pisosr_gpio_of_match_table,
 	},
 	.probe = pisosr_gpio_probe,
-	.remove = pisosr_gpio_remove,
 	.id_table = pisosr_gpio_id_table,
 };
 module_spi_driver(pisosr_gpio_driver);
 
-MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
+MODULE_AUTHOR("Andrew Davis <afd@ti.com>");
 MODULE_DESCRIPTION("SPI Compatible PISO Shift Register GPIO Driver");
 MODULE_LICENSE("GPL v2");
-- 
2.39.2

Re: [PATCH 4/6] gpio: pisosr: Use devm_gpiochip_add_data() to simplify remove path

[Andy Shevchenko]

On Tue, Mar 07, 2023 at 10:54:30AM -0600, Andrew Davis wrote:
> Use devm version of gpiochip add function to handle removal for us.
> 
> While here update copyright and module author.

...

> -	mutex_destroy(&gpio->lock);

You need to wrap this into devm.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/6] gpio: pisosr: Use devm_gpiochip_add_data() to simplify remove path

[Andrew Davis]

On 3/7/23 11:44 AM, Andy Shevchenko wrote:
> On Tue, Mar 07, 2023 at 10:54:30AM -0600, Andrew Davis wrote:
>> Use devm version of gpiochip add function to handle removal for us.
>>
>> While here update copyright and module author.
> 
> ...
> 
>> -	mutex_destroy(&gpio->lock);
> 
> You need to wrap this into devm.
> 

I was thinking that but it seems there is no such thing. Most drivers
just ignore unwinding mutex_init() since it doesn't allocate anything.

mutex_destroy() is a NOP unless you are doing DEBUG builds were
it sets a magic value to check for use-after-free issues.

Andrew

Re: [PATCH 4/6] gpio: pisosr: Use devm_gpiochip_add_data() to simplify remove path

[Andy Shevchenko]

On Tue, Mar 07, 2023 at 11:55:11AM -0600, Andrew Davis wrote:
> On 3/7/23 11:44 AM, Andy Shevchenko wrote:
> > On Tue, Mar 07, 2023 at 10:54:30AM -0600, Andrew Davis wrote:
> > > Use devm version of gpiochip add function to handle removal for us.
> > > 
> > > While here update copyright and module author.

...

> > > -	mutex_destroy(&gpio->lock);
> > 
> > You need to wrap this into devm.
> 
> I was thinking that but it seems there is no such thing. Most drivers
> just ignore unwinding mutex_init() since it doesn't allocate anything.
> 
> mutex_destroy() is a NOP unless you are doing DEBUG builds were
> it sets a magic value to check for use-after-free issues.

In any case it's correct to destroy it.

See, how it's done, for example, here a82c7cf803d9 ("leds: is31fl319x: Wrap
mutex_destroy() for devm_add_action_or_rest()").

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/6] gpio: pisosr: Use devm_gpiochip_add_data() to simplify remove path

[Bartosz Golaszewski]

On Tue, Mar 7, 2023 at 6:55 PM Andrew Davis <afd@ti.com> wrote:
>
> On 3/7/23 11:44 AM, Andy Shevchenko wrote:
> > On Tue, Mar 07, 2023 at 10:54:30AM -0600, Andrew Davis wrote:
> >> Use devm version of gpiochip add function to handle removal for us.
> >>
> >> While here update copyright and module author.
> >
> > ...
> >
> >> -    mutex_destroy(&gpio->lock);
> >
> > You need to wrap this into devm.
> >
>
> I was thinking that but it seems there is no such thing. Most drivers
> just ignore unwinding mutex_init() since it doesn't allocate anything.
>
> mutex_destroy() is a NOP unless you are doing DEBUG builds were
> it sets a magic value to check for use-after-free issues.
>
> Andrew

And this is precisely why it's useful to destroy it. :)

Bart

[PATCH 5/6] gpio: tpic2810: Use devm_gpiochip_add_data() to simplify remove path

[Andrew Davis]

Use devm version of gpiochip add function to handle removal for us.

While here update copyright and module author.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/gpio/gpio-tpic2810.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpio-tpic2810.c b/drivers/gpio/gpio-tpic2810.c
index 349c5fbd9b02..718053edd76a 100644
--- a/drivers/gpio/gpio-tpic2810.c
+++ b/drivers/gpio/gpio-tpic2810.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
- *	Andrew F. Davis <afd@ti.com>
+ * Copyright (C) 2015-2023 Texas Instruments Incorporated - https://www.ti.com/
+ *	Andrew Davis <afd@ti.com>
  */
 
 #include <linux/gpio/driver.h>
@@ -107,8 +107,6 @@ static int tpic2810_probe(struct i2c_client *client)
 	if (!gpio)
 		return -ENOMEM;
 
-	i2c_set_clientdata(client, gpio);
-
 	gpio->chip = template_chip;
 	gpio->chip.parent = &client->dev;
 
@@ -116,7 +114,7 @@ static int tpic2810_probe(struct i2c_client *client)
 
 	mutex_init(&gpio->lock);
 
-	ret = gpiochip_add_data(&gpio->chip, gpio);
+	ret = devm_gpiochip_add_data(&client->dev, &gpio->chip, gpio);
 	if (ret < 0) {
 		dev_err(&client->dev, "Unable to register gpiochip\n");
 		return ret;
@@ -125,13 +123,6 @@ static int tpic2810_probe(struct i2c_client *client)
 	return 0;
 }
 
-static void tpic2810_remove(struct i2c_client *client)
-{
-	struct tpic2810 *gpio = i2c_get_clientdata(client);
-
-	gpiochip_remove(&gpio->chip);
-}
-
 static const struct i2c_device_id tpic2810_id_table[] = {
 	{ "tpic2810", },
 	{ /* sentinel */ }
@@ -144,11 +135,10 @@ static struct i2c_driver tpic2810_driver = {
 		.of_match_table = tpic2810_of_match_table,
 	},
 	.probe_new = tpic2810_probe,
-	.remove = tpic2810_remove,
 	.id_table = tpic2810_id_table,
 };
 module_i2c_driver(tpic2810_driver);
 
-MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
+MODULE_AUTHOR("Andrew Davis <afd@ti.com>");
 MODULE_DESCRIPTION("TPIC2810 8-Bit LED Driver GPIO Driver");
 MODULE_LICENSE("GPL v2");
-- 
2.39.2

Re: [PATCH 5/6] gpio: tpic2810: Use devm_gpiochip_add_data() to simplify remove path

[Bartosz Golaszewski]

On Tue, Mar 7, 2023 at 5:54 PM Andrew Davis <afd@ti.com> wrote:
>
> Use devm version of gpiochip add function to handle removal for us.
>
> While here update copyright and module author.
>
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/gpio/gpio-tpic2810.c | 18 ++++--------------
>  1 file changed, 4 insertions(+), 14 deletions(-)

Applied, thanks!

Bart

Re: [PATCH 5/6] gpio: tpic2810: Use devm_gpiochip_add_data() to simplify remove path

[Bartosz Golaszewski]

On Wed, Mar 8, 2023 at 11:28 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Tue, Mar 7, 2023 at 5:54 PM Andrew Davis <afd@ti.com> wrote:
> >
> > Use devm version of gpiochip add function to handle removal for us.
> >
> > While here update copyright and module author.
> >
> > Signed-off-by: Andrew Davis <afd@ti.com>
> > ---
> >  drivers/gpio/gpio-tpic2810.c | 18 ++++--------------
> >  1 file changed, 4 insertions(+), 14 deletions(-)
>
> Applied, thanks!
>
> Bart

Scratch that, please do the same as for patch 6/6.

Bart

[PATCH 6/6] gpio: tps65086: Use devm_gpiochip_add_data() to simplify remove path

[Andrew Davis]

Use devm version of gpiochip add function to handle removal for us.

While here update copyright and module author.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/gpio/gpio-tps65086.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/gpio/gpio-tps65086.c b/drivers/gpio/gpio-tps65086.c
index 1e9d8262d0ff..0b8b631441ae 100644
--- a/drivers/gpio/gpio-tps65086.c
+++ b/drivers/gpio/gpio-tps65086.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
- *	Andrew F. Davis <afd@ti.com>
+ * Copyright (C) 2015-2023 Texas Instruments Incorporated - https://www.ti.com/
+ *	Andrew Davis <afd@ti.com>
  *
  * Based on the TPS65912 driver
  */
@@ -86,13 +86,11 @@ static int tps65086_gpio_probe(struct platform_device *pdev)
 	if (!gpio)
 		return -ENOMEM;
 
-	platform_set_drvdata(pdev, gpio);
-
 	gpio->tps = dev_get_drvdata(pdev->dev.parent);
 	gpio->chip = template_chip;
 	gpio->chip.parent = gpio->tps->dev;
 
-	ret = gpiochip_add_data(&gpio->chip, gpio);
+	ret = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Could not register gpiochip, %d\n", ret);
 		return ret;
@@ -101,15 +99,6 @@ static int tps65086_gpio_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int tps65086_gpio_remove(struct platform_device *pdev)
-{
-	struct tps65086_gpio *gpio = platform_get_drvdata(pdev);
-
-	gpiochip_remove(&gpio->chip);
-
-	return 0;
-}
-
 static const struct platform_device_id tps65086_gpio_id_table[] = {
 	{ "tps65086-gpio", },
 	{ /* sentinel */ }
@@ -121,11 +110,10 @@ static struct platform_driver tps65086_gpio_driver = {
 		.name = "tps65086-gpio",
 	},
 	.probe = tps65086_gpio_probe,
-	.remove = tps65086_gpio_remove,
 	.id_table = tps65086_gpio_id_table,
 };
 module_platform_driver(tps65086_gpio_driver);
 
-MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
+MODULE_AUTHOR("Andrew Davis <afd@ti.com>");
 MODULE_DESCRIPTION("TPS65086 GPIO driver");
 MODULE_LICENSE("GPL v2");
-- 
2.39.2

Re: [PATCH 6/6] gpio: tps65086: Use devm_gpiochip_add_data() to simplify remove path

[Bartosz Golaszewski]

On Tue, Mar 7, 2023 at 5:54 PM Andrew Davis <afd@ti.com> wrote:
>
> Use devm version of gpiochip add function to handle removal for us.
>
> While here update copyright and module author.
>
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/gpio/gpio-tps65086.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpio/gpio-tps65086.c b/drivers/gpio/gpio-tps65086.c
> index 1e9d8262d0ff..0b8b631441ae 100644
> --- a/drivers/gpio/gpio-tps65086.c
> +++ b/drivers/gpio/gpio-tps65086.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
> - *     Andrew F. Davis <afd@ti.com>
> + * Copyright (C) 2015-2023 Texas Instruments Incorporated - https://www.ti.com/
> + *     Andrew Davis <afd@ti.com>
>   *
>   * Based on the TPS65912 driver
>   */
> @@ -86,13 +86,11 @@ static int tps65086_gpio_probe(struct platform_device *pdev)
>         if (!gpio)
>                 return -ENOMEM;
>
> -       platform_set_drvdata(pdev, gpio);
> -
>         gpio->tps = dev_get_drvdata(pdev->dev.parent);
>         gpio->chip = template_chip;
>         gpio->chip.parent = gpio->tps->dev;
>
> -       ret = gpiochip_add_data(&gpio->chip, gpio);
> +       ret = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
>         if (ret < 0) {
>                 dev_err(&pdev->dev, "Could not register gpiochip, %d\n", ret);

Drop this log and just return directly the result of
devm_gpiochip_add_data() to save a couple more lines.

Bart

>                 return ret;
> @@ -101,15 +99,6 @@ static int tps65086_gpio_probe(struct platform_device *pdev)
>         return 0;
>  }
>
> -static int tps65086_gpio_remove(struct platform_device *pdev)
> -{
> -       struct tps65086_gpio *gpio = platform_get_drvdata(pdev);
> -
> -       gpiochip_remove(&gpio->chip);
> -
> -       return 0;
> -}
> -
>  static const struct platform_device_id tps65086_gpio_id_table[] = {
>         { "tps65086-gpio", },
>         { /* sentinel */ }
> @@ -121,11 +110,10 @@ static struct platform_driver tps65086_gpio_driver = {
>                 .name = "tps65086-gpio",
>         },
>         .probe = tps65086_gpio_probe,
> -       .remove = tps65086_gpio_remove,
>         .id_table = tps65086_gpio_id_table,
>  };
>  module_platform_driver(tps65086_gpio_driver);
>
> -MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
> +MODULE_AUTHOR("Andrew Davis <afd@ti.com>");
>  MODULE_DESCRIPTION("TPS65086 GPIO driver");
>  MODULE_LICENSE("GPL v2");
> --
> 2.39.2
>

Re: [PATCH 1/6] gpio: ich: Use devm_gpiochip_add_data() to simplify remove path

[Andy Shevchenko]

On Tue, Mar 07, 2023 at 10:54:27AM -0600, Andrew Davis wrote:
> Use devm version of gpiochip add function to handle removal for us.

OK, everything else is already using devm, so we are fine to have this also
being managed.

Pushed to my review and testing queue, thanks!

-- 
With Best Regards,
Andy Shevchenko