iio: health: afe4403: retrieve a valid iio_dev in suspend/resume

PATCH Applied Alison Schofield 3 participants 6 messages Jan 15, 2017 – Jan 21, 2017 IIO

Share & tools

Alison Schofield

author

2017-01-15 03:51 UTC #0 lore raw

PATCH 1/1 iio: health: afe4403: retrieve a valid iio_dev in suspend/resume

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 <[email protected]>

---
 drivers/iio/health/afe4403.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/iio/health/afe4403.c +2 -2

index 9a08146..6bb23a4 100644
@@ -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;

Signature · 1 line

2.1.4

Andrew F. Davis

Acked-by on v1 1/1

2017-01-16 16:38 UTC #1 lore raw

Quoted 5 lines · Alison Schofield

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 <[email protected]>

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 <[email protected]>

Quoted 27 lines

---
 drivers/iio/health/afe4403.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/iio/health/afe4403.c +2 -2

index 9a08146..6bb23a4 100644
@@ -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;

Alison Schofield

author

2017-01-16 18:12 UTC #2 lore raw

Quoted 5 lines · Andrew F. Davis

Quoted 4 lines · Alison Schofield
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.

Quoted 8 lines

Quoted 1 line
Signed-off-by Alison Schofield <[email protected]>
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 <[email protected]>

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

Quoted 27 lines

---
 drivers/iio/health/afe4403.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/iio/health/afe4403.c +2 -2

index 9a08146..6bb23a4 100644
@@ -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;

Andrew F. Davis

2017-01-16 18:22 UTC #3 lore raw

Quoted 14 lines · Alison Schofield

Quoted 5 lines · Andrew F. Davis
Quoted 4 lines · Alison Schofield
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.
Quoted 8 lines
Quoted 1 line
Signed-off-by Alison Schofield <[email protected]>
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 <[email protected]>

Quoted 8 lines

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.

Quoted 6 lines

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...

Quoted 35 lines

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

Quoted 27 lines

---
 drivers/iio/health/afe4403.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/iio/health/afe4403.c +2 -2

index 9a08146..6bb23a4 100644
@@ -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;

Alison Schofield

author

2017-01-16 18:36 UTC #4 lore raw

Quoted 24 lines · Andrew F. Davis

Quoted 14 lines · Alison Schofield
Quoted 5 lines · Andrew F. Davis
Quoted 4 lines · Alison Schofield
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.
Quoted 8 lines
Quoted 1 line
Signed-off-by Alison Schofield <[email protected]>
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 <[email protected]>
Quoted 8 lines
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.

Quoted 6 lines

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

Quoted 48 lines

Quoted 7 lines
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...
Quoted 35 lines
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
Quoted 27 lines
Quoted 27 lines
---
 drivers/iio/health/afe4403.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/health/afe4403.c +2 -2
index 9a08146..6bb23a4 100644
@@ -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;
 

Jonathan Cameron

maintainer Applied

2017-01-21 13:12 UTC #5 lore raw

In reply to Andrew F. Davis

Quoted 5 lines · Andrew F. Davis

Quoted 4 lines · Alison Schofield
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.

Quoted 7 lines

Quoted 1 line
Signed-off-by Alison Schofield <[email protected]>
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 <[email protected]>

Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan

Quoted 33 lines

Quoted 20 lines

---
 drivers/iio/health/afe4403.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/iio/health/afe4403.c +1 -1

index 9a08146..6bb23a4 100644
@@ -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;

drivers/iio/health/afe4403.c static int __maybe_unused afe4403_suspend(struct device *dev) · line 449

Quoted 8 lines

drivers/iio/health/afe4403.c +1 -1

@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html