diff mbox series

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

Message ID 20230307165432.25484-1-afd@ti.com
State Accepted
Commit 56b16a9a80232fc70bebe31ded52c2f6fd3f7ddf
Headers show
Series [1/6] gpio: ich: Use devm_gpiochip_add_data() to simplify remove path | expand

Commit Message

Andrew Davis March 7, 2023, 4:54 p.m. UTC
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(-)

Comments

Bartosz Golaszewski March 8, 2023, 10:24 a.m. UTC | #1
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
Bartosz Golaszewski March 8, 2023, 10:27 a.m. UTC | #2
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
Bartosz Golaszewski March 8, 2023, 10:33 a.m. UTC | #3
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
Andy Shevchenko March 8, 2023, 3:53 p.m. UTC | #4
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.
Andy Shevchenko March 8, 2023, 4:32 p.m. UTC | #5
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.
diff mbox series

Patch

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);