Re: [PATCH v3 1/2] iio: accel: add support for FXLS8962AF/FXLS8964AF accelerometers

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 28 Apr 2021 13:34:01 +0200
Sean Nyekjaer <sean@geanix.com> wrote:

> On 28/04/2021 10.01, Sean Nyekjaer wrote:
> > |static int fxls8962af_read_raw(struct iio_dev *indio_dev, + struct 
> > iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + 
> > struct fxls8962af_data *data = iio_priv(indio_dev); + int ret; + + 
> > switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = 
> > iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + 
> > switch (chan->type) { + case IIO_TEMP: + ret = 
> > fxls8962af_get_temp(data, val); + break; + case IIO_ACCEL: + ret = 
> > fxls8962af_get_axis(data, chan, val); + break; + default: + ret = 
> > -EINVAL; + } + + iio_device_release_direct_mode(indio_dev); + return ret;|  
> |Maybe I should remove iio_device_claim_direct_mode() and release(),
> there is actually nothing in the datasheet that block us from reading 
> temp and raw
> axis data with streaming enabled to the internal fifo.
> We have a use-case for reading temperature data while streaming 
> accelerometer data.
> 
> Could I just check for ||iio_buffer_enabled||() and skip the drdy and 
> power_on/off in
> ||fxls8962af_get_temp() and ||fxls8962af_get_axis()||?

Absolutely fine to have these enabled whilst doing fifo accesses as
long as they don't changing timing etc (they do on some devices).

Sounds like you'd need to reference count power_on / off rather than
basing it on whether the buffer is enabled (to avoid a race condition).

I think runtime pm should handle that for you anyway. 

> 
> /Sean
> |