Use devm version of gpiochip add function to handle removal for us.
Signed-off-by: Andrew Davis <[email protected]>
---
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
Use devm version of gpiochip add function to handle removal for us.
While here update copyright and module author.
Signed-off-by: Andrew Davis <[email protected]>
---
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 <[email protected]>
+ * Copyright (C) 2015-2023 Texas Instruments Incorporated - https://www.ti.com/
+ * Andrew Davis <[email protected]>
*/
#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 <[email protected]>");
+MODULE_AUTHOR("Andrew Davis <[email protected]>");
MODULE_DESCRIPTION("SPI Compatible PISO Shift Register GPIO Driver");
MODULE_LICENSE("GPL v2");
--
2.39.2
Use devm version of gpiochip add function to handle removal for us.
Signed-off-by: Andrew Davis <[email protected]>
---
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
Use devm version of gpiochip add function to handle removal for us.
Signed-off-by: Andrew Davis <[email protected]>
---
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
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
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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)
>
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
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
On Tue, Mar 7, 2023 at 5:54 PM Andrew Davis <[email protected]> wrote:
>
> Use devm version of gpiochip add function to handle removal for us.
>
> Signed-off-by: Andrew Davis <[email protected]>
> ---
> drivers/gpio/gpio-sch311x.c | 25 ++-----------------------
> 1 file changed, 2 insertions(+), 23 deletions(-)
>
Applied, thanks!
Bart
On Tue, Mar 7, 2023 at 6:55 PM Andrew Davis <[email protected]> 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
On Wed, Mar 8, 2023 at 11:24 AM Bartosz Golaszewski <[email protected]> wrote:
>
> On Tue, Mar 7, 2023 at 5:54 PM Andrew Davis <[email protected]> wrote:
> >
> > Use devm version of gpiochip add function to handle removal for us.
> >
> > Signed-off-by: Andrew Davis <[email protected]>
> > ---
> > 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
On 3/8/23 4:32 AM, Bartosz Golaszewski wrote:
> On Wed, Mar 8, 2023 at 11:24 AM Bartosz Golaszewski <[email protected]> wrote:
>>
>> On Tue, Mar 7, 2023 at 5:54 PM Andrew Davis <[email protected]> wrote:
>>>
>>> Use devm version of gpiochip add function to handle removal for us.
>>>
>>> Signed-off-by: Andrew Davis <[email protected]>
>>> ---
>>> 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
On Wed, Mar 8, 2023 at 5:50 PM Andrew Davis <[email protected]> wrote:
> On 3/8/23 4:32 AM, Bartosz Golaszewski wrote:
> > On Wed, Mar 8, 2023 at 11:24 AM Bartosz Golaszewski <[email protected]> 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
On 3/8/23 9:53 AM, Andy Shevchenko wrote:
> On Wed, Mar 8, 2023 at 5:50 PM Andrew Davis <[email protected]> wrote:
>> On 3/8/23 4:32 AM, Bartosz Golaszewski wrote:
>>> On Wed, Mar 8, 2023 at 11:24 AM Bartosz Golaszewski <[email protected]> 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
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 <[email protected]> wrote:
> > > On 3/8/23 4:32 AM, Bartosz Golaszewski wrote:
> > > > On Wed, Mar 8, 2023 at 11:24 AM Bartosz Golaszewski <[email protected]> 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