[PATCH] staging: iio: accel: Replace iio_device_register with devm_iio_device_register

[PATCH] staging: iio: accel: Replace iio_device_register with devm_iio_device_register

[Amitoj Kaur Chawla]

Devm_ functions allocate memory that is released when a driver
detaches. Replace iio_device_register with the resource-managed 
version devm_iio_device_register. Subsequently, remove 
iio_device_unregister from probe and remove functions of this driver.

Also, an unnecessary label was dropped.

Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
 drivers/staging/iio/accel/sca3000_core.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/accel/sca3000_core.c b/drivers/staging/iio/accel/sca3000_core.c
index a8f533a..f7dd359 100644
--- a/drivers/staging/iio/accel/sca3000_core.c
+++ b/drivers/staging/iio/accel/sca3000_core.c
@@ -1123,7 +1123,7 @@ static int sca3000_probe(struct spi_device *spi)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	sca3000_configure_ring(indio_dev);
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 	if (ret < 0)
 		return ret;
 
@@ -1135,7 +1135,7 @@ static int sca3000_probe(struct spi_device *spi)
 					   "sca3000",
 					   indio_dev);
 		if (ret)
-			goto error_unregister_dev;
+			return ret;
 	}
 	sca3000_register_ring_funcs(indio_dev);
 	ret = sca3000_clean_setup(st);
@@ -1146,8 +1146,6 @@ static int sca3000_probe(struct spi_device *spi)
 error_free_irq:
 	if (spi->irq)
 		free_irq(spi->irq, indio_dev);
-error_unregister_dev:
-	iio_device_unregister(indio_dev);
 	return ret;
 }
 
@@ -1178,7 +1176,6 @@ static int sca3000_remove(struct spi_device *spi)
 	sca3000_stop_all_interrupts(st);
 	if (spi->irq)
 		free_irq(spi->irq, indio_dev);
-	iio_device_unregister(indio_dev);
 	sca3000_unconfigure_ring(indio_dev);
 
 	return 0;
-- 
1.9.1

reordering remove actions in iio drivers

[Alison Schofield]

Hi Amitoj,

I just worked on a similar patch, and yours caught my attention.
It's not clear that it is OK to reorder the .remove actions.
Although it may seem counterintuitive to do anything *after*
you've unregistered your device, it is indeed done purposely
and safely in many iio drivers.  It usually needs to stay that way.

Check out more discussions of this on linux-iio.

And, consider the source...I'm just trying to figure this stuff
out too!

alisons

Re: [Outreachy kernel] reordering remove actions in iio drivers

[Julia Lawall]

On Sun, 28 Feb 2016, Alison Schofield wrote:

> Hi Amitoj,
> 
> I just worked on a similar patch, and yours caught my attention.
> It's not clear that it is OK to reorder the .remove actions.
> Although it may seem counterintuitive to do anything *after*
> you've unregistered your device, it is indeed done purposely
> and safely in many iio drivers.  It usually needs to stay that way.
> 
> Check out more discussions of this on linux-iio.
> 
> And, consider the source...I'm just trying to figure this stuff
> out too!

Which patch of Amitoj's are you referring to (or which file), and if 
possible do you have a more specific link for the discussion on linux-iio?

thanks,
julia

Re: [Outreachy kernel] reordering remove actions in iio drivers

[Alison Schofield]

On Sun, Feb 28, 2016 at 06:56:56PM +0100, Julia Lawall wrote:
> On Sun, 28 Feb 2016, Alison Schofield wrote:
> 
> > Hi Amitoj,
> > 
> > I just worked on a similar patch, and yours caught my attention.
> > It's not clear that it is OK to reorder the .remove actions.
> > Although it may seem counterintuitive to do anything *after*
> > you've unregistered your device, it is indeed done purposely
> > and safely in many iio drivers.  It usually needs to stay that way.
> > 
> > Check out more discussions of this on linux-iio.
> > 
> > And, consider the source...I'm just trying to figure this stuff
> > out too!
> 
> Which patch of Amitoj's are you referring to (or which file), and if 
> possible do you have a more specific link for the discussion on linux-iio?
> 
> thanks,
> julia

I'm seeing this threaded with original patch, but I guess you're not.
It is in reference to:
	[PATCH] staging: iio: accel: Replace iio_device_register with
	devm_iio_device_register

There is a lot of linux-iio discussion when this devm was originally
introduced with 30+ patches to migrate. Search devm_iio_device_register

and another later on when an intern tried to apply it:
http://article.gmane.org/gmane.linux.kernel.iio/13354/match=devm_iio_register

The accel case might be ok...just a heads up on the considerations.

alisons

Re: [Outreachy kernel] reordering remove actions in iio drivers

[Julia Lawall]

On Sun, 28 Feb 2016, Alison Schofield wrote:

> On Sun, Feb 28, 2016 at 06:56:56PM +0100, Julia Lawall wrote:
> > On Sun, 28 Feb 2016, Alison Schofield wrote:
> > 
> > > Hi Amitoj,
> > > 
> > > I just worked on a similar patch, and yours caught my attention.
> > > It's not clear that it is OK to reorder the .remove actions.
> > > Although it may seem counterintuitive to do anything *after*
> > > you've unregistered your device, it is indeed done purposely
> > > and safely in many iio drivers.  It usually needs to stay that way.
> > > 
> > > Check out more discussions of this on linux-iio.
> > > 
> > > And, consider the source...I'm just trying to figure this stuff
> > > out too!
> > 
> > Which patch of Amitoj's are you referring to (or which file), and if 
> > possible do you have a more specific link for the discussion on linux-iio?
> > 
> > thanks,
> > julia
> 
> I'm seeing this threaded with original patch, but I guess you're not.
> It is in reference to:
> 	[PATCH] staging: iio: accel: Replace iio_device_register with
> 	devm_iio_device_register

Thanks for the pointer.  Another question about this code is why there is 
no call to sca3000_unconfigure_ring in the probe function.  It loos like 
there should be one under the unregister call, if it is useful to do so in 
the remove function.

julia

> 
> There is a lot of linux-iio discussion when this devm was originally
> introduced with 30+ patches to migrate. Search devm_iio_device_register
> 
> and another later on when an intern tried to apply it:
> http://article.gmane.org/gmane.linux.kernel.iio/13354/match=devm_iio_register
> 
> The accel case might be ok...just a heads up on the considerations.
> 
> alisons
> 
> 

Re: [Outreachy kernel] reordering remove actions in iio drivers

[Amitoj Kaur Chawla]

On Sun, Feb 28, 2016 at 11:50 PM, Alison Schofield <amsfield22@gmail.com> wrote:
> On Sun, Feb 28, 2016 at 06:56:56PM +0100, Julia Lawall wrote:
>> On Sun, 28 Feb 2016, Alison Schofield wrote:
>>
>> > Hi Amitoj,
>> >
>> > I just worked on a similar patch, and yours caught my attention.
>> > It's not clear that it is OK to reorder the .remove actions.
>> > Although it may seem counterintuitive to do anything *after*
>> > you've unregistered your device, it is indeed done purposely
>> > and safely in many iio drivers.  It usually needs to stay that way.
>> >
>> > Check out more discussions of this on linux-iio.
>> >
>> > And, consider the source...I'm just trying to figure this stuff
>> > out too!
>>
>> Which patch of Amitoj's are you referring to (or which file), and if
>> possible do you have a more specific link for the discussion on linux-iio?
>>
>> thanks,
>> julia
>
> I'm seeing this threaded with original patch, but I guess you're not.
> It is in reference to:
>         [PATCH] staging: iio: accel: Replace iio_device_register with
>         devm_iio_device_register
>
> There is a lot of linux-iio discussion when this devm was originally
> introduced with 30+ patches to migrate. Search devm_iio_device_register
>
> and another later on when an intern tried to apply it:
> http://article.gmane.org/gmane.linux.kernel.iio/13354/match=devm_iio_register
>
> The accel case might be ok...just a heads up on the considerations.
>
> alisons
I did not know this, but after your heads up I did check previous such
patches accepted for iio drivers and I believe you're right. Thanks
for pointing it out. Will be careful about this in the future.

Thanks,
Amitoj

Re: [PATCH] staging: iio: accel: Replace iio_device_register with devm_iio_device_register

[Amitoj Kaur Chawla]

On Tue, Feb 23, 2016 at 1:13 AM, Amitoj Kaur Chawla
<amitoj1606@gmail.com> wrote:
> Devm_ functions allocate memory that is released when a driver
> detaches. Replace iio_device_register with the resource-managed
> version devm_iio_device_register. Subsequently, remove
> iio_device_unregister from probe and remove functions of this driver.
>
> Also, an unnecessary label was dropped.
>
> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> ---
This patch is incorrect and should be dropped.

Amitoj