Received: by 2002:a05:6a10:8c0a:0:0:0:0 with SMTP id go10csp455264pxb; Wed, 27 Jan 2021 11:52:25 -0800 (PST) X-Google-Smtp-Source: ABdhPJw0S+lLmG9XNn8IaoB5CprrVjzCVKJ+LCa/gio7lL73AkUN5EYFryuq4ECN6vTd3fFkLwi1 X-Received: by 2002:a17:906:6087:: with SMTP id t7mr8251374ejj.90.1611777144856; Wed, 27 Jan 2021 11:52:24 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1611777144; cv=none; d=google.com; s=arc-20160816; b=pDMxGlLEUnCnqY5NTFNf78RtxHs0jHpoPMqinIm9dHvPtuMtaATWIuI58kD7fhAkNx erVa5APiuaQuMkEqeX08/ASotzkUDQQ03trK57gnMnuHrtIMbQRhupI61Gw55t6U1gVm m72IpXe+9hyNSl2jVPcIefd55joR7HqVqlo6xpY4iW7WAHnx2acEHYubyK68Tk1GggAd Iw54FZnKZ+DwsA6RccXfYUxN9Ke+m5r/1WOnCTxCrnNNcHCkeBIhxOYDM8i/OOdwHCdH 5lH9IbSCygFVBOrnpoFPY8yzKBXrLueId/jq7idmDmCUlgR6te6tKIy5J3j/3/EsUC1R VJ5A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=M2I9sdcBhNd2ZDpjOFUA3JTjooXZgJestl6B5WtLqYw=; b=Klak5Py5DQ2BwuhlVeCZTSwOAUvGrp7EsyRn9bFB2BgFAo0/LlG2XFwO7It30b7Mm2 zAMsQXj/Gp7jmc07yyGMSae86vxDbblSwYQrM2Kn18m0Bar5juMtXf8SV7alTMTqnnFW 750XnN4lDuCChIipN/lO0NtzHe43JoPRc6/HdPLyUldX5gH3UtqUxBo+lk9Kvg3tJ/b4 PQQappfcWlmUKFuM07SDPws83kuYH2eRH4XBWPk1GT3XKvwsVpXKv0GgqI494WmKX06R U6lD8M/AkRf/Za519herc8B/Z7rvmkbCu2uQoIUVQ3Bj32ZB3UdhY0J7OniTgcXaPFhy Nk5Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=gxFNJhfN; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id t24si1250612ejr.255.2021.01.27.11.51.59; Wed, 27 Jan 2021 11:52:24 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=gxFNJhfN; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235842AbhA0DZx (ORCPT + 99 others); Tue, 26 Jan 2021 22:25:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45354 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2405857AbhAZVAr (ORCPT ); Tue, 26 Jan 2021 16:00:47 -0500 Received: from mail-io1-xd2c.google.com (mail-io1-xd2c.google.com [IPv6:2607:f8b0:4864:20::d2c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D6A53C0613ED for ; Tue, 26 Jan 2021 13:00:02 -0800 (PST) Received: by mail-io1-xd2c.google.com with SMTP id u7so5212434iol.8 for ; Tue, 26 Jan 2021 13:00:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=M2I9sdcBhNd2ZDpjOFUA3JTjooXZgJestl6B5WtLqYw=; b=gxFNJhfN4Chrhg5kdayB/ftyn/fnw2+Wb29YdrTDNZLITfcjnAYGg/UCD9KQ9iBKVq BTLngUgnymBGRjQPJU1AR8aQOyBGqngK8WBqyzvUVBBi2UozLEtk86hlfMu1L/tZvD8F LHE3q/aRZAw4CozvujuR16fcm3Ff9Ar9NX+HI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=M2I9sdcBhNd2ZDpjOFUA3JTjooXZgJestl6B5WtLqYw=; b=YBwv6LCbkgYcO/9ns+OBw5yyxID7F8pJ0JkLUa8wpv11LzuZbrbrhdrlowUq1UGzak DpU6M+mByD81wuWNaUKvGmFxFGaudzbdYETXI7OgU/CWOnKz0QohTq4hxhiyyk3e0Uuq F55locNjY1TeQ+RrQcbX4LbyGfUb0kMQveMdpyh9g1LkHvaSpCga4ob165NSMLo2DUtf QDFeFYikw9wBZAfl2IqHdZKJgrp8zgpD4P2VaYhvfejpeBRLmoUIaSui5rZxEpCb/mxN XaDczOu41GqchhHaT0q0aJO018vnWPznlg2HIyxXEHi9XxDSEiBz5hzvOLxMiFjCYB/0 uMOQ== X-Gm-Message-State: AOAM533QJ7pLHb+OVPbiYaxXY2EJamYHNyTWrx9ISKo3YyaFu9MzvoS2 /4zJ2GmESMR34zq0YpLPjBlXDJDKPwd4CeITTI/LIpsEzTr41Q== X-Received: by 2002:a05:6e02:6ce:: with SMTP id p14mr5849847ils.50.1611694802243; Tue, 26 Jan 2021 13:00:02 -0800 (PST) MIME-Version: 1.0 References: <20210122225443.186184-1-swboyd@chromium.org> <20210122225443.186184-4-swboyd@chromium.org> <20210124173820.4528b9c9@archlinux> <161160076017.76967.4467861058817044169@swboyd.mtv.corp.google.com> <161161826068.76967.15170332425672601158@swboyd.mtv.corp.google.com> In-Reply-To: <161161826068.76967.15170332425672601158@swboyd.mtv.corp.google.com> From: Gwendal Grignou Date: Tue, 26 Jan 2021 12:59:50 -0800 Message-ID: Subject: Re: [PATCH 3/3] iio: proximity: Add a ChromeOS EC MKBP proximity driver To: Stephen Boyd Cc: Jonathan Cameron , linux-kernel , linux-iio , Dmitry Torokhov , Benson Leung , Guenter Roeck , Douglas Anderson Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 25, 2021 at 3:44 PM Stephen Boyd wrote: > > Quoting Gwendal Grignou (2021-01-25 14:28:46) > > On Mon, Jan 25, 2021 at 10:52 AM Stephen Boyd wrote: > > > > > > Quoting Gwendal Grignou (2021-01-24 13:41:44) > > > > On Sun, Jan 24, 2021 at 9:38 AM Jonathan Cameron wrote: > > > > > > > > > > On Fri, 22 Jan 2021 14:54:43 -0800 > > > > > Stephen Boyd wrote: > > > > > > + if (event_type == EC_MKBP_EVENT_SWITCH) { > > > > > > + data = container_of(nb, struct cros_ec_proximity_data, notifier); > > > > > > + indio_dev = data->indio_dev; > > > > > > + > > > > > > + mutex_lock(&data->lock); > > > > > > + if (data->enabled) { > > > > > > + timestamp = iio_get_time_ns(indio_dev); > > > > For Android, given the timestamp must be time it happens, not reported > > > > [https://source.android.com/devices/sensors/sensors-hal2] """The > > > > timestamp must be accurate and correspond to the time at which the > > > > event physically happened, not the time it was reported.""", consider > > > > using ec_dev->last_event_time and apply a delta if the iio clock base > > > > is different from CLOCK_BOOTTIME. > > > > > > Ah alright. Is there a reason why cros_ec_get_time_ns() is using > > > boottime instead of plain ktime_get(), i.e. CLOCK_MONOTONIC? Otherwise I > > > suppose some sort of cros_ec API should be exposed to convert the > > > last_event_time to whatever clock base is desired. Does that exist? > > CLOCK_BOOTTIME was chosen to be Android compliant, as it includes > > suspend time and match elapsedRealtime() [see > > https://developer.android.com/reference/android/os/SystemClock#elapsedRealtime()] > > Chromebook set iio clock reference for all sensor to CLOCK_BOOTTIME > > (see https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/mems_setup/configuration.cc#127). > > In case the iio device clock_id is not CLOCK_BOOTTIME/"bootime", we > > need to add a delta, like in cros_ec_sensors_push_data() > > [https://elixir.bootlin.com/linux/latest/source/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c#L210] > > The delta may help but what if the clock is adjusted in the time between > the event is timestamped and this driver reading the timestamp? That > could lead to some odd behavior where the timestamp is in the future > because we don't know what sort of adjustment was made, e.g. the > realtime clock being moved back in time. > > I'd rather have a way to request that cros_ec core timestamp the event > with some clock base that this driver desires, ec_dev->last_event_time is collected outside of the IIO stack, and can be used by multiple drivers. > instead of having to do > an offset after the fact. For now I'll use ec_dev->last_event_time > because this is only used on chromeos and that should work until > userspace is changed, but in the future I think we'll need to have a way > for this IIO device to be notified when the clock base changes in > iio_device_set_clock() and then have this driver call into cros_ec to > request that such a timestamp be made when this event is seen. Or even > better have a way to request that cros_ec timestamp the event itself on > the EC side, but I don't know if that's possible. One way would be use the EC sensor stack that collect such timestamp, but that would be more firmware changes. On second thought, to keep it simple and consistent with other IIO drivers, I suggest to keep using iio_get_time_ns() when the sensor clock is not CLOCK_BOOTTIME, ec_dev->last_event_time when it is. > > > > > > > +static const struct of_device_id cros_ec_proximity_of_match[] = { > > > > > > + { .compatible = "google,cros-ec-proximity" }, > > > > > > + {} > > > > > > +}; > > > > > > +MODULE_DEVICE_TABLE(of, cros_ec_proximity_of_match); > > > > > > +#endif > > > > > > + > > > > > > +static struct platform_driver cros_ec_proximity_driver = { > > > > > > + .driver = { > > > > > > + .name = "cros-ec-proximity", > > > > > > + .of_match_table = of_match_ptr(cros_ec_proximity_of_match), > > > > Add a ACPI match table to match. > > > > > > I don't have an ACPI system in hand. What should the ACPI table look > > > like? Can ACPI use the of_match_table logic? > > AFAIK, ACPI uses .acpi_match_table, see > > drivers/iio/magnetometer/ak8975.c for a simple example. > > Ok. I'm leaning towards punting on this. I don't have an ACPI system to > test and I don't know what the ACPI match table should have in it. If > you can tell me what to put in the acpi_match_table then I can add it.