>
> 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?
>
ack, got it. Thx for the explanation. I will fix in v2
Regards,
Lorenzo
