On Sun, 4 Nov 2018 19:00:39 +0100
Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxx> wrote:
> >
> > On Sun, 4 Nov 2018 15:39:04 +0100
> > Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxx> wrote:
> >
> > > i2c controller embedded in lsm6dx series can connect up to four
> > > slave devices using accelerometer sensor as trigger for i2c
> > > read/write operations.
> > > Introduce sensor hub support for lsm6dso sensor. Add register map
> > > for lis2mdl magnetometer sensor
> > >
> > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxx>
> >
> > I'd completely forgotten how this driver did the multiple device registration
> > so this had me completely confused initially ;)
> >
> > Perhaps just state here that the result is an entirely separate apparent device
> > and what it's functionality is at this point in the patch set?
> >
>
> ack, will do in v2
>
> > A few minor comments inline. This looks like it is coming together quite
> > nicely to me.
> >
>
> Thx :). I will address your comments in v2, just one question inline.
> Regards,
>
> Lorenzo
>
> > Jonathan
...
> > > +
> > > +static ssize_t st_lsm6dsx_shub_scale_avail(struct device *dev,
> > > + struct device_attribute *attr,
> > > + char *buf)
> > > +{
> > > + struct st_lsm6dsx_sensor *sensor = iio_priv(dev_get_drvdata(dev));
> > > + const struct st_lsm6dsx_ext_dev_settings *settings;
> > > + int i, len = 0;
> > > +
> > > + settings = sensor->ext_info.settings;
> > > + for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++) {
> > > + u16 val = settings->fs_table.fs_avl[i].gain;
> > > +
> > > + if (val > 0)
> > > + len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
> > > + val);
> > > + }
> > > + buf[len - 1] = '\n';
> > > +
> > > + return len;
> > > +}
> > > +
> > > +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(st_lsm6dsx_shub_sampling_freq_avail);
> > > +static IIO_DEVICE_ATTR(in_ext_scale_available, 0444,
> > > + st_lsm6dsx_shub_scale_avail, NULL, 0);
> > > +static struct attribute *st_lsm6dsx_ext_attributes[] = {
> > > + &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> > > + &iio_dev_attr_in_ext_scale_available.dev_attr.attr,
> >
> > What's this abi element?
> >
>
> what do you mean here? I did not get you sorry :)
>
Seems to be creating a sysfs file called
in_ext_scale_available which I don't think is documented anywhere?
Intent was probably in_scale_available? Or maybe in_magn_scale_available?
