[PATCH 0/4] Input: elan - add support for SMBus Host Notify and trackstick

[PATCH 0/4] Input: elan - add support for SMBus Host Notify and trackstick

[Benjamin Tissoires]

Hi,

So it looks like Elantech devices also suffer from issues when used over PS/2.
They also appear to behave properly when used over SMBus. On many systems, these
touchpads are not enumerated by ACPI, but they are actually correctly working
(after a few fixes from this series).

I have been working with an owner to automatically bind the touchpad from PS/2,
but mentoring a new comer takes time. Anyway, right now, it looks like adding
"echo elan_i2c 0x15 > /sys/bus/i2c/devices/i2c-N/new_device" (N being the SMBus
adapter number) at boot works well enough to have the touchpad bound.

Currently the trackstick needs to be manually added by an extra patch but the
information should be provided through PS/2 when the rest of the series comes.

Link: https://bugzilla.redhat.com/show_bug.cgi?id=1326577
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1313939

Cheers,
Benjamin

Benjamin Tissoires (4):
  Input: elan_i2c - fix return tests of i2c_smbus_read_block_data()
  Input: elan_i2c - always output the device information
  Input: elan_i2c - add Host Notify support
  Input: elan_i2c - add trackstick report

 drivers/input/mouse/elan_i2c.h       |  10 ++
 drivers/input/mouse/elan_i2c_core.c  | 193 ++++++++++++++++++++++++++++++-----
 drivers/input/mouse/elan_i2c_smbus.c |   6 +-
 3 files changed, 180 insertions(+), 29 deletions(-)

-- 
2.7.4

[PATCH 1/4] Input: elan_i2c - fix return tests of i2c_smbus_read_block_data()

[Benjamin Tissoires]

i2c_smbus_read_block_data() returns negative errno else the number of
data bytes in the slave's response.

Checking for error not null means the function always fails if the device
answers properly.

So given that we read 3 bytes and access those, better check that we
actually read those 3 bytes.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/mouse/elan_i2c_smbus.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/elan_i2c_smbus.c b/drivers/input/mouse/elan_i2c_smbus.c
index cb6aecb..9b43b55 100644
--- a/drivers/input/mouse/elan_i2c_smbus.c
+++ b/drivers/input/mouse/elan_i2c_smbus.c
@@ -226,7 +226,7 @@ static int elan_smbus_get_max(struct i2c_client *client,
 	u8 val[3];
 
 	error = i2c_smbus_read_block_data(client, ETP_SMBUS_RANGE_CMD, val);
-	if (error) {
+	if (error != 3) {
 		dev_err(&client->dev, "failed to get dimensions: %d\n", error);
 		return error;
 	}
@@ -245,7 +245,7 @@ static int elan_smbus_get_resolution(struct i2c_client *client,
 
 	error = i2c_smbus_read_block_data(client,
 					  ETP_SMBUS_RESOLUTION_CMD, val);
-	if (error) {
+	if (error != 3) {
 		dev_err(&client->dev, "failed to get resolution: %d\n", error);
 		return error;
 	}
@@ -265,7 +265,7 @@ static int elan_smbus_get_num_traces(struct i2c_client *client,
 
 	error = i2c_smbus_read_block_data(client,
 					  ETP_SMBUS_XY_TRACENUM_CMD, val);
-	if (error) {
+	if (error != 3) {
 		dev_err(&client->dev, "failed to get trace info: %d\n", error);
 		return error;
 	}
-- 
2.7.4

[PATCH 2/4] Input: elan_i2c - always output the device information

[Benjamin Tissoires]

it's always easier to retrieve these information in bug reports when
it is always printed in the dmesg.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/mouse/elan_i2c_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index d15b338..2de1f75 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -1093,7 +1093,7 @@ static int elan_probe(struct i2c_client *client,
 	if (error)
 		return error;
 
-	dev_dbg(&client->dev,
+	dev_info(&client->dev,
 		"Elan Touchpad Information:\n"
 		"    Module product ID:  0x%04x\n"
 		"    Firmware Version:  0x%04x\n"
-- 
2.7.4

[PATCH 3/4] Input: elan_i2c - add Host Notify support

[Benjamin Tissoires]

The Thinkpad series 13 uses Host Notify to report the interrupt.
Add elan_smb_alert() to handle those interrupts and disable the irq
handling on this case.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/mouse/elan_i2c_core.c | 100 ++++++++++++++++++++++++++++--------
 1 file changed, 78 insertions(+), 22 deletions(-)

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 2de1f75..11671c7 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -96,6 +96,34 @@ struct elan_tp_data {
 	bool			baseline_ready;
 };
 
+static inline void elan_enable_irq(struct elan_tp_data *tp)
+{
+	if (tp->client->irq)
+		enable_irq(tp->client->irq);
+}
+
+static inline void elan_disable_irq(struct elan_tp_data *tp)
+{
+	if (tp->client->irq)
+		disable_irq(tp->client->irq);
+}
+
+static inline int elan_enable_irq_wake(struct elan_tp_data *tp)
+{
+	if (tp->client->irq)
+		return enable_irq_wake(tp->client->irq);
+
+	return 0;
+}
+
+static inline int elan_disable_irq_wake(struct elan_tp_data *tp)
+{
+	if (tp->client->irq)
+		return disable_irq_wake(tp->client->irq);
+
+	return 0;
+}
+
 static int elan_get_fwinfo(u8 iap_version, u16 *validpage_count,
 			   u16 *signature_address)
 {
@@ -457,7 +485,7 @@ static int elan_update_firmware(struct elan_tp_data *data,
 
 	dev_dbg(&client->dev, "Starting firmware update....\n");
 
-	disable_irq(client->irq);
+	elan_disable_irq(data);
 	data->in_fw_update = true;
 
 	retval = __elan_update_firmware(data, fw);
@@ -471,7 +499,7 @@ static int elan_update_firmware(struct elan_tp_data *data,
 	}
 
 	data->in_fw_update = false;
-	enable_irq(client->irq);
+	elan_enable_irq(data);
 
 	return retval;
 }
@@ -599,7 +627,7 @@ static ssize_t calibrate_store(struct device *dev,
 	if (retval)
 		return retval;
 
-	disable_irq(client->irq);
+	elan_disable_irq(data);
 
 	data->mode |= ETP_ENABLE_CALIBRATE;
 	retval = data->ops->set_mode(client, data->mode);
@@ -645,7 +673,7 @@ out_disable_calibrate:
 			retval = error;
 	}
 out:
-	enable_irq(client->irq);
+	elan_enable_irq(data);
 	mutex_unlock(&data->sysfs_mutex);
 	return retval ?: count;
 }
@@ -711,7 +739,7 @@ static ssize_t acquire_store(struct device *dev, struct device_attribute *attr,
 	if (retval)
 		return retval;
 
-	disable_irq(client->irq);
+	elan_disable_irq(data);
 
 	data->baseline_ready = false;
 
@@ -753,7 +781,7 @@ out_disable_calibrate:
 			retval = error;
 	}
 out:
-	enable_irq(client->irq);
+	elan_enable_irq(data);
 	mutex_unlock(&data->sysfs_mutex);
 	return retval ?: count;
 }
@@ -943,6 +971,23 @@ out:
 	return IRQ_HANDLED;
 }
 
+static void elan_smb_alert(struct i2c_client *client,
+			   enum i2c_alert_protocol type, unsigned int data)
+{
+	struct elan_tp_data *tp_data = i2c_get_clientdata(client);
+
+	if (type != I2C_PROTOCOL_SMBUS_HOST_NOTIFY)
+		return;
+
+	if (!tp_data) {
+		dev_err(&client->dev,
+			"Something went wrong, driver data is NULL.\n");
+		return;
+	}
+
+	elan_isr(0, tp_data);
+}
+
 /*
  ******************************************************************
  * Elan initialization functions
@@ -1036,6 +1081,13 @@ static int elan_probe(struct i2c_client *client,
 						I2C_FUNC_SMBUS_BLOCK_DATA |
 						I2C_FUNC_SMBUS_I2C_BLOCK)) {
 		transport_ops = &elan_smbus_ops;
+
+		if (!client->irq &&
+		    !i2c_check_functionality(client->adapter,
+					I2C_FUNC_SMBUS_HOST_NOTIFY)) {
+			dev_err(dev, "no Host Notify support\n");
+			return -ENODEV;
+		}
 	} else {
 		dev_err(dev, "not a supported I2C/SMBus adapter\n");
 		return -EIO;
@@ -1115,19 +1167,22 @@ static int elan_probe(struct i2c_client *client,
 	if (error)
 		return error;
 
-	/*
-	 * Systems using device tree should set up interrupt via DTS,
-	 * the rest will use the default falling edge interrupts.
-	 */
-	irqflags = client->dev.of_node ? 0 : IRQF_TRIGGER_FALLING;
+	if (client->irq) {
+		/*
+		 * Systems using device tree should set up interrupt via DTS,
+		 * the rest will use the default falling edge interrupts.
+		 */
+		irqflags = client->dev.of_node ? 0 : IRQF_TRIGGER_FALLING;
 
-	error = devm_request_threaded_irq(&client->dev, client->irq,
-					  NULL, elan_isr,
-					  irqflags | IRQF_ONESHOT,
-					  client->name, data);
-	if (error) {
-		dev_err(&client->dev, "cannot register irq=%d\n", client->irq);
-		return error;
+		error = devm_request_threaded_irq(&client->dev, client->irq,
+						  NULL, elan_isr,
+						  irqflags | IRQF_ONESHOT,
+						  client->name, data);
+		if (error) {
+			dev_err(&client->dev, "cannot register irq=%d\n",
+				client->irq);
+			return error;
+		}
 	}
 
 	error = sysfs_create_groups(&client->dev.kobj, elan_sysfs_groups);
@@ -1179,12 +1234,12 @@ static int __maybe_unused elan_suspend(struct device *dev)
 	if (ret)
 		return ret;
 
-	disable_irq(client->irq);
+	elan_disable_irq(data);
 
 	if (device_may_wakeup(dev)) {
 		ret = elan_sleep(data);
 		/* Enable wake from IRQ */
-		data->irq_wake = (enable_irq_wake(client->irq) == 0);
+		data->irq_wake = (elan_enable_irq_wake(data) == 0);
 	} else {
 		ret = elan_disable_power(data);
 	}
@@ -1200,7 +1255,7 @@ static int __maybe_unused elan_resume(struct device *dev)
 	int error;
 
 	if (device_may_wakeup(dev) && data->irq_wake) {
-		disable_irq_wake(client->irq);
+		elan_disable_irq_wake(data);
 		data->irq_wake = false;
 	}
 
@@ -1215,7 +1270,7 @@ static int __maybe_unused elan_resume(struct device *dev)
 		dev_err(dev, "initialize when resuming failed: %d\n", error);
 
 err:
-	enable_irq(data->client->irq);
+	elan_enable_irq(data);
 	return error;
 }
 
@@ -1256,6 +1311,7 @@ static struct i2c_driver elan_driver = {
 	},
 	.probe		= elan_probe,
 	.id_table	= elan_id,
+	.alert		= elan_smb_alert,
 };
 
 module_i2c_driver(elan_driver);
-- 
2.7.4

[PATCH 4/4] Input: elan_i2c - add trackstick report

[Benjamin Tissoires]

https://bugzilla.redhat.com/show_bug.cgi?id=1313939

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/mouse/elan_i2c.h      | 10 ++++
 drivers/input/mouse/elan_i2c_core.c | 91 +++++++++++++++++++++++++++++++++++--
 2 files changed, 98 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/elan_i2c.h b/drivers/input/mouse/elan_i2c.h
index c0ec261..468763a 100644
--- a/drivers/input/mouse/elan_i2c.h
+++ b/drivers/input/mouse/elan_i2c.h
@@ -86,4 +86,14 @@ struct elan_transport_ops {
 
 extern const struct elan_transport_ops elan_smbus_ops, elan_i2c_ops;
 
+/**
+ * struct elan_platform_data - system specific configuration info.
+ *
+ * @trackpoint - specify whether or not you want the trackstick input node to
+ * be created and handled by the driver.
+ */
+struct elan_platform_data {
+	bool trackpoint;
+};
+
 #endif /* _ELAN_I2C_H */
diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 11671c7..7aeb648 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -51,6 +51,7 @@
 #define ETP_MAX_FINGERS		5
 #define ETP_FINGER_DATA_LEN	5
 #define ETP_REPORT_ID		0x5D
+#define ETP_PT_REPORT_ID	0x5E
 #define ETP_REPORT_ID_OFFSET	2
 #define ETP_TOUCH_INFO_OFFSET	3
 #define ETP_FINGER_DATA_OFFSET	4
@@ -61,6 +62,7 @@
 struct elan_tp_data {
 	struct i2c_client	*client;
 	struct input_dev	*input;
+	struct input_dev	*tp_input; /* trackpoint input node */
 	struct regulator	*vcc;
 
 	const struct elan_transport_ops *ops;
@@ -940,6 +942,34 @@ static void elan_report_absolute(struct elan_tp_data *data, u8 *packet)
 	input_sync(input);
 }
 
+static void elan_report_trackpoint(struct elan_tp_data *data, u8 *report)
+{
+	struct input_dev *input = data->tp_input;
+	u8 *packet = &report[ETP_REPORT_ID_OFFSET + 1];
+	int x, y;
+
+	if (!data->tp_input) {
+		dev_warn_once(&data->client->dev,
+			      "received a trackpoint report while no trackpoint device has been created.\n"
+			      "Please report upstream.\n");
+		return;
+	}
+
+	input_report_key(input, BTN_LEFT, packet[0] & 0x01);
+	input_report_key(input, BTN_RIGHT, packet[0] & 0x02);
+	input_report_key(input, BTN_MIDDLE, packet[0] & 0x04);
+
+	if ((packet[3] & 0x0F) == 0x06) {
+		x = packet[4] - (int)((packet[1]^0x80) << 1);
+		y = (int)((packet[2]^0x80) << 1) - packet[5];
+
+		input_report_rel(input, REL_X, x);
+		input_report_rel(input, REL_Y, y);
+	}
+
+	input_sync(input);
+}
+
 static irqreturn_t elan_isr(int irq, void *dev_id)
 {
 	struct elan_tp_data *data = dev_id;
@@ -961,11 +991,17 @@ static irqreturn_t elan_isr(int irq, void *dev_id)
 	if (error)
 		goto out;
 
-	if (report[ETP_REPORT_ID_OFFSET] != ETP_REPORT_ID)
+	switch (report[ETP_REPORT_ID_OFFSET]) {
+	case ETP_REPORT_ID:
+		elan_report_absolute(data, report);
+		break;
+	case ETP_PT_REPORT_ID:
+		elan_report_trackpoint(data, report);
+		break;
+	default:
 		dev_err(dev, "invalid report id data (%x)\n",
 			report[ETP_REPORT_ID_OFFSET]);
-	else
-		elan_report_absolute(data, report);
+	}
 
 out:
 	return IRQ_HANDLED;
@@ -993,6 +1029,37 @@ static void elan_smb_alert(struct i2c_client *client,
  * Elan initialization functions
  ******************************************************************
  */
+
+static int elan_setup_trackpoint_input_device(struct elan_tp_data *data)
+{
+	struct device *dev = &data->client->dev;
+	struct input_dev *input;
+
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return -ENOMEM;
+
+	input->name = "Elan TrackPoint";
+	input->id.bustype = BUS_I2C;
+	input->id.vendor = ELAN_VENDOR_ID;
+	input->id.product = data->product_id;
+	input_set_drvdata(input, data);
+
+	input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
+	input->relbit[BIT_WORD(REL_X)] =
+		BIT_MASK(REL_X) | BIT_MASK(REL_Y);
+	input->keybit[BIT_WORD(BTN_LEFT)] =
+		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) |
+		BIT_MASK(BTN_RIGHT);
+
+	__set_bit(INPUT_PROP_POINTER, input->propbit);
+	__set_bit(INPUT_PROP_POINTING_STICK, input->propbit);
+
+	data->tp_input = input;
+
+	return 0;
+}
+
 static int elan_setup_input_device(struct elan_tp_data *data)
 {
 	struct device *dev = &data->client->dev;
@@ -1066,10 +1133,12 @@ static void elan_remove_sysfs_groups(void *_data)
 static int elan_probe(struct i2c_client *client,
 		      const struct i2c_device_id *dev_id)
 {
+	struct elan_platform_data *pdata = dev_get_platdata(&client->dev);
 	const struct elan_transport_ops *transport_ops;
 	struct device *dev = &client->dev;
 	struct elan_tp_data *data;
 	unsigned long irqflags;
+	bool has_trackpoint = pdata && pdata->trackpoint;
 	int error;
 
 	if (IS_ENABLED(CONFIG_MOUSE_ELAN_I2C_I2C) &&
@@ -1167,6 +1236,12 @@ static int elan_probe(struct i2c_client *client,
 	if (error)
 		return error;
 
+	if (has_trackpoint) {
+		error = elan_setup_trackpoint_input_device(data);
+		if (error)
+			return error;
+	}
+
 	if (client->irq) {
 		/*
 		 * Systems using device tree should set up interrupt via DTS,
@@ -1209,6 +1284,16 @@ static int elan_probe(struct i2c_client *client,
 		return error;
 	}
 
+	if (data->tp_input) {
+		error = input_register_device(data->tp_input);
+		if (error) {
+			dev_err(&client->dev,
+				"failed to register TrackPoint input device: %d\n",
+				error);
+			return error;
+		}
+	}
+
 	/*
 	 * Systems using device tree should set up wakeup via DTS,
 	 * the rest will configure device as wakeup source by default.
-- 
2.7.4

Re: [PATCH 2/4] Input: elan_i2c - always output the device information

[Dmitry Torokhov]

On Wed, Sep 28, 2016 at 04:34:02PM +0200, Benjamin Tissoires wrote:
> it's always easier to retrieve these information in bug reports when
> it is always printed in the dmesg.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>  drivers/input/mouse/elan_i2c_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index d15b338..2de1f75 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -1093,7 +1093,7 @@ static int elan_probe(struct i2c_client *client,
>  	if (error)
>  		return error;
>  
> -	dev_dbg(&client->dev,
> +	dev_info(&client->dev,
>  		"Elan Touchpad Information:\n"
>  		"    Module product ID:  0x%04x\n"
>  		"    Firmware Version:  0x%04x\n"

The format is acceptable for dev_dbg, but this multi-line output is not
too nice for normal logging, especially as we probe the device
asynchronously so there can easily be more output from other kernel
threads.

Thanks.

-- 
Dmitry

Re: [PATCH 3/4] Input: elan_i2c - add Host Notify support

[Dmitry Torokhov]

On Wed, Sep 28, 2016 at 04:34:03PM +0200, Benjamin Tissoires wrote:
> The Thinkpad series 13 uses Host Notify to report the interrupt.
> Add elan_smb_alert() to handle those interrupts and disable the irq
> handling on this case.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>  drivers/input/mouse/elan_i2c_core.c | 100 ++++++++++++++++++++++++++++--------
>  1 file changed, 78 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index 2de1f75..11671c7 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -96,6 +96,34 @@ struct elan_tp_data {
>  	bool			baseline_ready;
>  };
>  
> +static inline void elan_enable_irq(struct elan_tp_data *tp)
> +{
> +	if (tp->client->irq)
> +		enable_irq(tp->client->irq);

Hmm, so I wonder, why alert is not implemented as irqchip? Then clients
would not need to be bothered with these details, they'd simply requster
and manipulate irqs.

Thanks.

-- 
Dmitry

Re: [PATCH 1/4] Input: elan_i2c - fix return tests of i2c_smbus_read_block_data()

[Dmitry Torokhov]

On Wed, Sep 28, 2016 at 04:34:01PM +0200, Benjamin Tissoires wrote:
> i2c_smbus_read_block_data() returns negative errno else the number of
> data bytes in the slave's response.
> 
> Checking for error not null means the function always fails if the device
> answers properly.
> 
> So given that we read 3 bytes and access those, better check that we
> actually read those 3 bytes.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>  drivers/input/mouse/elan_i2c_smbus.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_smbus.c b/drivers/input/mouse/elan_i2c_smbus.c
> index cb6aecb..9b43b55 100644
> --- a/drivers/input/mouse/elan_i2c_smbus.c
> +++ b/drivers/input/mouse/elan_i2c_smbus.c
> @@ -226,7 +226,7 @@ static int elan_smbus_get_max(struct i2c_client *client,
>  	u8 val[3];
>  
>  	error = i2c_smbus_read_block_data(client, ETP_SMBUS_RANGE_CMD, val);
> -	if (error) {
> +	if (error != 3) {
>  		dev_err(&client->dev, "failed to get dimensions: %d\n", error);
>  		return error;

Unfortunately that means you'll report success if you get 0 from
i2c_smbus_read_block_data() or confuse upper layers with positive
errors. I'll change this to:

	ret = i2c_smbus_read_block_data();
	if (ret != 3) {
		error = ret < 0 ? ret : -EIO;
		...
	}

Thanks.

>  	}
> @@ -245,7 +245,7 @@ static int elan_smbus_get_resolution(struct i2c_client *client,
>  
>  	error = i2c_smbus_read_block_data(client,
>  					  ETP_SMBUS_RESOLUTION_CMD, val);
> -	if (error) {
> +	if (error != 3) {
>  		dev_err(&client->dev, "failed to get resolution: %d\n", error);
>  		return error;
>  	}
> @@ -265,7 +265,7 @@ static int elan_smbus_get_num_traces(struct i2c_client *client,
>  
>  	error = i2c_smbus_read_block_data(client,
>  					  ETP_SMBUS_XY_TRACENUM_CMD, val);
> -	if (error) {
> +	if (error != 3) {
>  		dev_err(&client->dev, "failed to get trace info: %d\n", error);
>  		return error;
>  	}
> -- 
> 2.7.4
> 

-- 
Dmitry

Re: [PATCH 3/4] Input: elan_i2c - add Host Notify support

[Benjamin Tissoires]

[Adding the I2C folks]

On Sep 30 2016 or thereabouts, Dmitry Torokhov wrote:
> On Wed, Sep 28, 2016 at 04:34:03PM +0200, Benjamin Tissoires wrote:
> > The Thinkpad series 13 uses Host Notify to report the interrupt.
> > Add elan_smb_alert() to handle those interrupts and disable the irq
> > handling on this case.
> > 
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > ---
> >  drivers/input/mouse/elan_i2c_core.c | 100 ++++++++++++++++++++++++++++--------
> >  1 file changed, 78 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> > index 2de1f75..11671c7 100644
> > --- a/drivers/input/mouse/elan_i2c_core.c
> > +++ b/drivers/input/mouse/elan_i2c_core.c
> > @@ -96,6 +96,34 @@ struct elan_tp_data {
> >  	bool			baseline_ready;
> >  };
> >  
> > +static inline void elan_enable_irq(struct elan_tp_data *tp)
> > +{
> > +	if (tp->client->irq)
> > +		enable_irq(tp->client->irq);
> 
> Hmm, so I wonder, why alert is not implemented as irqchip? Then clients
> would not need to be bothered with these details, they'd simply requster
> and manipulate irqs.
> 

Sounds like a good idea. There are few things to be aware:
- I don't think it will be OK to blindly add a Host Notify irq when none
  is provided by ACPI, platform or device tree. I think we would need to
  add an API (i2c_host_notify_to_irq()) that will be called by the driver
- On both systems I have seen using Host Notify (Synaptics and Elan),
  none is using the payload available in Host Notify. That means
  that we can use in those case an irq but it'll add more complexity
  when we actually need this payload to be retrieved
- Host Notify uses the same .alert mechanism than SMBus Alert. I checked
  the users of this mechanism (lm90 and ipmi_ssif), and none uses the
  payload provided by SMbus Alert
- lm90 can use a provided irq in addition to SMBus Alert, which is my
  main concern if we override the client->irq in i2c-core.c

The more I think of it, the more I think this is a good idea given that
the mechanism provided by .alert are similar to irq (without the payload
option), and would allow to reduce the code in i2c-smbus.

I'd be fine to implement an irqchip for Host Notify, but do we want to:
- remove the current (yet unused) .alert Host Notify support?
- keep .alert and have an irqchip available depending on how the user
  wants to address these notifications?
- also switch SMBus Alert into an irq, which would mean we will lose the
  payload availability (or we will need some API to retrieve it)?

Any thoughts?

Cheers,
Benjamin