Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL)
From: Joe Perches @ 2019-06-12  8:35 UTC (permalink / raw)
  To: Pali Rohár, Dmitry Torokhov; +Cc: Kefeng Wang, linux-kernel, linux-input
In-Reply-To: <20190612071444.5uih6em5o73dbvtf@pali>

On Wed, 2019-06-12 at 09:14 +0200, Pali Rohár wrote:
> On Tuesday 11 June 2019 17:59:13 Dmitry Torokhov wrote:
> > Hi Joe,
> > 
> > On Wed, Jun 05, 2019 at 07:28:53PM -0700, Joe Perches wrote:
> > > On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote:
> > > > On 2019/6/5 22:42, Pali Rohár wrote:
> > > > > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote:
> > > > > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
> > > > > > so no need to do that again from its callers. Drop it.
> > > > > Hi! I already reviewed this patch and rejected it, see:
> > > > > https://patchwork.kernel.org/patch/10817475/
> > > > OK, please ignore it.
> > > 
> > > I think the stated reason of better readability isn't
> > > particularly sensible as the object code produced is
> > > actually slightly larger.
> > > 
> > > x86-64 defconfig (gcc 8.3.0)
> > > 
> > > $ size drivers/input/mouse/alps.o*
> > >    text	   data	    bss	    dec	    hex	filename
> > >   29416	     56	      0	  29472	   7320	drivers/input/mouse/alps.o.new
> > >   29432	     56	      0	  29488	   7330	drivers/input/mouse/alps.o.old
> > 
> > If gcc produces worse code for double unlikely, you should probably
> > report it to gcc folks, no? Or double unlikely turns into likely?
> 
> Is measured size of stripped or unstripped binary? Plus with or without
> debug symbols? Double unlikely version should have more debug symbols
> and therefore also larger size.
> 
> But if unstripped version with double unlikely is larger then it is for
> sure compiler bug.

defconfig so no debug symbols.

It's not necessarily a gcc bug as gcc doesn't
guarantee compiler repeatability.

^ permalink raw reply

* [PATCH v2 3/3] iio: light: apple-ib-als: Add driver for ALS on iBridge chip.
From: Ronald Tschalär @ 2019-06-12  8:34 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler
  Cc: linux-input, linux-iio, linux-kernel
In-Reply-To: <20190612083400.1015-1-ronald@innovation.ch>

On 2016/2017 MacBook Pro's with a Touch Bar the ALS is attached to,
and exposed via the iBridge device. This provides the driver for that
sensor.

Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
 drivers/iio/light/Kconfig        |  12 +
 drivers/iio/light/Makefile       |   1 +
 drivers/iio/light/apple-ib-als.c | 607 +++++++++++++++++++++++++++++++
 3 files changed, 620 insertions(+)
 create mode 100644 drivers/iio/light/apple-ib-als.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 5190eacfeb0a..b477aa5d2024 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -64,6 +64,18 @@ config APDS9960
 	  To compile this driver as a module, choose M here: the
 	  module will be called apds9960
 
+config APPLE_IBRIDGE_ALS
+	tristate "Apple iBridge ambient light sensor"
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	depends on HID_APPLE_IBRIDGE
+	help
+	  Say Y here to build the driver for the Apple iBridge ALS
+	  sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called apple-ib-als.
+
 config BH1750
 	tristate "ROHM BH1750 ambient light sensor"
 	depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index e40794fbb435..cd6cd5ba6da5 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_ADJD_S311)		+= adjd_s311.o
 obj-$(CONFIG_AL3320A)		+= al3320a.o
 obj-$(CONFIG_APDS9300)		+= apds9300.o
 obj-$(CONFIG_APDS9960)		+= apds9960.o
+obj-$(CONFIG_APPLE_IBRIDGE_ALS)	+= apple-ib-als.o
 obj-$(CONFIG_BH1750)		+= bh1750.o
 obj-$(CONFIG_BH1780)		+= bh1780.o
 obj-$(CONFIG_CM32181)		+= cm32181.o
diff --git a/drivers/iio/light/apple-ib-als.c b/drivers/iio/light/apple-ib-als.c
new file mode 100644
index 000000000000..b84be0076e0f
--- /dev/null
+++ b/drivers/iio/light/apple-ib-als.c
@@ -0,0 +1,607 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Apple Ambient Light Sensor Driver
+ *
+ * Copyright (c) 2017-2018 Ronald Tschalär
+ */
+
+/*
+ * MacBookPro models with an iBridge chip (13,[23] and 14,[23]) have an
+ * ambient light sensor that is exposed via one of the USB interfaces on
+ * the iBridge as a standard HID light sensor. However, we cannot use the
+ * existing hid-sensor-als driver, for two reasons:
+ *
+ * 1. The hid-sensor-als driver is part of the hid-sensor-hub which in turn
+ *    is a hid driver, but you can't have more than one hid driver per hid
+ *    device, which is a problem because the touch bar also needs to
+ *    register as a driver for this hid device.
+ *
+ * 2. While the hid-sensors-als driver stores sensor readings received via
+ *    interrupt in an iio buffer, reads on the sysfs
+ *    .../iio:deviceX/in_illuminance_YYY attribute result in a get of the
+ *    feature report; however, in the case of this sensor here the
+ *    illuminance field of that report is always 0. Instead, the input
+ *    report needs to be requested.
+ */
+
+#define dev_fmt(fmt) "als: " fmt
+
+#include <linux/apple-ibridge.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/hid-sensor-ids.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#define APPLEALS_DYN_SENS		0	/* our dynamic sensitivity */
+#define APPLEALS_DEF_CHANGE_SENS	APPLEALS_DYN_SENS
+
+struct appleals_device {
+	struct hid_device	*hid_dev;
+	struct hid_report	*cfg_report;
+	struct hid_field	*illum_field;
+	struct iio_dev		*iio_dev;
+	int			cur_sensitivity;
+	int			cur_hysteresis;
+	bool			events_enabled;
+};
+
+static struct hid_driver appleals_hid_driver;
+
+/*
+ * This is a primitive way to get a relative sensitivity, one where we get
+ * notified when the value changes by a certain percentage rather than some
+ * absolute value. MacOS somehow manages to configure the sensor to work this
+ * way (with a 15% relative sensitivity), but I haven't been able to figure
+ * out how so far. So until we do, this provides a less-than-perfect
+ * simulation.
+ *
+ * When the brightness value is within one of the ranges, the sensitivity is
+ * set to that range's sensitivity. But in order to reduce flapping when the
+ * brightness is right on the border between two ranges, the ranges overlap
+ * somewhat (by at least one sensitivity), and sensitivity is only changed if
+ * the value leaves the current sensitivity's range.
+ *
+ * The values chosen for the map are somewhat arbitrary: a compromise of not
+ * too many ranges (and hence changing the sensitivity) but not too small or
+ * large of a percentage of the min and max values in the range (currently
+ * from 7.5% to 30%, i.e. within a factor of 2 of 15%), as well as just plain
+ * "this feels reasonable to me".
+ */
+struct appleals_sensitivity_map {
+	int	sensitivity;
+	int	illum_low;
+	int	illum_high;
+};
+
+static const struct appleals_sensitivity_map appleals_sensitivity_map[] = {
+	{   1,    0,   14 },
+	{   3,   10,   40 },
+	{   9,   30,  120 },
+	{  27,   90,  360 },
+	{  81,  270, 1080 },
+	{ 243,  810, 3240 },
+	{ 729, 2430, 9720 },
+};
+
+static int appleals_compute_sensitivity(int cur_illum, int cur_sens)
+{
+	const struct appleals_sensitivity_map *entry;
+	int i;
+
+	/* see if we're still in current range */
+	for (i = 0; i < ARRAY_SIZE(appleals_sensitivity_map); i++) {
+		entry = &appleals_sensitivity_map[i];
+
+		if (entry->sensitivity == cur_sens &&
+		    entry->illum_low <= cur_illum &&
+		    entry->illum_high >= cur_illum)
+			return cur_sens;
+		else if (entry->sensitivity > cur_sens)
+			break;
+	}
+
+	/* not in current range, so find new sensitivity */
+	for (i = 0; i < ARRAY_SIZE(appleals_sensitivity_map); i++) {
+		entry = &appleals_sensitivity_map[i];
+
+		if (entry->illum_low <= cur_illum &&
+		    entry->illum_high >= cur_illum)
+			return entry->sensitivity;
+	}
+
+	/* hmm, not in table, so assume we are above highest range */
+	i = ARRAY_SIZE(appleals_sensitivity_map) - 1;
+	return appleals_sensitivity_map[i].sensitivity;
+}
+
+static int appleals_get_field_value_for_usage(struct hid_field *field,
+					      unsigned int usage)
+{
+	int u;
+
+	if (!field)
+		return -1;
+
+	for (u = 0; u < field->maxusage; u++) {
+		if (field->usage[u].hid == usage)
+			return u + field->logical_minimum;
+	}
+
+	return -1;
+}
+
+static __s32 appleals_get_field_value(struct appleals_device *als_dev,
+				      struct hid_field *field)
+{
+	bool powered_on = !hid_hw_power(als_dev->hid_dev, PM_HINT_FULLON);
+
+	hid_hw_request(als_dev->hid_dev, field->report, HID_REQ_GET_REPORT);
+	hid_hw_wait(als_dev->hid_dev);
+
+	if (powered_on)
+		hid_hw_power(als_dev->hid_dev, PM_HINT_NORMAL);
+
+	return field->value[0];
+}
+
+static void appleals_set_field_value(struct appleals_device *als_dev,
+				     struct hid_field *field, __s32 value)
+{
+	hid_set_field(field, 0, value);
+	hid_hw_request(als_dev->hid_dev, field->report, HID_REQ_SET_REPORT);
+}
+
+static int appleals_get_config(struct appleals_device *als_dev,
+			       unsigned int field_usage, __s32 *value)
+{
+	struct hid_field *field;
+
+	field = appleib_find_report_field(als_dev->cfg_report, field_usage);
+	if (!field)
+		return -EINVAL;
+
+	*value = appleals_get_field_value(als_dev, field);
+
+	return 0;
+}
+
+static int appleals_set_config(struct appleals_device *als_dev,
+			       unsigned int field_usage, __s32 value)
+{
+	struct hid_field *field;
+
+	field = appleib_find_report_field(als_dev->cfg_report, field_usage);
+	if (!field)
+		return -EINVAL;
+
+	appleals_set_field_value(als_dev, field, value);
+
+	return 0;
+}
+
+static int appleals_set_enum_config(struct appleals_device *als_dev,
+				    unsigned int field_usage,
+				    unsigned int value_usage)
+{
+	struct hid_field *field;
+	int value;
+
+	field = appleib_find_report_field(als_dev->cfg_report, field_usage);
+	if (!field)
+		return -EINVAL;
+
+	value = appleals_get_field_value_for_usage(field, value_usage);
+	if (value >= 0)
+		appleals_set_field_value(als_dev, field, value);
+
+	return 0;
+}
+
+static void appleals_update_dyn_sensitivity(struct appleals_device *als_dev,
+					    __s32 value)
+{
+	int new_sens;
+	int rc;
+
+	new_sens = appleals_compute_sensitivity(value,
+						als_dev->cur_sensitivity);
+	if (new_sens != als_dev->cur_sensitivity) {
+		rc = appleals_set_config(als_dev,
+			HID_USAGE_SENSOR_LIGHT_ILLUM |
+			HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS,
+			new_sens);
+		if (!rc)
+			als_dev->cur_sensitivity = new_sens;
+	}
+}
+
+static void appleals_push_new_value(struct appleals_device *als_dev,
+				    __s32 value)
+{
+	__s32 buf[2] = { value, value };
+
+	iio_push_to_buffers(als_dev->iio_dev, buf);
+
+	if (als_dev->cur_hysteresis == APPLEALS_DYN_SENS)
+		appleals_update_dyn_sensitivity(als_dev, value);
+}
+
+static int appleals_hid_event(struct hid_device *hdev, struct hid_field *field,
+			      struct hid_usage *usage, __s32 value)
+{
+	struct appleals_device *als_dev = hid_get_drvdata(hdev);
+	int rc = 0;
+
+	if ((usage->hid & HID_USAGE_PAGE) != HID_UP_SENSOR)
+		return 0;
+
+	if (usage->hid == HID_USAGE_SENSOR_LIGHT_ILLUM) {
+		appleals_push_new_value(als_dev, value);
+		rc = 1;
+	}
+
+	return rc;
+}
+
+static int appleals_enable_events(struct iio_trigger *trig, bool enable)
+{
+	struct appleals_device *als_dev = iio_trigger_get_drvdata(trig);
+	int value;
+
+	appleals_set_enum_config(als_dev, HID_USAGE_SENSOR_PROP_REPORT_STATE,
+		enable ? HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM :
+			 HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM);
+	als_dev->events_enabled = enable;
+
+	/* if the sensor was enabled, push an initial value */
+	if (enable) {
+		value = appleals_get_field_value(als_dev, als_dev->illum_field);
+		appleals_push_new_value(als_dev, value);
+	}
+
+	return 0;
+}
+
+static int appleals_read_raw(struct iio_dev *iio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct appleals_device **priv = iio_priv(iio_dev);
+	struct appleals_device *als_dev = *priv;
+	__s32 value;
+	int rc;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_PROCESSED:
+		*val = appleals_get_field_value(als_dev, als_dev->illum_field);
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		rc = appleals_get_config(als_dev,
+					 HID_USAGE_SENSOR_PROP_REPORT_INTERVAL,
+					 &value);
+		if (rc)
+			return rc;
+
+		/* interval is in ms; val is in HZ, val2 in µHZ */
+		value = 1000000000 / value;
+		*val = value / 1000000;
+		*val2 = value - (*val * 1000000);
+
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_CHAN_INFO_HYSTERESIS:
+		if (als_dev->cur_hysteresis == APPLEALS_DYN_SENS) {
+			*val = als_dev->cur_hysteresis;
+			return IIO_VAL_INT;
+		}
+
+		rc = appleals_get_config(als_dev,
+			HID_USAGE_SENSOR_LIGHT_ILLUM |
+			HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS,
+			val);
+		if (!rc) {
+			als_dev->cur_sensitivity = *val;
+			als_dev->cur_hysteresis = *val;
+		}
+		return rc ? rc : IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int appleals_write_raw(struct iio_dev *iio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
+{
+	struct appleals_device **priv = iio_priv(iio_dev);
+	struct appleals_device *als_dev = *priv;
+	__s32 illum;
+	int rc;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		rc = appleals_set_config(als_dev,
+					 HID_USAGE_SENSOR_PROP_REPORT_INTERVAL,
+					 1000000000 / (val * 1000000 + val2));
+		return rc;
+
+	case IIO_CHAN_INFO_HYSTERESIS:
+		if (val == APPLEALS_DYN_SENS) {
+			if (als_dev->cur_hysteresis != APPLEALS_DYN_SENS) {
+				als_dev->cur_hysteresis = val;
+				illum = appleals_get_field_value(als_dev,
+							als_dev->illum_field);
+				appleals_update_dyn_sensitivity(als_dev, illum);
+			}
+
+			return 0;
+		}
+
+		rc = appleals_set_config(als_dev,
+			HID_USAGE_SENSOR_LIGHT_ILLUM |
+			HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS,
+			val);
+		if (!rc) {
+			als_dev->cur_sensitivity = val;
+			als_dev->cur_hysteresis = val;
+		}
+
+		return rc;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_chan_spec appleals_channels[] = {
+	{
+		.type = IIO_INTENSITY,
+		.modified = 1,
+		.channel2 = IIO_MOD_LIGHT_BOTH,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+			BIT(IIO_CHAN_INFO_HYSTERESIS),
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 32,
+		},
+		.scan_index = 0,
+	},
+	{
+		.type = IIO_LIGHT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+			BIT(IIO_CHAN_INFO_HYSTERESIS),
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 32,
+		},
+		.scan_index = 1,
+	}
+};
+
+static const struct iio_trigger_ops appleals_trigger_ops = {
+	.set_trigger_state = &appleals_enable_events,
+};
+
+static const struct iio_info appleals_info = {
+	.read_raw = &appleals_read_raw,
+	.write_raw = &appleals_write_raw,
+};
+
+static void appleals_config_sensor(struct appleals_device *als_dev,
+				   bool events_enabled, int sensitivity)
+{
+	struct hid_field *field;
+	bool powered_on;
+	__s32 val;
+
+	powered_on = !hid_hw_power(als_dev->hid_dev, PM_HINT_FULLON);
+
+	hid_hw_request(als_dev->hid_dev, als_dev->cfg_report,
+		       HID_REQ_GET_REPORT);
+	hid_hw_wait(als_dev->hid_dev);
+
+	field = appleib_find_report_field(als_dev->cfg_report,
+					  HID_USAGE_SENSOR_PROY_POWER_STATE);
+	val = appleals_get_field_value_for_usage(field,
+			HID_USAGE_SENSOR_PROP_POWER_STATE_D0_FULL_POWER_ENUM);
+	if (val >= 0)
+		hid_set_field(field, 0, val);
+
+	field = appleib_find_report_field(als_dev->cfg_report,
+					  HID_USAGE_SENSOR_PROP_REPORT_STATE);
+	val = appleals_get_field_value_for_usage(field,
+		events_enabled ?
+			HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM :
+			HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM);
+	if (val >= 0)
+		hid_set_field(field, 0, val);
+
+	field = appleib_find_report_field(als_dev->cfg_report,
+					 HID_USAGE_SENSOR_PROP_REPORT_INTERVAL);
+	hid_set_field(field, 0, field->logical_minimum);
+
+	/*
+	 * Set initial change sensitivity; if dynamic, enabling trigger will set
+	 * it instead.
+	 */
+	if (sensitivity != APPLEALS_DYN_SENS) {
+		field = appleib_find_report_field(als_dev->cfg_report,
+			HID_USAGE_SENSOR_LIGHT_ILLUM |
+			HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS);
+
+		hid_set_field(field, 0, sensitivity);
+	}
+
+	hid_hw_request(als_dev->hid_dev, als_dev->cfg_report,
+		       HID_REQ_SET_REPORT);
+
+	if (powered_on)
+		hid_hw_power(als_dev->hid_dev, PM_HINT_NORMAL);
+}
+
+static int appleals_config_iio(struct appleals_device *als_dev)
+{
+	struct iio_dev *iio_dev;
+	struct iio_trigger *iio_trig;
+	struct appleals_device **priv;
+	struct device *parent = &als_dev->hid_dev->dev;
+	int rc;
+
+	iio_dev = devm_iio_device_alloc(parent, sizeof(als_dev));
+	if (!iio_dev)
+		return -ENOMEM;
+
+	priv = iio_priv(iio_dev);
+	*priv = als_dev;
+
+	iio_dev->channels = appleals_channels;
+	iio_dev->num_channels = ARRAY_SIZE(appleals_channels);
+	iio_dev->dev.parent = parent;
+	iio_dev->info = &appleals_info;
+	iio_dev->name = "als";
+	iio_dev->modes = INDIO_DIRECT_MODE;
+
+	rc = devm_iio_triggered_buffer_setup(parent, iio_dev,
+					     &iio_pollfunc_store_time, NULL,
+					     NULL);
+	if (rc) {
+		hid_err(als_dev->hid_dev,
+			"Failed to set up iio triggered buffer: %d\n", rc);
+		return rc;
+	}
+
+	iio_trig = devm_iio_trigger_alloc(parent, "%s-dev%d", iio_dev->name,
+					  iio_dev->id);
+	if (!iio_trig)
+		return -ENOMEM;
+
+	iio_trig->dev.parent = parent;
+	iio_trig->ops = &appleals_trigger_ops;
+	iio_trigger_set_drvdata(iio_trig, als_dev);
+
+	rc = devm_iio_trigger_register(parent, iio_trig);
+	if (rc) {
+		hid_err(als_dev->hid_dev,
+			"Failed to register iio trigger: %d\n",
+			rc);
+		return rc;
+	}
+
+	rc = devm_iio_device_register(parent, iio_dev);
+	if (rc) {
+		hid_err(als_dev->hid_dev, "Failed to register iio device: %d\n",
+			rc);
+		return rc;
+	}
+
+	als_dev->iio_dev = iio_dev;
+
+	return 0;
+}
+
+static int appleals_probe(struct hid_device *hdev,
+			  const struct hid_device_id *id)
+{
+	struct appleals_device *als_dev;
+	struct hid_field *state_field;
+	struct hid_field *illum_field;
+	int rc;
+
+	/* find als fields and reports */
+	rc = hid_parse(hdev);
+	if (rc) {
+		hid_err(hdev, "als: hid parse failed (%d)\n", rc);
+		return rc;
+	}
+
+	state_field = appleib_find_hid_field(hdev, HID_USAGE_SENSOR_ALS,
+					    HID_USAGE_SENSOR_PROP_REPORT_STATE);
+	illum_field = appleib_find_hid_field(hdev, HID_USAGE_SENSOR_ALS,
+					     HID_USAGE_SENSOR_LIGHT_ILLUM);
+	if (!state_field || !illum_field)
+		return -ENODEV;
+
+	hid_dbg(hdev, "Found ambient light sensor\n");
+
+	/* initialize device */
+	als_dev = devm_kzalloc(&hdev->dev, sizeof(*als_dev), GFP_KERNEL);
+	if (!als_dev)
+		return -ENOMEM;
+
+	als_dev->hid_dev = hdev;
+	als_dev->cfg_report = state_field->report;
+	als_dev->illum_field = illum_field;
+
+	als_dev->cur_hysteresis = APPLEALS_DEF_CHANGE_SENS;
+	als_dev->cur_sensitivity = APPLEALS_DEF_CHANGE_SENS;
+
+	hid_set_drvdata(hdev, als_dev);
+
+	rc = hid_hw_start(hdev, HID_CONNECT_DRIVER);
+	if (rc) {
+		hid_err(hdev, "als: hw start failed (%d)\n", rc);
+		return rc;
+	}
+
+	hid_device_io_start(hdev);
+	appleals_config_sensor(als_dev, false, als_dev->cur_sensitivity);
+	hid_device_io_stop(hdev);
+
+	rc = appleals_config_iio(als_dev);
+	if (rc)
+		return rc;
+
+	return hid_hw_open(hdev);
+}
+
+#ifdef CONFIG_PM
+static int appleals_reset_resume(struct hid_device *hdev)
+{
+	struct appleals_device *als_dev = hid_get_drvdata(hdev);
+	__s32 illum;
+
+	appleals_config_sensor(als_dev, als_dev->events_enabled,
+			       als_dev->cur_sensitivity);
+
+	illum = appleals_get_field_value(als_dev, als_dev->illum_field);
+	appleals_push_new_value(als_dev, illum);
+
+	return 0;
+}
+#endif
+
+static const struct hid_device_id appleals_hid_ids[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LINUX_FOUNDATION,
+			 USB_DEVICE_ID_IBRIDGE_ALS) },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(hid, appleals_hid_ids);
+
+static struct hid_driver appleals_hid_driver = {
+	.name = "apple-ib-als",
+	.id_table = appleals_hid_ids,
+	.probe = appleals_probe,
+	.event = appleals_hid_event,
+#ifdef CONFIG_PM
+	.reset_resume = appleals_reset_resume,
+#endif
+};
+
+module_hid_driver(appleals_hid_driver);
+
+MODULE_AUTHOR("Ronald Tschalär");
+MODULE_DESCRIPTION("Apple iBridge ALS driver");
+MODULE_LICENSE("GPL v2");
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 2/3] HID: apple-ib-tb: Add driver for the Touch Bar on MacBook Pro's.
From: Ronald Tschalär @ 2019-06-12  8:33 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler
  Cc: linux-input, linux-iio, linux-kernel
In-Reply-To: <20190612083400.1015-1-ronald@innovation.ch>

This driver enables basic touch bar functionality: enabling it, switching
between modes on FN key press, and dimming and turning the display
off/on when idle/active.

Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
 drivers/hid/Kconfig       |   10 +
 drivers/hid/Makefile      |    1 +
 drivers/hid/apple-ib-tb.c | 1389 +++++++++++++++++++++++++++++++++++++
 3 files changed, 1400 insertions(+)
 create mode 100644 drivers/hid/apple-ib-tb.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 545d3691fc1c..7621c2500d71 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -149,6 +149,16 @@ config HID_APPLE_IBRIDGE
 	  To compile this driver as a module, choose M here: the
 	  module will be called apple-ibridge.
 
+config HID_APPLE_IBRIDGE_TB
+	tristate "Apple iBridge Touch Bar"
+	depends on HID_APPLE_IBRIDGE
+	---help---
+	Say Y here if you want support for the Touch Bar on recent
+	MacBook Pros.
+
+	To compile this driver as a module, choose M here: the
+	module will be called apple-ib-tb.
+
 config HID_APPLEIR
 	tristate "Apple infrared receiver"
 	depends on (USB_HID)
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index a4da5663a541..0c46e5f70db1 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_HID_ALPS)		+= hid-alps.o
 obj-$(CONFIG_HID_ACRUX)		+= hid-axff.o
 obj-$(CONFIG_HID_APPLE)		+= hid-apple.o
 obj-$(CONFIG_HID_APPLE_IBRIDGE)	+= apple-ibridge.o
+obj-$(CONFIG_HID_APPLE_IBRIDGE_TB)	+= apple-ib-tb.o
 obj-$(CONFIG_HID_APPLEIR)	+= hid-appleir.o
 obj-$(CONFIG_HID_ASUS)		+= hid-asus.o
 obj-$(CONFIG_HID_AUREAL)	+= hid-aureal.o
diff --git a/drivers/hid/apple-ib-tb.c b/drivers/hid/apple-ib-tb.c
new file mode 100644
index 000000000000..6daee80060ce
--- /dev/null
+++ b/drivers/hid/apple-ib-tb.c
@@ -0,0 +1,1389 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Apple Touch Bar Driver
+ *
+ * Copyright (c) 2017-2018 Ronald Tschalär
+ */
+
+/*
+ * Recent MacBookPro models (13,[23] and 14,[23]) have a touch bar, which
+ * is exposed via several USB interfaces. MacOS supports a fancy mode
+ * where arbitrary buttons can be defined; this driver currently only
+ * supports the simple mode that consists of 3 predefined layouts
+ * (escape-only, esc + special keys, and esc + function keys).
+ *
+ * The first USB HID interface supports two reports, an input report that
+ * is used to report the key presses, and an output report which can be
+ * used to set the touch bar "mode": touch bar off (in which case no touches
+ * are reported at all), escape key only, escape + 12 function keys, and
+ * escape + several special keys (including brightness, audio volume,
+ * etc). The second interface supports several, complex reports, most of
+ * which are unknown at this time, but one of which has been determined to
+ * allow for controlling of the touch bar's brightness: off (though touches
+ * are still reported), dimmed, and full brightness. This driver makes
+ * use of these two reports.
+ */
+
+#define dev_fmt(fmt) "tb: " fmt
+
+#include <linux/apple-ibridge.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/input.h>
+#include <linux/jiffies.h>
+#include <linux/ktime.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/usb/ch9.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#define HID_UP_APPLE		0xff120000
+#define HID_USAGE_MODE		(HID_UP_CUSTOM | 0x0004)
+#define HID_USAGE_APPLE_APP	(HID_UP_APPLE  | 0x0001)
+#define HID_USAGE_DISP		(HID_UP_APPLE  | 0x0021)
+#define HID_USAGE_DISP_AUX1	(HID_UP_APPLE  | 0x0020)
+
+#define APPLETB_MAX_TB_KEYS	13	/* ESC, F1-F12 */
+
+#define APPLETB_CMD_MODE_ESC	0
+#define APPLETB_CMD_MODE_FN	1
+#define APPLETB_CMD_MODE_SPCL	2
+#define APPLETB_CMD_MODE_OFF	3
+#define APPLETB_CMD_MODE_UPD	254
+#define APPLETB_CMD_MODE_NONE	255
+
+#define APPLETB_CMD_DISP_ON	1
+#define APPLETB_CMD_DISP_DIM	2
+#define APPLETB_CMD_DISP_OFF	4
+#define APPLETB_CMD_DISP_UPD	254
+#define APPLETB_CMD_DISP_NONE	255
+
+#define APPLETB_FN_MODE_FKEYS	0
+#define APPLETB_FN_MODE_NORM	1
+#define APPLETB_FN_MODE_INV	2
+#define APPLETB_FN_MODE_SPCL	3
+#define APPLETB_FN_MODE_MAX	APPLETB_FN_MODE_SPCL
+
+#define APPLETB_DEVID_KEYBOARD	1
+#define APPLETB_DEVID_TOUCHPAD	2
+
+#define APPLETB_MAX_DIM_TIME	30
+
+static int appletb_tb_def_idle_timeout = 5 * 60;
+module_param_named(idle_timeout, appletb_tb_def_idle_timeout, int, 0444);
+MODULE_PARM_DESC(idle_timeout, "Default touch bar idle timeout:\n"
+			       "    >0 - turn touch bar display off after no keyboard, trackpad, or touch bar input has been received for this many seconds;\n"
+			       "         the display will be turned back on as soon as new input is received\n"
+			       "     0 - turn touch bar display off (input does not turn it on again)\n"
+			       "    -1 - turn touch bar display on (does not turn off automatically)\n"
+			       "    -2 - disable touch bar completely");
+
+static int appletb_tb_def_dim_timeout = -2;
+module_param_named(dim_timeout, appletb_tb_def_dim_timeout, int, 0444);
+MODULE_PARM_DESC(dim_timeout, "Default touch bar dim timeout:\n"
+			      "    >0 - dim touch bar display after no keyboard, trackpad, or touch bar input has been received for this many seconds\n"
+			      "         the display will be returned to full brightness as soon as new input is received\n"
+			      "     0 - dim touch bar display (input does not return it to full brightness)\n"
+			      "    -1 - disable timeout (touch bar never dimmed)\n"
+			      "    [-2] - calculate timeout based on idle-timeout");
+
+static int appletb_tb_def_fn_mode = APPLETB_FN_MODE_NORM;
+module_param_named(fnmode, appletb_tb_def_fn_mode, int, 0444);
+MODULE_PARM_DESC(fnmode, "Default Fn key mode:\n"
+			 "    0 - function-keys only\n"
+			 "    [1] - fn key switches from special to function-keys\n"
+			 "    2 - inverse of 1\n"
+			 "    3 - special keys only");
+
+static ssize_t idle_timeout_show(struct device *dev,
+				 struct device_attribute *attr, char *buf);
+static ssize_t idle_timeout_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t size);
+static DEVICE_ATTR_RW(idle_timeout);
+
+static ssize_t dim_timeout_show(struct device *dev,
+				struct device_attribute *attr, char *buf);
+static ssize_t dim_timeout_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t size);
+static DEVICE_ATTR_RW(dim_timeout);
+
+static ssize_t fnmode_show(struct device *dev, struct device_attribute *attr,
+			   char *buf);
+static ssize_t fnmode_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t size);
+static DEVICE_ATTR_RW(fnmode);
+
+static struct attribute *appletb_attrs[] = {
+	&dev_attr_idle_timeout.attr,
+	&dev_attr_dim_timeout.attr,
+	&dev_attr_fnmode.attr,
+	NULL,
+};
+
+static const struct attribute_group appletb_attr_group = {
+	.attrs = appletb_attrs,
+};
+
+struct appletb_device {
+	bool			active;
+	struct device		*log_dev;
+
+	struct hid_field	*mode_field;
+	struct hid_field	*disp_field;
+	struct hid_field	*disp_field_aux1;
+	struct appletb_iface_info {
+		struct hid_device	*hdev;
+		struct usb_interface	*usb_iface;
+		bool			suspended;
+	}			mode_iface, disp_iface;
+
+	struct input_handler	inp_handler;
+	struct input_handle	kbd_handle;
+	struct input_handle	tpd_handle;
+
+	bool			last_tb_keys_pressed[APPLETB_MAX_TB_KEYS];
+	bool			last_tb_keys_translated[APPLETB_MAX_TB_KEYS];
+	bool			last_fn_pressed;
+
+	ktime_t			last_event_time;
+
+	unsigned char		cur_tb_mode;
+	unsigned char		pnd_tb_mode;
+	unsigned char		cur_tb_disp;
+	unsigned char		pnd_tb_disp;
+	bool			tb_autopm_off;
+	bool			restore_autopm;
+	struct delayed_work	tb_work;
+	/* protects most of the above */
+	spinlock_t		tb_lock;
+
+	int			dim_timeout;
+	int			idle_timeout;
+	bool			dim_to_is_calc;
+	int			fn_mode;
+};
+
+struct appletb_key_translation {
+	u16 from;
+	u16 to;
+};
+
+static const struct appletb_key_translation appletb_fn_codes[] = {
+	{ KEY_F1,  KEY_BRIGHTNESSDOWN },
+	{ KEY_F2,  KEY_BRIGHTNESSUP },
+	{ KEY_F3,  KEY_SCALE },		/* not used */
+	{ KEY_F4,  KEY_DASHBOARD },	/* not used */
+	{ KEY_F5,  KEY_KBDILLUMDOWN },
+	{ KEY_F6,  KEY_KBDILLUMUP },
+	{ KEY_F7,  KEY_PREVIOUSSONG },
+	{ KEY_F8,  KEY_PLAYPAUSE },
+	{ KEY_F9,  KEY_NEXTSONG },
+	{ KEY_F10, KEY_MUTE },
+	{ KEY_F11, KEY_VOLUMEDOWN },
+	{ KEY_F12, KEY_VOLUMEUP },
+};
+
+static struct appletb_device *appletb_dev;
+
+static int appletb_send_usb_ctrl(struct appletb_iface_info *iface_info,
+				 __u8 requesttype, struct hid_report *report,
+				 void *data, __u16 size)
+{
+	struct usb_device *dev = interface_to_usbdev(iface_info->usb_iface);
+	u8 ifnum = iface_info->usb_iface->cur_altsetting->desc.bInterfaceNumber;
+	int tries = 0;
+	int rc;
+
+	do {
+		rc = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+				     HID_REQ_SET_REPORT, requesttype,
+				     (report->type + 1) << 8 | report->id,
+				     ifnum, data, size, 2000);
+		if (rc != -EPIPE)
+			break;
+
+		usleep_range(1000 << tries, 3000 << tries);
+	} while (++tries < 5);
+
+	return (rc > 0) ? 0 : rc;
+}
+
+static bool appletb_disable_autopm(struct hid_device *hdev)
+{
+	int rc;
+
+	rc = hid_hw_power(hdev, PM_HINT_FULLON);
+
+	if (rc == 0)
+		return true;
+
+	hid_err(hdev,
+		"Failed to disable auto-pm on touch bar device (%d)\n", rc);
+	return false;
+}
+
+/*
+ * While the mode functionality is listed as a valid hid report in the usb
+ * interface descriptor, it's not sent that way. Instead it's sent with
+ * different request-type and without a leading report-id in the data. Hence
+ * we need to send it as a custom usb control message rather via any of the
+ * standard hid_hw_*request() functions.
+ */
+static int appletb_set_tb_mode(struct appletb_device *tb_dev,
+			       unsigned char mode)
+{
+	void *buf;
+	bool autopm_off = false;
+	int rc;
+
+	if (!tb_dev->mode_iface.hdev)
+		return -ENOTCONN;
+
+	buf = kmemdup(&mode, 1, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	autopm_off = appletb_disable_autopm(tb_dev->mode_iface.hdev);
+
+	rc = appletb_send_usb_ctrl(&tb_dev->mode_iface,
+				   USB_DIR_OUT | USB_TYPE_VENDOR |
+							USB_RECIP_DEVICE,
+				   tb_dev->mode_field->report, buf, 1);
+	if (rc < 0)
+		dev_err(tb_dev->log_dev,
+			"Failed to set touch bar mode to %u (%d)\n", mode, rc);
+
+	if (autopm_off)
+		hid_hw_power(tb_dev->mode_iface.hdev, PM_HINT_NORMAL);
+
+	kfree(buf);
+
+	return rc;
+}
+
+/*
+ * We don't use hid_hw_request() because that doesn't allow us to get the
+ * returned status from the usb-control request; we also don't use
+ * hid_hw_raw_request() because would mean duplicating the retry-on-EPIPE
+ * in our appletb_send_usb_ctrl().
+ */
+static int appletb_send_hid_report(struct appletb_iface_info *iface_info,
+				   struct hid_report *report)
+{
+	unsigned char *buf;
+	int rc;
+
+	buf = hid_alloc_report_buf(report, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	hid_output_report(report, buf);
+
+	rc = appletb_send_usb_ctrl(iface_info,
+				   USB_DIR_OUT | USB_TYPE_CLASS |
+							USB_RECIP_INTERFACE,
+				   report, buf, hid_report_len(report));
+
+	kfree(buf);
+
+	return rc;
+}
+
+static int appletb_set_tb_disp(struct appletb_device *tb_dev,
+			       unsigned char disp)
+{
+	struct hid_report *report;
+	int rc;
+
+	if (!tb_dev->disp_iface.hdev)
+		return -ENOTCONN;
+
+	report = tb_dev->disp_field->report;
+
+	rc = hid_set_field(tb_dev->disp_field_aux1, 0, 1);
+	if (!rc)
+		rc = hid_set_field(tb_dev->disp_field, 0, disp);
+	if (rc) {
+		dev_err(tb_dev->log_dev,
+			"Failed to set display report fields (%d)\n", rc);
+		return rc;
+	}
+
+	/*
+	 * Keep the USB interface powered on while the touch bar display is on
+	 * for better responsiveness.
+	 */
+	if (disp != APPLETB_CMD_DISP_OFF && !tb_dev->tb_autopm_off)
+		tb_dev->tb_autopm_off =
+			appletb_disable_autopm(report->device);
+
+	rc = appletb_send_hid_report(&tb_dev->disp_iface, report);
+
+	if (rc < 0)
+		dev_err(tb_dev->log_dev,
+			"Failed to set touch bar display to %u (%d)\n", disp,
+			rc);
+
+	if (disp == APPLETB_CMD_DISP_OFF && tb_dev->tb_autopm_off) {
+		hid_hw_power(tb_dev->disp_iface.hdev, PM_HINT_NORMAL);
+		tb_dev->tb_autopm_off = false;
+	}
+
+	return rc;
+}
+
+static bool appletb_any_tb_key_pressed(struct appletb_device *tb_dev)
+{
+	return !!memchr_inv(tb_dev->last_tb_keys_pressed, 0,
+			    sizeof(tb_dev->last_tb_keys_pressed));
+}
+
+static void appletb_schedule_tb_update(struct appletb_device *tb_dev, s64 secs)
+{
+	schedule_delayed_work(&tb_dev->tb_work, msecs_to_jiffies(secs * 1000));
+}
+
+static void appletb_set_tb_worker(struct work_struct *work)
+{
+	struct appletb_device *tb_dev =
+		container_of(work, struct appletb_device, tb_work.work);
+	s64 time_left = 0, min_timeout, time_to_off;
+	unsigned char pending_mode;
+	unsigned char pending_disp;
+	unsigned char current_disp;
+	bool restore_autopm;
+	bool any_tb_key_pressed, need_reschedule;
+	int rc1 = 1, rc2 = 1;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	/* handle explicit mode-change request */
+	pending_mode = tb_dev->pnd_tb_mode;
+	pending_disp = tb_dev->pnd_tb_disp;
+	restore_autopm = tb_dev->restore_autopm;
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	if (pending_mode != APPLETB_CMD_MODE_NONE)
+		rc1 = appletb_set_tb_mode(tb_dev, pending_mode);
+	if (pending_mode != APPLETB_CMD_MODE_NONE &&
+	    pending_disp != APPLETB_CMD_DISP_NONE)
+		msleep(25);
+	if (pending_disp != APPLETB_CMD_DISP_NONE)
+		rc2 = appletb_set_tb_disp(tb_dev, pending_disp);
+
+	if (restore_autopm && tb_dev->tb_autopm_off)
+		appletb_disable_autopm(tb_dev->disp_field->report->device);
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	need_reschedule = false;
+
+	if (rc1 == 0) {
+		tb_dev->cur_tb_mode = pending_mode;
+
+		if (tb_dev->pnd_tb_mode == pending_mode)
+			tb_dev->pnd_tb_mode = APPLETB_CMD_MODE_NONE;
+		else
+			need_reschedule = true;
+	}
+
+	if (rc2 == 0) {
+		tb_dev->cur_tb_disp = pending_disp;
+
+		if (tb_dev->pnd_tb_disp == pending_disp)
+			tb_dev->pnd_tb_disp = APPLETB_CMD_DISP_NONE;
+		else
+			need_reschedule = true;
+	}
+	current_disp = tb_dev->cur_tb_disp;
+
+	tb_dev->restore_autopm = false;
+
+	/* calculate time left to next timeout */
+	if (tb_dev->idle_timeout == -2 || tb_dev->idle_timeout == 0)
+		min_timeout = -1;
+	else if (tb_dev->idle_timeout == -1)
+		min_timeout = tb_dev->dim_timeout;
+	else if (tb_dev->dim_timeout <= 0)
+		min_timeout = tb_dev->idle_timeout;
+	else
+		min_timeout = min(tb_dev->dim_timeout, tb_dev->idle_timeout);
+
+	if (min_timeout > 0) {
+		s64 idle_time =
+			(ktime_ms_delta(ktime_get(), tb_dev->last_event_time) +
+			 500) / 1000;
+
+		time_left = max(min_timeout - idle_time, 0LL);
+		if (tb_dev->idle_timeout <= 0)
+			time_to_off = -1;
+		else if (idle_time >= tb_dev->idle_timeout)
+			time_to_off = 0;
+		else
+			time_to_off = tb_dev->idle_timeout - idle_time;
+	} else {
+		/* not used - just to appease the compiler */
+		time_to_off = 0;
+	}
+
+	any_tb_key_pressed = appletb_any_tb_key_pressed(tb_dev);
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	dev_dbg(tb_dev->log_dev, "timeout calc: idle_timeout=%d dim_timeout=%d min_timeout=%lld time_left=%lld need_reschedule=%d any_tb_key_pressed=%d\n",
+		tb_dev->idle_timeout, tb_dev->dim_timeout, min_timeout,
+		time_left, need_reschedule, any_tb_key_pressed);
+
+	/* a new command arrived while we were busy - handle it */
+	if (need_reschedule) {
+		appletb_schedule_tb_update(tb_dev, 0);
+		return;
+	}
+
+	/* if no idle/dim timeout, we're done */
+	if (min_timeout <= 0)
+		return;
+
+	/* manage idle/dim timeout */
+	if (time_left > 0) {
+		/* we fired too soon or had a mode-change - re-schedule */
+		appletb_schedule_tb_update(tb_dev, time_left);
+	} else if (any_tb_key_pressed) {
+		/* keys are still pressed - re-schedule */
+		appletb_schedule_tb_update(tb_dev, min_timeout);
+	} else {
+		/* dim or idle timeout reached */
+		int next_disp = (time_to_off == 0) ? APPLETB_CMD_DISP_OFF :
+						     APPLETB_CMD_DISP_DIM;
+		if (next_disp != current_disp &&
+		    appletb_set_tb_disp(tb_dev, next_disp) == 0) {
+			spin_lock_irqsave(&tb_dev->tb_lock, flags);
+			tb_dev->cur_tb_disp = next_disp;
+			spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+		}
+
+		if (time_to_off > 0)
+			appletb_schedule_tb_update(tb_dev, time_to_off);
+	}
+}
+
+static u16 appletb_fn_to_special(u16 code)
+{
+	int idx;
+
+	for (idx = 0; idx < ARRAY_SIZE(appletb_fn_codes); idx++) {
+		if (appletb_fn_codes[idx].from == code)
+			return appletb_fn_codes[idx].to;
+	}
+
+	return 0;
+}
+
+static unsigned char appletb_get_cur_tb_mode(struct appletb_device *tb_dev)
+{
+	return tb_dev->pnd_tb_mode != APPLETB_CMD_MODE_NONE ?
+				tb_dev->pnd_tb_mode : tb_dev->cur_tb_mode;
+}
+
+static unsigned char appletb_get_cur_tb_disp(struct appletb_device *tb_dev)
+{
+	return tb_dev->pnd_tb_disp != APPLETB_CMD_DISP_NONE ?
+				tb_dev->pnd_tb_disp : tb_dev->cur_tb_disp;
+}
+
+static unsigned char appletb_get_fn_tb_mode(struct appletb_device *tb_dev)
+{
+	switch (tb_dev->fn_mode) {
+	case APPLETB_FN_MODE_FKEYS:
+		return APPLETB_CMD_MODE_FN;
+
+	case APPLETB_FN_MODE_SPCL:
+		return APPLETB_CMD_MODE_SPCL;
+
+	case APPLETB_FN_MODE_INV:
+		return (tb_dev->last_fn_pressed) ? APPLETB_CMD_MODE_SPCL :
+						   APPLETB_CMD_MODE_FN;
+
+	case APPLETB_FN_MODE_NORM:
+	default:
+		return (tb_dev->last_fn_pressed) ? APPLETB_CMD_MODE_FN :
+						   APPLETB_CMD_MODE_SPCL;
+	}
+}
+
+/*
+ * Switch touch bar mode and display when mode or display not the desired ones.
+ */
+static void appletb_update_touchbar_no_lock(struct appletb_device *tb_dev,
+					    bool force)
+{
+	unsigned char want_mode;
+	unsigned char want_disp;
+	bool need_update = false;
+
+	/*
+	 * Calculate the new modes:
+	 *   idle_timeout:
+	 *     -2  mode/disp off
+	 *     -1  mode on, disp on/dim
+	 *      0  mode on, disp off
+	 *     >0  mode on, disp off after idle_timeout seconds
+	 *   dim_timeout (only valid if idle_timeout > 0 || idle_timeout == -1):
+	 *     -1  disp never dimmed
+	 *      0  disp always dimmed
+	 *     >0  disp dim after dim_timeout seconds
+	 */
+	if (tb_dev->idle_timeout == -2) {
+		want_mode = APPLETB_CMD_MODE_OFF;
+		want_disp = APPLETB_CMD_DISP_OFF;
+	} else {
+		want_mode = appletb_get_fn_tb_mode(tb_dev);
+		want_disp = tb_dev->idle_timeout ==  0 ? APPLETB_CMD_DISP_OFF :
+			    tb_dev->dim_timeout  ==  0 ? APPLETB_CMD_DISP_DIM :
+							 APPLETB_CMD_DISP_ON;
+	}
+
+	/*
+	 * See if we need to update the touch bar, taking into account that we
+	 * generally don't want to switch modes while a touch bar key is
+	 * pressed.
+	 */
+	if (appletb_get_cur_tb_mode(tb_dev) != want_mode &&
+	    !appletb_any_tb_key_pressed(tb_dev)) {
+		tb_dev->pnd_tb_mode = want_mode;
+		need_update = true;
+	}
+
+	if (appletb_get_cur_tb_disp(tb_dev) != want_disp &&
+	    (!appletb_any_tb_key_pressed(tb_dev) ||
+	     want_disp != APPLETB_CMD_DISP_OFF)) {
+		tb_dev->pnd_tb_disp = want_disp;
+		need_update = true;
+	}
+
+	if (force)
+		need_update = true;
+
+	/* schedule the update if desired */
+	dev_dbg_ratelimited(tb_dev->log_dev,
+			    "update: need_update=%d, want_mode=%d, cur-mode=%d, want_disp=%d, cur-disp=%d\n",
+			    need_update, want_mode, tb_dev->cur_tb_mode,
+			    want_disp, tb_dev->cur_tb_disp);
+
+	if (need_update) {
+		cancel_delayed_work(&tb_dev->tb_work);
+		appletb_schedule_tb_update(tb_dev, 0);
+	}
+}
+
+static void appletb_update_touchbar(struct appletb_device *tb_dev, bool force)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (tb_dev->active)
+		appletb_update_touchbar_no_lock(tb_dev, force);
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+}
+
+static void appletb_set_idle_timeout(struct appletb_device *tb_dev, int new)
+{
+	tb_dev->idle_timeout = new;
+
+	if (tb_dev->dim_to_is_calc && tb_dev->idle_timeout > 0)
+		tb_dev->dim_timeout = new - min(APPLETB_MAX_DIM_TIME, new / 3);
+	else if (tb_dev->dim_to_is_calc)
+		tb_dev->dim_timeout = -1;
+}
+
+static ssize_t idle_timeout_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", tb_dev->idle_timeout);
+}
+
+static ssize_t idle_timeout_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t size)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+	long new;
+	int rc;
+
+	rc = kstrtol(buf, 0, &new);
+	if (rc || new > INT_MAX || new < -2)
+		return -EINVAL;
+
+	appletb_set_idle_timeout(tb_dev, new);
+	appletb_update_touchbar(tb_dev, true);
+
+	return size;
+}
+
+static void appletb_set_dim_timeout(struct appletb_device *tb_dev, int new)
+{
+	if (new == -2) {
+		tb_dev->dim_to_is_calc = true;
+		appletb_set_idle_timeout(tb_dev, tb_dev->idle_timeout);
+	} else {
+		tb_dev->dim_to_is_calc = false;
+		tb_dev->dim_timeout = new;
+	}
+}
+
+static ssize_t dim_timeout_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			tb_dev->dim_to_is_calc ? -2 : tb_dev->dim_timeout);
+}
+
+static ssize_t dim_timeout_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t size)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+	long new;
+	int rc;
+
+	rc = kstrtol(buf, 0, &new);
+	if (rc || new > INT_MAX || new < -2)
+		return -EINVAL;
+
+	appletb_set_dim_timeout(tb_dev, new);
+	appletb_update_touchbar(tb_dev, true);
+
+	return size;
+}
+
+static ssize_t fnmode_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", tb_dev->fn_mode);
+}
+
+static ssize_t fnmode_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t size)
+{
+	struct appletb_device *tb_dev = dev_get_drvdata(dev);
+	long new;
+	int rc;
+
+	rc = kstrtol(buf, 0, &new);
+	if (rc || new > APPLETB_FN_MODE_MAX || new < 0)
+		return -EINVAL;
+
+	tb_dev->fn_mode = new;
+	appletb_update_touchbar(tb_dev, false);
+
+	return size;
+}
+
+static int appletb_tb_key_to_slot(unsigned int code)
+{
+	switch (code) {
+	case KEY_ESC:
+		return 0;
+	case KEY_F1:
+	case KEY_F2:
+	case KEY_F3:
+	case KEY_F4:
+	case KEY_F5:
+	case KEY_F6:
+	case KEY_F7:
+	case KEY_F8:
+	case KEY_F9:
+	case KEY_F10:
+		return code - KEY_F1 + 1;
+	case KEY_F11:
+	case KEY_F12:
+		return code - KEY_F11 + 11;
+	default:
+		return -1;
+	}
+}
+
+static int appletb_hid_event(struct hid_device *hdev, struct hid_field *field,
+			     struct hid_usage *usage, __s32 value)
+{
+	struct appletb_device *tb_dev = hid_get_drvdata(hdev);
+	unsigned int new_code = 0;
+	unsigned long flags;
+	bool send_dummy = false;
+	bool send_trnsl = false;
+	int slot;
+	int rc = 0;
+
+	if ((usage->hid & HID_USAGE_PAGE) != HID_UP_KEYBOARD ||
+	    usage->type != EV_KEY)
+		return 0;
+
+	/*
+	 * Skip non-touch-bar keys.
+	 *
+	 * Either the touch bar itself or usbhid generate a slew of key-down
+	 * events for all the meta keys. None of which we're at all interested
+	 * in.
+	 */
+	slot = appletb_tb_key_to_slot(usage->code);
+	if (slot < 0)
+		return 0;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (!tb_dev->active) {
+		spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+		return 0;
+	}
+
+	new_code = appletb_fn_to_special(usage->code);
+
+	if (value != 2)
+		tb_dev->last_tb_keys_pressed[slot] = value;
+
+	tb_dev->last_event_time = ktime_get();
+
+	appletb_update_touchbar_no_lock(tb_dev, false);
+
+	/*
+	 * We want to suppress touch bar keys while the touch bar is off, but
+	 * we do want to wake up the screen if it's asleep, so generate a dummy
+	 * event in that case.
+	 */
+	if (tb_dev->cur_tb_mode == APPLETB_CMD_MODE_OFF ||
+	    tb_dev->cur_tb_disp == APPLETB_CMD_DISP_OFF) {
+		send_dummy = true;
+		rc = 1;
+	/* translate special keys */
+	} else if (new_code &&
+		   ((value > 0 &&
+		     appletb_get_cur_tb_mode(tb_dev) == APPLETB_CMD_MODE_SPCL)
+		    ||
+		    (value == 0 && tb_dev->last_tb_keys_translated[slot]))) {
+		tb_dev->last_tb_keys_translated[slot] = true;
+		send_trnsl = true;
+		rc = 1;
+	/* everything else handled normally */
+	} else {
+		tb_dev->last_tb_keys_translated[slot] = false;
+	}
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	/*
+	 * Need to send these input events outside of the lock, as otherwise
+	 * we can run into the following deadlock:
+	 *            Task 1                         Task 2
+	 *     appletb_hid_event()            input_event()
+	 *       acquire tb_lock                acquire dev->event_lock
+	 *       input_event()                  appletb_inp_event()
+	 *         acquire dev->event_lock        acquire tb_lock
+	 */
+	if (send_dummy) {
+		input_event(field->hidinput->input, EV_KEY, KEY_UNKNOWN, 1);
+		input_event(field->hidinput->input, EV_KEY, KEY_UNKNOWN, 0);
+	} else if (send_trnsl) {
+		input_event(field->hidinput->input, usage->type, new_code,
+			    value);
+	}
+
+	return rc;
+}
+
+static void appletb_inp_event(struct input_handle *handle, unsigned int type,
+			      unsigned int code, int value)
+{
+	struct appletb_device *tb_dev = handle->private;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (!tb_dev->active) {
+		spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+		return;
+	}
+
+	if (type == EV_KEY && code == KEY_FN && value != 2)
+		tb_dev->last_fn_pressed = value;
+
+	tb_dev->last_event_time = ktime_get();
+
+	appletb_update_touchbar_no_lock(tb_dev, false);
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+}
+
+/* Find and save the usb-device associated with the touch bar input device */
+static struct usb_interface *appletb_get_usb_iface(struct hid_device *hdev)
+{
+	struct device *dev = &hdev->dev;
+
+	/* iBridge virt-hid */
+	if (!dev->bus || strcmp(dev->bus->name, "hid") != 0)
+		return NULL;
+
+	/* real hid */
+	dev = dev->parent;
+	if (!dev || !dev->bus || strcmp(dev->bus->name, "hid") != 0)
+		return NULL;
+
+	/* usb dev */
+	dev = dev->parent;
+	if (!dev || !dev->bus || strcmp(dev->bus->name, "usb") != 0)
+		return NULL;
+
+	return to_usb_interface(dev);
+}
+
+static int appletb_inp_connect(struct input_handler *handler,
+			       struct input_dev *dev,
+			       const struct input_device_id *id)
+{
+	struct appletb_device *tb_dev = handler->private;
+	struct input_handle *handle;
+	int rc;
+
+	if (id->driver_info == APPLETB_DEVID_KEYBOARD) {
+		handle = &tb_dev->kbd_handle;
+		handle->name = "tbkbd";
+	} else if (id->driver_info == APPLETB_DEVID_TOUCHPAD) {
+		handle = &tb_dev->tpd_handle;
+		handle->name = "tbtpad";
+	} else {
+		dev_err(tb_dev->log_dev, "Unknown device id (%lu)\n",
+			id->driver_info);
+		return -ENOENT;
+	}
+
+	if (handle->dev) {
+		dev_err(tb_dev->log_dev,
+			"Duplicate connect to %s input device\n", handle->name);
+		return -EEXIST;
+	}
+
+	handle->open = 0;
+	handle->dev = input_get_device(dev);
+	handle->handler = handler;
+	handle->private = tb_dev;
+
+	rc = input_register_handle(handle);
+	if (rc)
+		goto err_free_dev;
+
+	rc = input_open_device(handle);
+	if (rc)
+		goto err_unregister_handle;
+
+	dev_dbg(tb_dev->log_dev, "Connected to %s input device\n",
+		handle == &tb_dev->kbd_handle ? "keyboard" : "touchpad");
+
+	return 0;
+
+ err_unregister_handle:
+	input_unregister_handle(handle);
+ err_free_dev:
+	input_put_device(handle->dev);
+	handle->dev = NULL;
+	return rc;
+}
+
+static void appletb_inp_disconnect(struct input_handle *handle)
+{
+	struct appletb_device *tb_dev = handle->private;
+
+	input_close_device(handle);
+	input_unregister_handle(handle);
+
+	dev_dbg(tb_dev->log_dev, "Disconnected from %s input device\n",
+		handle == &tb_dev->kbd_handle ? "keyboard" : "touchpad");
+
+	input_put_device(handle->dev);
+	handle->dev = NULL;
+}
+
+static int appletb_input_configured(struct hid_device *hdev,
+				    struct hid_input *hidinput)
+{
+	int idx;
+	struct input_dev *input = hidinput->input;
+
+	/*
+	 * Clear various input capabilities that are blindly set by the hid
+	 * driver (usbkbd.c)
+	 */
+	memset(input->evbit, 0, sizeof(input->evbit));
+	memset(input->keybit, 0, sizeof(input->keybit));
+	memset(input->ledbit, 0, sizeof(input->ledbit));
+
+	/* set our actual capabilities */
+	__set_bit(EV_KEY, input->evbit);
+	__set_bit(EV_REP, input->evbit);
+	__set_bit(EV_MSC, input->evbit);  /* hid-input generates MSC_SCAN */
+
+	for (idx = 0; idx < ARRAY_SIZE(appletb_fn_codes); idx++) {
+		input_set_capability(input, EV_KEY, appletb_fn_codes[idx].from);
+		input_set_capability(input, EV_KEY, appletb_fn_codes[idx].to);
+	}
+
+	input_set_capability(input, EV_KEY, KEY_ESC);
+	input_set_capability(input, EV_KEY, KEY_UNKNOWN);
+
+	return 0;
+}
+
+static int appletb_extract_report_info(struct appletb_device *tb_dev,
+				       struct hid_device *hdev)
+{
+	struct appletb_iface_info *iface_info;
+	struct usb_interface *usb_iface;
+	struct hid_field *field;
+
+	field = appleib_find_hid_field(hdev, HID_GD_KEYBOARD, HID_USAGE_MODE);
+	if (field) {
+		iface_info = &tb_dev->mode_iface;
+		tb_dev->mode_field = field;
+	} else {
+		field = appleib_find_hid_field(hdev, HID_USAGE_APPLE_APP,
+					       HID_USAGE_DISP);
+		if (!field)
+			return 0;
+
+		iface_info = &tb_dev->disp_iface;
+		tb_dev->disp_field = field;
+		tb_dev->disp_field_aux1 =
+			appleib_find_hid_field(hdev, HID_USAGE_APPLE_APP,
+					       HID_USAGE_DISP_AUX1);
+
+		if (!tb_dev->disp_field_aux1 ||
+		    tb_dev->disp_field_aux1->report !=
+						tb_dev->disp_field->report) {
+			dev_err(tb_dev->log_dev,
+				"Unexpected report structure for report %u in device %s\n",
+				tb_dev->disp_field->report->id,
+				dev_name(&hdev->dev));
+			return -ENODEV;
+		}
+	}
+
+	usb_iface = appletb_get_usb_iface(hdev);
+	if (!usb_iface) {
+		dev_err(tb_dev->log_dev,
+			"Failed to find usb interface for hid device %s\n",
+			dev_name(&hdev->dev));
+		return -ENODEV;
+	}
+
+	iface_info->hdev = hdev;
+	iface_info->usb_iface = usb_get_intf(usb_iface);
+	iface_info->suspended = false;
+
+	return 1;
+}
+
+static struct appletb_iface_info *
+appletb_get_iface_info(struct appletb_device *tb_dev, struct hid_device *hdev)
+{
+	if (hdev == tb_dev->mode_iface.hdev)
+		return &tb_dev->mode_iface;
+	if (hdev == tb_dev->disp_iface.hdev)
+		return &tb_dev->disp_iface;
+	return NULL;
+}
+
+static bool appletb_test_and_mark_active(struct appletb_device *tb_dev)
+{
+	unsigned long flags;
+	bool activated = false;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (tb_dev->mode_iface.hdev && tb_dev->disp_iface.hdev &&
+	    !tb_dev->active) {
+		tb_dev->active = true;
+		activated = true;
+	}
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	return activated;
+}
+
+static bool appletb_test_and_mark_inactive(struct appletb_device *tb_dev,
+					   struct hid_device *hdev)
+{
+	unsigned long flags;
+	bool deactivated = false;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (tb_dev->mode_iface.hdev && tb_dev->disp_iface.hdev &&
+	    tb_dev->active &&
+	    (hdev == tb_dev->mode_iface.hdev ||
+	     hdev == tb_dev->disp_iface.hdev)) {
+		tb_dev->active = false;
+		deactivated = true;
+	}
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	return deactivated;
+}
+
+static const struct input_device_id appletb_input_devices[] = {
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_BUS |
+			INPUT_DEVICE_ID_MATCH_KEYBIT,
+		.bustype = BUS_SPI,
+		.keybit = { [BIT_WORD(KEY_FN)] = BIT_MASK(KEY_FN) },
+		.driver_info = APPLETB_DEVID_KEYBOARD,
+	},			/* Builtin keyboard device */
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_BUS |
+			INPUT_DEVICE_ID_MATCH_KEYBIT,
+		.bustype = BUS_SPI,
+		.keybit = { [BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH) },
+		.driver_info = APPLETB_DEVID_TOUCHPAD,
+	},			/* Builtin touchpad device */
+	{ },			/* Terminating zero entry */
+};
+
+static int appletb_probe(struct hid_device *hdev,
+			 const struct hid_device_id *id)
+{
+	struct appletb_device *tb_dev = appletb_dev;
+	struct appletb_iface_info *iface_info;
+	unsigned long flags;
+	int rc;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (!tb_dev->log_dev)
+		tb_dev->log_dev = &hdev->dev;
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	hid_set_drvdata(hdev, tb_dev);
+
+	/* initialize the report info */
+	rc = hid_parse(hdev);
+	if (rc) {
+		dev_err(tb_dev->log_dev, "als: hid parse failed (%d)\n", rc);
+		goto error;
+	}
+
+	rc = appletb_extract_report_info(tb_dev, hdev);
+	if (rc < 0)
+		goto error;
+
+	rc = hid_hw_start(hdev, HID_CONNECT_DRIVER | HID_CONNECT_HIDINPUT);
+	if (rc) {
+		dev_err(tb_dev->log_dev, "hw start failed (%d)\n", rc);
+		goto clear_iface_info;
+	}
+
+	rc = hid_hw_open(hdev);
+	if (rc) {
+		dev_err(tb_dev->log_dev, "hw open failed (%d)\n", rc);
+		goto stop_hid;
+	}
+
+	/* do setup if we have both interfaces */
+	if (appletb_test_and_mark_active(tb_dev)) {
+		/* initialize the touch bar */
+		if (appletb_tb_def_fn_mode >= 0 &&
+		    appletb_tb_def_fn_mode <= APPLETB_FN_MODE_MAX)
+			tb_dev->fn_mode = appletb_tb_def_fn_mode;
+		else
+			tb_dev->fn_mode = APPLETB_FN_MODE_NORM;
+		appletb_set_idle_timeout(tb_dev, appletb_tb_def_idle_timeout);
+		appletb_set_dim_timeout(tb_dev, appletb_tb_def_dim_timeout);
+		tb_dev->last_event_time = ktime_get();
+
+		tb_dev->pnd_tb_mode = APPLETB_CMD_MODE_UPD;
+		tb_dev->pnd_tb_disp = APPLETB_CMD_DISP_UPD;
+
+		appletb_update_touchbar(tb_dev, false);
+
+		/* set up the input handler */
+		tb_dev->inp_handler.event = appletb_inp_event;
+		tb_dev->inp_handler.connect = appletb_inp_connect;
+		tb_dev->inp_handler.disconnect = appletb_inp_disconnect;
+		tb_dev->inp_handler.name = "appletb";
+		tb_dev->inp_handler.id_table = appletb_input_devices;
+		tb_dev->inp_handler.private = tb_dev;
+
+		rc = input_register_handler(&tb_dev->inp_handler);
+		if (rc) {
+			dev_err(tb_dev->log_dev,
+				"Unable to register keyboard handler (%d)\n",
+				rc);
+			goto mark_inactive;
+		}
+
+		/* initialize sysfs attributes */
+		rc = sysfs_create_group(&tb_dev->mode_iface.hdev->dev.kobj,
+					&appletb_attr_group);
+		if (rc) {
+			dev_err(tb_dev->log_dev,
+				"Failed to create sysfs attributes (%d)\n", rc);
+			goto unreg_handler;
+		}
+
+		dev_dbg(tb_dev->log_dev, "Touchbar activated\n");
+	}
+
+	return 0;
+
+unreg_handler:
+	input_unregister_handler(&tb_dev->inp_handler);
+mark_inactive:
+	appletb_test_and_mark_inactive(tb_dev, hdev);
+	cancel_delayed_work_sync(&tb_dev->tb_work);
+	hid_hw_close(hdev);
+stop_hid:
+	hid_hw_stop(hdev);
+clear_iface_info:
+	iface_info = appletb_get_iface_info(tb_dev, hdev);
+	if (iface_info) {
+		usb_put_intf(iface_info->usb_iface);
+		iface_info->usb_iface = NULL;
+		iface_info->hdev = NULL;
+	}
+error:
+	return rc;
+}
+
+static void appletb_remove(struct hid_device *hdev)
+{
+	struct appletb_device *tb_dev = hid_get_drvdata(hdev);
+	struct appletb_iface_info *iface_info;
+	unsigned long flags;
+
+	if (appletb_test_and_mark_inactive(tb_dev, hdev)) {
+		sysfs_remove_group(&tb_dev->mode_iface.hdev->dev.kobj,
+				   &appletb_attr_group);
+
+		input_unregister_handler(&tb_dev->inp_handler);
+
+		cancel_delayed_work_sync(&tb_dev->tb_work);
+		appletb_set_tb_mode(tb_dev, APPLETB_CMD_MODE_OFF);
+		appletb_set_tb_disp(tb_dev, APPLETB_CMD_DISP_ON);
+
+		if (tb_dev->tb_autopm_off)
+			hid_hw_power(tb_dev->disp_iface.hdev, PM_HINT_NORMAL);
+
+		dev_info(tb_dev->log_dev, "Touchbar deactivated\n");
+	}
+
+	hid_hw_close(hdev);
+	hid_hw_stop(hdev);
+
+	iface_info = appletb_get_iface_info(tb_dev, hdev);
+	if (iface_info) {
+		usb_put_intf(iface_info->usb_iface);
+		iface_info->usb_iface = NULL;
+		iface_info->hdev = NULL;
+	}
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (tb_dev->log_dev == &hdev->dev) {
+		if (tb_dev->mode_iface.hdev)
+			tb_dev->log_dev = &tb_dev->mode_iface.hdev->dev;
+		else if (tb_dev->disp_iface.hdev)
+			tb_dev->log_dev = &tb_dev->disp_iface.hdev->dev;
+		else
+			tb_dev->log_dev = NULL;
+	}
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+}
+
+#ifdef CONFIG_PM
+static int appletb_suspend(struct hid_device *hdev, pm_message_t message)
+{
+	struct appletb_device *tb_dev = hid_get_drvdata(hdev);
+	struct appletb_iface_info *iface_info;
+	unsigned long flags;
+	bool all_suspended = false;
+
+	if (message.event != PM_EVENT_SUSPEND &&
+	    message.event != PM_EVENT_FREEZE)
+		return 0;
+
+	/*
+	 * Wait for both interfaces to be suspended and no more async work
+	 * in progress.
+	 */
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	if (!tb_dev->mode_iface.suspended && !tb_dev->disp_iface.suspended) {
+		tb_dev->active = false;
+		cancel_delayed_work(&tb_dev->tb_work);
+	}
+
+	iface_info = appletb_get_iface_info(tb_dev, hdev);
+	if (iface_info)
+		iface_info->suspended = true;
+
+	if ((!tb_dev->mode_iface.hdev || tb_dev->mode_iface.suspended) &&
+	    (!tb_dev->disp_iface.hdev || tb_dev->disp_iface.suspended))
+		all_suspended = true;
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	flush_delayed_work(&tb_dev->tb_work);
+
+	if (!all_suspended)
+		return 0;
+
+	/*
+	 * The touch bar device itself remembers the last state when suspended
+	 * in some cases, but in others (e.g. when mode != off and disp == off)
+	 * it resumes with a different state; furthermore it may be only
+	 * partially responsive in that state. By turning both mode and disp
+	 * off we ensure it is in a good state when resuming (and this happens
+	 * to be the same state after booting/resuming-from-hibernate, so less
+	 * special casing between the two).
+	 */
+	if (message.event == PM_EVENT_SUSPEND) {
+		appletb_set_tb_mode(tb_dev, APPLETB_CMD_MODE_OFF);
+		appletb_set_tb_disp(tb_dev, APPLETB_CMD_DISP_OFF);
+	}
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	tb_dev->cur_tb_mode = APPLETB_CMD_MODE_OFF;
+	tb_dev->cur_tb_disp = APPLETB_CMD_DISP_OFF;
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	dev_info(tb_dev->log_dev, "Touchbar suspended.\n");
+
+	return 0;
+}
+
+static int appletb_reset_resume(struct hid_device *hdev)
+{
+	struct appletb_device *tb_dev = hid_get_drvdata(hdev);
+	struct appletb_iface_info *iface_info;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tb_dev->tb_lock, flags);
+
+	iface_info = appletb_get_iface_info(tb_dev, hdev);
+	if (iface_info)
+		iface_info->suspended = false;
+
+	if ((tb_dev->mode_iface.hdev && !tb_dev->mode_iface.suspended) &&
+	    (tb_dev->disp_iface.hdev && !tb_dev->disp_iface.suspended)) {
+		/*
+		 * Restore touch bar state. Note that autopm state is not
+		 * preserved, so need explicitly restore that here.
+		 */
+		tb_dev->active = true;
+		tb_dev->restore_autopm = true;
+		tb_dev->last_event_time = ktime_get();
+
+		appletb_update_touchbar_no_lock(tb_dev, true);
+
+		dev_info(tb_dev->log_dev, "Touchbar resumed.\n");
+	}
+
+	spin_unlock_irqrestore(&tb_dev->tb_lock, flags);
+
+	return 0;
+}
+#endif
+
+static struct appletb_device *appletb_alloc_device(void)
+{
+	struct appletb_device *tb_dev;
+
+	tb_dev = kzalloc(sizeof(*tb_dev), GFP_KERNEL);
+	if (!tb_dev)
+		return NULL;
+
+	spin_lock_init(&tb_dev->tb_lock);
+	INIT_DELAYED_WORK(&tb_dev->tb_work, appletb_set_tb_worker);
+
+	return tb_dev;
+}
+
+static void appletb_free_device(struct appletb_device *tb_dev)
+{
+	cancel_delayed_work_sync(&tb_dev->tb_work);
+	kfree(tb_dev);
+}
+
+static const struct hid_device_id appletb_hid_ids[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LINUX_FOUNDATION,
+			 USB_DEVICE_ID_IBRIDGE_TB) },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(hid, appletb_hid_ids);
+
+static struct hid_driver appletb_hid_driver = {
+	.name = "apple-ib-touchbar",
+	.id_table = appletb_hid_ids,
+	.probe = appletb_probe,
+	.remove = appletb_remove,
+	.event = appletb_hid_event,
+	.input_configured = appletb_input_configured,
+#ifdef CONFIG_PM
+	.suspend = appletb_suspend,
+	.reset_resume = appletb_reset_resume,
+#endif
+};
+
+static int __init appletb_init(void)
+{
+	struct appletb_device *tb_dev;
+	int rc;
+
+	tb_dev = appletb_alloc_device();
+	if (!tb_dev)
+		return -ENOMEM;
+
+	appletb_dev = tb_dev;
+
+	rc = hid_register_driver(&appletb_hid_driver);
+	if (rc)
+		goto error;
+
+	return 0;
+
+error:
+	appletb_free_device(tb_dev);
+	return rc;
+}
+
+static void __exit appletb_exit(void)
+{
+	hid_unregister_driver(&appletb_hid_driver);
+	appletb_free_device(appletb_dev);
+}
+
+module_init(appletb_init);
+module_exit(appletb_exit);
+
+MODULE_AUTHOR("Ronald Tschalär");
+MODULE_DESCRIPTION("MacBookPro Touch Bar driver");
+MODULE_LICENSE("GPL v2");
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 1/3] HID: apple-ibridge: Add Apple iBridge HID driver.
From: Ronald Tschalär @ 2019-06-12  8:33 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler
  Cc: linux-input, linux-iio, linux-kernel
In-Reply-To: <20190612083400.1015-1-ronald@innovation.ch>

The iBridge device provides access to several devices, including:
- the Touch Bar
- the iSight webcam
- the light sensor
- the fingerprint sensor

This driver provides the core support for managing the iBridge device
and the access to the underlying devices. In particular, since the
functionality for the touch bar and light sensor is exposed via USB HID
interfaces, and the same HID device is used for multiple functions, this
driver creates virtual HID devices, one per real HID device and
sub-driver pair (for a total of 4 virtual HID devices). The sub-drivers
then bind to these virtual HID devices.

This way the Touch Bar and ALS drivers can be kept in their own modules,
while at the same time making them look very much like as if they were
connected to the real HID devices; e.g. in particular the Touch Bar
driver is aware of the fact that it is dealing with two HID devices that
need to managed differently.

Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
 drivers/hid/Kconfig           |  14 +
 drivers/hid/Makefile          |   1 +
 drivers/hid/apple-ibridge.c   | 585 ++++++++++++++++++++++++++++++++++
 drivers/hid/hid-ids.h         |   1 +
 include/linux/apple-ibridge.h |  23 ++
 5 files changed, 624 insertions(+)
 create mode 100644 drivers/hid/apple-ibridge.c
 create mode 100644 include/linux/apple-ibridge.h

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ca0cdfa6b33..545d3691fc1c 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -135,6 +135,20 @@ config HID_APPLE
 	Say Y here if you want support for keyboards of	Apple iBooks, PowerBooks,
 	MacBooks, MacBook Pros and Apple Aluminum.
 
+config HID_APPLE_IBRIDGE
+	tristate "Apple iBridge"
+	depends on ACPI
+	depends on USB_HID
+	depends on X86 || COMPILE_TEST
+	help
+	  This module provides the core support for the Apple T1 chip found
+	  on recent MacBookPro's, also known as the iBridge. The drivers for
+	  the Touch Bar (apple-ib-tb) and light sensor (apple-ib-als) need to
+	  be enabled separately.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called apple-ibridge.
+
 config HID_APPLEIR
 	tristate "Apple infrared receiver"
 	depends on (USB_HID)
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 170163b41303..a4da5663a541 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_HID_ACCUTOUCH)	+= hid-accutouch.o
 obj-$(CONFIG_HID_ALPS)		+= hid-alps.o
 obj-$(CONFIG_HID_ACRUX)		+= hid-axff.o
 obj-$(CONFIG_HID_APPLE)		+= hid-apple.o
+obj-$(CONFIG_HID_APPLE_IBRIDGE)	+= apple-ibridge.o
 obj-$(CONFIG_HID_APPLEIR)	+= hid-appleir.o
 obj-$(CONFIG_HID_ASUS)		+= hid-asus.o
 obj-$(CONFIG_HID_AUREAL)	+= hid-aureal.o
diff --git a/drivers/hid/apple-ibridge.c b/drivers/hid/apple-ibridge.c
new file mode 100644
index 000000000000..565f080a38d6
--- /dev/null
+++ b/drivers/hid/apple-ibridge.c
@@ -0,0 +1,585 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Apple iBridge Driver
+ *
+ * Copyright (c) 2018 Ronald Tschalär
+ */
+
+/**
+ * DOC: Overview
+ *
+ * MacBookPro models with a Touch Bar (13,[23] and 14,[23]) have an Apple
+ * iBridge chip (also known as T1 chip) which exposes the touch bar,
+ * built-in webcam (iSight), ambient light sensor, and Secure Enclave
+ * Processor (SEP) for TouchID. It shows up in the system as a USB device
+ * with 3 configurations: 'Default iBridge Interfaces', 'Default iBridge
+ * Interfaces(OS X)', and 'Default iBridge Interfaces(Recovery)'. While
+ * the second one is used by MacOS to provide the fancy touch bar
+ * functionality with custom buttons etc, this driver just uses the first.
+ *
+ * In the first (default after boot) configuration, 4 usb interfaces are
+ * exposed: 2 related to the webcam, and 2 USB HID interfaces representing
+ * the touch bar and the ambient light sensor. The webcam interfaces are
+ * already handled by the uvcvideo driver; furthermore, the handling of the
+ * input reports when "keys" on the touch bar are pressed is already handled
+ * properly by the generic USB HID core. This leaves the management of the
+ * touch bar modes (e.g. switching between function and special keys when the
+ * FN key is pressed), the touch bar display (dimming and turning off), the
+ * key-remapping when the FN key is pressed, and handling of the light sensor.
+ *
+ * This driver is implemented as a HID driver that creates virtual HID devices
+ * for the ALS and touch bar functionality, and the ALS and touch bar drivers
+ * are HID drivers which in turn attach to these virtual HID devices. This
+ * driver then relays the calls on the real HID devices to the virtual ones,
+ * and visa versa.
+ *
+ * Lastly, this driver also takes care of the power-management for the
+ * iBridge when suspending and resuming.
+ */
+
+#include <linux/acpi.h>
+#include <linux/apple-ibridge.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+
+#include "hid-ids.h"
+#include "../hid/usbhid/usbhid.h"
+
+#define APPLEIB_BASIC_CONFIG	1
+
+#define	LOG_DEV(ib_dev)		(&(ib_dev)->acpi_dev->dev)
+
+static struct hid_device_id appleib_sub_hid_ids[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LINUX_FOUNDATION,
+			 USB_DEVICE_ID_IBRIDGE_TB) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LINUX_FOUNDATION,
+			 USB_DEVICE_ID_IBRIDGE_ALS) },
+};
+
+struct appleib_device {
+	struct acpi_device		*acpi_dev;
+	acpi_handle			asoc_socw;
+};
+
+struct appleib_hid_dev_info {
+	struct hid_device	*hdev;
+	struct hid_device	*sub_hdevs[ARRAY_SIZE(appleib_sub_hid_ids)];
+};
+
+static void appleib_remove_device(struct hid_device *hdev)
+{
+	struct appleib_hid_dev_info *hdev_info = hid_get_drvdata(hdev);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++)
+		hid_destroy_device(hdev_info->sub_hdevs[i]);
+
+	hid_set_drvdata(hdev, NULL);
+}
+
+/**
+ * appleib_forward_int_op() - Forward a hid-driver callback to all drivers on
+ * all virtual HID devices attached to the given real HID device.
+ * @hdev the real hid-device
+ * @forward a function that calls the callback on the given driver
+ * @args arguments for the forward function
+ *
+ * This is for callbacks that return a status as an int.
+ *
+ * Returns: 0 on success, or the first error returned by the @forward function.
+ */
+static int appleib_forward_int_op(struct hid_device *hdev,
+				  int (*forward)(struct hid_driver *,
+						 struct hid_device *, void *),
+				  void *args)
+{
+	struct appleib_hid_dev_info *hdev_info = hid_get_drvdata(hdev);
+	struct hid_device *sub_hdev;
+	int rc = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++) {
+		sub_hdev = hdev_info->sub_hdevs[i];
+		if (sub_hdev->driver) {
+			rc = forward(sub_hdev->driver, sub_hdev, args);
+			if (rc)
+				break;
+		}
+
+		break;
+	}
+
+	return rc;
+}
+
+static int appleib_hid_raw_event(struct hid_device *hdev,
+				 struct hid_report *report, u8 *data, int size)
+{
+	struct appleib_hid_dev_info *hdev_info = hid_get_drvdata(hdev);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++)
+		hid_input_report(hdev_info->sub_hdevs[i], report->type, data,
+				 size, 0);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int appleib_hid_suspend_fwd(struct hid_driver *drv,
+				   struct hid_device *hdev, void *args)
+{
+	int rc = 0;
+
+	if (drv->suspend)
+		rc = drv->suspend(hdev, *(pm_message_t *)args);
+
+	return rc;
+}
+
+static int appleib_hid_suspend(struct hid_device *hdev, pm_message_t message)
+{
+	return appleib_forward_int_op(hdev, appleib_hid_suspend_fwd, &message);
+}
+
+static int appleib_hid_resume_fwd(struct hid_driver *drv,
+				  struct hid_device *hdev, void *args)
+{
+	int rc = 0;
+
+	if (drv->resume)
+		rc = drv->resume(hdev);
+
+	return rc;
+}
+
+static int appleib_hid_resume(struct hid_device *hdev)
+{
+	return appleib_forward_int_op(hdev, appleib_hid_resume_fwd, NULL);
+}
+
+static int appleib_hid_reset_resume_fwd(struct hid_driver *drv,
+					struct hid_device *hdev, void *args)
+{
+	int rc = 0;
+
+	if (drv->reset_resume)
+		rc = drv->reset_resume(hdev);
+
+	return rc;
+}
+
+static int appleib_hid_reset_resume(struct hid_device *hdev)
+{
+	return appleib_forward_int_op(hdev, appleib_hid_reset_resume_fwd, NULL);
+}
+#endif /* CONFIG_PM */
+
+/**
+ * appleib_find_report_field() - Find the field in the report with the given
+ * usage.
+ * @report: the report to search
+ * @field_usage: the usage of the field to search for
+ *
+ * Returns: the hid field if found, or NULL if none found.
+ */
+struct hid_field *appleib_find_report_field(struct hid_report *report,
+					    unsigned int field_usage)
+{
+	int f, u;
+
+	for (f = 0; f < report->maxfield; f++) {
+		struct hid_field *field = report->field[f];
+
+		if (field->logical == field_usage)
+			return field;
+
+		for (u = 0; u < field->maxusage; u++) {
+			if (field->usage[u].hid == field_usage)
+				return field;
+		}
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(appleib_find_report_field);
+
+/**
+ * appleib_find_hid_field() - Search all the reports of the device for the
+ * field with the given usage.
+ * @hdev: the device whose reports to search
+ * @application: the usage of application collection that the field must
+ *               belong to
+ * @field_usage: the usage of the field to search for
+ *
+ * Returns: the hid field if found, or NULL if none found.
+ */
+struct hid_field *appleib_find_hid_field(struct hid_device *hdev,
+					 unsigned int application,
+					 unsigned int field_usage)
+{
+	static const int report_types[] = { HID_INPUT_REPORT, HID_OUTPUT_REPORT,
+					    HID_FEATURE_REPORT };
+	struct hid_report *report;
+	struct hid_field *field;
+	int t;
+
+	for (t = 0; t < ARRAY_SIZE(report_types); t++) {
+		struct list_head *report_list =
+			    &hdev->report_enum[report_types[t]].report_list;
+		list_for_each_entry(report, report_list, list) {
+			if (report->application != application)
+				continue;
+
+			field = appleib_find_report_field(report, field_usage);
+			if (field)
+				return field;
+		}
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(appleib_find_hid_field);
+
+static int appleib_ll_start(struct hid_device *hdev)
+{
+	return 0;
+}
+
+static void appleib_ll_stop(struct hid_device *hdev)
+{
+}
+
+static int appleib_ll_open(struct hid_device *hdev)
+{
+	// TODO: allow event reporting
+	return 0;
+}
+
+static void appleib_ll_close(struct hid_device *hdev)
+{
+	// TODO: disallow event reporting
+}
+
+static int appleib_ll_power(struct hid_device *hdev, int level)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	return hid_hw_power(hdev_info->hdev, level);
+}
+
+static int appleib_ll_parse(struct hid_device *hdev)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	return hid_parse_report(hdev, hdev_info->hdev->rdesc,
+				hdev_info->hdev->rsize);
+}
+
+static void appleib_ll_request(struct hid_device *hdev,
+			       struct hid_report *report, int reqtype)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	hid_hw_request(hdev_info->hdev, report, reqtype);
+}
+
+static int appleib_ll_wait(struct hid_device *hdev)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	hid_hw_wait(hdev_info->hdev);
+	return 0;
+}
+
+static int appleib_ll_raw_request(struct hid_device *hdev,
+				  unsigned char reportnum, __u8 *buf,
+				  size_t len, unsigned char rtype, int reqtype)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	return hid_hw_raw_request(hdev_info->hdev, reportnum, buf, len, rtype,
+				  reqtype);
+}
+
+static int appleib_ll_output_report(struct hid_device *hdev, __u8 *buf,
+				    size_t len)
+{
+	struct appleib_hid_dev_info *hdev_info = hdev->driver_data;
+
+	return hid_hw_output_report(hdev_info->hdev, buf, len);
+}
+
+static struct hid_ll_driver appleib_ll_driver = {
+	.start = appleib_ll_start,
+	.stop = appleib_ll_stop,
+	.open = appleib_ll_open,
+	.close = appleib_ll_close,
+	.power = appleib_ll_power,
+	.parse = appleib_ll_parse,
+	.request = appleib_ll_request,
+	.wait = appleib_ll_wait,
+	.raw_request = appleib_ll_raw_request,
+	.output_report = appleib_ll_output_report,
+};
+
+static struct hid_device *
+appleib_add_sub_dev(struct appleib_hid_dev_info *hdev_info,
+		    struct hid_device_id *dev_id)
+{
+	struct hid_device *sub_hdev;
+	int rc;
+
+	sub_hdev = hid_allocate_device();
+	if (IS_ERR(sub_hdev))
+		return sub_hdev;
+
+	sub_hdev->dev.parent = &hdev_info->hdev->dev;
+
+	sub_hdev->bus = dev_id->bus;
+	sub_hdev->group = dev_id->group;
+	sub_hdev->vendor = dev_id->vendor;
+	sub_hdev->product = dev_id->product;
+
+	sub_hdev->ll_driver = &appleib_ll_driver;
+
+	snprintf(sub_hdev->name, sizeof(sub_hdev->name),
+		 "iBridge Virtual HID %s/%04x:%04x",
+		 dev_name(sub_hdev->dev.parent), sub_hdev->vendor,
+		 sub_hdev->product);
+
+	sub_hdev->driver_data = hdev_info;
+
+	rc = hid_add_device(sub_hdev);
+	if (rc) {
+		hid_destroy_device(sub_hdev);
+		return ERR_PTR(rc);
+	}
+
+	return sub_hdev;
+}
+
+static struct appleib_hid_dev_info *appleib_add_device(struct hid_device *hdev)
+{
+	struct appleib_hid_dev_info *hdev_info;
+	int i;
+
+	hdev_info = devm_kzalloc(&hdev->dev, sizeof(*hdev_info), GFP_KERNEL);
+	if (!hdev_info)
+		return ERR_PTR(-ENOMEM);
+
+	hdev_info->hdev = hdev;
+
+	for (i = 0; i < ARRAY_SIZE(hdev_info->sub_hdevs); i++) {
+		hdev_info->sub_hdevs[i] =
+			appleib_add_sub_dev(hdev_info, &appleib_sub_hid_ids[i]);
+
+		if (IS_ERR(hdev_info->sub_hdevs[i])) {
+			while (i-- > 0)
+				hid_destroy_device(hdev_info->sub_hdevs[i]);
+			return (void *)hdev_info->sub_hdevs[i];
+		}
+	}
+
+	return hdev_info;
+}
+
+static int appleib_hid_probe(struct hid_device *hdev,
+			     const struct hid_device_id *id)
+{
+	struct appleib_hid_dev_info *hdev_info;
+	struct usb_device *udev;
+	int rc;
+
+	/* check and set usb config first */
+	udev = hid_to_usb_dev(hdev);
+
+	if (udev->actconfig->desc.bConfigurationValue != APPLEIB_BASIC_CONFIG) {
+		rc = usb_driver_set_configuration(udev, APPLEIB_BASIC_CONFIG);
+		return rc ? rc : -ENODEV;
+	}
+
+	rc = hid_parse(hdev);
+	if (rc) {
+		hid_err(hdev, "ib: hid parse failed (%d)\n", rc);
+		goto error;
+	}
+
+	rc = hid_hw_start(hdev, HID_CONNECT_DRIVER);
+	if (rc) {
+		hid_err(hdev, "ib: hw start failed (%d)\n", rc);
+		goto error;
+	}
+
+	hdev_info = appleib_add_device(hdev);
+	if (IS_ERR(hdev_info)) {
+		rc = PTR_ERR(hdev_info);
+		goto stop_hw;
+	}
+
+	hid_set_drvdata(hdev, hdev_info);
+
+	rc = hid_hw_open(hdev);
+	if (rc) {
+		hid_err(hdev, "ib: failed to open hid: %d\n", rc);
+		goto remove_dev;
+	}
+
+	return 0;
+
+remove_dev:
+	appleib_remove_device(hdev);
+stop_hw:
+	hid_hw_stop(hdev);
+error:
+	return rc;
+}
+
+static void appleib_hid_remove(struct hid_device *hdev)
+{
+	hid_hw_close(hdev);
+	appleib_remove_device(hdev);
+	hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id appleib_hid_ids[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IBRIDGE) },
+	{ },
+};
+
+static struct hid_driver appleib_hid_driver = {
+	.name = "apple-ibridge-hid",
+	.id_table = appleib_hid_ids,
+	.probe = appleib_hid_probe,
+	.remove = appleib_hid_remove,
+	.raw_event = appleib_hid_raw_event,
+#ifdef CONFIG_PM
+	.suspend = appleib_hid_suspend,
+	.resume = appleib_hid_resume,
+	.reset_resume = appleib_hid_reset_resume,
+#endif
+};
+
+static struct appleib_device *appleib_alloc_device(struct acpi_device *acpi_dev)
+{
+	struct appleib_device *ib_dev;
+	acpi_status sts;
+
+	ib_dev = devm_kzalloc(&acpi_dev->dev, sizeof(*ib_dev), GFP_KERNEL);
+	if (!ib_dev)
+		return ERR_PTR(-ENOMEM);
+
+	ib_dev->acpi_dev = acpi_dev;
+
+	/* get iBridge acpi power control method for suspend/resume */
+	sts = acpi_get_handle(acpi_dev->handle, "SOCW", &ib_dev->asoc_socw);
+	if (ACPI_FAILURE(sts)) {
+		dev_err(LOG_DEV(ib_dev),
+			"Error getting handle for ASOC.SOCW method: %s\n",
+			acpi_format_exception(sts));
+		return ERR_PTR(-ENXIO);
+	}
+
+	/* ensure iBridge is powered on */
+	sts = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 1);
+	if (ACPI_FAILURE(sts))
+		dev_warn(LOG_DEV(ib_dev), "SOCW(1) failed: %s\n",
+			 acpi_format_exception(sts));
+
+	return ib_dev;
+}
+
+static int appleib_probe(struct acpi_device *acpi)
+{
+	struct appleib_device *ib_dev;
+	int ret;
+
+	ib_dev = appleib_alloc_device(acpi);
+	if (IS_ERR(ib_dev))
+		return PTR_ERR(ib_dev);
+
+	ret = hid_register_driver(&appleib_hid_driver);
+	if (ret) {
+		dev_err(LOG_DEV(ib_dev), "Error registering hid driver: %d\n",
+			ret);
+		return ret;
+	}
+
+	acpi->driver_data = ib_dev;
+
+	return 0;
+}
+
+static int appleib_remove(struct acpi_device *acpi)
+{
+	hid_unregister_driver(&appleib_hid_driver);
+
+	return 0;
+}
+
+static int appleib_suspend(struct device *dev)
+{
+	struct appleib_device *ib_dev;
+	int rc;
+
+	ib_dev = acpi_driver_data(to_acpi_device(dev));
+
+	rc = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 0);
+	if (ACPI_FAILURE(rc))
+		dev_warn(dev, "SOCW(0) failed: %s\n",
+			 acpi_format_exception(rc));
+
+	return 0;
+}
+
+static int appleib_resume(struct device *dev)
+{
+	struct appleib_device *ib_dev;
+	int rc;
+
+	ib_dev = acpi_driver_data(to_acpi_device(dev));
+
+	rc = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 1);
+	if (ACPI_FAILURE(rc))
+		dev_warn(dev, "SOCW(1) failed: %s\n",
+			 acpi_format_exception(rc));
+
+	return 0;
+}
+
+static const struct dev_pm_ops appleib_pm = {
+	.suspend = appleib_suspend,
+	.resume = appleib_resume,
+	.restore = appleib_resume,
+};
+
+static const struct acpi_device_id appleib_acpi_match[] = {
+	{ "APP7777", 0 },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(acpi, appleib_acpi_match);
+
+static struct acpi_driver appleib_driver = {
+	.name		= "apple-ibridge",
+	.class		= "topcase", /* ? */
+	.owner		= THIS_MODULE,
+	.ids		= appleib_acpi_match,
+	.ops		= {
+		.add		= appleib_probe,
+		.remove		= appleib_remove,
+	},
+	.drv		= {
+		.pm		= &appleib_pm,
+	},
+};
+
+module_acpi_driver(appleib_driver)
+
+MODULE_AUTHOR("Ronald Tschalär");
+MODULE_DESCRIPTION("Apple iBridge driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index adce58f24f76..f963bb02c477 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -177,6 +177,7 @@
 #define USB_DEVICE_ID_APPLE_IRCONTROL3	0x8241
 #define USB_DEVICE_ID_APPLE_IRCONTROL4	0x8242
 #define USB_DEVICE_ID_APPLE_IRCONTROL5	0x8243
+#define USB_DEVICE_ID_APPLE_IBRIDGE	0x8600
 
 #define USB_VENDOR_ID_ASUS		0x0486
 #define USB_DEVICE_ID_ASUS_T91MT	0x0185
diff --git a/include/linux/apple-ibridge.h b/include/linux/apple-ibridge.h
new file mode 100644
index 000000000000..07ded8c68776
--- /dev/null
+++ b/include/linux/apple-ibridge.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Apple iBridge Driver
+ *
+ * Copyright (c) 2018 Ronald Tschalär
+ */
+
+#ifndef __LINUX_APPLE_IBRDIGE_H
+#define __LINUX_APPLE_IBRDIGE_H
+
+#include <linux/hid.h>
+
+#define USB_VENDOR_ID_LINUX_FOUNDATION	0x1d6b
+#define USB_DEVICE_ID_IBRIDGE_TB	0x0301
+#define USB_DEVICE_ID_IBRIDGE_ALS	0x0302
+
+struct hid_field *appleib_find_report_field(struct hid_report *report,
+					    unsigned int field_usage);
+struct hid_field *appleib_find_hid_field(struct hid_device *hdev,
+					 unsigned int application,
+					 unsigned int field_usage);
+
+#endif
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 0/3] Apple iBridge support
From: Ronald Tschalär @ 2019-06-12  8:33 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler
  Cc: linux-input, linux-iio, linux-kernel

2016 and 2017 MacBook Pro's have a T1 chip that drives the Touch Bar,
ambient light sensor, webcam, and fingerprint sensor; this shows up
as an iBridge USB device in the system. These patches provide initial
support for the Touch Bar and ALS - the webcam is already handled by
existing drivers, and no information is currently known on how to access
the fingerprint sensor (other than it's apparently via one of the extra
interfaces available in the OS X USB configuration).

One thing of note here is that both the ALS and (some of) the Touch Bar
functionality are exposed via the same USB interface (and hence same
hid_device), so both drivers need to share this device. This is solved
by having the iBridge driver create multiple virtual HID devices for
each real HID device to which the Touch Bar and ALS drivers attach, and 
then forwarding the calls between the real and virtual HID devices, so
we end up with a structure like this:

    iBridge (HID) driver    Sub drivers

           --  vhdev0  --   (unbound)
          /
   hdev1  ---  vhdev1  ---  tb-drv
                         /
           --  vhdev2  --
          /
   hdev2  ---  vhdev3  ---  als-drv


Changes in v2:
  - Changed iBridge driver from an MFD driver to a HID driver. Instead
    of creating virtual HID drivers and (de)multiplexing their calls,
    this now create virtual HID devices and (de)multiplexing the
    operations on them. This is from the feedback by Benjamin Tissoires.
  - Applied all feedback on ALS driver from Jonathan Cameron
  - Applied all feedback on Touch Bar driver from Jonathan Cameron
    and Peter Meerwald-Stadler
  - various smaller cleanups

Ronald Tschalär (3):
  HID: apple-ibridge: Add Apple iBridge HID driver.
  HID: apple-ib-tb: Add driver for the Touch Bar on MacBook Pro's.
  iio: light: apple-ib-als: Add driver for ALS on iBridge chip.

 drivers/hid/Kconfig              |   24 +
 drivers/hid/Makefile             |    2 +
 drivers/hid/apple-ib-tb.c        | 1389 ++++++++++++++++++++++++++++++
 drivers/hid/apple-ibridge.c      |  588 +++++++++++++
 drivers/hid/hid-ids.h            |    1 +
 drivers/iio/light/Kconfig        |   12 +
 drivers/iio/light/Makefile       |    1 +
 drivers/iio/light/apple-ib-als.c |  607 +++++++++++++
 include/linux/apple-ibridge.h    |   23 +
 9 files changed, 2647 insertions(+)
 create mode 100644 drivers/hid/apple-ib-tb.c
 create mode 100644 drivers/hid/apple-ibridge.c
 create mode 100644 drivers/iio/light/apple-ib-als.c
 create mode 100644 include/linux/apple-ibridge.h

-- 
2.21.0

^ permalink raw reply

* Re: Strange regression in hid_llogitech_dj (was: Re: Linux 5.2-rc4)
From: Jiri Kosina @ 2019-06-12  8:31 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Hans de Goede, Linus Torvalds, Linux Kernel Mailing,
	Benjamin Tissoires, linux-input
In-Reply-To: <1875376.0DUQQ8o03D@kreacher>

On Wed, 12 Jun 2019, Rafael J. Wysocki wrote:

> It kind of helps, but there is a catch.
> 
> hid-logitech-dj is not loaded after a fresh boot, so I need to modprobe it manually and that
> appears to be blocking (apparently indefinitely) until terminated with ^C.  But then it turns
> out that hid-logitech-dj is there in the list of modules and it is in use (by usbhid) and the
> mouse works.

My bad, I should've asked you to test with the complete 'for-5.2/fixes' 
branch which contains two reverts [1] [2] that should fix this issue as 
well.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for-5.2/fixes&id=e0b7f9bc0246bc642d1de2ff3ff133730584c956
[2] https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for-5.2/fixes&id=f9482dabfd1686987cc6044e06ae0e4c05915518

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: Strange regression in hid_llogitech_dj (was: Re: Linux 5.2-rc4)
From: Rafael J. Wysocki @ 2019-06-12  8:24 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Hans de Goede, Linus Torvalds, Linux Kernel Mailing,
	Benjamin Tissoires, linux-input
In-Reply-To: <nycvar.YFH.7.76.1906112358580.27227@cbobk.fhfr.pm>

On Wednesday, June 12, 2019 12:02:21 AM CEST Jiri Kosina wrote:
> On Tue, 11 Jun 2019, Rafael J. Wysocki wrote:
> 
> > I noticed that the cordless mouse used by me with one of the machines here
> > stopped to work in 5.2-rc (up to and including the -rc4).
> > 
> > Bisection turned up commit 74808f9115ce ("HID: logitech-dj: add support for non
> > unifying receivers").
> > 
> > Of course, that commit does not revert cleanly from 5.2-rc4, but I have reverted
> > the changes made by it in hid/hid-ids.h and I took the version of hid/hid-logitech-dj.c
> > from commit b6aeeddef68d ("HID: logitech-dj: add logi_dj_recv_queue_unknown_work
> > helper"), which is the parent of commit 74808f9115ce, and that made the mouse
> > work again for me.
> > 
> > Here's the output of "dmesg | grep -i logitech" from 5.2-rc4 with the above changes:
> > 
> > [    4.288905] usb 1-2: Manufacturer: Logitech
> > [    5.444621] input: Logitech USB Receiver as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:046D:C52F.0002/input/input23
> > [    5.446960] hid-generic 0003:046D:C52F.0002: input,hidraw1: USB HID v1.11 Mouse [Logitech USB Receiver] on usb-0000:00:14.0-2/input0
> > [    5.451265] input: Logitech USB Receiver Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C52F.0003/input/input24
> > [    5.507545] hid-generic 0003:046D:C52F.0003: input,hiddev96,hidraw2: USB HID v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-2/input1
> 
> Hi Rafael,
> 
> 0x046d/0xc52f is known to have issues in 5.2-rcX. There is a patch queued 
> [1] that is believed to fix all this; my plan is to send it to Linus in 
> the coming 1-2 days. If you could report whether it fixes the issues 
> you've been seeing yourself as well, it'd be helpful.
> 
> Thanks.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for-5.2/fixes&id=3ed224e273ac5880eeab4c3043a6b06b0478dd56

It kind of helps, but there is a catch.

hid-logitech-dj is not loaded after a fresh boot, so I need to modprobe it manually and that
appears to be blocking (apparently indefinitely) until terminated with ^C.  But then it turns
out that hid-logitech-dj is there in the list of modules and it is in use (by usbhid) and the
mouse works.

I guess I need to update the mkinitrd configuration, but even so that is not exactly
straightforward IMO. :-)

Cheers!

^ permalink raw reply

* Re: [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL)
From: Pali Rohár @ 2019-06-12  7:14 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Joe Perches, Kefeng Wang, linux-kernel, linux-input
In-Reply-To: <20190612005913.GJ143729@dtor-ws>

On Tuesday 11 June 2019 17:59:13 Dmitry Torokhov wrote:
> Hi Joe,
> 
> On Wed, Jun 05, 2019 at 07:28:53PM -0700, Joe Perches wrote:
> > On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote:
> > > On 2019/6/5 22:42, Pali Rohár wrote:
> > > > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote:
> > > > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
> > > > > so no need to do that again from its callers. Drop it.
> > > > Hi! I already reviewed this patch and rejected it, see:
> > > > https://patchwork.kernel.org/patch/10817475/
> > > OK, please ignore it.
> > 
> > I think the stated reason of better readability isn't
> > particularly sensible as the object code produced is
> > actually slightly larger.
> > 
> > x86-64 defconfig (gcc 8.3.0)
> > 
> > $ size drivers/input/mouse/alps.o*
> >    text	   data	    bss	    dec	    hex	filename
> >   29416	     56	      0	  29472	   7320	drivers/input/mouse/alps.o.new
> >   29432	     56	      0	  29488	   7330	drivers/input/mouse/alps.o.old
> 
> If gcc produces worse code for double unlikely, you should probably
> report it to gcc folks, no? Or double unlikely turns into likely?

Is measured size of stripped or unstripped binary? Plus with or without
debug symbols? Double unlikely version should have more debug symbols
and therefore also larger size.

But if unstripped version with double unlikely is larger then it is for
sure compiler bug.

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply

* Re: [PATCH 2/2] HID: wacom: Don't report anything prior to the tool entering range
From: Greg KH @ 2019-06-12  7:10 UTC (permalink / raw)
  To: Jason Gerecke
  Cc: # v3.17+, Sasha Levin, Linux Input, Ping Cheng,
	Aaron Armstrong Skomra, Jason Gerecke, Aaron Armstrong Skomra
In-Reply-To: <CANRwn3SNAQccFQEaw7Qg=hNojbFVkFRHLT3tzq_BtxpUNRiMUw@mail.gmail.com>

On Tue, Jun 11, 2019 at 01:45:36PM -0700, Jason Gerecke wrote:
> On Tue, Jun 11, 2019 at 12:22 PM Greg KH <greg@kroah.com> wrote:
> >
> > On Tue, Jun 11, 2019 at 12:02:47PM -0700, Jason Gerecke wrote:
> > > I haven't been keeping a close eye on this and just noticed that this
> > > patch set doesn't seem to have been merged into stable. There's also a
> > > second patch series (beginning with "[PATCH 1/3] HID: wacom: Send
> > > BTN_TOUCH in response to INTUOSP2_BT eraser contact") that hasn't seen
> > > any stable activity either.
> > >
> > > Any idea what's up?
> >
> > I don't see these in my queue at all.
> >
> > What is the git commit id of these patches in Linus's tree?
> >
> > thanks,
> >
> > greg k-h
> 
> Ah, looks like the HID tree's "for-5.2/fixes" branch hasn't been
> pulled yet. That could explain things.
> 
> 69dbdfffef20c715df9f381b2cee4e9e0a4efd93 HID: wacom: Sync INTUOSP2_BT
> touch state after each frame if necessary
> 6441fc781c344df61402be1fde582c4491fa35fa HID: wacom: Correct button
> numbering 2nd-gen Intuos Pro over Bluetooth
> fe7f8d73d1af19b678171170e4e5384deb57833d HID: wacom: Send BTN_TOUCH in
> response to INTUOSP2_BT eraser contact
> e92a7be7fe5b2510fa60965eaf25f9e3dc08b8cc HID: wacom: Don't report
> anything prior to the tool entering range
> 2cc08800a6b9fcda7c7afbcf2da1a6e8808da725 HID: wacom: Don't set tool
> type until we're in range

There is nothing I can do until the patches are in Linus's tree, sorry.

greg k-h

^ permalink raw reply

* Re: [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL)
From: Dmitry Torokhov @ 2019-06-12  0:59 UTC (permalink / raw)
  To: Joe Perches; +Cc: Kefeng Wang, Pali Rohár, linux-kernel, linux-input
In-Reply-To: <a1908a998b6d1d1e4cdd097a50b9c9ac9b00caae.camel@perches.com>

Hi Joe,

On Wed, Jun 05, 2019 at 07:28:53PM -0700, Joe Perches wrote:
> On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote:
> > On 2019/6/5 22:42, Pali Rohár wrote:
> > > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote:
> > > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
> > > > so no need to do that again from its callers. Drop it.
> > > Hi! I already reviewed this patch and rejected it, see:
> > > https://patchwork.kernel.org/patch/10817475/
> > OK, please ignore it.
> 
> I think the stated reason of better readability isn't
> particularly sensible as the object code produced is
> actually slightly larger.
> 
> x86-64 defconfig (gcc 8.3.0)
> 
> $ size drivers/input/mouse/alps.o*
>    text	   data	    bss	    dec	    hex	filename
>   29416	     56	      0	  29472	   7320	drivers/input/mouse/alps.o.new
>   29432	     56	      0	  29488	   7330	drivers/input/mouse/alps.o.old

If gcc produces worse code for double unlikely, you should probably
report it to gcc folks, no? Or double unlikely turns into likely?

> 
> Also if this unlikely is _really_ useful, perhaps the
> !IS_ERR immediately after could also use likely as the
> test seems only done for an OOM condition.

No, once you take the IS_ERR_OR_NULL(priv->dev3) == true branch it stops
being hot path and additional annotations are completely unneeded.

And if we failed to create and register priv->dev3 device - that's an
error and system is degraded. Can't do much here.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [RESEND] input: keyboard: imx: make sure keyboard can always wake up system
From: dmitry.torokhov @ 2019-06-12  0:51 UTC (permalink / raw)
  To: Anson Huang
  Cc: shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	linux-input@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, dl-linux-imx
In-Reply-To: <1554341727-16084-1-git-send-email-Anson.Huang@nxp.com>

On Thu, Apr 04, 2019 at 01:40:16AM +0000, Anson Huang wrote:
> There are several scenarios that keyboard can NOT wake up system
> from suspend, e.g., if a keyboard is depressed between system
> device suspend phase and device noirq suspend phase, the keyboard
> ISR will be called and both keyboard depress and release interrupts
> will be disabled, then keyboard will no longer be able to wake up
> system. Another scenario would be, if a keyboard is kept depressed,
> and then system goes into suspend, the expected behavior would be
> when keyboard is released, system will be waked up, but current
> implementation can NOT achieve that, because both depress and release
> interrupts are disabled in ISR, and the event check is still in
> progress.
> 
> To fix these issues, need to make sure keyboard's depress or release
> interrupt is enabled after noirq device suspend phase, this patch
> moves the suspend/resume callback to noirq suspend/resume phase, and
> enable the corresponding interrupt according to current keyboard status.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Applied, thank you.

> ---
>  drivers/input/keyboard/imx_keypad.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
> index cf08f4a..97500a2 100644
> --- a/drivers/input/keyboard/imx_keypad.c
> +++ b/drivers/input/keyboard/imx_keypad.c
> @@ -524,11 +524,12 @@ static int imx_keypad_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static int __maybe_unused imx_kbd_suspend(struct device *dev)
> +static int __maybe_unused imx_kbd_noirq_suspend(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct imx_keypad *kbd = platform_get_drvdata(pdev);
>  	struct input_dev *input_dev = kbd->input_dev;
> +	unsigned short reg_val = readw(kbd->mmio_base + KPSR);
>  
>  	/* imx kbd can wake up system even clock is disabled */
>  	mutex_lock(&input_dev->mutex);
> @@ -538,13 +539,20 @@ static int __maybe_unused imx_kbd_suspend(struct device *dev)
>  
>  	mutex_unlock(&input_dev->mutex);
>  
> -	if (device_may_wakeup(&pdev->dev))
> +	if (device_may_wakeup(&pdev->dev)) {
> +		if (reg_val & KBD_STAT_KPKD)
> +			reg_val |= KBD_STAT_KRIE;
> +		if (reg_val & KBD_STAT_KPKR)
> +			reg_val |= KBD_STAT_KDIE;
> +		writew(reg_val, kbd->mmio_base + KPSR);
> +
>  		enable_irq_wake(kbd->irq);
> +	}
>  
>  	return 0;
>  }
>  
> -static int __maybe_unused imx_kbd_resume(struct device *dev)
> +static int __maybe_unused imx_kbd_noirq_resume(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct imx_keypad *kbd = platform_get_drvdata(pdev);
> @@ -568,7 +576,9 @@ static int __maybe_unused imx_kbd_resume(struct device *dev)
>  	return ret;
>  }
>  
> -static SIMPLE_DEV_PM_OPS(imx_kbd_pm_ops, imx_kbd_suspend, imx_kbd_resume);
> +static const struct dev_pm_ops imx_kbd_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_kbd_noirq_suspend, imx_kbd_noirq_resume)
> +};
>  
>  static struct platform_driver imx_keypad_driver = {
>  	.driver		= {
> -- 
> 2.7.4
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling
From: Dmitry Torokhov @ 2019-06-12  0:35 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: benjamin.tissoires, jikos, bjorn.andersson, lee.jones, robh+dt,
	mark.rutland, agross, david.brown, hdegoede, linux-input,
	devicetree, linux-arm-msm, linux-kernel
In-Reply-To: <20190606161322.47192-1-jeffrey.l.hugo@gmail.com>

On Thu, Jun 06, 2019 at 09:13:22AM -0700, Jeffrey Hugo wrote:
> There needs to be coordination between hid-quirks and the elan_i2c driver
> about which devices are handled by what drivers.  Currently, both use
> whitelists, which results in valid devices being unhandled by default,
> when they should not be rejected by hid-quirks.  This is quickly becoming
> an issue.
> 
> Since elan_i2c has a maintained whitelist of what devices it will handle,
> use that to implement a blacklist in hid-quirks so that only the devices
> that need to be handled by elan_i2c get rejected by hid-quirks, and
> everything else is handled by default.  The downside is the whitelist and
> blacklist need to be kept in sync.
> 
> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/hid/hid-quirks.c | 78 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 67 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index e5ca6fe2ca57..edebd0700e3d 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -912,8 +912,66 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
>  	{ }
>  };
>  
> +/* 
> + * List of device names that elan_i2c is handling and HID should ignore.  Must
> + * be kept in sync with elan_i2c
> + */
> +static const char *hid_elan_i2c_ignore[] = {

If this is a copy of elan whitelist, then, if we do not want to bother
with sharing it in object form (as a elan-i2c-ids module), can we at
least move it into include/linux/input/elan-i2c-ids.h and consume from
hid-quirks.c?

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [RFC 0/2] Support for buttons on newer MS Surface devices
From: Maximilian Luz @ 2019-06-11 23:06 UTC (permalink / raw)
  Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
	Hans de Goede, Chen Yu, Darren Hart, Andy Shevchenko
In-Reply-To: <a0f93af3-c587-40d5-2a85-fdc0f9e6b79f@gmail.com>

Since there are no comments on this, should I simply submit this as patch?

Maximilian


On 6/1/19 9:07 PM, Maximilian Luz wrote:
> Hi,
> 
> any comments on this?
> 
> I should also mention that this has been tested via
> https://github.com/jakeday/linux-surface.
> 
> Maximilian

^ permalink raw reply

* Re: Strange regression in hid_llogitech_dj (was: Re: Linux 5.2-rc4)
From: Jiri Kosina @ 2019-06-11 22:02 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Hans de Goede, Linus Torvalds, Linux Kernel Mailing,
	Benjamin Tissoires, linux-input
In-Reply-To: <2268131.Lc39eCoc3j@kreacher>

On Tue, 11 Jun 2019, Rafael J. Wysocki wrote:

> I noticed that the cordless mouse used by me with one of the machines here
> stopped to work in 5.2-rc (up to and including the -rc4).
> 
> Bisection turned up commit 74808f9115ce ("HID: logitech-dj: add support for non
> unifying receivers").
> 
> Of course, that commit does not revert cleanly from 5.2-rc4, but I have reverted
> the changes made by it in hid/hid-ids.h and I took the version of hid/hid-logitech-dj.c
> from commit b6aeeddef68d ("HID: logitech-dj: add logi_dj_recv_queue_unknown_work
> helper"), which is the parent of commit 74808f9115ce, and that made the mouse
> work again for me.
> 
> Here's the output of "dmesg | grep -i logitech" from 5.2-rc4 with the above changes:
> 
> [    4.288905] usb 1-2: Manufacturer: Logitech
> [    5.444621] input: Logitech USB Receiver as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:046D:C52F.0002/input/input23
> [    5.446960] hid-generic 0003:046D:C52F.0002: input,hidraw1: USB HID v1.11 Mouse [Logitech USB Receiver] on usb-0000:00:14.0-2/input0
> [    5.451265] input: Logitech USB Receiver Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C52F.0003/input/input24
> [    5.507545] hid-generic 0003:046D:C52F.0003: input,hiddev96,hidraw2: USB HID v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-2/input1

Hi Rafael,

0x046d/0xc52f is known to have issues in 5.2-rcX. There is a patch queued 
[1] that is believed to fix all this; my plan is to send it to Linus in 
the coming 1-2 days. If you could report whether it fixes the issues 
you've been seeing yourself as well, it'd be helpful.

Thanks.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for-5.2/fixes&id=3ed224e273ac5880eeab4c3043a6b06b0478dd56

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Strange regression in hid_llogitech_dj (was: Re: Linux 5.2-rc4)
From: Rafael J. Wysocki @ 2019-06-11 21:53 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Linus Torvalds, Linux Kernel Mailing, Benjamin Tissoires,
	Jiri Kosina, linux-input
In-Reply-To: <CAHk-=wjm7FQxdF=RKa8Xe23CLNNpbGDOACewgo8e-hwDJ8TyQg@mail.gmail.com>

On Sunday, June 9, 2019 5:46:48 AM CEST Linus Torvalds wrote:
> No, I'm not confused, and I haven't lost track of what day it is, I do
> actually know that it's still Saturday here, not Sunday, and I'm just
> doing rc4 a bit early because I'll be on an airplane during my normal
> release time. And while I've done releases on airports and airplanes
> before, I looked at my empty queue of pull requests and went "let's
> just do it now".
> 
> We've had a fairly calm release so far, and on the whole that seems to
> hold. rc4 isn't smaller than rc3 was (it's a bit bigger), but rc3 was
> fairly small, so the size increase isn't all that worrisome. I do hope
> that we'll start actually shrinking now, though.

I noticed that the cordless mouse used by me with one of the machines here
stopped to work in 5.2-rc (up to and including the -rc4).

Bisection turned up commit 74808f9115ce ("HID: logitech-dj: add support for non
unifying receivers").

Of course, that commit does not revert cleanly from 5.2-rc4, but I have reverted
the changes made by it in hid/hid-ids.h and I took the version of hid/hid-logitech-dj.c
from commit b6aeeddef68d ("HID: logitech-dj: add logi_dj_recv_queue_unknown_work
helper"), which is the parent of commit 74808f9115ce, and that made the mouse
work again for me.

Here's the output of "dmesg | grep -i logitech" from 5.2-rc4 with the above changes:

[    4.288905] usb 1-2: Manufacturer: Logitech
[    5.444621] input: Logitech USB Receiver as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:046D:C52F.0002/input/input23
[    5.446960] hid-generic 0003:046D:C52F.0002: input,hidraw1: USB HID v1.11 Mouse [Logitech USB Receiver] on usb-0000:00:14.0-2/input0
[    5.451265] input: Logitech USB Receiver Consumer Control as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.1/0003:046D:C52F.0003/input/input24
[    5.507545] hid-generic 0003:046D:C52F.0003: input,hiddev96,hidraw2: USB HID v1.11 Device [Logitech USB Receiver] on usb-0000:00:14.0-2/input1

Please let me know what you need to diagnose this.

Thanks,
Rafael

^ permalink raw reply

* Re: [PATCH 2/2] HID: wacom: Don't report anything prior to the tool entering range
From: Jason Gerecke @ 2019-06-11 20:45 UTC (permalink / raw)
  To: Greg KH
  Cc: # v3.17+, Sasha Levin, Linux Input, Ping Cheng,
	Aaron Armstrong Skomra, Jason Gerecke, Aaron Armstrong Skomra
In-Reply-To: <20190611192248.GB19775@kroah.com>

On Tue, Jun 11, 2019 at 12:22 PM Greg KH <greg@kroah.com> wrote:
>
> On Tue, Jun 11, 2019 at 12:02:47PM -0700, Jason Gerecke wrote:
> > I haven't been keeping a close eye on this and just noticed that this
> > patch set doesn't seem to have been merged into stable. There's also a
> > second patch series (beginning with "[PATCH 1/3] HID: wacom: Send
> > BTN_TOUCH in response to INTUOSP2_BT eraser contact") that hasn't seen
> > any stable activity either.
> >
> > Any idea what's up?
>
> I don't see these in my queue at all.
>
> What is the git commit id of these patches in Linus's tree?
>
> thanks,
>
> greg k-h

Ah, looks like the HID tree's "for-5.2/fixes" branch hasn't been
pulled yet. That could explain things.

69dbdfffef20c715df9f381b2cee4e9e0a4efd93 HID: wacom: Sync INTUOSP2_BT
touch state after each frame if necessary
6441fc781c344df61402be1fde582c4491fa35fa HID: wacom: Correct button
numbering 2nd-gen Intuos Pro over Bluetooth
fe7f8d73d1af19b678171170e4e5384deb57833d HID: wacom: Send BTN_TOUCH in
response to INTUOSP2_BT eraser contact
e92a7be7fe5b2510fa60965eaf25f9e3dc08b8cc HID: wacom: Don't report
anything prior to the tool entering range
2cc08800a6b9fcda7c7afbcf2da1a6e8808da725 HID: wacom: Don't set tool
type until we're in range

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

^ permalink raw reply

* Re: [PATCH 06/10] mfd / platform: cros_ec: Reorganize platform and mfd includes
From: Sebastian Reichel @ 2019-06-11 19:54 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: linux-kernel, gwendal, Guenter Roeck, Benson Leung, Lee Jones,
	kernel, dtor, Mauro Carvalho Chehab, alsa-devel, Alessandro Zummo,
	linux-iio, Fabien Lahoudere, Alexandre Belloni, linux-i2c,
	linux-rtc, Heiko Stuebner, Brian Norris, Chanwoo Choi,
	Benjamin Tissoires, Gustavo A. R. Silva, Rushikesh S Kadam
In-Reply-To: <20190604152019.16100-7-enric.balletbo@collabora.com>

[-- Attachment #1: Type: text/plain, Size: 48544 bytes --]

Hi,

On Tue, Jun 04, 2019 at 05:20:15PM +0200, Enric Balletbo i Serra wrote:
> There is a bit of mess between cros-ec mfd includes and platform
> includes. For example, we have a linux/mfd/cros_ec.h include that
> exports the interface implemented in platform/chrome/cros_ec_proto.c. Or
> we have a linux/mfd/cros_ec_commands.h file that is non related to the
> multifunction device (in the sense that is not exporting any function of
> the mfd device). This causes crossed includes between mfd and
> platform/chrome subsystems and makes the code difficult to read, apart
> from creating 'curious' situations where a platform/chrome driver includes
> a linux/mfd/cros_ec.h file just to get the exported functions that are
> implemented in another platform/chrome driver.
> 
> In order to have a better separation on what the cros-ec multifunction
> driver does and what the cros-ec core provides move and rework the
> affected includes doing:
> 
>  - Move cros_ec_commands.h to include/linux/platform_data/cros_ec_commands.h
>  - Get rid of the parts that are implemented in the platform/chrome/cros_ec_proto.c
>    driver from include/linux/mfd/cros_ec.h to a new file
>    include/linux/platform_data/cros_ec_proto.h
>  - Update all the drivers with the new includes, so
>    - Drivers that only need to know about the protocol include
>      - linux/platform_data/cros_ec_proto.h
>      - linux/platform_data/cros_ec_commands.h
>    - Drivers that need to know about the cros-ec mfd device also include
>      - linux/mfd/cros_ec.h
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---

Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/extcon/extcon-usbc-cros-ec.c          |   3 +-
>  drivers/hid/hid-google-hammer.c               |   4 +-
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c       |   4 +-
>  drivers/iio/accel/cros_ec_accel_legacy.c      |   3 +-
>  .../common/cros_ec_sensors/cros_ec_sensors.c  |   3 +-
>  .../cros_ec_sensors/cros_ec_sensors_core.c    |   3 +-
>  drivers/iio/light/cros_ec_light_prox.c        |   3 +-
>  drivers/iio/pressure/cros_ec_baro.c           |   3 +-
>  drivers/input/keyboard/cros_ec_keyb.c         |   4 +-
>  .../media/platform/cros-ec-cec/cros-ec-cec.c  |   4 +-
>  drivers/mfd/cros_ec_dev.c                     |   3 +-
>  drivers/platform/chrome/cros_ec.c             |   3 +-
>  drivers/platform/chrome/cros_ec_chardev.c     |   4 +-
>  drivers/platform/chrome/cros_ec_debugfs.c     |   3 +-
>  drivers/platform/chrome/cros_ec_i2c.c         |   4 +-
>  drivers/platform/chrome/cros_ec_lightbar.c    |   3 +-
>  drivers/platform/chrome/cros_ec_lpc.c         |   4 +-
>  drivers/platform/chrome/cros_ec_lpc_reg.c     |   4 +-
>  drivers/platform/chrome/cros_ec_proto.c       |   3 +-
>  drivers/platform/chrome/cros_ec_rpmsg.c       |   4 +-
>  drivers/platform/chrome/cros_ec_spi.c         |   4 +-
>  drivers/platform/chrome/cros_ec_sysfs.c       |   3 +-
>  drivers/platform/chrome/cros_ec_trace.c       |   2 +-
>  drivers/platform/chrome/cros_ec_trace.h       |   4 +-
>  drivers/platform/chrome/cros_ec_vbc.c         |   3 +-
>  drivers/platform/chrome/cros_usbpd_logger.c   |   5 +-
>  drivers/power/supply/cros_usbpd-charger.c     |   5 +-
>  drivers/pwm/pwm-cros-ec.c                     |   4 +-
>  drivers/rtc/rtc-cros-ec.c                     |   3 +-
>  .../linux/iio/common/cros_ec_sensors_core.h   |   3 +-
>  include/linux/mfd/cros_ec.h                   | 306 -----------------
>  .../{mfd => platform_data}/cros_ec_commands.h |   0
>  include/linux/platform_data/cros_ec_proto.h   | 315 ++++++++++++++++++
>  sound/soc/codecs/cros_ec_codec.c              |   4 +-
>  34 files changed, 379 insertions(+), 351 deletions(-)
>  rename include/linux/{mfd => platform_data}/cros_ec_commands.h (100%)
>  create mode 100644 include/linux/platform_data/cros_ec_proto.h
> 
> diff --git a/drivers/extcon/extcon-usbc-cros-ec.c b/drivers/extcon/extcon-usbc-cros-ec.c
> index 43c0a936ab82..5290cc2d19d9 100644
> --- a/drivers/extcon/extcon-usbc-cros-ec.c
> +++ b/drivers/extcon/extcon-usbc-cros-ec.c
> @@ -6,10 +6,11 @@
>  
>  #include <linux/extcon-provider.h>
>  #include <linux/kernel.h>
> -#include <linux/mfd/cros_ec.h>
>  #include <linux/module.h>
>  #include <linux/notifier.h>
>  #include <linux/of.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/sched.h>
> diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
> index ee5e0bdcf078..84f8c127ebdc 100644
> --- a/drivers/hid/hid-google-hammer.c
> +++ b/drivers/hid/hid-google-hammer.c
> @@ -16,9 +16,9 @@
>  #include <linux/acpi.h>
>  #include <linux/hid.h>
>  #include <linux/leds.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_wakeup.h>
>  #include <asm/unaligned.h>
> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index 82bcd9a78759..c551aa96a2e3 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -5,8 +5,8 @@
>  
>  #include <linux/module.h>
>  #include <linux/i2c.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> index 46bb2e421bb9..fd9a634f741e 100644
> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -18,9 +18,10 @@
>  #include <linux/iio/triggered_buffer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  
>  #define DRV_NAME	"cros-ec-accel-legacy"
> 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 17af4e0fd5f8..40dc24ff0ee5 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -17,8 +17,9 @@
>  #include <linux/iio/triggered_buffer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> 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 719a0df5aeeb..fd63315399ac 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
> @@ -14,9 +14,10 @@
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  
>  static char *cros_ec_loc[] = {
> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> index 308ee6ff2e22..437e0eae9178 100644
> --- a/drivers/iio/light/cros_ec_light_prox.c
> +++ b/drivers/iio/light/cros_ec_light_prox.c
> @@ -15,8 +15,9 @@
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
> index 034ce98d6e97..956dc01f1295 100644
> --- a/drivers/iio/pressure/cros_ec_baro.c
> +++ b/drivers/iio/pressure/cros_ec_baro.c
> @@ -15,9 +15,10 @@
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  
>  /*
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index d56001181598..2b71c5a51f90 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -22,8 +22,8 @@
>  #include <linux/slab.h>
>  #include <linux/sysrq.h>
>  #include <linux/input/matrix_keypad.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  
>  #include <asm/unaligned.h>
>  
> diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> index 068df9888dbf..2e4e263a4a94 100644
> --- a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> +++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> @@ -16,8 +16,8 @@
>  #include <linux/interrupt.h>
>  #include <media/cec.h>
>  #include <media/cec-notifier.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  
>  #define DRV_NAME	"cros-ec-cec"
>  
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index c7a5dfa36874..5481df4e1216 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -7,11 +7,12 @@
>  
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/slab.h>
>  
>  #define DRV_NAME "cros-ec-dev"
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index 11fced7917fc..9800597ccd96 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -21,7 +21,8 @@
>  #include <linux/interrupt.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> -#include <linux/mfd/cros_ec.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/suspend.h>
>  #include <asm/unaligned.h>
>  
> diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
> index 1a0a27080026..786b941a60df 100644
> --- a/drivers/platform/chrome/cros_ec_chardev.c
> +++ b/drivers/platform/chrome/cros_ec_chardev.c
> @@ -9,10 +9,10 @@
>  #include <linux/device.h>
>  #include <linux/fs.h>
>  #include <linux/list.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
> index 4c2a27f6a6d0..b088d91be9c9 100644
> --- a/drivers/platform/chrome/cros_ec_debugfs.c
> +++ b/drivers/platform/chrome/cros_ec_debugfs.c
> @@ -8,9 +8,10 @@
>  #include <linux/delay.h>
>  #include <linux/fs.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/poll.h>
>  #include <linux/sched.h>
> diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c
> index 6bb82dfa7dae..9bd97bc8454b 100644
> --- a/drivers/platform/chrome/cros_ec_i2c.c
> +++ b/drivers/platform/chrome/cros_ec_i2c.c
> @@ -9,8 +9,8 @@
>  #include <linux/module.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> index d30a6650b0b5..caa26da2c788 100644
> --- a/drivers/platform/chrome/cros_ec_lightbar.c
> +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> @@ -9,8 +9,9 @@
>  #include <linux/fs.h>
>  #include <linux/kobject.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/sched.h>
>  #include <linux/types.h>
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 2c7e654cf89c..0c976e95998a 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -16,9 +16,9 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/interrupt.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/printk.h>
>  #include <linux/suspend.h>
> diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
> index 0f5cd0ac8b49..dec9a779e209 100644
> --- a/drivers/platform/chrome/cros_ec_lpc_reg.c
> +++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
> @@ -4,8 +4,8 @@
>  // Copyright (C) 2016 Google, Inc
>  
>  #include <linux/io.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  
>  #include "cros_ec_lpc_mec.h"
>  
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 3d2325197a68..f659f96bda12 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -3,10 +3,11 @@
>  //
>  // Copyright (C) 2015 Google, Inc
>  
> -#include <linux/mfd/cros_ec.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/slab.h>
>  #include <asm/unaligned.h>
>  
> diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c
> index 520e507bfa54..9633e5417686 100644
> --- a/drivers/platform/chrome/cros_ec_rpmsg.c
> +++ b/drivers/platform/chrome/cros_ec_rpmsg.c
> @@ -6,9 +6,9 @@
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/of.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/rpmsg.h>
>  #include <linux/slab.h>
> diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
> index 2e21f2776063..9006e1872942 100644
> --- a/drivers/platform/chrome/cros_ec_spi.c
> +++ b/drivers/platform/chrome/cros_ec_spi.c
> @@ -6,9 +6,9 @@
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/of.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/spi/spi.h>
> diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
> index fe0b7614ae1b..0caeb8d0989d 100644
> --- a/drivers/platform/chrome/cros_ec_sysfs.c
> +++ b/drivers/platform/chrome/cros_ec_sysfs.c
> @@ -9,8 +9,9 @@
>  #include <linux/fs.h>
>  #include <linux/kobject.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/printk.h>
>  #include <linux/slab.h>
> diff --git a/drivers/platform/chrome/cros_ec_trace.c b/drivers/platform/chrome/cros_ec_trace.c
> index 0a76412095a9..6f80ff4532ae 100644
> --- a/drivers/platform/chrome/cros_ec_trace.c
> +++ b/drivers/platform/chrome/cros_ec_trace.c
> @@ -6,7 +6,7 @@
>  #define TRACE_SYMBOL(a) {a, #a}
>  
>  // Generate the list using the following script:
> -// sed -n 's/^#define \(EC_CMD_[[:alnum:]_]*\)\s.*/\tTRACE_SYMBOL(\1), \\/p' include/linux/mfd/cros_ec_commands.h
> +// sed -n 's/^#define \(EC_CMD_[[:alnum:]_]*\)\s.*/\tTRACE_SYMBOL(\1), \\/p' include/linux/platform_data/cros_ec_commands.h
>  #define EC_CMDS \
>  	TRACE_SYMBOL(EC_CMD_PROTO_VERSION), \
>  	TRACE_SYMBOL(EC_CMD_HELLO), \
> diff --git a/drivers/platform/chrome/cros_ec_trace.h b/drivers/platform/chrome/cros_ec_trace.h
> index 7ae3b89c78b9..0dd4df30fa89 100644
> --- a/drivers/platform/chrome/cros_ec_trace.h
> +++ b/drivers/platform/chrome/cros_ec_trace.h
> @@ -11,8 +11,10 @@
>  #if !defined(_CROS_EC_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
>  #define _CROS_EC_TRACE_H_
>  
> +#include <linux/bits.h>
>  #include <linux/types.h>
> -#include <linux/mfd/cros_ec.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  
>  #include <linux/tracepoint.h>
>  
> diff --git a/drivers/platform/chrome/cros_ec_vbc.c b/drivers/platform/chrome/cros_ec_vbc.c
> index 8392a1ec33a7..cffe119e7a7a 100644
> --- a/drivers/platform/chrome/cros_ec_vbc.c
> +++ b/drivers/platform/chrome/cros_ec_vbc.c
> @@ -7,8 +7,9 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/slab.h>
>  
>  #define DRV_NAME "cros-ec-vbc"
> diff --git a/drivers/platform/chrome/cros_usbpd_logger.c b/drivers/platform/chrome/cros_usbpd_logger.c
> index 7c7b267626a0..c549a9b49b56 100644
> --- a/drivers/platform/chrome/cros_usbpd_logger.c
> +++ b/drivers/platform/chrome/cros_usbpd_logger.c
> @@ -6,10 +6,11 @@
>   */
>  
>  #include <linux/ktime.h>
> -#include <linux/math64.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/rtc.h>
>  
> diff --git a/drivers/power/supply/cros_usbpd-charger.c b/drivers/power/supply/cros_usbpd-charger.c
> index 3a9ea94c3de3..6cc7c3910e09 100644
> --- a/drivers/power/supply/cros_usbpd-charger.c
> +++ b/drivers/power/supply/cros_usbpd-charger.c
> @@ -5,9 +5,10 @@
>   * Copyright (c) 2014 - 2018 Google, Inc
>   */
>  
> -#include <linux/module.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/power_supply.h>
>  #include <linux/slab.h>
> diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
> index 98f6ac6cf6ab..85bea2d40b7d 100644
> --- a/drivers/pwm/pwm-cros-ec.c
> +++ b/drivers/pwm/pwm-cros-ec.c
> @@ -6,8 +6,8 @@
>   */
>  
>  #include <linux/module.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/pwm.h>
>  #include <linux/slab.h>
> diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c
> index 4d6bf9304ceb..6909e01936d9 100644
> --- a/drivers/rtc/rtc-cros-ec.c
> +++ b/drivers/rtc/rtc-cros-ec.c
> @@ -6,8 +6,9 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/rtc.h>
>  #include <linux/slab.h>
> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> index ce16445411ac..8a91669f5bed 100644
> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> @@ -18,7 +18,8 @@
>  
>  #include <linux/iio/iio.h>
>  #include <linux/irqreturn.h>
> -#include <linux/mfd/cros_ec.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  
>  enum {
>  	CROS_EC_SENSOR_X,
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 2a1372d167b9..e0bae49535e1 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -16,184 +16,7 @@
>  #ifndef __LINUX_MFD_CROS_EC_H
>  #define __LINUX_MFD_CROS_EC_H
>  
> -#include <linux/cdev.h>
>  #include <linux/device.h>
> -#include <linux/notifier.h>
> -#include <linux/mfd/cros_ec_commands.h>
> -#include <linux/mutex.h>
> -
> -#define CROS_EC_DEV_NAME "cros_ec"
> -#define CROS_EC_DEV_FP_NAME "cros_fp"
> -#define CROS_EC_DEV_PD_NAME "cros_pd"
> -#define CROS_EC_DEV_TP_NAME "cros_tp"
> -#define CROS_EC_DEV_ISH_NAME "cros_ish"
> -
> -/*
> - * The EC is unresponsive for a time after a reboot command.  Add a
> - * simple delay to make sure that the bus stays locked.
> - */
> -#define EC_REBOOT_DELAY_MS             50
> -
> -/*
> - * Max bus-specific overhead incurred by request/responses.
> - * I2C requires 1 additional byte for requests.
> - * I2C requires 2 additional bytes for responses.
> - * SPI requires up to 32 additional bytes for responses.
> - */
> -#define EC_PROTO_VERSION_UNKNOWN	0
> -#define EC_MAX_REQUEST_OVERHEAD		1
> -#define EC_MAX_RESPONSE_OVERHEAD	32
> -
> -/*
> - * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
> - */
> -enum {
> -	EC_MSG_TX_HEADER_BYTES	= 3,
> -	EC_MSG_TX_TRAILER_BYTES	= 1,
> -	EC_MSG_TX_PROTO_BYTES	= EC_MSG_TX_HEADER_BYTES +
> -					EC_MSG_TX_TRAILER_BYTES,
> -	EC_MSG_RX_PROTO_BYTES	= 3,
> -
> -	/* Max length of messages for proto 2*/
> -	EC_PROTO2_MSG_BYTES		= EC_PROTO2_MAX_PARAM_SIZE +
> -					EC_MSG_TX_PROTO_BYTES,
> -
> -	EC_MAX_MSG_BYTES		= 64 * 1024,
> -};
> -
> -/**
> - * struct cros_ec_command - Information about a ChromeOS EC command.
> - * @version: Command version number (often 0).
> - * @command: Command to send (EC_CMD_...).
> - * @outsize: Outgoing length in bytes.
> - * @insize: Max number of bytes to accept from the EC.
> - * @result: EC's response to the command (separate from communication failure).
> - * @data: Where to put the incoming data from EC and outgoing data to EC.
> - */
> -struct cros_ec_command {
> -	uint32_t version;
> -	uint32_t command;
> -	uint32_t outsize;
> -	uint32_t insize;
> -	uint32_t result;
> -	uint8_t data[0];
> -};
> -
> -/**
> - * struct cros_ec_device - Information about a ChromeOS EC device.
> - * @phys_name: Name of physical comms layer (e.g. 'i2c-4').
> - * @dev: Device pointer for physical comms device
> - * @was_wake_device: True if this device was set to wake the system from
> - *                   sleep at the last suspend.
> - * @cros_class: The class structure for this device.
> - * @cmd_readmem: Direct read of the EC memory-mapped region, if supported.
> - *     @offset: Is within EC_LPC_ADDR_MEMMAP region.
> - *     @bytes: Number of bytes to read. zero means "read a string" (including
> - *             the trailing '\0'). At most only EC_MEMMAP_SIZE bytes can be
> - *             read. Caller must ensure that the buffer is large enough for the
> - *             result when reading a string.
> - * @max_request: Max size of message requested.
> - * @max_response: Max size of message response.
> - * @max_passthru: Max sice of passthru message.
> - * @proto_version: The protocol version used for this device.
> - * @priv: Private data.
> - * @irq: Interrupt to use.
> - * @id: Device id.
> - * @din: Input buffer (for data from EC). This buffer will always be
> - *       dword-aligned and include enough space for up to 7 word-alignment
> - *       bytes also, so we can ensure that the body of the message is always
> - *       dword-aligned (64-bit). We use this alignment to keep ARM and x86
> - *       happy. Probably word alignment would be OK, there might be a small
> - *       performance advantage to using dword.
> - * @dout: Output buffer (for data to EC). This buffer will always be
> - *        dword-aligned and include enough space for up to 7 word-alignment
> - *        bytes also, so we can ensure that the body of the message is always
> - *        dword-aligned (64-bit). We use this alignment to keep ARM and x86
> - *        happy. Probably word alignment would be OK, there might be a small
> - *        performance advantage to using dword.
> - * @din_size: Size of din buffer to allocate (zero to use static din).
> - * @dout_size: Size of dout buffer to allocate (zero to use static dout).
> - * @wake_enabled: True if this device can wake the system from sleep.
> - * @suspended: True if this device had been suspended.
> - * @cmd_xfer: Send command to EC and get response.
> - *            Returns the number of bytes received if the communication
> - *            succeeded, but that doesn't mean the EC was happy with the
> - *            command. The caller should check msg.result for the EC's result
> - *            code.
> - * @pkt_xfer: Send packet to EC and get response.
> - * @lock: One transaction at a time.
> - * @mkbp_event_supported: True if this EC supports the MKBP event protocol.
> - * @host_sleep_v1: True if this EC supports the sleep v1 command.
> - * @event_notifier: Interrupt event notifier for transport devices.
> - * @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.
> - * @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
> - *      PD behind an EC.
> - */
> -struct cros_ec_device {
> -	/* These are used by other drivers that want to talk to the EC */
> -	const char *phys_name;
> -	struct device *dev;
> -	bool was_wake_device;
> -	struct class *cros_class;
> -	int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
> -			   unsigned int bytes, void *dest);
> -
> -	/* These are used to implement the platform-specific interface */
> -	u16 max_request;
> -	u16 max_response;
> -	u16 max_passthru;
> -	u16 proto_version;
> -	void *priv;
> -	int irq;
> -	u8 *din;
> -	u8 *dout;
> -	int din_size;
> -	int dout_size;
> -	bool wake_enabled;
> -	bool suspended;
> -	int (*cmd_xfer)(struct cros_ec_device *ec,
> -			struct cros_ec_command *msg);
> -	int (*pkt_xfer)(struct cros_ec_device *ec,
> -			struct cros_ec_command *msg);
> -	struct mutex lock;
> -	bool mkbp_event_supported;
> -	bool host_sleep_v1;
> -	struct blocking_notifier_head event_notifier;
> -
> -	struct ec_response_get_next_event_v1 event_data;
> -	int event_size;
> -	u32 host_event_wake_mask;
> -
> -	/* The platform devices used by the mfd driver */
> -	struct platform_device *ec;
> -	struct platform_device *pd;
> -};
> -
> -/**
> - * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information.
> - * @sensor_num: Id of the sensor, as reported by the EC.
> - */
> -struct cros_ec_sensor_platform {
> -	u8 sensor_num;
> -};
> -
> -/**
> - * struct cros_ec_platform - ChromeOS EC platform information.
> - * @ec_name: Name of EC device (e.g. 'cros-ec', 'cros-pd', ...)
> - *           used in /dev/ and sysfs.
> - * @cmd_offset: Offset to apply for each command. Set when
> - *              registering a device behind another one.
> - */
> -struct cros_ec_platform {
> -	const char *ec_name;
> -	u16 cmd_offset;
> -};
> -
> -struct cros_ec_debugfs;
>  
>  /**
>   * struct cros_ec_dev - ChromeOS EC device entry point.
> @@ -217,133 +40,4 @@ struct cros_ec_dev {
>  
>  #define to_cros_ec_dev(dev)  container_of(dev, struct cros_ec_dev, class_dev)
>  
> -/**
> - * cros_ec_suspend() - Handle a suspend operation for the ChromeOS EC device.
> - * @ec_dev: Device to suspend.
> - *
> - * This can be called by drivers to handle a suspend event.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_suspend(struct cros_ec_device *ec_dev);
> -
> -/**
> - * cros_ec_resume() - Handle a resume operation for the ChromeOS EC device.
> - * @ec_dev: Device to resume.
> - *
> - * This can be called by drivers to handle a resume event.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_resume(struct cros_ec_device *ec_dev);
> -
> -/**
> - * cros_ec_prepare_tx() - Prepare an outgoing message in the output buffer.
> - * @ec_dev: Device to register.
> - * @msg: Message to write.
> - *
> - * This is intended to be used by all ChromeOS EC drivers, but at present
> - * only SPI uses it. Once LPC uses the same protocol it can start using it.
> - * I2C could use it now, with a refactor of the existing code.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
> -		       struct cros_ec_command *msg);
> -
> -/**
> - * cros_ec_check_result() - Check ec_msg->result.
> - * @ec_dev: EC device.
> - * @msg: Message to check.
> - *
> - * This is used by ChromeOS EC drivers to check the ec_msg->result for
> - * errors and to warn about them.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_check_result(struct cros_ec_device *ec_dev,
> -			 struct cros_ec_command *msg);
> -
> -/**
> - * cros_ec_cmd_xfer() - Send a command to the ChromeOS EC.
> - * @ec_dev: EC device.
> - * @msg: Message to write.
> - *
> - * Call this to send a command to the ChromeOS EC.  This should be used
> - * instead of calling the EC's cmd_xfer() callback directly.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> -		     struct cros_ec_command *msg);
> -
> -/**
> - * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
> - * @ec_dev: EC device.
> - * @msg: Message to write.
> - *
> - * This function is identical to cros_ec_cmd_xfer, except it returns success
> - * status only if both the command was transmitted successfully and the EC
> - * replied with success status. It's not necessary to check msg->result when
> - * using this function.
> - *
> - * Return: The number of bytes transferred on success or negative error code.
> - */
> -int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> -			    struct cros_ec_command *msg);
> -
> -/**
> - * cros_ec_register() - Register a new ChromeOS EC, using the provided info.
> - * @ec_dev: Device to register.
> - *
> - * Before calling this, allocate a pointer to a new device and then fill
> - * in all the fields up to the --private-- marker.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_register(struct cros_ec_device *ec_dev);
> -
> -/**
> - * cros_ec_unregister() - Remove a ChromeOS EC.
> - * @ec_dev: Device to unregister.
> - *
> - * Call this to deregister a ChromeOS EC, then clean up any private data.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_unregister(struct cros_ec_device *ec_dev);
> -
> -/**
> - * cros_ec_query_all() -  Query the protocol version supported by the
> - *         ChromeOS EC.
> - * @ec_dev: Device to register.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_query_all(struct cros_ec_device *ec_dev);
> -
> -/**
> - * cros_ec_get_next_event() - Fetch next event from the ChromeOS EC.
> - * @ec_dev: Device to fetch event from.
> - * @wake_event: Pointer to a bool set to true upon return if the event might be
> - *              treated as a wake event. Ignored if null.
> - *
> - * Return: negative error code on errors; 0 for no data; or else number of
> - * bytes received (i.e., an event was retrieved successfully). Event types are
> - * written out to @ec_dev->event_data.event_type on success.
> - */
> -int cros_ec_get_next_event(struct cros_ec_device *ec_dev, bool *wake_event);
> -
> -/**
> - * cros_ec_get_host_event() - Return a mask of event set by the ChromeOS EC.
> - * @ec_dev: Device to fetch event from.
> - *
> - * When MKBP is supported, when the EC raises an interrupt, we collect the
> - * events raised and call the functions in the ec notifier. This function
> - * is a helper to know which events are raised.
> - *
> - * Return: 0 on error or non-zero bitmask of one or more EC_HOST_EVENT_*.
> - */
> -u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
> -
>  #endif /* __LINUX_MFD_CROS_EC_H */
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
> similarity index 100%
> rename from include/linux/mfd/cros_ec_commands.h
> rename to include/linux/platform_data/cros_ec_commands.h
> diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
> new file mode 100644
> index 000000000000..34dd9e5c1779
> --- /dev/null
> +++ b/include/linux/platform_data/cros_ec_proto.h
> @@ -0,0 +1,315 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * ChromeOS Embedded Controller protocol interface.
> + *
> + * Copyright (C) 2012 Google, Inc
> + */
> +
> +#ifndef __LINUX_CROS_EC_PROTO_H
> +#define __LINUX_CROS_EC_PROTO_H
> +
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +
> +#define CROS_EC_DEV_NAME	"cros_ec"
> +#define CROS_EC_DEV_FP_NAME	"cros_fp"
> +#define CROS_EC_DEV_ISH_NAME	"cros_ish"
> +#define CROS_EC_DEV_PD_NAME	"cros_pd"
> +#define CROS_EC_DEV_TP_NAME	"cros_tp"
> +
> +/*
> + * The EC is unresponsive for a time after a reboot command.  Add a
> + * simple delay to make sure that the bus stays locked.
> + */
> +#define EC_REBOOT_DELAY_MS		50
> +
> +/*
> + * Max bus-specific overhead incurred by request/responses.
> + * I2C requires 1 additional byte for requests.
> + * I2C requires 2 additional bytes for responses.
> + * SPI requires up to 32 additional bytes for responses.
> + */
> +#define EC_PROTO_VERSION_UNKNOWN	0
> +#define EC_MAX_REQUEST_OVERHEAD		1
> +#define EC_MAX_RESPONSE_OVERHEAD	32
> +
> +/*
> + * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
> + */
> +enum {
> +	EC_MSG_TX_HEADER_BYTES	= 3,
> +	EC_MSG_TX_TRAILER_BYTES	= 1,
> +	EC_MSG_TX_PROTO_BYTES	= EC_MSG_TX_HEADER_BYTES +
> +				  EC_MSG_TX_TRAILER_BYTES,
> +	EC_MSG_RX_PROTO_BYTES	= 3,
> +
> +	/* Max length of messages for proto 2*/
> +	EC_PROTO2_MSG_BYTES	= EC_PROTO2_MAX_PARAM_SIZE +
> +				  EC_MSG_TX_PROTO_BYTES,
> +
> +	EC_MAX_MSG_BYTES	= 64 * 1024,
> +};
> +
> +/**
> + * struct cros_ec_command - Information about a ChromeOS EC command.
> + * @version: Command version number (often 0).
> + * @command: Command to send (EC_CMD_...).
> + * @outsize: Outgoing length in bytes.
> + * @insize: Max number of bytes to accept from the EC.
> + * @result: EC's response to the command (separate from communication failure).
> + * @data: Where to put the incoming data from EC and outgoing data to EC.
> + */
> +struct cros_ec_command {
> +	uint32_t version;
> +	uint32_t command;
> +	uint32_t outsize;
> +	uint32_t insize;
> +	uint32_t result;
> +	uint8_t data[0];
> +};
> +
> +/**
> + * struct cros_ec_device - Information about a ChromeOS EC device.
> + * @phys_name: Name of physical comms layer (e.g. 'i2c-4').
> + * @dev: Device pointer for physical comms device
> + * @was_wake_device: True if this device was set to wake the system from
> + *                   sleep at the last suspend.
> + * @cros_class: The class structure for this device.
> + * @cmd_readmem: Direct read of the EC memory-mapped region, if supported.
> + *     @offset: Is within EC_LPC_ADDR_MEMMAP region.
> + *     @bytes: Number of bytes to read. zero means "read a string" (including
> + *             the trailing '\0'). At most only EC_MEMMAP_SIZE bytes can be
> + *             read. Caller must ensure that the buffer is large enough for the
> + *             result when reading a string.
> + * @max_request: Max size of message requested.
> + * @max_response: Max size of message response.
> + * @max_passthru: Max sice of passthru message.
> + * @proto_version: The protocol version used for this device.
> + * @priv: Private data.
> + * @irq: Interrupt to use.
> + * @id: Device id.
> + * @din: Input buffer (for data from EC). This buffer will always be
> + *       dword-aligned and include enough space for up to 7 word-alignment
> + *       bytes also, so we can ensure that the body of the message is always
> + *       dword-aligned (64-bit). We use this alignment to keep ARM and x86
> + *       happy. Probably word alignment would be OK, there might be a small
> + *       performance advantage to using dword.
> + * @dout: Output buffer (for data to EC). This buffer will always be
> + *        dword-aligned and include enough space for up to 7 word-alignment
> + *        bytes also, so we can ensure that the body of the message is always
> + *        dword-aligned (64-bit). We use this alignment to keep ARM and x86
> + *        happy. Probably word alignment would be OK, there might be a small
> + *        performance advantage to using dword.
> + * @din_size: Size of din buffer to allocate (zero to use static din).
> + * @dout_size: Size of dout buffer to allocate (zero to use static dout).
> + * @wake_enabled: True if this device can wake the system from sleep.
> + * @suspended: True if this device had been suspended.
> + * @cmd_xfer: Send command to EC and get response.
> + *            Returns the number of bytes received if the communication
> + *            succeeded, but that doesn't mean the EC was happy with the
> + *            command. The caller should check msg.result for the EC's result
> + *            code.
> + * @pkt_xfer: Send packet to EC and get response.
> + * @lock: One transaction at a time.
> + * @mkbp_event_supported: True if this EC supports the MKBP event protocol.
> + * @host_sleep_v1: True if this EC supports the sleep v1 command.
> + * @event_notifier: Interrupt event notifier for transport devices.
> + * @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.
> + * @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
> + *      PD behind an EC.
> + */
> +struct cros_ec_device {
> +	/* These are used by other drivers that want to talk to the EC */
> +	const char *phys_name;
> +	struct device *dev;
> +	bool was_wake_device;
> +	struct class *cros_class;
> +	int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
> +			   unsigned int bytes, void *dest);
> +
> +	/* These are used to implement the platform-specific interface */
> +	u16 max_request;
> +	u16 max_response;
> +	u16 max_passthru;
> +	u16 proto_version;
> +	void *priv;
> +	int irq;
> +	u8 *din;
> +	u8 *dout;
> +	int din_size;
> +	int dout_size;
> +	bool wake_enabled;
> +	bool suspended;
> +	int (*cmd_xfer)(struct cros_ec_device *ec,
> +			struct cros_ec_command *msg);
> +	int (*pkt_xfer)(struct cros_ec_device *ec,
> +			struct cros_ec_command *msg);
> +	struct mutex lock;
> +	bool mkbp_event_supported;
> +	bool host_sleep_v1;
> +	struct blocking_notifier_head event_notifier;
> +
> +	struct ec_response_get_next_event_v1 event_data;
> +	int event_size;
> +	u32 host_event_wake_mask;
> +
> +	/* The platform devices used by the mfd driver */
> +	struct platform_device *ec;
> +	struct platform_device *pd;
> +};
> +
> +/**
> + * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information.
> + * @sensor_num: Id of the sensor, as reported by the EC.
> + */
> +struct cros_ec_sensor_platform {
> +	u8 sensor_num;
> +};
> +
> +/**
> + * struct cros_ec_platform - ChromeOS EC platform information.
> + * @ec_name: Name of EC device (e.g. 'cros-ec', 'cros-pd', ...)
> + *           used in /dev/ and sysfs.
> + * @cmd_offset: Offset to apply for each command. Set when
> + *              registering a device behind another one.
> + */
> +struct cros_ec_platform {
> +	const char *ec_name;
> +	u16 cmd_offset;
> +};
> +
> +/**
> + * cros_ec_suspend() - Handle a suspend operation for the ChromeOS EC device.
> + * @ec_dev: Device to suspend.
> + *
> + * This can be called by drivers to handle a suspend event.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_suspend(struct cros_ec_device *ec_dev);
> +
> +/**
> + * cros_ec_resume() - Handle a resume operation for the ChromeOS EC device.
> + * @ec_dev: Device to resume.
> + *
> + * This can be called by drivers to handle a resume event.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_resume(struct cros_ec_device *ec_dev);
> +
> +/**
> + * cros_ec_prepare_tx() - Prepare an outgoing message in the output buffer.
> + * @ec_dev: Device to register.
> + * @msg: Message to write.
> + *
> + * This is intended to be used by all ChromeOS EC drivers, but at present
> + * only SPI uses it. Once LPC uses the same protocol it can start using it.
> + * I2C could use it now, with a refactor of the existing code.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
> +		       struct cros_ec_command *msg);
> +
> +/**
> + * cros_ec_check_result() - Check ec_msg->result.
> + * @ec_dev: EC device.
> + * @msg: Message to check.
> + *
> + * This is used by ChromeOS EC drivers to check the ec_msg->result for
> + * errors and to warn about them.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_check_result(struct cros_ec_device *ec_dev,
> +			 struct cros_ec_command *msg);
> +
> +/**
> + * cros_ec_cmd_xfer() - Send a command to the ChromeOS EC.
> + * @ec_dev: EC device.
> + * @msg: Message to write.
> + *
> + * Call this to send a command to the ChromeOS EC.  This should be used
> + * instead of calling the EC's cmd_xfer() callback directly.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> +		     struct cros_ec_command *msg);
> +
> +/**
> + * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
> + * @ec_dev: EC device.
> + * @msg: Message to write.
> + *
> + * This function is identical to cros_ec_cmd_xfer, except it returns success
> + * status only if both the command was transmitted successfully and the EC
> + * replied with success status. It's not necessary to check msg->result when
> + * using this function.
> + *
> + * Return: The number of bytes transferred on success or negative error code.
> + */
> +int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> +			    struct cros_ec_command *msg);
> +
> +/**
> + * cros_ec_register() - Register a new ChromeOS EC, using the provided info.
> + * @ec_dev: Device to register.
> + *
> + * Before calling this, allocate a pointer to a new device and then fill
> + * in all the fields up to the --private-- marker.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_register(struct cros_ec_device *ec_dev);
> +
> +/**
> + * cros_ec_unregister() - Remove a ChromeOS EC.
> + * @ec_dev: Device to unregister.
> + *
> + * Call this to deregister a ChromeOS EC, then clean up any private data.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_unregister(struct cros_ec_device *ec_dev);
> +
> +/**
> + * cros_ec_query_all() -  Query the protocol version supported by the
> + *         ChromeOS EC.
> + * @ec_dev: Device to register.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_query_all(struct cros_ec_device *ec_dev);
> +
> +/**
> + * cros_ec_get_next_event() - Fetch next event from the ChromeOS EC.
> + * @ec_dev: Device to fetch event from.
> + * @wake_event: Pointer to a bool set to true upon return if the event might be
> + *              treated as a wake event. Ignored if null.
> + *
> + * Return: negative error code on errors; 0 for no data; or else number of
> + * bytes received (i.e., an event was retrieved successfully). Event types are
> + * written out to @ec_dev->event_data.event_type on success.
> + */
> +int cros_ec_get_next_event(struct cros_ec_device *ec_dev, bool *wake_event);
> +
> +/**
> + * cros_ec_get_host_event() - Return a mask of event set by the ChromeOS EC.
> + * @ec_dev: Device to fetch event from.
> + *
> + * When MKBP is supported, when the EC raises an interrupt, we collect the
> + * events raised and call the functions in the ec notifier. This function
> + * is a helper to know which events are raised.
> + *
> + * Return: 0 on error or non-zero bitmask of one or more EC_HOST_EVENT_*.
> + */
> +u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
> +
> +#endif /* __LINUX_CROS_EC_PROTO_H */
> diff --git a/sound/soc/codecs/cros_ec_codec.c b/sound/soc/codecs/cros_ec_codec.c
> index 87830ed5ebf4..79bb4081d3c2 100644
> --- a/sound/soc/codecs/cros_ec_codec.c
> +++ b/sound/soc/codecs/cros_ec_codec.c
> @@ -9,9 +9,9 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/kernel.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <sound/pcm.h>
>  #include <sound/pcm_params.h>
> -- 
> 2.20.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 02/10] mfd / platform: cros_ec: Move cros-ec core driver out from MFD
From: Sebastian Reichel @ 2019-06-11 19:52 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: gwendal, Banajit Goswami, Vignesh R, Alexandre Belloni,
	Wolfram Sang, linux-iio, Mark Brown, Juergen Fitschen, alsa-devel,
	Stefan Agner, Douglas Anderson, Benjamin Tissoires,
	Karthikeyan Ramasubramanian, linux-i2c, Peter Meerwald-Stadler,
	Manivannan Sadhasivam, Guenter Roeck, kernel, dtor,
	Lars-Peter Clausen, Jean Delvare, Jacky Bai, linux-rtc,
	Andy Shevchenko, Sean
In-Reply-To: <20190604152019.16100-3-enric.balletbo@collabora.com>


[-- Attachment #1.1: Type: text/plain, Size: 10857 bytes --]

Hi,

On Tue, Jun 04, 2019 at 05:20:11PM +0200, Enric Balletbo i Serra wrote:
> Now, the ChromeOS EC core driver has nothing related to an MFD device, so
> move that driver from the MFD subsystem to the platform/chrome subsystem.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---

Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/extcon/Kconfig                     |  2 +-
>  drivers/hid/Kconfig                        |  2 +-
>  drivers/i2c/busses/Kconfig                 |  2 +-
>  drivers/iio/common/cros_ec_sensors/Kconfig |  2 +-
>  drivers/input/keyboard/Kconfig             |  2 +-
>  drivers/media/platform/Kconfig             |  3 +--
>  drivers/mfd/Kconfig                        | 14 +-------------
>  drivers/mfd/Makefile                       |  2 --
>  drivers/platform/chrome/Kconfig            | 21 +++++++++++++++++----
>  drivers/platform/chrome/Makefile           |  1 +
>  drivers/{mfd => platform/chrome}/cros_ec.c |  0
>  drivers/power/supply/Kconfig               |  2 +-
>  drivers/pwm/Kconfig                        |  2 +-
>  drivers/rtc/Kconfig                        |  2 +-
>  sound/soc/qcom/Kconfig                     |  2 +-
>  15 files changed, 29 insertions(+), 30 deletions(-)
>  rename drivers/{mfd => platform/chrome}/cros_ec.c (100%)
> 
> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> index 6f5af4196b8d..0ebc599c5e51 100644
> --- a/drivers/extcon/Kconfig
> +++ b/drivers/extcon/Kconfig
> @@ -169,7 +169,7 @@ config EXTCON_USB_GPIO
>  
>  config EXTCON_USBC_CROS_EC
>  	tristate "ChromeOS Embedded Controller EXTCON support"
> -	depends on MFD_CROS_EC
> +	depends on CROS_EC
>  	help
>  	  Say Y here to enable USB Type C cable detection extcon support when
>  	  using Chrome OS EC based USB Type-C ports.
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 3872e03d9a59..a958b9625bba 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -376,7 +376,7 @@ config HOLTEK_FF
>  
>  config HID_GOOGLE_HAMMER
>  	tristate "Google Hammer Keyboard"
> -	depends on USB_HID && LEDS_CLASS && MFD_CROS_EC
> +	depends on USB_HID && LEDS_CLASS && CROS_EC
>  	---help---
>  	Say Y here if you have a Google Hammer device.
>  
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index ee5dfb5aee2a..42a224d08ec7 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -1336,7 +1336,7 @@ config I2C_SIBYTE
>  
>  config I2C_CROS_EC_TUNNEL
>  	tristate "ChromeOS EC tunnel I2C bus"
> -	depends on MFD_CROS_EC
> +	depends on CROS_EC
>  	help
>  	  If you say yes here you get an I2C bus that will tunnel i2c commands
>  	  through to the other side of the ChromeOS EC to the i2c bus
> diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig b/drivers/iio/common/cros_ec_sensors/Kconfig
> index f9bf7ff7fcaf..55999104cd44 100644
> --- a/drivers/iio/common/cros_ec_sensors/Kconfig
> +++ b/drivers/iio/common/cros_ec_sensors/Kconfig
> @@ -4,7 +4,7 @@
>  #
>  config IIO_CROS_EC_SENSORS_CORE
>  	tristate "ChromeOS EC Sensors Core"
> -	depends on SYSFS && MFD_CROS_EC
> +	depends on SYSFS && CROS_EC
>  	select IIO_BUFFER
>  	select IIO_TRIGGERED_BUFFER
>  	help
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 7c4f19dab34f..64555cc8d83e 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -729,7 +729,7 @@ config KEYBOARD_W90P910
>  config KEYBOARD_CROS_EC
>  	tristate "ChromeOS EC keyboard"
>  	select INPUT_MATRIXKMAP
> -	depends on MFD_CROS_EC
> +	depends on CROS_EC
>  	help
>  	  Say Y here to enable the matrix keyboard used by ChromeOS devices
>  	  and implemented on the ChromeOS EC. You must enable one bus option
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index f2b5f27ebacb..adec7a0bfe1e 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -558,10 +558,9 @@ if CEC_PLATFORM_DRIVERS
>  
>  config VIDEO_CROS_EC_CEC
>  	tristate "ChromeOS EC CEC driver"
> -	depends on MFD_CROS_EC
> +	depends on CROS_EC
>  	select CEC_CORE
>  	select CEC_NOTIFIER
> -	select CHROME_PLATFORMS
>  	select CROS_EC_PROTO
>  	help
>  	  If you say yes here you will get support for the
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index a17d275bf1d4..ad0a5de74ef2 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -211,21 +211,9 @@ config MFD_AXP20X_RSB
>  	  components like regulators or the PEK (Power Enable Key) under the
>  	  corresponding menus.
>  
> -config MFD_CROS_EC
> -	tristate "ChromeOS Embedded Controller"
> -	select MFD_CORE
> -	select CHROME_PLATFORMS
> -	select CROS_EC_PROTO
> -	depends on X86 || ARM || ARM64 || COMPILE_TEST
> -	help
> -	  If you say Y here you get support for the ChromeOS Embedded
> -	  Controller (EC) providing keyboard, battery and power services.
> -	  You also need to enable the driver for the bus you are using. The
> -	  protocol for talking to the EC is defined by the bus driver.
> -
>  config MFD_CROS_EC_CHARDEV
>  	tristate "Chrome OS Embedded Controller userspace device interface"
> -	depends on MFD_CROS_EC
> +	depends on CROS_EC
>  	---help---
>  	  This driver adds support to talk with the ChromeOS EC from userspace.
>  
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 52b1a90ff515..32327dc6bb45 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -13,8 +13,6 @@ obj-$(CONFIG_MFD_ASIC3)		+= asic3.o tmio_core.o
>  obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835-pm.o
>  obj-$(CONFIG_MFD_BCM590XX)	+= bcm590xx.o
>  obj-$(CONFIG_MFD_BD9571MWV)	+= bd9571mwv.o
> -cros_ec_core-objs		:= cros_ec.o
> -obj-$(CONFIG_MFD_CROS_EC)	+= cros_ec_core.o
>  obj-$(CONFIG_MFD_CROS_EC_CHARDEV) += cros_ec_dev.o
>  obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
>  
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 35bb5a2663f0..9417b982ad92 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -50,9 +50,22 @@ config CHROMEOS_TBMC
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called chromeos_tbmc.
>  
> +config CROS_EC
> +	tristate "ChromeOS Embedded Controller"
> +	select CROS_EC_PROTO
> +	depends on X86 || ARM || ARM64 || COMPILE_TEST
> +	help
> +	  If you say Y here you get support for the ChromeOS Embedded
> +	  Controller (EC) providing keyboard, battery and power services.
> +	  You also need to enable the driver for the bus you are using. The
> +	  protocol for talking to the EC is defined by the bus driver.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called cros_ec.
> +
>  config CROS_EC_I2C
>  	tristate "ChromeOS Embedded Controller (I2C)"
> -	depends on MFD_CROS_EC && I2C
> +	depends on CROS_EC && I2C
>  
>  	help
>  	  If you say Y here, you get support for talking to the ChromeOS
> @@ -62,7 +75,7 @@ config CROS_EC_I2C
>  
>  config CROS_EC_RPMSG
>  	tristate "ChromeOS Embedded Controller (rpmsg)"
> -	depends on MFD_CROS_EC && RPMSG && OF
> +	depends on CROS_EC && RPMSG && OF
>  	help
>  	  If you say Y here, you get support for talking to the ChromeOS EC
>  	  through rpmsg. This uses a simple byte-level protocol with a
> @@ -87,7 +100,7 @@ config CROS_EC_ISHTP
>  
>  config CROS_EC_SPI
>  	tristate "ChromeOS Embedded Controller (SPI)"
> -	depends on MFD_CROS_EC && SPI
> +	depends on CROS_EC && SPI
>  
>  	---help---
>  	  If you say Y here, you get support for talking to the ChromeOS EC
> @@ -97,7 +110,7 @@ config CROS_EC_SPI
>  
>  config CROS_EC_LPC
>          tristate "ChromeOS Embedded Controller (LPC)"
> -        depends on MFD_CROS_EC && ACPI && (X86 || COMPILE_TEST)
> +        depends on CROS_EC && ACPI && (X86 || COMPILE_TEST)
>          help
>            If you say Y here, you get support for talking to the ChromeOS EC
>            over an LPC bus. This uses a simple byte-level protocol with a
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index c5583c48d1e5..ebb57e21923b 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -6,6 +6,7 @@ CFLAGS_cros_ec_trace.o:=		-I$(src)
>  obj-$(CONFIG_CHROMEOS_LAPTOP)		+= chromeos_laptop.o
>  obj-$(CONFIG_CHROMEOS_PSTORE)		+= chromeos_pstore.o
>  obj-$(CONFIG_CHROMEOS_TBMC)		+= chromeos_tbmc.o
> +obj-$(CONFIG_CROS_EC)			+= cros_ec.o
>  obj-$(CONFIG_CROS_EC_I2C)		+= cros_ec_i2c.o
>  obj-$(CONFIG_CROS_EC_ISHTP)		+= cros_ec_ishtp.o
>  obj-$(CONFIG_CROS_EC_RPMSG)		+= cros_ec_rpmsg.o
> diff --git a/drivers/mfd/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> similarity index 100%
> rename from drivers/mfd/cros_ec.c
> rename to drivers/platform/chrome/cros_ec.c
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index dd7da41f230c..e05140771845 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -656,7 +656,7 @@ config CHARGER_RT9455
>  
>  config CHARGER_CROS_USBPD
>  	tristate "ChromeOS EC based USBPD charger"
> -	depends on MFD_CROS_EC
> +	depends on CROS_EC
>  	default n
>  	help
>  	  Say Y here to enable ChromeOS EC based USBPD charger
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index dff5a93f7daa..99946e1bcc73 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -145,7 +145,7 @@ config PWM_CRC
>  
>  config PWM_CROS_EC
>  	tristate "ChromeOS EC PWM driver"
> -	depends on MFD_CROS_EC
> +	depends on CROS_EC
>  	help
>  	  PWM driver for exposing a PWM attached to the ChromeOS Embedded
>  	  Controller.
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 5c0790eed656..4eb311569fc4 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1265,7 +1265,7 @@ config RTC_DRV_ZYNQMP
>  
>  config RTC_DRV_CROS_EC
>  	tristate "Chrome OS EC RTC driver"
> -	depends on MFD_CROS_EC
> +	depends on CROS_EC
>  	help
>  	  If you say yes here you will get support for the
>  	  Chrome OS Embedded Controller's RTC.
> diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
> index 8e3e86619b35..60086858e920 100644
> --- a/sound/soc/qcom/Kconfig
> +++ b/sound/soc/qcom/Kconfig
> @@ -99,7 +99,7 @@ config SND_SOC_MSM8996
>  
>  config SND_SOC_SDM845
>  	tristate "SoC Machine driver for SDM845 boards"
> -	depends on QCOM_APR && MFD_CROS_EC && I2C
> +	depends on QCOM_APR && CROS_EC && I2C
>  	select SND_SOC_QDSP6
>  	select SND_SOC_QCOM_COMMON
>  	select SND_SOC_RT5663
> -- 
> 2.20.1
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: [PATCH 2/2] HID: wacom: Don't report anything prior to the tool entering range
From: Greg KH @ 2019-06-11 19:22 UTC (permalink / raw)
  To: Jason Gerecke
  Cc: # v3.17+, Sasha Levin, Linux Input, Ping Cheng,
	Aaron Armstrong Skomra, Jason Gerecke, Aaron Armstrong Skomra
In-Reply-To: <CANRwn3RBK41mRJKUPDDptoq_So6_7UxR0toaauZvjT5U=OaHWw@mail.gmail.com>

On Tue, Jun 11, 2019 at 12:02:47PM -0700, Jason Gerecke wrote:
> I haven't been keeping a close eye on this and just noticed that this
> patch set doesn't seem to have been merged into stable. There's also a
> second patch series (beginning with "[PATCH 1/3] HID: wacom: Send
> BTN_TOUCH in response to INTUOSP2_BT eraser contact") that hasn't seen
> any stable activity either.
> 
> Any idea what's up?

I don't see these in my queue at all.

What is the git commit id of these patches in Linus's tree?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 2/2] HID: wacom: Don't report anything prior to the tool entering range
From: Jason Gerecke @ 2019-06-11 19:02 UTC (permalink / raw)
  To: # v3.17+, Sasha Levin
  Cc: Linux Input, Ping Cheng, Aaron Armstrong Skomra, Jason Gerecke,
	Aaron Armstrong Skomra
In-Reply-To: <20190426163531.9782-1-jason.gerecke@wacom.com>

I haven't been keeping a close eye on this and just noticed that this
patch set doesn't seem to have been merged into stable. There's also a
second patch series (beginning with "[PATCH 1/3] HID: wacom: Send
BTN_TOUCH in response to INTUOSP2_BT eraser contact") that hasn't seen
any stable activity either.

Any idea what's up?

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....
On Fri, Apr 26, 2019 at 9:35 AM Gerecke, Jason <killertofu@gmail.com> wrote:
>
> From: Jason Gerecke <jason.gerecke@wacom.com>
>
> If the tool spends some time in prox before entering range, a series of
> events (e.g. ABS_DISTANCE, MSC_SERIAL) can be sent before we or userspace
> have any clue about the pen whose data is being reported. We need to hold
> off on reporting anything until the pen has entered range. Since we still
> want to report events that occur "in prox" after the pen has *left* range
> we use 'wacom-tool[0]' as the indicator that the pen did at one point
> enter range and provide us/userspace with tool type and serial number
> information.
>
> Fixes: a48324de6d ("HID: wacom: Bluetooth IRQ for Intuos Pro should handle prox/range")
> Cc: <stable@vger.kernel.org> # 4.11+
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> Reviewed-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
> ---
> Version of patch specifically targeted to stable v4.14.113
>
>  drivers/hid/wacom_wac.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 03b04bc742dd..e4aeffa56018 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1271,17 +1271,20 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
>                         input_report_abs(pen_input, ABS_Z, rotation);
>                         input_report_abs(pen_input, ABS_WHEEL, get_unaligned_le16(&frame[11]));
>                 }
> -               input_report_abs(pen_input, ABS_PRESSURE, get_unaligned_le16(&frame[5]));
> -               input_report_abs(pen_input, ABS_DISTANCE, range ? frame[13] : wacom->features.distance_max);
>
> -               input_report_key(pen_input, BTN_TOUCH, frame[0] & 0x01);
> -               input_report_key(pen_input, BTN_STYLUS, frame[0] & 0x02);
> -               input_report_key(pen_input, BTN_STYLUS2, frame[0] & 0x04);
> +               if (wacom->tool[0]) {
> +                       input_report_abs(pen_input, ABS_PRESSURE, get_unaligned_le16(&frame[5]));
> +                       input_report_abs(pen_input, ABS_DISTANCE, range ? frame[13] : wacom->features.distance_max);
>
> -               input_report_key(pen_input, wacom->tool[0], prox);
> -               input_event(pen_input, EV_MSC, MSC_SERIAL, wacom->serial[0]);
> -               input_report_abs(pen_input, ABS_MISC,
> -                                wacom_intuos_id_mangle(wacom->id[0])); /* report tool id */
> +                       input_report_key(pen_input, BTN_TOUCH, frame[0] & 0x01);
> +                       input_report_key(pen_input, BTN_STYLUS, frame[0] & 0x02);
> +                       input_report_key(pen_input, BTN_STYLUS2, frame[0] & 0x04);
> +
> +                       input_report_key(pen_input, wacom->tool[0], prox);
> +                       input_event(pen_input, EV_MSC, MSC_SERIAL, wacom->serial[0]);
> +                       input_report_abs(pen_input, ABS_MISC,
> +                                        wacom_intuos_id_mangle(wacom->id[0])); /* report tool id */
> +               }
>
>                 wacom->shared->stylus_in_proximity = prox;
>
> --
> 2.21.0
>

^ permalink raw reply

* Re: 答复: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.
From: dmitry.torokhov @ 2019-06-11 17:45 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Hui Wang, Xiaoxiao Liu, XiaoXiao Liu, peter.hutterer@who-t.net,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Xiaojian Cao, zhangfp1@lenovo.com, Naoki Saito, Hideo Kawase
In-Reply-To: <20190611173856.jjwoagud6doxvpy3@pali>

On Tue, Jun 11, 2019 at 07:38:56PM +0200, Pali Rohár wrote:
> On Tuesday 11 June 2019 10:32:28 dmitry.torokhov@gmail.com wrote:
> > On Tue, Jun 11, 2019 at 07:17:07PM +0200, Pali Rohár wrote:
> > > On Tuesday 11 June 2019 10:07:07 dmitry.torokhov@gmail.com wrote:
> > > > On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> > > > > On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > > > > > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > > > > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > > > > > Hi Pali,
> > > > > > > > 
> > > > > > > > I discussed with our FW team about this problem.
> > > > > > > > We think the V8 method means a touchpad feature  and does not fit
> > > > > > > > the CS19 trackpoint device.
> > > > > > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually
> > > > > > > > use standard mouse data.
> > > > > > > > CS19 TrackPoint device is a completely different device with
> > > > > > > > DualPoint device of Dell/HP.
> > > > > > > > CS19 TrackPoint device is independent  of Touchpad. (Touchpad is
> > > > > > > > connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> > > > > > > > And it has completely another FW.
> > > > > > > > 
> > > > > > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > > > > > 
> > > > > > > Maybe here is some mis-understanding,  the mouse mode here doesn't mean
> > > > > > > we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use
> > > > > > > trackpoint.c to drive this HW, so this trackpoint has all features a
> > > > > > > trackpoint should have.
> > > > > > > 
> > > > > > And I sent a patch one month ago to let the the trackpoint.c to drive this
> > > > > > HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe that
> > > > > > patch is reference.
> > > > > 
> > > > > So instead of creating blacklist, you should check for TP_VARIANT_ALPS
> > > > > in alps.c and disallow its usage.
> > > > > 
> > > > > Or maybe better, move trackpoint.c detect code before alsp.c detect code
> > > > > in psmouse-base. And no changes in alps.c are needed.
> > > > 
> > > > I'd be very cautions of moving around the protocol detection. It is very
> > > > fragile, so if we can detect trackpoint-only case in alps.c and skip on
> > > > to trackpoint I would prefer it.
> > > 
> > > The main problem is that proposed trackpoint-only check in alps.c is
> > > basically what trackpoint.c is doing for checking if device is
> > > trackpoint (via function trackpoint_start_protocol()).
> > > 
> > > So I'm not sure now what is the best solution...
> > 
> > Unfortunately currently trackpoint is being probed only after we tried
> > Elan, Genius, and Logitech PS2++ protocols, and I am not sure if moving
> > trackpoint around will disturb them or not.
> > 
> > I do not think there is much code duplication by pulling limited version
> > of trackpoint detection code into alps.c and then have it bail out when
> > it sees trackpoint-only device so trackpoint.c can handle it properly.
> 
> Ok. Seems that it is the best solution.
> 
> The last question is, should be use ALPS or Trackpoint protocol? Because
> it looks like that device can be configured to one or other.
> 
> What are pros and cons of these two protocols?

As far as I know the device implements trackpoint protocol, although not
complete version. Several manufacturers started making trackponts once
IBM/Lenovo patents on the original one expired (I think).

The data stream is regular relative PS/2, bit it allows controlling
device behavior a bit, such as press-to-select option and device
sensitivity. IBM/Lenovo version has many more parameters.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: 答复: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.
From: Pali Rohár @ 2019-06-11 17:38 UTC (permalink / raw)
  To: dmitry.torokhov@gmail.com
  Cc: Hui Wang, Xiaoxiao Liu, XiaoXiao Liu, peter.hutterer@who-t.net,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Xiaojian Cao, zhangfp1@lenovo.com, Naoki Saito, Hideo Kawase
In-Reply-To: <20190611173228.GD143729@dtor-ws>

[-- Attachment #1: Type: text/plain, Size: 3202 bytes --]

On Tuesday 11 June 2019 10:32:28 dmitry.torokhov@gmail.com wrote:
> On Tue, Jun 11, 2019 at 07:17:07PM +0200, Pali Rohár wrote:
> > On Tuesday 11 June 2019 10:07:07 dmitry.torokhov@gmail.com wrote:
> > > On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> > > > On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > > > > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > > > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > > > > Hi Pali,
> > > > > > > 
> > > > > > > I discussed with our FW team about this problem.
> > > > > > > We think the V8 method means a touchpad feature  and does not fit
> > > > > > > the CS19 trackpoint device.
> > > > > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually
> > > > > > > use standard mouse data.
> > > > > > > CS19 TrackPoint device is a completely different device with
> > > > > > > DualPoint device of Dell/HP.
> > > > > > > CS19 TrackPoint device is independent  of Touchpad. (Touchpad is
> > > > > > > connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> > > > > > > And it has completely another FW.
> > > > > > > 
> > > > > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > > > > 
> > > > > > Maybe here is some mis-understanding,  the mouse mode here doesn't mean
> > > > > > we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use
> > > > > > trackpoint.c to drive this HW, so this trackpoint has all features a
> > > > > > trackpoint should have.
> > > > > > 
> > > > > And I sent a patch one month ago to let the the trackpoint.c to drive this
> > > > > HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe that
> > > > > patch is reference.
> > > > 
> > > > So instead of creating blacklist, you should check for TP_VARIANT_ALPS
> > > > in alps.c and disallow its usage.
> > > > 
> > > > Or maybe better, move trackpoint.c detect code before alsp.c detect code
> > > > in psmouse-base. And no changes in alps.c are needed.
> > > 
> > > I'd be very cautions of moving around the protocol detection. It is very
> > > fragile, so if we can detect trackpoint-only case in alps.c and skip on
> > > to trackpoint I would prefer it.
> > 
> > The main problem is that proposed trackpoint-only check in alps.c is
> > basically what trackpoint.c is doing for checking if device is
> > trackpoint (via function trackpoint_start_protocol()).
> > 
> > So I'm not sure now what is the best solution...
> 
> Unfortunately currently trackpoint is being probed only after we tried
> Elan, Genius, and Logitech PS2++ protocols, and I am not sure if moving
> trackpoint around will disturb them or not.
> 
> I do not think there is much code duplication by pulling limited version
> of trackpoint detection code into alps.c and then have it bail out when
> it sees trackpoint-only device so trackpoint.c can handle it properly.

Ok. Seems that it is the best solution.

The last question is, should be use ALPS or Trackpoint protocol? Because
it looks like that device can be configured to one or other.

What are pros and cons of these two protocols?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs
From: Dmitry Torokhov @ 2019-06-11 17:35 UTC (permalink / raw)
  To: Aaron Ma; +Cc: linux-input, linux-kernel, Cheiny, aduggan, benjamin.tissoires
In-Reply-To: <7da443d0-f433-c5a5-5194-707362eb2ee5@canonical.com>

On Tue, Jun 11, 2019 at 12:55:58AM +0800, Aaron Ma wrote:
> 
> On 6/10/19 12:55 AM, Dmitry Torokhov wrote:
> > Hi Aaron,
> > 
> > On Wed, Feb 20, 2019 at 05:41:59PM +0100, Aaron Ma wrote:
> >> rmi4 got spam data after S3 resume on some ThinkPads.
> >> Then TrackPoint lost when be detected by psmouse.
> >> Clear irqs status before set irqs will make TrackPoint back.
> > Could you please give me an idea as to what this spam data is?
> > 
> 
> It should be some data 0 during suspend/resume.
> Actually I don't know how these data 0 is produced.
> Not all synaptics touchpads have this issue.
> 
> > In F03 probe we clear all pending data before enabling the function,
> 
> Yes we did, but not after resume.

Yes, I understand that. The question I was asking: if we add code
consuming all pending data to f03->suspend(), similarly to what we are
doing at probe time, will it fix the issue with trackstick losing
synchronization and attempting disconnect?

> 
> > maybe the same needs to be done on resume, instead of changing the way
> > we handle IRQ bits?
> 
> This patch is supposed to clear irq status like it in fn probe. Not
> changing IRQ bits.

What I meant is changing how we enable IRQ bits. I would really prefer
we did not lose IRQ state for other functions when we enable interrupts
for given function.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: 答复: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.
From: dmitry.torokhov @ 2019-06-11 17:32 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Hui Wang, Xiaoxiao Liu, XiaoXiao Liu, peter.hutterer@who-t.net,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Xiaojian Cao, zhangfp1@lenovo.com, Naoki Saito, Hideo Kawase
In-Reply-To: <20190611171707.tydk7rsmtzmjohky@pali>

On Tue, Jun 11, 2019 at 07:17:07PM +0200, Pali Rohár wrote:
> On Tuesday 11 June 2019 10:07:07 dmitry.torokhov@gmail.com wrote:
> > On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> > > On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > > > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > > > Hi Pali,
> > > > > > 
> > > > > > I discussed with our FW team about this problem.
> > > > > > We think the V8 method means a touchpad feature  and does not fit
> > > > > > the CS19 trackpoint device.
> > > > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually
> > > > > > use standard mouse data.
> > > > > > CS19 TrackPoint device is a completely different device with
> > > > > > DualPoint device of Dell/HP.
> > > > > > CS19 TrackPoint device is independent  of Touchpad. (Touchpad is
> > > > > > connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> > > > > > And it has completely another FW.
> > > > > > 
> > > > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > > > 
> > > > > Maybe here is some mis-understanding,  the mouse mode here doesn't mean
> > > > > we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use
> > > > > trackpoint.c to drive this HW, so this trackpoint has all features a
> > > > > trackpoint should have.
> > > > > 
> > > > And I sent a patch one month ago to let the the trackpoint.c to drive this
> > > > HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe that
> > > > patch is reference.
> > > 
> > > So instead of creating blacklist, you should check for TP_VARIANT_ALPS
> > > in alps.c and disallow its usage.
> > > 
> > > Or maybe better, move trackpoint.c detect code before alsp.c detect code
> > > in psmouse-base. And no changes in alps.c are needed.
> > 
> > I'd be very cautions of moving around the protocol detection. It is very
> > fragile, so if we can detect trackpoint-only case in alps.c and skip on
> > to trackpoint I would prefer it.
> 
> The main problem is that proposed trackpoint-only check in alps.c is
> basically what trackpoint.c is doing for checking if device is
> trackpoint (via function trackpoint_start_protocol()).
> 
> So I'm not sure now what is the best solution...

Unfortunately currently trackpoint is being probed only after we tried
Elan, Genius, and Logitech PS2++ protocols, and I am not sure if moving
trackpoint around will disturb them or not.

I do not think there is much code duplication by pulling limited version
of trackpoint detection code into alps.c and then have it bail out when
it sees trackpoint-only device so trackpoint.c can handle it properly.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: 答复: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.
From: Pali Rohár @ 2019-06-11 17:17 UTC (permalink / raw)
  To: dmitry.torokhov@gmail.com
  Cc: Hui Wang, Xiaoxiao Liu, XiaoXiao Liu, peter.hutterer@who-t.net,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Xiaojian Cao, zhangfp1@lenovo.com, Naoki Saito, Hideo Kawase
In-Reply-To: <20190611170707.GA143729@dtor-ws>

[-- Attachment #1: Type: text/plain, Size: 2235 bytes --]

On Tuesday 11 June 2019 10:07:07 dmitry.torokhov@gmail.com wrote:
> On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> > On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > > Hi Pali,
> > > > > 
> > > > > I discussed with our FW team about this problem.
> > > > > We think the V8 method means a touchpad feature  and does not fit
> > > > > the CS19 trackpoint device.
> > > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually
> > > > > use standard mouse data.
> > > > > CS19 TrackPoint device is a completely different device with
> > > > > DualPoint device of Dell/HP.
> > > > > CS19 TrackPoint device is independent  of Touchpad. (Touchpad is
> > > > > connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> > > > > And it has completely another FW.
> > > > > 
> > > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > > 
> > > > Maybe here is some mis-understanding,  the mouse mode here doesn't mean
> > > > we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use
> > > > trackpoint.c to drive this HW, so this trackpoint has all features a
> > > > trackpoint should have.
> > > > 
> > > And I sent a patch one month ago to let the the trackpoint.c to drive this
> > > HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe that
> > > patch is reference.
> > 
> > So instead of creating blacklist, you should check for TP_VARIANT_ALPS
> > in alps.c and disallow its usage.
> > 
> > Or maybe better, move trackpoint.c detect code before alsp.c detect code
> > in psmouse-base. And no changes in alps.c are needed.
> 
> I'd be very cautions of moving around the protocol detection. It is very
> fragile, so if we can detect trackpoint-only case in alps.c and skip on
> to trackpoint I would prefer it.

The main problem is that proposed trackpoint-only check in alps.c is
basically what trackpoint.c is doing for checking if device is
trackpoint (via function trackpoint_start_protocol()).

So I'm not sure now what is the best solution...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox