* [PATCH] iio: health: afe4403: retrieve a valid iio_dev in suspend/resume
@ 2017-01-15 3:51 Alison Schofield
2017-01-16 16:38 ` Andrew F. Davis
0 siblings, 1 reply; 6+ messages in thread
From: Alison Schofield @ 2017-01-15 3:51 UTC (permalink / raw)
To: jic23; +Cc: afd, knaack.h, lars, pmeerw, linux-iio, linux-kernel
The suspend/resume functions were using dev_to_iio_dev() to get
the iio_dev. That only works on IIO dev's. Replace it with spi
functions to get the correct iio_dev.
Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
drivers/iio/health/afe4403.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index 9a08146..6bb23a4 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -422,7 +422,7 @@ MODULE_DEVICE_TABLE(of, afe4403_of_match);
static int __maybe_unused afe4403_suspend(struct device *dev)
{
- struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
struct afe4403_data *afe = iio_priv(indio_dev);
int ret;
@@ -443,7 +443,7 @@ static int __maybe_unused afe4403_suspend(struct device *dev)
static int __maybe_unused afe4403_resume(struct device *dev)
{
- struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
struct afe4403_data *afe = iio_priv(indio_dev);
int ret;
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: health: afe4403: retrieve a valid iio_dev in suspend/resume
2017-01-15 3:51 [PATCH] iio: health: afe4403: retrieve a valid iio_dev in suspend/resume Alison Schofield
@ 2017-01-16 16:38 ` Andrew F. Davis
2017-01-16 18:10 ` Alison Schofield
2017-01-21 13:12 ` Jonathan Cameron
0 siblings, 2 replies; 6+ messages in thread
From: Andrew F. Davis @ 2017-01-16 16:38 UTC (permalink / raw)
To: Alison Schofield, jic23; +Cc: knaack.h, lars, pmeerw, linux-iio, linux-kernel
On 01/14/2017 09:51 PM, Alison Schofield wrote:
> The suspend/resume functions were using dev_to_iio_dev() to get
> the iio_dev. That only works on IIO dev's. Replace it with spi
> functions to get the correct iio_dev.
>
> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
Was this found with an automated tool? If not, it might be nice to have
a Coccinelle style check for this. Anyway for this and the afe4404
version patch:
Acked-by: Andrew F. Davis <afd@ti.com>
> ---
> drivers/iio/health/afe4403.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index 9a08146..6bb23a4 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -422,7 +422,7 @@ MODULE_DEVICE_TABLE(of, afe4403_of_match);
>
> static int __maybe_unused afe4403_suspend(struct device *dev)
> {
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
> struct afe4403_data *afe = iio_priv(indio_dev);
> int ret;
>
> @@ -443,7 +443,7 @@ static int __maybe_unused afe4403_suspend(struct device *dev)
>
> static int __maybe_unused afe4403_resume(struct device *dev)
> {
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
> struct afe4403_data *afe = iio_priv(indio_dev);
> int ret;
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: health: afe4403: retrieve a valid iio_dev in suspend/resume
2017-01-16 16:38 ` Andrew F. Davis
@ 2017-01-16 18:10 ` Alison Schofield
2017-01-16 18:22 ` Andrew F. Davis
2017-01-21 13:12 ` Jonathan Cameron
1 sibling, 1 reply; 6+ messages in thread
From: Alison Schofield @ 2017-01-16 18:10 UTC (permalink / raw)
To: Andrew F. Davis; +Cc: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel
On Mon, Jan 16, 2017 at 10:38:27AM -0600, Andrew F. Davis wrote:
> On 01/14/2017 09:51 PM, Alison Schofield wrote:
> > The suspend/resume functions were using dev_to_iio_dev() to get
> > the iio_dev. That only works on IIO dev's. Replace it with spi
> > functions to get the correct iio_dev.
> >
> > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
>
> Was this found with an automated tool? If not, it might be nice to have
> a Coccinelle style check for this. Anyway for this and the afe4404
> version patch:
>
> Acked-by: Andrew F. Davis <afd@ti.com>
>
Hi Andrew,
Just caught my eye while looking at these drivers for another reason:
they popped up in a cocci scan looking for drivers with regmap and dev
struct in global data. The dev struct may be redundant since it
can be retrieved from regmap. I'm going to look at that a bit further
and send a patch if appropriate.
Back to dev_to_iio_dev...it's a case of history repeating itself, but
not so often anymore. I grep'd one more that I'll patch. So, although
it appears close to extinction, a cocci check would be great. It would
be even more valuble if worked with a more general check of the IIO
conversion/naming functions that are ripe for misuse.
I'll propose this as an intern task.
If anyone has read this far and wants to chime in with their favorite
misused funcs to add to the todo list...please do.
thanks,
alisons
> > ---
> > drivers/iio/health/afe4403.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> > index 9a08146..6bb23a4 100644
> > --- a/drivers/iio/health/afe4403.c
> > +++ b/drivers/iio/health/afe4403.c
> > @@ -422,7 +422,7 @@ MODULE_DEVICE_TABLE(of, afe4403_of_match);
> >
> > static int __maybe_unused afe4403_suspend(struct device *dev)
> > {
> > - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > + struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
> > struct afe4403_data *afe = iio_priv(indio_dev);
> > int ret;
> >
> > @@ -443,7 +443,7 @@ static int __maybe_unused afe4403_suspend(struct device *dev)
> >
> > static int __maybe_unused afe4403_resume(struct device *dev)
> > {
> > - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > + struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
> > struct afe4403_data *afe = iio_priv(indio_dev);
> > int ret;
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: health: afe4403: retrieve a valid iio_dev in suspend/resume
2017-01-16 18:10 ` Alison Schofield
@ 2017-01-16 18:22 ` Andrew F. Davis
2017-01-16 18:36 ` Alison Schofield
0 siblings, 1 reply; 6+ messages in thread
From: Andrew F. Davis @ 2017-01-16 18:22 UTC (permalink / raw)
To: Alison Schofield; +Cc: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel
On 01/16/2017 12:10 PM, Alison Schofield wrote:
> On Mon, Jan 16, 2017 at 10:38:27AM -0600, Andrew F. Davis wrote:
>> On 01/14/2017 09:51 PM, Alison Schofield wrote:
>>> The suspend/resume functions were using dev_to_iio_dev() to get
>>> the iio_dev. That only works on IIO dev's. Replace it with spi
>>> functions to get the correct iio_dev.
>>>
>>> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
>>
>> Was this found with an automated tool? If not, it might be nice to have
>> a Coccinelle style check for this. Anyway for this and the afe4404
>> version patch:
>>
>> Acked-by: Andrew F. Davis <afd@ti.com>
>>
> Hi Andrew,
>
> Just caught my eye while looking at these drivers for another reason:
> they popped up in a cocci scan looking for drivers with regmap and dev
> struct in global data. The dev struct may be redundant since it
> can be retrieved from regmap. I'm going to look at that a bit further
> and send a patch if appropriate.
>
I have seen these patches before, and I always NACK them when I can. My
stance is that just because we can find creative ways to recover
information doesn't mean we should. It doesn't and anything and it is
much more clear to me to just keep a copy of *dev to reference when
needed, rather than some round-about method of fetching it through other
entities.
> Back to dev_to_iio_dev...it's a case of history repeating itself, but
> not so often anymore. I grep'd one more that I'll patch. So, although
> it appears close to extinction, a cocci check would be great. It would
> be even more valuble if worked with a more general check of the IIO
> conversion/naming functions that are ripe for misuse.
>
I'm not a huge fan of all these macro tricks to fetch types for this
reason here, we lose visibility into the containing types. Like here we
have a function (dev_to_iio_dev) that goes from dev to iio_dev with no
real compile time way to know if it is the *right* dev type...
> I'll propose this as an intern task.
>
> If anyone has read this far and wants to chime in with their favorite
> misused funcs to add to the todo list...please do.
>
> thanks,
> alisons
>
>>> ---
>>> drivers/iio/health/afe4403.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
>>> index 9a08146..6bb23a4 100644
>>> --- a/drivers/iio/health/afe4403.c
>>> +++ b/drivers/iio/health/afe4403.c
>>> @@ -422,7 +422,7 @@ MODULE_DEVICE_TABLE(of, afe4403_of_match);
>>>
>>> static int __maybe_unused afe4403_suspend(struct device *dev)
>>> {
>>> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
>>> struct afe4403_data *afe = iio_priv(indio_dev);
>>> int ret;
>>>
>>> @@ -443,7 +443,7 @@ static int __maybe_unused afe4403_suspend(struct device *dev)
>>>
>>> static int __maybe_unused afe4403_resume(struct device *dev)
>>> {
>>> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
>>> struct afe4403_data *afe = iio_priv(indio_dev);
>>> int ret;
>>>
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: health: afe4403: retrieve a valid iio_dev in suspend/resume
2017-01-16 18:22 ` Andrew F. Davis
@ 2017-01-16 18:36 ` Alison Schofield
0 siblings, 0 replies; 6+ messages in thread
From: Alison Schofield @ 2017-01-16 18:36 UTC (permalink / raw)
To: Andrew F. Davis; +Cc: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel
On Mon, Jan 16, 2017 at 12:22:03PM -0600, Andrew F. Davis wrote:
> On 01/16/2017 12:10 PM, Alison Schofield wrote:
> > On Mon, Jan 16, 2017 at 10:38:27AM -0600, Andrew F. Davis wrote:
> >> On 01/14/2017 09:51 PM, Alison Schofield wrote:
> >>> The suspend/resume functions were using dev_to_iio_dev() to get
> >>> the iio_dev. That only works on IIO dev's. Replace it with spi
> >>> functions to get the correct iio_dev.
> >>>
> >>> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> >>
> >> Was this found with an automated tool? If not, it might be nice to have
> >> a Coccinelle style check for this. Anyway for this and the afe4404
> >> version patch:
> >>
> >> Acked-by: Andrew F. Davis <afd@ti.com>
> >>
> > Hi Andrew,
> >
> > Just caught my eye while looking at these drivers for another reason:
> > they popped up in a cocci scan looking for drivers with regmap and dev
> > struct in global data. The dev struct may be redundant since it
> > can be retrieved from regmap. I'm going to look at that a bit further
> > and send a patch if appropriate.
> >
>
> I have seen these patches before, and I always NACK them when I can. My
> stance is that just because we can find creative ways to recover
> information doesn't mean we should. It doesn't and anything and it is
> much more clear to me to just keep a copy of *dev to reference when
> needed, rather than some round-about method of fetching it through other
> entities.
Thanks for the heads up...removed from queue.
alisons
>
> > Back to dev_to_iio_dev...it's a case of history repeating itself, but
> > not so often anymore. I grep'd one more that I'll patch. So, although
> > it appears close to extinction, a cocci check would be great. It would
> > be even more valuble if worked with a more general check of the IIO
> > conversion/naming functions that are ripe for misuse.
> >
>
> I'm not a huge fan of all these macro tricks to fetch types for this
> reason here, we lose visibility into the containing types. Like here we
> have a function (dev_to_iio_dev) that goes from dev to iio_dev with no
> real compile time way to know if it is the *right* dev type...
>
> > I'll propose this as an intern task.
> >
> > If anyone has read this far and wants to chime in with their favorite
> > misused funcs to add to the todo list...please do.
> >
> > thanks,
> > alisons
> >
> >>> ---
> >>> drivers/iio/health/afe4403.c | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> >>> index 9a08146..6bb23a4 100644
> >>> --- a/drivers/iio/health/afe4403.c
> >>> +++ b/drivers/iio/health/afe4403.c
> >>> @@ -422,7 +422,7 @@ MODULE_DEVICE_TABLE(of, afe4403_of_match);
> >>>
> >>> static int __maybe_unused afe4403_suspend(struct device *dev)
> >>> {
> >>> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> >>> + struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
> >>> struct afe4403_data *afe = iio_priv(indio_dev);
> >>> int ret;
> >>>
> >>> @@ -443,7 +443,7 @@ static int __maybe_unused afe4403_suspend(struct device *dev)
> >>>
> >>> static int __maybe_unused afe4403_resume(struct device *dev)
> >>> {
> >>> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> >>> + struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
> >>> struct afe4403_data *afe = iio_priv(indio_dev);
> >>> int ret;
> >>>
> >>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: health: afe4403: retrieve a valid iio_dev in suspend/resume
2017-01-16 16:38 ` Andrew F. Davis
2017-01-16 18:10 ` Alison Schofield
@ 2017-01-21 13:12 ` Jonathan Cameron
1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2017-01-21 13:12 UTC (permalink / raw)
To: Andrew F. Davis, Alison Schofield
Cc: knaack.h, lars, pmeerw, linux-iio, linux-kernel
On 16/01/17 16:38, Andrew F. Davis wrote:
> On 01/14/2017 09:51 PM, Alison Schofield wrote:
>> The suspend/resume functions were using dev_to_iio_dev() to get
>> the iio_dev. That only works on IIO dev's. Replace it with spi
>> functions to get the correct iio_dev.
>>
>> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
>
> Was this found with an automated tool? If not, it might be nice to have
> a Coccinelle style check for this. Anyway for this and the afe4404
> version patch:
>
> Acked-by: Andrew F. Davis <afd@ti.com>
Applied to the fixes-togreg branch of iio.git.
Thanks,
Jonathan
>
>> ---
>> drivers/iio/health/afe4403.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
>> index 9a08146..6bb23a4 100644
>> --- a/drivers/iio/health/afe4403.c
>> +++ b/drivers/iio/health/afe4403.c
>> @@ -422,7 +422,7 @@ MODULE_DEVICE_TABLE(of, afe4403_of_match);
>>
>> static int __maybe_unused afe4403_suspend(struct device *dev)
>> {
>> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> + struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
>> struct afe4403_data *afe = iio_priv(indio_dev);
>> int ret;
>>
>> @@ -443,7 +443,7 @@ static int __maybe_unused afe4403_suspend(struct device *dev)
>>
>> static int __maybe_unused afe4403_resume(struct device *dev)
>> {
>> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> + struct iio_dev *indio_dev = spi_get_drvdata(to_spi_device(dev));
>> struct afe4403_data *afe = iio_priv(indio_dev);
>> int ret;
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-21 13:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-15 3:51 [PATCH] iio: health: afe4403: retrieve a valid iio_dev in suspend/resume Alison Schofield
2017-01-16 16:38 ` Andrew F. Davis
2017-01-16 18:10 ` Alison Schofield
2017-01-16 18:22 ` Andrew F. Davis
2017-01-16 18:36 ` Alison Schofield
2017-01-21 13:12 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.