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

Message ID 20230307165432.25484-3-afd@ti.com
State New
Headers
Series [1/6] gpio: ich: Use devm_gpiochip_add_data() to simplify remove path |

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-sch311x.c | 25 ++-----------------------
 1 file changed, 2 insertions(+), 23 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:32 a.m. UTC | #2
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
  
Andrew Davis March 8, 2023, 3:50 p.m. UTC | #3
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
  
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.
  
Andrew Davis March 8, 2023, 4:20 p.m. UTC | #5
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
  
Andy Shevchenko March 8, 2023, 4:32 p.m. UTC | #6
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.
  

Patch

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 = {