Received: by 2002:a25:8b12:0:0:0:0:0 with SMTP id i18csp3882420ybl; Mon, 26 Aug 2019 01:59:12 -0700 (PDT) X-Google-Smtp-Source: APXvYqyvgfKgI0ttWjaxMX882yYEjVRGX6mppRgYrcGYUb3SlfPCDDGcIlGfBx3TxPWDNaAhvWdj X-Received: by 2002:a63:1765:: with SMTP id 37mr15591944pgx.447.1566809952296; Mon, 26 Aug 2019 01:59:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1566809952; cv=none; d=google.com; s=arc-20160816; b=SVeblXOx9/1ZOC9ZCXcqod5z2yu6eZYUdjK+ww/omu3Nzfz1Aj/DuJpvZifrnYV/0m X6YhR2bhwqYVb0fPUDHwsI5dr8IaSUl0+PPk/Qvu8/CIjK55le9hYXgmBkfGTwURcC2d Ln42fmv+zHaAoNYPZp7IJ1QG2+vxUhO4K8SiUyu93fsPRUpX3cbt9vPj/uK+l7ResBaH nWETNS6lLGAvwLzz7TNUN3oIzoRdSkqWA4JE9E+B4XwQecQqxhfW4k/v7o/wQ5oMat3B b5GU4VtLSrvdvxYcS3YO6+rQUe+M1re5v1EhdCWXKwGw8nOeeU38Yrf5CFWExncDq3vP /wRQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :dkim-signature; bh=BGK/6Vgyst1Ti2q0+TLk7JrOoShJvppUxU4T8uG6WPc=; b=Hqtduq384SvgjfOLn17iDgfBfPpX0zzsSBJlUBHZ3SF+iCatI0yMoySPJgvTJqhBjB BkH73g9VONMeq60JuZTVwqazob/OPlu66eYQsOFwANTFlqC4+0VoCrlUebF/n8j5WTBS Bni++w3bVqRul6Hz/+eZLQtJeJaHAvSc3dcJcYrw+4tUFFbXq/j4toH4rMhW1aBcj8eP fSV8DYwMwn30Kg3kVQ2zbCgMpR/o7RP1lG4IARv4UbC8Z2n731ZNfh5hSNZCramGkFez BivKnRNoo+YySr6lOSPfE6Q59BJ22MJHugjAp4xqrpjCdr/jCe4P9sSYbuXyYaa4/LQ/ tS+w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=FjhT9Fc9; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id t17si8175527pgv.54.2019.08.26.01.58.57; Mon, 26 Aug 2019 01:59:12 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=FjhT9Fc9; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730349AbfHZI4W (ORCPT + 99 others); Mon, 26 Aug 2019 04:56:22 -0400 Received: from mail.kernel.org ([198.145.29.99]:34034 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727974AbfHZI4V (ORCPT ); Mon, 26 Aug 2019 04:56:21 -0400 Received: from archlinux (cpc149474-cmbg20-2-0-cust94.5-4.cable.virginm.net [82.4.196.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 03FCF206BA; Mon, 26 Aug 2019 08:56:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1566809780; bh=tcS8K/s1i8KfMqHuEbwek20gIkSr0U6AsqOiw1H+uGY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=FjhT9Fc94rYqwk9XFpiqM7EvmBJ14MhKXbYZXAy49AKT0YGFbcz6B7V/Cq766MYPt yacVsPVF/H3shzb70quG4tuEiaMocKM+yvfF8GQMrACWyYktOsCfcdQ/nRGOzSMKeI K0EVsOnHKqBDreww+hIxm2JHJZaBiquPi+zohLWo= Date: Mon, 26 Aug 2019 09:56:12 +0100 From: Jonathan Cameron To: Fabien Lahoudere Cc: gwendal@chromium.org, egranata@chromium.org, kernel@collabora.com, William Breathitt Gray , Jonathan Corbet , Benson Leung , Enric Balletbo i Serra , Guenter Roeck , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Lee Jones , Mauro Carvalho Chehab , "David S. Miller" , Greg Kroah-Hartman , Nicolas Ferre , Nick Vaccaro , linux-iio@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/1] counter: cros_ec: Add synchronization sensor Message-ID: <20190826095612.7455cb05@archlinux> In-Reply-To: References: X-Mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 23 Aug 2019 14:41:27 +0200 Fabien Lahoudere wrote: > From: Gwendal Grignou > > EC returns a counter when there is an event on camera vsync. > This patch comes from chromeos kernel 4.4 > > Signed-off-by: Gwendal Grignou > Signed-off-by: Fabien Lahoudere > > CROS EC sync sensor was originally designed as an IIO device. > Now that the counter subsystem will replace IIO_COUNTER, we > have to implement a new way to get sync count. I'm curious. What is this counter used for? This combined counter and iio driver isn't something we would normally want to support. What is the reasoning behind doing both interfaces? > > Signed-off-by: Fabien Lahoudere > --- > Documentation/driver-api/generic-counter.rst | 3 + > MAINTAINERS | 7 + > drivers/counter/Kconfig | 9 + > drivers/counter/Makefile | 1 + > drivers/counter/counter.c | 2 + > drivers/counter/cros_ec_sensors_sync.c | 208 ++++++++++++++++++ > .../cros_ec_sensors/cros_ec_sensors_core.c | 1 + > drivers/mfd/cros_ec_dev.c | 3 + > include/linux/counter.h | 1 + > 9 files changed, 235 insertions(+) > create mode 100644 drivers/counter/cros_ec_sensors_sync.c > > diff --git a/Documentation/driver-api/generic-counter.rst b/Documentation/driver-api/generic-counter.rst > index 8382f01a53e3..beb80714ac8b 100644 > --- a/Documentation/driver-api/generic-counter.rst > +++ b/Documentation/driver-api/generic-counter.rst > @@ -44,6 +44,9 @@ Counter interface provides the following available count data types: > * COUNT_POSITION: > Unsigned integer value representing position. > > +* COUNT_TALLY: > + Unsigned integer value representing tally. > + > A Count has a count function mode which represents the update behavior > for the count data. The Generic Counter interface provides the following > available count function modes: > diff --git a/MAINTAINERS b/MAINTAINERS > index e60f5c361969..83bd291d103e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3902,6 +3902,13 @@ R: Guenter Roeck > F: Documentation/devicetree/bindings/sound/google,cros-ec-codec.txt > F: sound/soc/codecs/cros_ec_codec.* > > +CHROMEOS EC COUNTER DRIVER > +M: Fabien Lahoudere > +M: William Breathitt Gray > +L: linux-iio@vger.kernel.org > +S: Maintained > +F: drivers/counter/cros_ec_sensors_sync.c > + > CIRRUS LOGIC AUDIO CODEC DRIVERS > M: Brian Austin > M: Paul Handrigan > diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig > index 2967d0a9ff91..22287f5715e5 100644 > --- a/drivers/counter/Kconfig > +++ b/drivers/counter/Kconfig > @@ -59,4 +59,13 @@ config FTM_QUADDEC > To compile this driver as a module, choose M here: the > module will be called ftm-quaddec. > > +config IIO_CROS_EC_SENSORS_SYNC > + tristate "ChromeOS EC Counter Sensors" > + depends on IIO_CROS_EC_SENSORS_CORE && IIO > + help > + Module to handle synchronisation sensors presented by the ChromeOS EC > + Sensor hub. > + Synchronisation sensors are counter sensors triggered when events > + occurs from other subsystems. > + > endif # COUNTER > diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile > index 40d35522937d..6fe4c98a446f 100644 > --- a/drivers/counter/Makefile > +++ b/drivers/counter/Makefile > @@ -9,3 +9,4 @@ obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o > obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o > obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o > obj-$(CONFIG_FTM_QUADDEC) += ftm-quaddec.o > +obj-$(CONFIG_IIO_CROS_EC_SENSORS_SYNC) += cros_ec_sensors_sync.o > diff --git a/drivers/counter/counter.c b/drivers/counter/counter.c > index 106bc7180cd8..53525b109094 100644 > --- a/drivers/counter/counter.c > +++ b/drivers/counter/counter.c > @@ -261,6 +261,7 @@ void counter_count_read_value_set(struct counter_count_read_value *const val, > { > switch (type) { > case COUNTER_COUNT_POSITION: > + case COUNTER_COUNT_TALLY: > val->len = sprintf(val->buf, "%lu\n", *(unsigned long *)data); > break; > default: > @@ -290,6 +291,7 @@ int counter_count_write_value_get(void *const data, > > switch (type) { > case COUNTER_COUNT_POSITION: > + case COUNTER_COUNT_TALLY: > err = kstrtoul(val->buf, 0, data); > if (err) > return err; > diff --git a/drivers/counter/cros_ec_sensors_sync.c b/drivers/counter/cros_ec_sensors_sync.c > new file mode 100644 > index 000000000000..b6f5e2c6da9f > --- /dev/null > +++ b/drivers/counter/cros_ec_sensors_sync.c > @@ -0,0 +1,208 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver of counter incremented after events on interrupt line in EC. > + * > + * Copyright 2018 Google, Inc > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define DRV_NAME "cros-ec-sync" > + > +/* > + * One channel for counter, the other for timestamp. > + */ > +#define MAX_CHANNELS (1) > + > +/* State data for ec_sensors iio driver. */ > +struct cros_ec_sensors_sync_state { > + /* Shared by all sensors */ > + struct cros_ec_sensors_core_state core; > + struct counter_device counter; > + struct iio_chan_spec channels[MAX_CHANNELS]; > +}; > + > +static int cros_ec_sensors_sync_read(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct cros_ec_sensors_sync_state *st = iio_priv(indio_dev); > + u16 data; > + int ret; > + > + mutex_lock(&st->core.cmd_lock); > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = cros_ec_sensors_read_cmd(indio_dev, BIT(0), &data); > + if (ret < 0) > + break; > + ret = IIO_VAL_INT; > + *val = data; > + break; > + default: > + ret = cros_ec_sensors_core_read(&st->core, chan, val, val2, > + mask); > + break; > + } > + mutex_unlock(&st->core.cmd_lock); > + return ret; > +} > + > +static struct iio_info cros_ec_sensors_sync_info = { > + .read_raw = &cros_ec_sensors_sync_read, > + .read_avail = &cros_ec_sensors_core_read_avail, > +}; > + > +static struct counter_count cros_ec_sync_counts = { > + .id = 0, > + .name = "Cros EC sync counter", > +}; > + > +static int cros_ec_sync_cnt_read(struct counter_device *counter, > + struct counter_count *count, > + struct counter_count_read_value *val) > +{ > + s16 cnt; > + int ret; > + struct iio_dev *indio_dev = counter->priv; > + struct cros_ec_sensors_sync_state *const st = iio_priv(indio_dev); > + unsigned long data; > + > + mutex_lock(&st->core.cmd_lock); > + ret = cros_ec_sensors_read_cmd(indio_dev, BIT(0), &cnt); > + mutex_unlock(&st->core.cmd_lock); > + if (ret != 0) { > + dev_warn(&indio_dev->dev, "Unable to read sensor data\n"); > + return ret; > + } > + > + data = (unsigned long) cnt; > + counter_count_read_value_set(val, COUNTER_COUNT_TALLY, &data); > + > + return 0; > +} > + > +static const struct counter_ops cros_ec_sync_cnt_ops = { > + .count_read = cros_ec_sync_cnt_read, > +}; > + > +static char *cros_ec_loc[] = { > + [MOTIONSENSE_LOC_BASE] = "base", > + [MOTIONSENSE_LOC_LID] = "lid", > + [MOTIONSENSE_LOC_CAMERA] = "camera", > + [MOTIONSENSE_LOC_MAX] = "unknown", > +}; > + > +static ssize_t cros_ec_sync_id(struct counter_device *counter, > + void *private, char *buf) > +{ > + struct iio_dev *indio_dev = counter->priv; > + struct cros_ec_sensors_sync_state *const st = iio_priv(indio_dev); > + > + return snprintf(buf, PAGE_SIZE, "%d\n", st->core.param.info.sensor_num); > +} > + > +static ssize_t cros_ec_sync_loc(struct counter_device *counter, > + void *private, char *buf) > +{ > + struct iio_dev *indio_dev = counter->priv; > + struct cros_ec_sensors_sync_state *const st = iio_priv(indio_dev); > + > + return snprintf(buf, PAGE_SIZE, "%s\n", cros_ec_loc[st->core.loc]); > +} > + > +static struct counter_device_ext cros_ec_sync_cnt_ext[] = { > + { > + .name = "id", > + .read = cros_ec_sync_id > + }, > + { > + .name = "location", > + .read = cros_ec_sync_loc > + }, > +}; > + > +static int cros_ec_sensors_sync_probe(struct platform_device *pdev) > +{ > + struct cros_ec_sensors_sync_state *state; > + struct device *dev = &pdev->dev; > + struct iio_chan_spec *channel; > + struct iio_dev *indio_dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*state)); > + if (!indio_dev) > + return -ENOMEM; > + > + ret = cros_ec_sensors_core_init(pdev, indio_dev, true); > + if (ret) > + return ret; > + > + indio_dev->info = &cros_ec_sensors_sync_info; > + state = iio_priv(indio_dev); > + > + if (state->core.type != MOTIONSENSE_TYPE_SYNC) > + return -EINVAL; > + > + /* Initialize IIO device */ > + channel = state->channels; > + channel->type = IIO_TIMESTAMP; > + channel->channel = -1; > + channel->scan_index = 1; > + channel->scan_type.sign = 's'; > + channel->scan_type.realbits = 64; > + channel->scan_type.storagebits = 64; > + > + indio_dev->channels = state->channels; > + indio_dev->num_channels = MAX_CHANNELS; > + > + 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; > + > + ret = devm_iio_device_register(dev, indio_dev); Hmm. Wasn't expecting to see that here if it's a counter device. > + if (ret) > + return ret; > + > + /* Initialize counter device */ > + state->counter.name = dev_name(&pdev->dev); > + state->counter.parent = &pdev->dev; > + state->counter.counts = &cros_ec_sync_counts; > + state->counter.num_counts = 1; > + state->counter.priv = indio_dev; > + state->counter.ops = &cros_ec_sync_cnt_ops; > + state->counter.ext = cros_ec_sync_cnt_ext; > + state->counter.num_ext = ARRAY_SIZE(cros_ec_sync_cnt_ext); > + > + return devm_counter_register(&pdev->dev, &state->counter); > +} > + > +static const struct platform_device_id cros_ec_sensors_sync_ids[] = { > + { .name = DRV_NAME, }, > + { } > +}; > +MODULE_DEVICE_TABLE(platform, cros_ec_sensors_sync_ids); > + > +static struct platform_driver cros_ec_sensors_sync_platform_driver = { > + .driver = { > + .name = DRV_NAME, > + .pm = &cros_ec_sensors_pm_ops, > + }, > + .probe = cros_ec_sensors_sync_probe, > + .id_table = cros_ec_sensors_sync_ids, > +}; > +module_platform_driver(cros_ec_sensors_sync_platform_driver); > + > +MODULE_DESCRIPTION("ChromeOS EC synchronisation sensor driver"); > +MODULE_ALIAS("platform:" DRV_NAME); > +MODULE_LICENSE("GPL v2"); > 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 805652250960..2bf183425eaf 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 > @@ -22,6 +22,7 @@ > static char *cros_ec_loc[] = { > [MOTIONSENSE_LOC_BASE] = "base", > [MOTIONSENSE_LOC_LID] = "lid", > + [MOTIONSENSE_LOC_CAMERA] = "camera", > [MOTIONSENSE_LOC_MAX] = "unknown", > }; > > diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c > index 41dccced5026..1c5c2c38af88 100644 > --- a/drivers/mfd/cros_ec_dev.c > +++ b/drivers/mfd/cros_ec_dev.c > @@ -332,6 +332,9 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec) > case MOTIONSENSE_TYPE_ACTIVITY: > sensor_cells[id].name = "cros-ec-activity"; > break; > + case MOTIONSENSE_TYPE_SYNC: > + sensor_cells[id].name = "cros-ec-sync"; > + break; > default: > dev_warn(ec->dev, "unknown type %d\n", resp->info.type); > continue; > diff --git a/include/linux/counter.h b/include/linux/counter.h > index a061cdcdef7c..1198e675306f 100644 > --- a/include/linux/counter.h > +++ b/include/linux/counter.h > @@ -488,6 +488,7 @@ enum counter_signal_value_type { > > enum counter_count_value_type { > COUNTER_COUNT_POSITION = 0, > + COUNTER_COUNT_TALLY > }; > > void counter_signal_read_value_set(struct counter_signal_read_value *const val,