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@xxxxxxxxx>
> >>
> >> 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@xxxxxx>
> >>
> > 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;
> >>>
> >>>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
