Re: [PATCH] iio: imu: st_lsm6dsx: introduce sw trigger support

[Jonathan Cameron <jic23@kernel.org>]

...

> > > +static irqreturn_t st_lsm6dsx_sw_trigger_handler_thread(int irq,
> > > +							void *private)
> > > +{
> > > +	struct iio_poll_func *pf = private;
> > > +	struct iio_dev *iio_dev = pf->indio_dev;
> > > +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> > > +	struct st_lsm6dsx_hw *hw = sensor->hw;
> > > +
> > > +	if (sensor->id == ST_LSM6DSX_ID_EXT0 ||
> > > +	    sensor->id == ST_LSM6DSX_ID_EXT1 ||
> > > +	    sensor->id == ST_LSM6DSX_ID_EXT2)
> > > +		st_lsm6dsx_shub_read_output(hw,
> > > +					    (u8 *)hw->scan[sensor->id].channels,
> > > +					    sizeof(hw->scan[sensor->id].channels));  
> > 
> > Are we guaranteed this particular size of readback?  I'm guessing a bit
> > as it's been a long time since I looked at this driver in detail, but could
> > we have sensors with either a different number of axes or different number
> > of registers per axis?
> > 
> > It might be neater to have two handlers, one for the EXTN cases and one
> > for the main sensors.  That would push this conditional down to the
> > point of registration.  I'm not sure it's worth it however so up to you...  
> 
> Hi Jonathan,
> 
> so far we support just magnetometers on sensor-hub (LIS2MDL and LIS3MDL).
> Both LIS2MDL and LIS3MDL have 3 axis, each of them is le16, so it is fine as it
> is for the moment. Do you prefer to be more generic and take into account new
> possible sensors? I am not sure when they will arrive :)

Fine as it stands.  You've thought about it and decided to postpone such a
change until it is necessary and that's fine by me.

Applied to the togreg branch of iio.git and pushed out as testing for 0-day
to poke at the tree and see if we missed anything.

Thanks,

Jonathan