From: Jonathan Cameron <jic23@kernel.org>
To: Gwendal Grignou <gwendal@chromium.org>
Cc: dmitry.torokhov@gmail.com, groeck@chromium.org,
briannorris@chromium.org, knaack.h@gmx.de, lars@metafoo.de,
pmeerw@pmeerw.net, lee.jones@linaro.org, bleung@chromium.org,
enric.balletbo@collabora.com, dianders@chromium.org,
fabien.lahoudere@collabora.com, linux-kernel@vger.kernel.org,
linux-iio@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH v5 14/18] iio: cros_ec: Register to cros_ec_sensorhub when EC supports FIFO
Date: Sat, 16 Nov 2019 11:44:27 +0000 [thread overview]
Message-ID: <20191116114427.4d0f5d45@archlinux> (raw)
In-Reply-To: <20191115093412.144922-15-gwendal@chromium.org>
On Fri, 15 Nov 2019 01:34:08 -0800
Gwendal Grignou <gwendal@chromium.org> wrote:
> When EC supports FIFO, each IIO device registers a callback, to put
> samples in the buffer when they arrives from the FIFO.
> We can still use a trigger to collect samples, but there may be some
> duplications in the buffer: EC has a single FIFO, so once one sensor is
> using it, all sensors event will be in the FIFO.
>
> When no FIFO, the user space app needs to call trigger_new, or better
> register a high precision timer.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Looks good to me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> Changes in v5:
> - Revert modes setting that were removed by mistake
> - Remove unnecessary check when receving events from callback
> - Set clock_id to boottime by default, as it is what sensorhub_ring is
> using.
> - Update commit messages reference to time domain: trigger and hub
> generated events are always in the same time domain.
> Changes in v4:
> - Fix a logic error when the sensor is not "physical", for instance lig
> angle: core_init() would return !0 even if there was no error.
> - Check patch with --strict option
> Use sizeof(*obj) instead of sizeof(struct ...obj)
> Alignement
> Change in v3:
> - Remove double line
> - Fix indentation
> - Add code to support iio clock_id setting. Optimized for
> CLOCK_BOOTTIME.
> Change in v2 from "Use triggered buffer only when EC does not support
> FIFO":
> - Keep trigger all the time.
> - Add devm_add_action to cleanup callback registration.
> - EC that "reports" legacy sensors do not have FIFO.
> - Use iiio_is_buffer_enabled instead of checking the scan_mask
> before sending samples to buffer.
> - Add empty lines for visibility.
>
> drivers/iio/accel/cros_ec_accel_legacy.c | 8 +-
> .../cros_ec_sensors/cros_ec_lid_angle.c | 2 +-
> .../common/cros_ec_sensors/cros_ec_sensors.c | 9 +--
> .../cros_ec_sensors/cros_ec_sensors_core.c | 81 ++++++++++++++++++-
> drivers/iio/light/cros_ec_light_prox.c | 9 +--
> drivers/iio/pressure/cros_ec_baro.c | 9 +--
> .../linux/iio/common/cros_ec_sensors_core.h | 10 ++-
> 7 files changed, 101 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> index 65f85faf6f31..b9f651e4ce99 100644
> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -171,7 +171,8 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> if (!indio_dev)
> return -ENOMEM;
>
> - ret = cros_ec_sensors_core_init(pdev, indio_dev, true);
> + ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> + cros_ec_sensors_capture, NULL);
> if (ret)
> return ret;
>
> @@ -191,11 +192,6 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> state->sign[CROS_EC_SENSOR_Z] = -1;
> }
>
> - ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> - cros_ec_sensors_capture, NULL);
> - if (ret)
> - return ret;
> -
> return devm_iio_device_register(dev, indio_dev);
> }
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> index 1dcc2a16ab2d..e30a59fcf0f9 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> @@ -97,7 +97,7 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
> if (!indio_dev)
> return -ENOMEM;
>
> - ret = cros_ec_sensors_core_init(pdev, indio_dev, false);
> + ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL, NULL);
> if (ret)
> return ret;
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> index 7dce04473467..62a0dd970988 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -231,7 +231,9 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
> if (!indio_dev)
> return -ENOMEM;
>
> - ret = cros_ec_sensors_core_init(pdev, indio_dev, true);
> + ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> + cros_ec_sensors_capture,
> + cros_ec_sensors_push_data);
> if (ret)
> return ret;
>
> @@ -293,11 +295,6 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
> else
> state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
>
> - ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> - cros_ec_sensors_capture, NULL);
> - if (ret)
> - return ret;
> -
> return devm_iio_device_register(dev, indio_dev);
> }
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index b47da497a3c3..cffc712c32a7 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -12,6 +12,7 @@
> #include <linux/iio/iio.h>
> #include <linux/iio/kfifo_buf.h>
> #include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> #include <linux/kernel.h>
> #include <linux/mfd/cros_ec.h>
> #include <linux/module.h>
> @@ -83,17 +84,71 @@ static void get_default_min_max_freq(enum motionsensor_type type,
> }
> }
>
> +int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
> + s16 *data,
> + s64 timestamp)
> +{
> + struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> + s16 *out;
> + s64 delta;
> + unsigned int i;
> +
> + /*
> + * Ignore samples if the buffer is not set: it is needed if the ODR is
> + * set but the buffer is not enabled yet.
> + */
> + if (!iio_buffer_enabled(indio_dev))
> + return 0;
> +
> + out = (s16 *)st->samples;
> + for_each_set_bit(i,
> + indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + *out = data[i];
> + out++;
> + }
> +
> + if (iio_device_get_clock(indio_dev) != CLOCK_BOOTTIME)
> + delta = iio_get_time_ns(indio_dev) - cros_ec_get_time_ns();
> + else
> + delta = 0;
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, st->samples,
> + timestamp + delta);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(cros_ec_sensors_push_data);
> +
> +static void cros_ec_sensors_core_clean(void *arg)
> +{
> + struct platform_device *pdev = (struct platform_device *)arg;
> + struct cros_ec_sensorhub *sensor_hub =
> + dev_get_drvdata(pdev->dev.parent);
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> + u8 sensor_num = st->param.info.sensor_num;
> +
> + cros_ec_sensorhub_unregister_push_data(sensor_hub, sensor_num);
> +}
> +
> /**
> * cros_ec_sensors_core_init() - basic initialization of the core structure
> * @pdev: platform device created for the sensors
> * @indio_dev: iio device structure of the device
> * @physical_device: true if the device refers to a physical device
> + * @trigger_capture: function pointer to call buffer is triggered,
> + * for backward compatibility.
> + * @push_data: function to call when cros_ec_sensorhub receives
> + * a sample for that sensor.
> *
> * Return: 0 on success, -errno on failure.
> */
> int cros_ec_sensors_core_init(struct platform_device *pdev,
> struct iio_dev *indio_dev,
> - bool physical_device)
> + bool physical_device,
> + cros_ec_sensors_capture_t trigger_capture,
> + cros_ec_sensorhub_push_data_cb_t push_data)
> {
> struct device *dev = &pdev->dev;
> struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
> @@ -162,6 +217,30 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> state->frequencies[2] =
> state->resp->info_3.max_frequency;
> }
> +
> + ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> + trigger_capture, NULL);
> + if (ret)
> + return ret;
> +
> + if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) {
> + ret = cros_ec_sensorhub_register_push_data(sensor_hub,
> + sensor_platform->sensor_num,
> + indio_dev, push_data);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(dev,
> + cros_ec_sensors_core_clean,
> + pdev);
> + if (ret)
> + return ret;
> +
> + /* Timestamp coming from FIFO are in ns since boot. */
> + ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME);
> + if (ret)
> + return ret;
> + }
> }
>
> return 0;
> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> index d85a391e50c5..698b2ee81ebf 100644
> --- a/drivers/iio/light/cros_ec_light_prox.c
> +++ b/drivers/iio/light/cros_ec_light_prox.c
> @@ -178,7 +178,9 @@ static int cros_ec_light_prox_probe(struct platform_device *pdev)
> if (!indio_dev)
> return -ENOMEM;
>
> - ret = cros_ec_sensors_core_init(pdev, indio_dev, true);
> + ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> + cros_ec_sensors_capture,
> + cros_ec_sensors_push_data);
> if (ret)
> return ret;
>
> @@ -237,11 +239,6 @@ static int cros_ec_light_prox_probe(struct platform_device *pdev)
>
> state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
>
> - ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> - cros_ec_sensors_capture, NULL);
> - if (ret)
> - return ret;
> -
> return devm_iio_device_register(dev, indio_dev);
> }
>
> diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
> index 2354302375de..e1c86b22676c 100644
> --- a/drivers/iio/pressure/cros_ec_baro.c
> +++ b/drivers/iio/pressure/cros_ec_baro.c
> @@ -134,7 +134,9 @@ static int cros_ec_baro_probe(struct platform_device *pdev)
> if (!indio_dev)
> return -ENOMEM;
>
> - ret = cros_ec_sensors_core_init(pdev, indio_dev, true);
> + ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> + cros_ec_sensors_capture,
> + cros_ec_sensors_push_data);
> if (ret)
> return ret;
>
> @@ -180,11 +182,6 @@ static int cros_ec_baro_probe(struct platform_device *pdev)
>
> state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
>
> - ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> - cros_ec_sensors_capture, NULL);
> - if (ret)
> - return ret;
> -
> return devm_iio_device_register(dev, indio_dev);
> }
>
> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> index 0af918978f97..b8f573ca9dcc 100644
> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> @@ -12,6 +12,7 @@
> #include <linux/irqreturn.h>
> #include <linux/platform_data/cros_ec_commands.h>
> #include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/platform_data/cros_ec_sensorhub.h>
>
> enum {
> CROS_EC_SENSOR_X,
> @@ -32,6 +33,8 @@ enum {
> /* Minimum sampling period to use when device is suspending */
> #define CROS_EC_MIN_SUSPEND_SAMPLING_FREQUENCY 1000 /* 1 second */
>
> +typedef irqreturn_t (*cros_ec_sensors_capture_t)(int irq, void *p);
> +
> /**
> * struct cros_ec_sensors_core_state - state data for EC sensors IIO driver
> * @ec: cros EC device structure
> @@ -87,9 +90,14 @@ int cros_ec_sensors_read_cmd(struct iio_dev *indio_dev, unsigned long scan_mask,
>
> struct platform_device;
> int cros_ec_sensors_core_init(struct platform_device *pdev,
> - struct iio_dev *indio_dev, bool physical_device);
> + struct iio_dev *indio_dev, bool physical_device,
> + cros_ec_sensors_capture_t trigger_capture,
> + cros_ec_sensorhub_push_data_cb_t push_data);
>
> irqreturn_t cros_ec_sensors_capture(int irq, void *p);
> +int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
> + s16 *data,
> + s64 timestamp);
>
> int cros_ec_motion_send_host_cmd(struct cros_ec_sensors_core_state *st,
> u16 opt_length);
next prev parent reply other threads:[~2019-11-16 11:44 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-15 9:33 [PATCH v5 00/18] cros_ec: Add sensorhub driver and FIFO processing Gwendal Grignou
2019-11-15 9:33 ` [PATCH v5 01/18] mfd: cros_ec: Add sensor_count and make check_features public Gwendal Grignou
2019-11-15 9:33 ` [PATCH v5 02/18] platform: cros_ec: Add cros_ec_sensor_hub driver Gwendal Grignou
2019-11-16 11:32 ` Jonathan Cameron
2019-11-15 9:33 ` [PATCH v5 03/18] platform/mfd:iio: cros_ec: Register sensor through sensorhub Gwendal Grignou
2019-11-15 9:33 ` [PATCH v5 04/18] platform: chrome: cros-ec: record event timestamp in the hard irq Gwendal Grignou
2019-11-15 9:33 ` [PATCH v5 05/18] platform: chrome: cros_ec: Do not attempt to register a non-positive IRQ number Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 06/18] platform: chrome: cros_ec: handle MKBP more events flag Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 07/18] Revert "Input: cros_ec_keyb - add back missing mask for event_type" Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 08/18] Revert "Input: cros_ec_keyb: mask out extra flags in event_type" Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 09/18] platform: chrome: sensorhub: Add FIFO support Gwendal Grignou
2019-11-16 11:39 ` Jonathan Cameron
2019-11-22 11:35 ` Enric Balletbo i Serra
2020-03-24 20:31 ` Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 10/18] platform: chrome: sensorhub: Add code to spread timestmap Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 11/18] platform: chrome: sensorhub: Add median filter Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 12/18] iio: cros_ec: Move function description to .c file Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 13/18] iio: expose iio_device_set_clock Gwendal Grignou
2019-11-16 11:42 ` Jonathan Cameron
2019-11-15 9:34 ` [PATCH v5 14/18] iio: cros_ec: Register to cros_ec_sensorhub when EC supports FIFO Gwendal Grignou
2019-11-16 11:44 ` Jonathan Cameron [this message]
2019-11-15 9:34 ` [PATCH v5 15/18] iio: cros_ec: Remove pm function Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 16/18] iio: cros_ec: Expose hwfifo_timeout Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 17/18] iio: cros_ec: Report hwfifo_watermark_max Gwendal Grignou
2019-11-15 9:34 ` [PATCH v5 18/18] iio: cros_ec: Use Hertz as unit for sampling frequency Gwendal Grignou
2019-11-22 11:11 ` [PATCH v5 00/18] cros_ec: Add sensorhub driver and FIFO processing Enric Balletbo i Serra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191116114427.4d0f5d45@archlinux \
--to=jic23@kernel.org \
--cc=bleung@chromium.org \
--cc=briannorris@chromium.org \
--cc=dianders@chromium.org \
--cc=dmitry.torokhov@gmail.com \
--cc=enric.balletbo@collabora.com \
--cc=fabien.lahoudere@collabora.com \
--cc=groeck@chromium.org \
--cc=gwendal@chromium.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=lee.jones@linaro.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pmeerw@pmeerw.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.