Re: [PATCH v2 10/18] platform: chrome: sensorhub: Add FIFO support

[Jonathan Cameron <jic23@kernel.org>]

...
> > > + * @sensor_hub : Sensor Hub object
> > > + */
> > > +int cros_ec_sensorhub_ring_add(struct cros_ec_sensorhub *sensorhub)
> > > +{
> > > +     struct cros_ec_dev *ec = sensorhub->ec;
> > > +     int ret;
> > > +
> > > +     /* Retrieve FIFO information */
> > > +     sensorhub->msg->version = 2;
> > > +     sensorhub->params->cmd = MOTIONSENSE_CMD_FIFO_INFO;
> > > +     sensorhub->msg->outsize = 1;
> > > +     sensorhub->msg->insize =
> > > +             sizeof(struct ec_response_motion_sense_fifo_info) +
> > > +             sizeof(u16) * CROS_EC_SENSOR_MAX;
> > > +
> > > +     ret = cros_ec_cmd_xfer_status(ec->ec_dev, sensorhub->msg);
> > > +     if (ret < 0)
> > > +             return ret;
> > > +
> > > +     /*
> > > +      * Allocate the full fifo.
> > > +      * We need to copy the whole FIFO to set timestamps properly *
> > > +      */
> > > +     sensorhub->fifo_size = sensorhub->resp->fifo_info.size;
> > > +     sensorhub->ring = devm_kcalloc(sensorhub->dev, sensorhub->fifo_size,
> > > +                     sizeof(*sensorhub->ring), GFP_KERNEL);
> > > +     if (!sensorhub->ring)
> > > +             return -ENOMEM;
> > > +
> > > +     sensorhub->fifo_timestamp[CROS_EC_SENSOR_LAST_TS] =
> > > +             cros_ec_get_time_ns();  
> >
> > Hmm. Is the IIO standard timestamp selection attribute being exposed?
> > If so this is going to be confusing as we aren't obeying the selection
> > of clock from that..  
> 
> You're right, I did not find an elegant solution.
> cros_ec_get_time_ns() is an inline version of ktime_get_boottime_ns
> aka iio_get_time_ns( with indio_dev->clock_id set to  CLOCK_BOOTTIME).
> But I can not call iio_get_time_ns() here, as I don't have a
> indio_dev. Besides, sensors could have a different
> current_timestamp_clock attribute. I will convert in the callback
> routine (cros_ec_sensors_push_data() by adding an offset
> (iio_get_time_ns(indio_dev) - cros_ec_get_time_ns())
> >  

If you can special case that (perhaps adding some functionality to IIO)
to not do anything if both are CLOCK_BOOTTIME that would be great.

Thanks,

Jonathan