Re: [PATCH 5/7] iio: imu: st_lsm6dsx: add i2c embedded controller support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>
> 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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux