linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio:dac:max517: Use devm_iio_device_register()
@ 2021-03-14  9:33 Mugilraj Dhavachelvan
  2021-03-14 11:07 ` Alexandru Ardelean
  2021-03-14 17:57 ` [PATCH v2] iio:dac:max517.c: " Mugilraj Dhavachelvan
  0 siblings, 2 replies; 6+ messages in thread
From: Mugilraj Dhavachelvan @ 2021-03-14  9:33 UTC (permalink / raw)
  To: ardeleanalex, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	Mugilraj Dhavachelvan, linux-iio, linux-kernel

Use devm_iio_device_register() to avoid remove function and 
drop explicit call to iio_device_unregister().

Signed-off-by: Mugilraj Dhavachelvan <dmugil2000@gmail.com>
---
 drivers/iio/dac/max517.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
index 7e01838ef4d0..5f72f126162d 100644
--- a/drivers/iio/dac/max517.c
+++ b/drivers/iio/dac/max517.c
@@ -189,13 +189,7 @@ static int max517_probe(struct i2c_client *client,
 			data->vref_mv[chan] = platform_data->vref_mv[chan];
 	}
 
-	return iio_device_register(indio_dev);
-}
-
-static int max517_remove(struct i2c_client *client)
-{
-	iio_device_unregister(i2c_get_clientdata(client));
-	return 0;
+	return devm_iio_device_register(&client->dev, indio_dev);
 }
 
 static const struct i2c_device_id max517_id[] = {
@@ -214,7 +208,6 @@ static struct i2c_driver max517_driver = {
 		.pm	= &max517_pm_ops,
 	},
 	.probe		= max517_probe,
-	.remove		= max517_remove,
 	.id_table	= max517_id,
 };
 module_i2c_driver(max517_driver);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] iio:dac:max517: Use devm_iio_device_register()
  2021-03-14  9:33 [PATCH] iio:dac:max517: Use devm_iio_device_register() Mugilraj Dhavachelvan
@ 2021-03-14 11:07 ` Alexandru Ardelean
  2021-03-14 13:31   ` Mugilraj D
  2021-03-14 17:57 ` [PATCH v2] iio:dac:max517.c: " Mugilraj Dhavachelvan
  1 sibling, 1 reply; 6+ messages in thread
From: Alexandru Ardelean @ 2021-03-14 11:07 UTC (permalink / raw)
  To: Mugilraj Dhavachelvan
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio, LKML

On Sun, Mar 14, 2021 at 11:34 AM Mugilraj Dhavachelvan
<dmugil2000@gmail.com> wrote:
>
> Use devm_iio_device_register() to avoid remove function and
> drop explicit call to iio_device_unregister().
>
> Signed-off-by: Mugilraj Dhavachelvan <dmugil2000@gmail.com>
> ---
>  drivers/iio/dac/max517.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
> index 7e01838ef4d0..5f72f126162d 100644
> --- a/drivers/iio/dac/max517.c
> +++ b/drivers/iio/dac/max517.c
> @@ -189,13 +189,7 @@ static int max517_probe(struct i2c_client *client,
>                         data->vref_mv[chan] = platform_data->vref_mv[chan];
>         }
>
> -       return iio_device_register(indio_dev);
> -}
> -

In this case you can also remove the line with
     i2c_set_clientdata(client, indio_dev);

A lot of times, when functions like i2c_get_clientdata() get removed,
the i2c_set_clientdata() function becomes useless.
i.e. it stores some data that will never be used in the lifetime of the driver.

It isn't always the case that you can do that; so, some care must be
taken to avoid special cases.
But in this case, you can remove the i2c_set_clientdata() call.


> -static int max517_remove(struct i2c_client *client)
> -{
> -       iio_device_unregister(i2c_get_clientdata(client));
> -       return 0;
> +       return devm_iio_device_register(&client->dev, indio_dev);
>  }
>
>  static const struct i2c_device_id max517_id[] = {
> @@ -214,7 +208,6 @@ static struct i2c_driver max517_driver = {
>                 .pm     = &max517_pm_ops,
>         },
>         .probe          = max517_probe,
> -       .remove         = max517_remove,
>         .id_table       = max517_id,
>  };
>  module_i2c_driver(max517_driver);
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iio:dac:max517: Use devm_iio_device_register()
  2021-03-14 11:07 ` Alexandru Ardelean
@ 2021-03-14 13:31   ` Mugilraj D
  0 siblings, 0 replies; 6+ messages in thread
From: Mugilraj D @ 2021-03-14 13:31 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio, LKML



On 14/03/21 4:37 pm, Alexandru Ardelean wrote:
> On Sun, Mar 14, 2021 at 11:34 AM Mugilraj Dhavachelvan
> <dmugil2000@gmail.com> wrote:
>>
>> Use devm_iio_device_register() to avoid remove function and
>> drop explicit call to iio_device_unregister().
>>
>> Signed-off-by: Mugilraj Dhavachelvan <dmugil2000@gmail.com>
>> ---
>>  drivers/iio/dac/max517.c | 9 +--------
>>  1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
>> index 7e01838ef4d0..5f72f126162d 100644
>> --- a/drivers/iio/dac/max517.c
>> +++ b/drivers/iio/dac/max517.c
>> @@ -189,13 +189,7 @@ static int max517_probe(struct i2c_client *client,
>>                         data->vref_mv[chan] = platform_data->vref_mv[chan];
>>         }
>>
>> -       return iio_device_register(indio_dev);
>> -}
>> -
> 
> In this case you can also remove the line with
>      i2c_set_clientdata(client, indio_dev);
> 
> A lot of times, when functions like i2c_get_clientdata() get removed,
> the i2c_set_clientdata() function becomes useless.
> i.e. it stores some data that will never be used in the lifetime of the driver.
> 
> It isn't always the case that you can do that; so, some care must be
> taken to avoid special cases.
> But in this case, you can remove the i2c_set_clientdata() call.
> 
Ack, I'll send that in a while. I should send with [V2] tag right.
> 
>> -static int max517_remove(struct i2c_client *client)
>> -{
>> -       iio_device_unregister(i2c_get_clientdata(client));
>> -       return 0;
>> +       return devm_iio_device_register(&client->dev, indio_dev);
>>  }
>>
>>  static const struct i2c_device_id max517_id[] = {
>> @@ -214,7 +208,6 @@ static struct i2c_driver max517_driver = {
>>                 .pm     = &max517_pm_ops,
>>         },
>>         .probe          = max517_probe,
>> -       .remove         = max517_remove,
>>         .id_table       = max517_id,
>>  };
>>  module_i2c_driver(max517_driver);
>> --
>> 2.25.1
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] iio:dac:max517.c: Use devm_iio_device_register()
  2021-03-14  9:33 [PATCH] iio:dac:max517: Use devm_iio_device_register() Mugilraj Dhavachelvan
  2021-03-14 11:07 ` Alexandru Ardelean
@ 2021-03-14 17:57 ` Mugilraj Dhavachelvan
  2021-03-15  5:55   ` Lars-Peter Clausen
  1 sibling, 1 reply; 6+ messages in thread
From: Mugilraj Dhavachelvan @ 2021-03-14 17:57 UTC (permalink / raw)
  To: ardeleanalex, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	Mugilraj Dhavachelvan, linux-iio, linux-kernel

Use devm_iio_device_register() to avoid remove function and
drop explicit call to iio_device_unregister().

Signed-off-by: Mugilraj Dhavachelvan <dmugil2000@gmail.com>

changes v1->v2:
	-As sugested by Alexandru removed i2c_set_clientdata() because the stored
	 data will be never used.
---
 drivers/iio/dac/max517.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
index 7e01838ef4d0..00f0062a0298 100644
--- a/drivers/iio/dac/max517.c
+++ b/drivers/iio/dac/max517.c
@@ -153,7 +153,6 @@ static int max517_probe(struct i2c_client *client,
 	if (!indio_dev)
 		return -ENOMEM;
 	data = iio_priv(indio_dev);
-	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
 
 	/* establish that the iio_dev is a child of the i2c device */
@@ -189,13 +188,7 @@ static int max517_probe(struct i2c_client *client,
 			data->vref_mv[chan] = platform_data->vref_mv[chan];
 	}
 
-	return iio_device_register(indio_dev);
-}
-
-static int max517_remove(struct i2c_client *client)
-{
-	iio_device_unregister(i2c_get_clientdata(client));
-	return 0;
+	return devm_iio_device_register(&client->dev, indio_dev);
 }
 
 static const struct i2c_device_id max517_id[] = {
@@ -214,7 +207,6 @@ static struct i2c_driver max517_driver = {
 		.pm	= &max517_pm_ops,
 	},
 	.probe		= max517_probe,
-	.remove		= max517_remove,
 	.id_table	= max517_id,
 };
 module_i2c_driver(max517_driver);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] iio:dac:max517.c: Use devm_iio_device_register()
  2021-03-14 17:57 ` [PATCH v2] iio:dac:max517.c: " Mugilraj Dhavachelvan
@ 2021-03-15  5:55   ` Lars-Peter Clausen
  2021-03-20 14:39     ` Jonathan Cameron
  0 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2021-03-15  5:55 UTC (permalink / raw)
  To: Mugilraj Dhavachelvan, ardeleanalex, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, linux-iio, linux-kernel

On 3/14/21 6:57 PM, Mugilraj Dhavachelvan wrote:
> Use devm_iio_device_register() to avoid remove function and
> drop explicit call to iio_device_unregister().
>
> Signed-off-by: Mugilraj Dhavachelvan <dmugil2000@gmail.com>
>
> changes v1->v2:
> 	-As sugested by Alexandru removed i2c_set_clientdata() because the stored
> 	 data will be never used.

Hi,

This looks good!

Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>

One thing process wise. I know it is tempting to send version 2 as a 
reply to version 1, but this way it is also easy for the messages to get 
lost in longer threads. At least for the IIO mailinglist we have decided 
that it is best to send new versions of a patch series as their own 
threads so that they stand own and get noticed.

- Lars

> ---
>   drivers/iio/dac/max517.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
> index 7e01838ef4d0..00f0062a0298 100644
> --- a/drivers/iio/dac/max517.c
> +++ b/drivers/iio/dac/max517.c
> @@ -153,7 +153,6 @@ static int max517_probe(struct i2c_client *client,
>   	if (!indio_dev)
>   		return -ENOMEM;
>   	data = iio_priv(indio_dev);
> -	i2c_set_clientdata(client, indio_dev);
>   	data->client = client;
>   
>   	/* establish that the iio_dev is a child of the i2c device */
> @@ -189,13 +188,7 @@ static int max517_probe(struct i2c_client *client,
>   			data->vref_mv[chan] = platform_data->vref_mv[chan];
>   	}
>   
> -	return iio_device_register(indio_dev);
> -}
> -
> -static int max517_remove(struct i2c_client *client)
> -{
> -	iio_device_unregister(i2c_get_clientdata(client));
> -	return 0;
> +	return devm_iio_device_register(&client->dev, indio_dev);
>   }
>   
>   static const struct i2c_device_id max517_id[] = {
> @@ -214,7 +207,6 @@ static struct i2c_driver max517_driver = {
>   		.pm	= &max517_pm_ops,
>   	},
>   	.probe		= max517_probe,
> -	.remove		= max517_remove,
>   	.id_table	= max517_id,
>   };
>   module_i2c_driver(max517_driver);



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] iio:dac:max517.c: Use devm_iio_device_register()
  2021-03-15  5:55   ` Lars-Peter Clausen
@ 2021-03-20 14:39     ` Jonathan Cameron
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2021-03-20 14:39 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Mugilraj Dhavachelvan, ardeleanalex, Hartmut Knaack,
	Peter Meerwald-Stadler, linux-iio, linux-kernel

On Mon, 15 Mar 2021 06:55:53 +0100
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 3/14/21 6:57 PM, Mugilraj Dhavachelvan wrote:
> > Use devm_iio_device_register() to avoid remove function and
> > drop explicit call to iio_device_unregister().
> >
> > Signed-off-by: Mugilraj Dhavachelvan <dmugil2000@gmail.com>
> >
> > changes v1->v2:
> > 	-As sugested by Alexandru removed i2c_set_clientdata() because the stored
> > 	 data will be never used.  
This changelog belongs below the -- so that it doesn't get picked
up for the permanent log in by git am.  I fixed that up whilst applying.

Otherwise looks good.

Applied to the togreg branch of iio.git and pushed out as testing
to let the autobuilder bots like 0-day see if they can find anything
we missed.  That usually takes about a day and I need to fix up any
resulting issues before pushing this out as the non-rebasing togreg
branch on which pull requests are then based.

Thanks,

Jonathan

> 
> Hi,
> 
> This looks good!
> 
> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> One thing process wise. I know it is tempting to send version 2 as a 
> reply to version 1, but this way it is also easy for the messages to get 
> lost in longer threads. At least for the IIO mailinglist we have decided 
> that it is best to send new versions of a patch series as their own 
> threads so that they stand own and get noticed.
> 
> - Lars
> 
> > ---
> >   drivers/iio/dac/max517.c | 10 +---------
> >   1 file changed, 1 insertion(+), 9 deletions(-)
> >
> > diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
> > index 7e01838ef4d0..00f0062a0298 100644
> > --- a/drivers/iio/dac/max517.c
> > +++ b/drivers/iio/dac/max517.c
> > @@ -153,7 +153,6 @@ static int max517_probe(struct i2c_client *client,
> >   	if (!indio_dev)
> >   		return -ENOMEM;
> >   	data = iio_priv(indio_dev);
> > -	i2c_set_clientdata(client, indio_dev);
> >   	data->client = client;
> >   
> >   	/* establish that the iio_dev is a child of the i2c device */
> > @@ -189,13 +188,7 @@ static int max517_probe(struct i2c_client *client,
> >   			data->vref_mv[chan] = platform_data->vref_mv[chan];
> >   	}
> >   
> > -	return iio_device_register(indio_dev);
> > -}
> > -
> > -static int max517_remove(struct i2c_client *client)
> > -{
> > -	iio_device_unregister(i2c_get_clientdata(client));
> > -	return 0;
> > +	return devm_iio_device_register(&client->dev, indio_dev);
> >   }
> >   
> >   static const struct i2c_device_id max517_id[] = {
> > @@ -214,7 +207,6 @@ static struct i2c_driver max517_driver = {
> >   		.pm	= &max517_pm_ops,
> >   	},
> >   	.probe		= max517_probe,
> > -	.remove		= max517_remove,
> >   	.id_table	= max517_id,
> >   };
> >   module_i2c_driver(max517_driver);  
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-20 14:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-14  9:33 [PATCH] iio:dac:max517: Use devm_iio_device_register() Mugilraj Dhavachelvan
2021-03-14 11:07 ` Alexandru Ardelean
2021-03-14 13:31   ` Mugilraj D
2021-03-14 17:57 ` [PATCH v2] iio:dac:max517.c: " Mugilraj Dhavachelvan
2021-03-15  5:55   ` Lars-Peter Clausen
2021-03-20 14:39     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).