Re: [PATCH v2 10/18] platform: chrome: sensorhub: Add FIFO support
...
> > > + * @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
Re: [PATCH v2 10/18] platform: chrome: sensorhub: Add FIFO support
On Mon, Oct 21, 2019 at 9:27 AM Jonathan Cameron wrote:
>
> On Sun, 20 Oct 2019 22:53:55 -0700
> Gwendal Grignou wrote:
>
> > cros_ec_sensorhub registers a listener and query motion sense FIFO,
> > spread to iio sensors registers.
> >
> > To test, we can use libiio:
> > iiod&
> > iio_readdev -u ip:localhost -T 1 -s 25 -b 16 cros-ec-gyro | od -x
> >
> > Signed-off-by: Gwendal Grignou
>
> A few new bits inline. I'm not an expert in power management, but
> using prepare as done here doesn't immediately seem the right choice
> to me (I'd not come across it before so had to read the docs ;)
>
> Thanks,
>
> Jonathan
>
>
> > ---
> > Changes sunce v2:
> > - Do not register a .remove routinge in plaform_driver. A
> > devm_action_add is added later patch IIO driver register their
> > callback.
> > - Remove double lines, add lines before return calls.
> > - Handle FLUSH flag from EC.
> > - Use ktime_t for most timestamp measurements.
> > - Add doxygen comments
> > - Cleanup timestamp collection when processing FIFO.
> > - Rename fifo_toggle to fifo_enable
> >
> > drivers/platform/chrome/Makefile | 3 +-
> > drivers/platform/chrome/cros_ec_sensorhub.c | 123 --
> > .../platform/chrome/cros_ec_sensorhub_ring.c | 412 ++
> > .../linux/platform_data/cros_ec_sensorhub.h | 77
> > 4 files changed, 583 insertions(+), 32 deletions(-)
> > create mode 100644 drivers/platform/chrome/cros_ec_sensorhub_ring.c
> >
> > diff --git a/drivers/platform/chrome/Makefile
> > b/drivers/platform/chrome/Makefile
> > index a164c40dc0996..cb709048c003e 100644
> > --- a/drivers/platform/chrome/Makefile
> > +++ b/drivers/platform/chrome/Makefile
> > @@ -17,7 +17,8 @@ obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o
> > cros_ec_trace.o
> > obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
> > obj-$(CONFIG_CROS_EC_CHARDEV)+= cros_ec_chardev.o
> > obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o
> > -obj-$(CONFIG_CROS_EC_SENSORHUB) += cros_ec_sensorhub.o
> > +cros_ec_sensorsupport-objs := cros_ec_sensorhub_ring.o
> > cros_ec_sensorhub.o
> > +obj-$(CONFIG_CROS_EC_SENSORHUB) += cros_ec_sensorsupport.o
> > obj-$(CONFIG_CROS_EC_VBC)+= cros_ec_vbc.o
> > obj-$(CONFIG_CROS_EC_DEBUGFS)+= cros_ec_debugfs.o
> > obj-$(CONFIG_CROS_EC_SYSFS) += cros_ec_sysfs.o
> > diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c
> > b/drivers/platform/chrome/cros_ec_sensorhub.c
> > index 5fea4c28c5c95..87308dadc32c5 100644
> > --- a/drivers/platform/chrome/cros_ec_sensorhub.c
> > +++ b/drivers/platform/chrome/cros_ec_sensorhub.c
> > @@ -24,7 +24,6 @@
> >
> > #define DRV_NAME "cros-ec-sensorhub"
> >
> > -
> > static struct device_type cros_ec_sensorhub_dev_type = {
> > .name = "cros_ec_iio_sensor",
> > };
> > @@ -64,14 +63,13 @@ static int cros_ec_sensorhub_allocate_single_sensor(
> > }
> >
> > static int cros_ec_sensorhub_register(struct device *dev,
> > - struct cros_ec_dev *ec)
> > + struct cros_ec_sensorhub *sensorhub)
> > {
> > int ret, i, id, sensor_num;
> > int sensor_type[MOTIONSENSE_TYPE_MAX] = { 0 };
> > - struct ec_params_motion_sense *params;
> > - struct ec_response_motion_sense *resp;
> > - struct cros_ec_command *msg;
> > char *name;
> > + struct cros_ec_command *msg = sensorhub->msg;
> > + struct cros_ec_dev *ec = sensorhub->ec;
> >
> > sensor_num = cros_ec_get_sensor_count(ec);
> > if (sensor_num < 0) {
> > @@ -86,32 +84,21 @@ static int cros_ec_sensorhub_register(struct device
> > *dev,
> > return -EINVAL;
> > }
> >
> > - /* Prepare a message to send INFO command to each sensor. */
> > - msg = kzalloc(sizeof(struct cros_ec_command) +
> > - max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
> > - if (!msg) {
> > - ret = -ENOMEM;
> > - goto error;
> > - }
> > -
> > msg->version = 1;
> > - msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
> > - msg->outsize = sizeof(*params);
> > - msg->insize = sizeof(*resp);
> > - params = (struct ec_params_motion_sense *)msg->data;
> > - resp = (struct ec_response_motion_sense *)msg->data;
> > + msg->insize = sizeof(struct ec_response_motion_sense);
> > + msg->outsize = sizeof(struct ec_params_motion_sense);
> >
> > id = 0;
> > for (i = 0; i < sensor_num; i++) {
> > - params->cmd = MOTIONSENSE_CMD_INFO;
> > - params->info.sensor_num = i;
> > + sensorhub->params->cmd = MOTIONSENSE_CMD_INFO;
> > + sensorhub->params->info.sensor_num = i;
> > ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
> > if (ret < 0) {
> > dev_warn(dev, "no info for EC sensor %d : %d/%d\n",
> >i,
Re: [PATCH v2 10/18] platform: chrome: sensorhub: Add FIFO support
On Sun, 20 Oct 2019 22:53:55 -0700
Gwendal Grignou wrote:
> cros_ec_sensorhub registers a listener and query motion sense FIFO,
> spread to iio sensors registers.
>
> To test, we can use libiio:
> iiod&
> iio_readdev -u ip:localhost -T 1 -s 25 -b 16 cros-ec-gyro | od -x
>
> Signed-off-by: Gwendal Grignou
A few new bits inline. I'm not an expert in power management, but
using prepare as done here doesn't immediately seem the right choice
to me (I'd not come across it before so had to read the docs ;)
Thanks,
Jonathan
> ---
> Changes sunce v2:
> - Do not register a .remove routinge in plaform_driver. A
> devm_action_add is added later patch IIO driver register their
> callback.
> - Remove double lines, add lines before return calls.
> - Handle FLUSH flag from EC.
> - Use ktime_t for most timestamp measurements.
> - Add doxygen comments
> - Cleanup timestamp collection when processing FIFO.
> - Rename fifo_toggle to fifo_enable
>
> drivers/platform/chrome/Makefile | 3 +-
> drivers/platform/chrome/cros_ec_sensorhub.c | 123 --
> .../platform/chrome/cros_ec_sensorhub_ring.c | 412 ++
> .../linux/platform_data/cros_ec_sensorhub.h | 77
> 4 files changed, 583 insertions(+), 32 deletions(-)
> create mode 100644 drivers/platform/chrome/cros_ec_sensorhub_ring.c
>
> diff --git a/drivers/platform/chrome/Makefile
> b/drivers/platform/chrome/Makefile
> index a164c40dc0996..cb709048c003e 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -17,7 +17,8 @@ obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o
> cros_ec_trace.o
> obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
> obj-$(CONFIG_CROS_EC_CHARDEV)+= cros_ec_chardev.o
> obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o
> -obj-$(CONFIG_CROS_EC_SENSORHUB) += cros_ec_sensorhub.o
> +cros_ec_sensorsupport-objs := cros_ec_sensorhub_ring.o
> cros_ec_sensorhub.o
> +obj-$(CONFIG_CROS_EC_SENSORHUB) += cros_ec_sensorsupport.o
> obj-$(CONFIG_CROS_EC_VBC)+= cros_ec_vbc.o
> obj-$(CONFIG_CROS_EC_DEBUGFS)+= cros_ec_debugfs.o
> obj-$(CONFIG_CROS_EC_SYSFS) += cros_ec_sysfs.o
> diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c
> b/drivers/platform/chrome/cros_ec_sensorhub.c
> index 5fea4c28c5c95..87308dadc32c5 100644
> --- a/drivers/platform/chrome/cros_ec_sensorhub.c
> +++ b/drivers/platform/chrome/cros_ec_sensorhub.c
> @@ -24,7 +24,6 @@
>
> #define DRV_NAME "cros-ec-sensorhub"
>
> -
> static struct device_type cros_ec_sensorhub_dev_type = {
> .name = "cros_ec_iio_sensor",
> };
> @@ -64,14 +63,13 @@ static int cros_ec_sensorhub_allocate_single_sensor(
> }
>
> static int cros_ec_sensorhub_register(struct device *dev,
> - struct cros_ec_dev *ec)
> + struct cros_ec_sensorhub *sensorhub)
> {
> int ret, i, id, sensor_num;
> int sensor_type[MOTIONSENSE_TYPE_MAX] = { 0 };
> - struct ec_params_motion_sense *params;
> - struct ec_response_motion_sense *resp;
> - struct cros_ec_command *msg;
> char *name;
> + struct cros_ec_command *msg = sensorhub->msg;
> + struct cros_ec_dev *ec = sensorhub->ec;
>
> sensor_num = cros_ec_get_sensor_count(ec);
> if (sensor_num < 0) {
> @@ -86,32 +84,21 @@ static int cros_ec_sensorhub_register(struct device *dev,
> return -EINVAL;
> }
>
> - /* Prepare a message to send INFO command to each sensor. */
> - msg = kzalloc(sizeof(struct cros_ec_command) +
> - max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
> - if (!msg) {
> - ret = -ENOMEM;
> - goto error;
> - }
> -
> msg->version = 1;
> - msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
> - msg->outsize = sizeof(*params);
> - msg->insize = sizeof(*resp);
> - params = (struct ec_params_motion_sense *)msg->data;
> - resp = (struct ec_response_motion_sense *)msg->data;
> + msg->insize = sizeof(struct ec_response_motion_sense);
> + msg->outsize = sizeof(struct ec_params_motion_sense);
>
> id = 0;
> for (i = 0; i < sensor_num; i++) {
> - params->cmd = MOTIONSENSE_CMD_INFO;
> - params->info.sensor_num = i;
> + sensorhub->params->cmd = MOTIONSENSE_CMD_INFO;
> + sensorhub->params->info.sensor_num = i;
> ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
> if (ret < 0) {
> dev_warn(dev, "no info for EC sensor %d : %d/%d\n",
>i, ret, msg->result);
> continue;
> }
> - switch (resp->info.type) {
> + switch (sensorhub->resp->info.type) {
> case MOTIONSENSE_TYPE_ACCEL:
> name = "cros-ec-accel";
>
[PATCH v2 10/18] platform: chrome: sensorhub: Add FIFO support
cros_ec_sensorhub registers a listener and query motion sense FIFO,
spread to iio sensors registers.
To test, we can use libiio:
iiod&
iio_readdev -u ip:localhost -T 1 -s 25 -b 16 cros-ec-gyro | od -x
Signed-off-by: Gwendal Grignou
---
Changes sunce v2:
- Do not register a .remove routinge in plaform_driver. A
devm_action_add is added later patch IIO driver register their
callback.
- Remove double lines, add lines before return calls.
- Handle FLUSH flag from EC.
- Use ktime_t for most timestamp measurements.
- Add doxygen comments
- Cleanup timestamp collection when processing FIFO.
- Rename fifo_toggle to fifo_enable
drivers/platform/chrome/Makefile | 3 +-
drivers/platform/chrome/cros_ec_sensorhub.c | 123 --
.../platform/chrome/cros_ec_sensorhub_ring.c | 412 ++
.../linux/platform_data/cros_ec_sensorhub.h | 77
4 files changed, 583 insertions(+), 32 deletions(-)
create mode 100644 drivers/platform/chrome/cros_ec_sensorhub_ring.c
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index a164c40dc0996..cb709048c003e 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -17,7 +17,8 @@ obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o
cros_ec_trace.o
obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
obj-$(CONFIG_CROS_EC_CHARDEV) += cros_ec_chardev.o
obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o
-obj-$(CONFIG_CROS_EC_SENSORHUB)+= cros_ec_sensorhub.o
+cros_ec_sensorsupport-objs := cros_ec_sensorhub_ring.o
cros_ec_sensorhub.o
+obj-$(CONFIG_CROS_EC_SENSORHUB)+= cros_ec_sensorsupport.o
obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o
obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o
obj-$(CONFIG_CROS_EC_SYSFS)+= cros_ec_sysfs.o
diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c
b/drivers/platform/chrome/cros_ec_sensorhub.c
index 5fea4c28c5c95..87308dadc32c5 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub.c
@@ -24,7 +24,6 @@
#define DRV_NAME "cros-ec-sensorhub"
-
static struct device_type cros_ec_sensorhub_dev_type = {
.name = "cros_ec_iio_sensor",
};
@@ -64,14 +63,13 @@ static int cros_ec_sensorhub_allocate_single_sensor(
}
static int cros_ec_sensorhub_register(struct device *dev,
- struct cros_ec_dev *ec)
+ struct cros_ec_sensorhub *sensorhub)
{
int ret, i, id, sensor_num;
int sensor_type[MOTIONSENSE_TYPE_MAX] = { 0 };
- struct ec_params_motion_sense *params;
- struct ec_response_motion_sense *resp;
- struct cros_ec_command *msg;
char *name;
+ struct cros_ec_command *msg = sensorhub->msg;
+ struct cros_ec_dev *ec = sensorhub->ec;
sensor_num = cros_ec_get_sensor_count(ec);
if (sensor_num < 0) {
@@ -86,32 +84,21 @@ static int cros_ec_sensorhub_register(struct device *dev,
return -EINVAL;
}
- /* Prepare a message to send INFO command to each sensor. */
- msg = kzalloc(sizeof(struct cros_ec_command) +
- max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
- if (!msg) {
- ret = -ENOMEM;
- goto error;
- }
-
msg->version = 1;
- msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
- msg->outsize = sizeof(*params);
- msg->insize = sizeof(*resp);
- params = (struct ec_params_motion_sense *)msg->data;
- resp = (struct ec_response_motion_sense *)msg->data;
+ msg->insize = sizeof(struct ec_response_motion_sense);
+ msg->outsize = sizeof(struct ec_params_motion_sense);
id = 0;
for (i = 0; i < sensor_num; i++) {
- params->cmd = MOTIONSENSE_CMD_INFO;
- params->info.sensor_num = i;
+ sensorhub->params->cmd = MOTIONSENSE_CMD_INFO;
+ sensorhub->params->info.sensor_num = i;
ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
if (ret < 0) {
dev_warn(dev, "no info for EC sensor %d : %d/%d\n",
i, ret, msg->result);
continue;
}
- switch (resp->info.type) {
+ switch (sensorhub->resp->info.type) {
case MOTIONSENSE_TYPE_ACCEL:
name = "cros-ec-accel";
break;
@@ -134,14 +121,15 @@ static int cros_ec_sensorhub_register(struct device *dev,
name = "cros-ec-activity";
break;
default:
- dev_warn(dev, "unknown type %d\n", resp->info.type);
+ dev_warn(dev, "unknown type %d\n",
+sensorhub->resp->info.type);
