Re: [PATCH 15/16] iio: adc: at91-sama5d2_adc: add runtime pm support

Jonathan Cameron
2022-06-14T12:05:51Z

...

> >> @@ -1268,11 +1309,15 @@ static int at91_adc_buffer_prepare(struct iio_dev *indio_dev)
> >> if (!(iio_device_get_current_mode(indio_dev) & INDIO_ALL_TRIGGERED_MODES))
> >> return -EINVAL;
> >>
> >> + ret = pm_runtime_resume_and_get(st->dev);
> >
> > This seems unusual. I'd normally expect to see runtime pm left on whenever
> > a buffer is in use, but in this case you seem to let it autosuspend.
> >
> > That 'might' be fine as you might hit it often enough that it stays up whilst
> > doing DMA but it certainly seems odd and less than efficient.
> > Or possibly the use of the trigger is enough to keep it up.
>
> This is here because at91_adc_buffer_prepare() is called though
> .hwfifo_set_watermark which is called in iio_enable_buffers() before
> iio_trigger_attach_poll_func() which calls in turn
> at91_adc_configure_trigger() that turns on the power for the whole duration
> the buffers are enabled.
>
> It is necessary to have runtime resume here to be able to do proper
> register settings.

Runtime pm is reference counted so I'd take the view that either
* buffer being enabled or
* trigger being enabled

are reasons to leave the device powered up. That'll result in
needing it to be powered down in both buffer and trigger disables
but that shoudl be fine.

...

Thanks,

Jonathan