From: Enric Balletbo i Serra <enric.balletbo@collabora.com>
To: Jonathan Cameron <jic23@kernel.org>,
Gwendal Grignou <gwendal@chromium.org>
Cc: briannorris@chromium.org, knaack.h@gmx.de, lars@metafoo.de,
pmeerw@pmeerw.net, lee.jones@linaro.org, bleung@chromium.org,
dianders@chromium.org, groeck@chromium.org,
fabien.lahoudere@collabora.com, linux-kernel@vger.kernel.org,
linux-iio@vger.kernel.org
Subject: Re: [PATCH v4 04/17] platform: chrome: cros-ec: record event timestamp in the hard irq
Date: Mon, 11 Nov 2019 10:27:07 +0100 [thread overview]
Message-ID: <de768b67-406c-9862-2550-7af8cbd7355f@collabora.com> (raw)
In-Reply-To: <20191110121608.330bc0f7@archlinux>
On 10/11/19 13:16, Jonathan Cameron wrote:
> On Tue, 5 Nov 2019 14:26:39 -0800
> Gwendal Grignou <gwendal@chromium.org> wrote:
>
>> To improve sensor timestamp precision, given EC and AP are in
>> different time domains, the AP needs to try to record the exact
>> moment an event was signalled to the AP by the EC as soon as
>> possible after it happens.
>>
>> First thing in the hard irq is the best place for this.
>>
>> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> A minor issue with kernel-doc (I think...)
>
> Otherwise,
> Acked-by: Jonathan Cameron <Jonathan.Cameron@kernel.org>
With the Jonathan's comment solved.
Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Thanks,
Enric
>> ---
>> Changes in v4:
>> Check patch with --strict option
>> Alignement
>> No changes in v3.
>> Changes in v2:
>> Make cros_ec_get_time_ns inline.
>> Using ktime_t instead of s64 when dealing with time.
>> Added code in ishtp to gather timestamp.
>>
>> drivers/platform/chrome/cros_ec.c | 17 ++++++++++++++---
>> drivers/platform/chrome/cros_ec_ishtp.c | 17 +++++++++++++++--
>> drivers/platform/chrome/cros_ec_lpc.c | 2 ++
>> include/linux/platform_data/cros_ec_proto.h | 16 ++++++++++++++++
>> 4 files changed, 47 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
>> index 9b2d07422e17..925f84dbf621 100644
>> --- a/drivers/platform/chrome/cros_ec.c
>> +++ b/drivers/platform/chrome/cros_ec.c
>> @@ -31,6 +31,15 @@ static struct cros_ec_platform pd_p = {
>> .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX),
>> };
>>
>> +static irqreturn_t ec_irq_handler(int irq, void *data)
>> +{
>> + struct cros_ec_device *ec_dev = data;
>> +
>> + ec_dev->last_event_time = cros_ec_get_time_ns();
>> +
>> + return IRQ_WAKE_THREAD;
>> +}
>> +
>> static irqreturn_t ec_irq_thread(int irq, void *data)
>> {
>> struct cros_ec_device *ec_dev = data;
>> @@ -141,9 +150,11 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>> }
>>
>> if (ec_dev->irq) {
>> - err = devm_request_threaded_irq(dev, ec_dev->irq, NULL,
>> - ec_irq_thread, IRQF_TRIGGER_LOW | IRQF_ONESHOT,
>> - "chromeos-ec", ec_dev);
>> + err = devm_request_threaded_irq(dev, ec_dev->irq,
>> + ec_irq_handler,
>> + ec_irq_thread,
>> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
>> + "chromeos-ec", ec_dev);
>> if (err) {
>> dev_err(dev, "Failed to request IRQ %d: %d",
>> ec_dev->irq, err);
>> diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
>> index 25ca2c894b4d..5c848f22b44b 100644
>> --- a/drivers/platform/chrome/cros_ec_ishtp.c
>> +++ b/drivers/platform/chrome/cros_ec_ishtp.c
>> @@ -200,13 +200,14 @@ static int ish_send(struct ishtp_cl_data *client_data,
>> * process_recv() - Received and parse incoming packet
>> * @cros_ish_cl: Client instance to get stats
>> * @rb_in_proc: Host interface message buffer
>> + * @timestamp: Timestamp of when parent callback started
>> *
>> * Parse the incoming packet. If it is a response packet then it will
>> * update per instance flags and wake up the caller waiting to for the
>> * response. If it is an event packet then it will schedule event work.
>> */
>> static void process_recv(struct ishtp_cl *cros_ish_cl,
>> - struct ishtp_cl_rb *rb_in_proc)
>> + struct ishtp_cl_rb *rb_in_proc, ktime_t timestamp)
>> {
>> size_t data_len = rb_in_proc->buf_idx;
>> struct ishtp_cl_data *client_data =
>> @@ -295,6 +296,11 @@ static void process_recv(struct ishtp_cl *cros_ish_cl,
>> break;
>>
>> case CROS_MKBP_EVENT:
>> + /*
>> + * Set timestamp from beginning of function since we actually
>> + * got an incoming MKBP event
>> + */
>> + client_data->ec_dev->last_event_time = timestamp;
>> /* The event system doesn't send any data in buffer */
>> schedule_work(&client_data->work_ec_evt);
>>
>> @@ -322,10 +328,17 @@ static void ish_event_cb(struct ishtp_cl_device *cl_device)
>> {
>> struct ishtp_cl_rb *rb_in_proc;
>> struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
>> + ktime_t timestamp;
>> +
>> + /*
>> + * Take timestamp as close to hardware interrupt as possible for sensor
>> + * timestamps.
>> + */
>> + timestamp = cros_ec_get_time_ns();
>>
>> while ((rb_in_proc = ishtp_cl_rx_get_rb(cros_ish_cl)) != NULL) {
>> /* Decide what to do with received data */
>> - process_recv(cros_ish_cl, rb_in_proc);
>> + process_recv(cros_ish_cl, rb_in_proc, timestamp);
>> }
>> }
>>
>> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
>> index 7d10d909435f..3c77496e164d 100644
>> --- a/drivers/platform/chrome/cros_ec_lpc.c
>> +++ b/drivers/platform/chrome/cros_ec_lpc.c
>> @@ -313,6 +313,8 @@ static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data)
>> {
>> struct cros_ec_device *ec_dev = data;
>>
>> + ec_dev->last_event_time = cros_ec_get_time_ns();
>> +
>> if (ec_dev->mkbp_event_supported &&
>> cros_ec_get_next_event(ec_dev, NULL) > 0)
>> blocking_notifier_call_chain(&ec_dev->event_notifier, 0,
>> diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
>> index 691f9e953a96..b183024fef1f 100644
>> --- a/include/linux/platform_data/cros_ec_proto.h
>> +++ b/include/linux/platform_data/cros_ec_proto.h
>> @@ -122,6 +122,8 @@ struct cros_ec_command {
>> * @event_data: Raw payload transferred with the MKBP event.
>> * @event_size: Size in bytes of the event data.
>> * @host_event_wake_mask: Mask of host events that cause wake from suspend.
>> + * @last_event_time: exact time from the hard irq when we got notified of
>> + * a new event.
>> * @ec: The platform_device used by the mfd driver to interface with the
>> * main EC.
>> * @pd: The platform_device used by the mfd driver to interface with the
>> @@ -162,6 +164,7 @@ struct cros_ec_device {
>> int event_size;
>> u32 host_event_wake_mask;
>> u32 last_resume_result;
>> + ktime_t last_event_time;
>>
>> /* The platform devices used by the mfd driver */
>> struct platform_device *ec;
>> @@ -210,4 +213,17 @@ int cros_ec_check_features(struct cros_ec_dev *ec, int feature);
>>
>> int cros_ec_get_sensor_count(struct cros_ec_dev *ec);
>>
>> +/**
>> + * cros_ec_get_time_ns - Return time in ns.
>
> Fairly sure that's not correct kernel-doc. Please run
> ../scripts/kernel-doc over the files and fix the warnings.
>
> * cros_ec_get_time_ns() - Return time in ns.
>
>> + *
>> + * This is the function used to record the time for last_event_time in struct
>> + * cros_ec_device during the hard irq.
>> + *
>> + * Return: ktime_t format since boot.
>> + */
>> +static inline ktime_t cros_ec_get_time_ns(void)
>> +{
>> + return ktime_get_boottime_ns();
>> +}
>> +
>> #endif /* __LINUX_CROS_EC_PROTO_H */
>
next prev parent reply other threads:[~2019-11-11 9:27 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-05 22:26 [PATCH v4 00/17] cros_ec: Add sensorhub driver and FIFO processing Gwendal Grignou
2019-11-05 22:26 ` [PATCH v4 01/17] mfd: cros_ec: Add sensor_count and make check_features public Gwendal Grignou
2019-11-08 22:03 ` Enric Balletbo Serra
2019-11-11 11:44 ` Lee Jones
2019-11-16 11:49 ` Jonathan Cameron
2019-11-05 22:26 ` [PATCH v4 02/17] platform: cros_ec: Add cros_ec_sensor_hub driver Gwendal Grignou
2019-11-10 12:10 ` Jonathan Cameron
2019-11-11 9:24 ` Enric Balletbo i Serra
2019-11-11 11:55 ` Jonathan Cameron
2019-11-05 22:26 ` [PATCH v4 03/17] platform/mfd:iio: cros_ec: Register sensor through sensorhub Gwendal Grignou
2019-11-10 12:13 ` Jonathan Cameron
2019-11-11 9:25 ` Enric Balletbo i Serra
2019-11-11 11:43 ` Lee Jones
2019-11-05 22:26 ` [PATCH v4 04/17] platform: chrome: cros-ec: record event timestamp in the hard irq Gwendal Grignou
2019-11-10 12:16 ` Jonathan Cameron
2019-11-11 9:27 ` Enric Balletbo i Serra [this message]
2019-11-05 22:26 ` [PATCH v4 05/17] platform: chrome: cros_ec: Do not attempt to register a non-positive IRQ number Gwendal Grignou
2019-11-10 12:17 ` Jonathan Cameron
2019-11-11 9:29 ` Enric Balletbo i Serra
2019-11-14 0:58 ` Gwendal Grignou
2019-11-05 22:26 ` [PATCH v4 06/17] platform: chrome: cros_ec: handle MKBP more events flag Gwendal Grignou
2019-11-10 12:28 ` Jonathan Cameron
2019-11-11 9:30 ` Enric Balletbo i Serra
2019-11-05 22:26 ` [PATCH v4 07/17] Revert "Input: cros_ec_keyb - add back missing mask for event_type" Gwendal Grignou
2019-11-11 9:20 ` Enric Balletbo i Serra
2019-11-11 19:23 ` Dmitry Torokhov
2019-11-05 22:26 ` [PATCH v4 08/17] Revert "Input: cros_ec_keyb: mask out extra flags in event_type" Gwendal Grignou
2019-11-11 9:20 ` Enric Balletbo i Serra
2019-11-11 19:23 ` Dmitry Torokhov
2019-11-05 22:26 ` [PATCH v4 09/17] platform: chrome: sensorhub: Add FIFO support Gwendal Grignou
2019-11-06 21:13 ` Gwendal Grignou
2019-11-10 12:54 ` Jonathan Cameron
2019-11-14 1:01 ` Gwendal Grignou
2019-11-05 22:26 ` [PATCH v4 10/17] platform: chrome: sensorhub: Add code to spread timestmap Gwendal Grignou
2019-11-10 12:57 ` Jonathan Cameron
2019-11-05 22:26 ` [PATCH v4 11/17] platform: chrome: sensorhub: Add median filter Gwendal Grignou
2019-11-10 13:07 ` Jonathan Cameron
2019-11-05 22:26 ` [PATCH v4 12/17] iio: cros_ec: Move function description to .c file Gwendal Grignou
2019-11-10 13:08 ` Jonathan Cameron
2019-11-11 9:35 ` Enric Balletbo i Serra
2019-11-05 22:26 ` [PATCH v4 13/17] iio: cros_ec: Register to cros_ec_sensorhub when EC supports FIFO Gwendal Grignou
2019-11-10 13:17 ` Jonathan Cameron
2019-11-14 18:17 ` Gwendal Grignou
2019-11-05 22:26 ` [PATCH v4 14/17] iio: cros_ec: Remove pm function Gwendal Grignou
2019-11-10 13:18 ` Jonathan Cameron
2019-11-11 9:37 ` Enric Balletbo i Serra
2019-11-05 22:26 ` [PATCH v4 15/17] iio: cros_ec: Expose hwfifo_timeout Gwendal Grignou
2019-11-10 13:21 ` Jonathan Cameron
2019-11-05 22:26 ` [PATCH v4 16/17] iio: cros_ec: Report hwfifo_watermark_max Gwendal Grignou
2019-11-10 13:22 ` Jonathan Cameron
2019-11-05 22:26 ` [PATCH v4 17/17] iio: cros_ec: Use Hertz as unit for sampling frequency Gwendal Grignou
2019-11-10 13:24 ` Jonathan Cameron
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=de768b67-406c-9862-2550-7af8cbd7355f@collabora.com \
--to=enric.balletbo@collabora.com \
--cc=bleung@chromium.org \
--cc=briannorris@chromium.org \
--cc=dianders@chromium.org \
--cc=fabien.lahoudere@collabora.com \
--cc=groeck@chromium.org \
--cc=gwendal@chromium.org \
--cc=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=lee.jones@linaro.org \
--cc=linux-iio@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).