linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core
@ 2016-11-29 10:08 Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 01/13] Input: synaptics-rmi4 - fix documentation of rmi_2d_sensor_platform_data Benjamin Tissoires
                   ` (12 more replies)
  0 siblings, 13 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

Hi guys,

This series applies on top of Dmitry's synaptics-rmi4 branch.

Patches 1-6 are cleanups which should be applied on this branch before it
gets merged with linux-next IMO. Nick, I updated the patch since our last
tests from last week. I'd be glad if you could give an other spin at testing
(patch 3 is a brancd new one which simplifies the rest of the series).

Patches 7-10 are requirements for hid-rmi to switch to rmi4_core.
It adds the PS/2 pass-through support and the API for hid-rmi to provide
the attention data.

Patches 10-13 are the HID ones. I wish they could go in together, but given
that we are getting closer than the merge window, I don't have many hopes for
them to be taken. Anyway, they are here and I'd like to get some feedback for
them too. I am able to only test these with a Synaptics test touchpad, so any
real hardware test (with F11 or F12) would be good.

I still have few rmi4 patches to switch the Lenovo Thinkpad to rmi-smbus,
but given they are touching serio+psmouse I guess they are more v4.11 material.
(for those wondering I already submitted those as 14-17 / 18 in the v3 of
rmi4-smbus).

I have pushed the series on my github account (with the smbus/psmouse too):
https://github.com/bentiss/linux/commits/synaptics-rmi4-v4.9-rc7%2B
(branch synaptics-rmi4-v4.9-rc7+)

Cheers,
Benjamin


Andrew Duggan (3):
  HID: rmi: Make hid-rmi a transport driver for synaptics-rmi4
  HID: rmi: Handle all Synaptics touchpads using hid-rmi
  HID: rmi: Support the Lenovo Thinkpad X1 Tablet dock using hid-rmi

Benjamin Tissoires (8):
  Input: synaptics-rmi4 - fix documentation of
    rmi_2d_sensor_platform_data
  Input: synaptics-rmi4 - remove unused fields in struct rmi_driver_data
  Input: synaptics-rmi4 - add rmi_enable/disable_irq
  Input: synaptics-rmi4 - remove mutex calls while updating the firmware
  Input: synaptics-rmi4 - remove EXPORT_SYMBOL_GPL for internal
    functions
  Input: synaptics-rmi4 - have only one struct platform data
  Input: synaptics-rmi4 - allow to add attention data
  Input: synaptics-rmi4 - store the attn data in the driver

Dennis Wassenberg (1):
  Input: synaptics-rmi4 - f03: grab data passed by transport device

Lyude Paul (1):
  Input: synaptics-rmi4 - add support for F03

 drivers/hid/Kconfig                |   4 +
 drivers/hid/hid-core.c             |   4 +-
 drivers/hid/hid-ids.h              |   1 +
 drivers/hid/hid-rmi.c              | 974 ++++++-------------------------------
 drivers/input/mouse/psmouse-base.c |   6 +
 drivers/input/rmi4/Kconfig         |   9 +
 drivers/input/rmi4/Makefile        |   1 +
 drivers/input/rmi4/rmi_bus.c       |   3 +
 drivers/input/rmi4/rmi_driver.c    | 186 ++++---
 drivers/input/rmi4/rmi_driver.h    |   6 +-
 drivers/input/rmi4/rmi_f03.c       | 245 ++++++++++
 drivers/input/rmi4/rmi_f11.c       |  16 +-
 drivers/input/rmi4/rmi_f12.c       |  47 +-
 drivers/input/rmi4/rmi_f30.c       |  20 +-
 drivers/input/rmi4/rmi_f34.c       |  19 +-
 include/linux/rmi.h                |  25 +-
 include/uapi/linux/serio.h         |   1 +
 17 files changed, 609 insertions(+), 958 deletions(-)
 create mode 100644 drivers/input/rmi4/rmi_f03.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/13] Input: synaptics-rmi4 - fix documentation of rmi_2d_sensor_platform_data
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 02/13] Input: synaptics-rmi4 - remove unused fields in struct rmi_driver_data Benjamin Tissoires
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

Typos...

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---
 include/linux/rmi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 8499b6a..27dd9aa 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -108,7 +108,7 @@ struct rmi_2d_sensor_platform_data {
  * @buttonpad - the touchpad is a buttonpad, so enable only the first actual
  * button that is found.
  * @trackstick_buttons - Set when the function 30 is handling the physical
- * buttons of the trackstick (as a PD/2 passthrough device.
+ * buttons of the trackstick (as a PS/2 passthrough device).
  * @disable - the touchpad incorrectly reports F30 and it should be ignored.
  * This is a special case which is due to misconfigured firmware.
  */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/13] Input: synaptics-rmi4 - remove unused fields in struct rmi_driver_data
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 01/13] Input: synaptics-rmi4 - fix documentation of rmi_2d_sensor_platform_data Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 03/13] Input: synaptics-rmi4 - add rmi_enable/disable_irq Benjamin Tissoires
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

These fields are not used anywhere, there is no point in carrying them.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 include/linux/rmi.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 27dd9aa..0b118ab 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -340,7 +340,6 @@ struct rmi_driver_data {
 	struct rmi_function *f34_container;
 	bool f01_bootloader_mode;
 
-	u32 attn_count;
 	int num_of_irq_regs;
 	int irq_count;
 	void *irq_memory;
@@ -352,14 +351,11 @@ struct rmi_driver_data {
 	struct input_dev *input;
 
 	u8 pdt_props;
-	u8 bsr;
 
 	u8 num_rx_electrodes;
 	u8 num_tx_electrodes;
 
 	bool enabled;
-
-	void *data;
 };
 
 int rmi_register_transport_device(struct rmi_transport_dev *xport);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/13] Input: synaptics-rmi4 - add rmi_enable/disable_irq
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 01/13] Input: synaptics-rmi4 - fix documentation of rmi_2d_sensor_platform_data Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 02/13] Input: synaptics-rmi4 - remove unused fields in struct rmi_driver_data Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 04/13] Input: synaptics-rmi4 - remove mutex calls while updating the firmware Benjamin Tissoires
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

Set the .enabled boolean and trigger an event processing when enabling
for edge-triggered systems.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_driver.c | 83 +++++++++++++++++++++++++++++++----------
 drivers/input/rmi4/rmi_driver.h |  2 +
 include/linux/rmi.h             |  1 +
 3 files changed, 67 insertions(+), 19 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 2b17d8c..f04fc41 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -215,6 +215,7 @@ static irqreturn_t rmi_irq_fn(int irq, void *dev_id)
 static int rmi_irq_init(struct rmi_device *rmi_dev)
 {
 	struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
 	int irq_flags = irq_get_trigger_type(pdata->irq);
 	int ret;
 
@@ -232,6 +233,8 @@ static int rmi_irq_init(struct rmi_device *rmi_dev)
 		return ret;
 	}
 
+	data->enabled = true;
+
 	return 0;
 }
 
@@ -866,17 +869,54 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
 	return error;
 }
 
-int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake)
+void rmi_enable_irq(struct rmi_device *rmi_dev, bool clear_wake)
 {
 	struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
 	int irq = pdata->irq;
-	int retval = 0;
+	int irq_flags;
+	int retval;
 
-	retval = rmi_suspend_functions(rmi_dev);
-	if (retval)
-		dev_warn(&rmi_dev->dev, "Failed to suspend functions: %d\n",
-			retval);
+	mutex_lock(&data->enabled_mutex);
+
+	if (data->enabled)
+		goto out;
+
+	enable_irq(irq);
+	data->enabled = true;
+	if (clear_wake && device_may_wakeup(rmi_dev->xport->dev)) {
+		retval = disable_irq_wake(irq);
+		if (!retval)
+			dev_warn(&rmi_dev->dev,
+				 "Failed to disable irq for wake: %d\n",
+				 retval);
+	}
+
+	/*
+	 * Call rmi_process_interrupt_requests() after enabling irq,
+	 * otherwise we may lose interrupt on edge-triggered systems.
+	 */
+	irq_flags = irq_get_trigger_type(pdata->irq);
+	if (irq_flags & IRQ_TYPE_EDGE_BOTH)
+		rmi_process_interrupt_requests(rmi_dev);
+
+out:
+	mutex_unlock(&data->enabled_mutex);
+}
+
+void rmi_disable_irq(struct rmi_device *rmi_dev, bool enable_wake)
+{
+	struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
+	int irq = pdata->irq;
+	int retval;
+
+	mutex_lock(&data->enabled_mutex);
+
+	if (!data->enabled)
+		goto out;
 
+	data->enabled = false;
 	disable_irq(irq);
 	if (enable_wake && device_may_wakeup(rmi_dev->xport->dev)) {
 		retval = enable_irq_wake(irq);
@@ -885,24 +925,30 @@ int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake)
 				 "Failed to enable irq for wake: %d\n",
 				 retval);
 	}
+
+out:
+	mutex_unlock(&data->enabled_mutex);
+}
+
+int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake)
+{
+	int retval;
+
+	retval = rmi_suspend_functions(rmi_dev);
+	if (retval)
+		dev_warn(&rmi_dev->dev, "Failed to suspend functions: %d\n",
+			retval);
+
+	rmi_disable_irq(rmi_dev, enable_wake);
 	return retval;
 }
 EXPORT_SYMBOL_GPL(rmi_driver_suspend);
 
 int rmi_driver_resume(struct rmi_device *rmi_dev, bool clear_wake)
 {
-	struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
-	int irq = pdata->irq;
 	int retval;
 
-	enable_irq(irq);
-	if (clear_wake && device_may_wakeup(rmi_dev->xport->dev)) {
-		retval = disable_irq_wake(irq);
-		if (!retval)
-			dev_warn(&rmi_dev->dev,
-				 "Failed to disable irq for wake: %d\n",
-				 retval);
-	}
+	rmi_enable_irq(rmi_dev, clear_wake);
 
 	retval = rmi_resume_functions(rmi_dev);
 	if (retval)
@@ -916,10 +962,8 @@ EXPORT_SYMBOL_GPL(rmi_driver_resume);
 static int rmi_driver_remove(struct device *dev)
 {
 	struct rmi_device *rmi_dev = to_rmi_device(dev);
-	struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
-	int irq = pdata->irq;
 
-	disable_irq(irq);
+	rmi_disable_irq(rmi_dev, false);
 
 	rmi_f34_remove_sysfs(rmi_dev);
 	rmi_free_function_list(rmi_dev);
@@ -1108,6 +1152,7 @@ static int rmi_driver_probe(struct device *dev)
 	}
 
 	mutex_init(&data->irq_mutex);
+	mutex_init(&data->enabled_mutex);
 
 	retval = rmi_probe_interrupts(data);
 	if (retval)
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 5b201f3..c9fe3d3 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -101,6 +101,8 @@ int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
 		 int (*callback)(struct rmi_device *rmi_dev, void *ctx,
 		 const struct pdt_entry *entry));
 int rmi_probe_interrupts(struct rmi_driver_data *data);
+void rmi_enable_irq(struct rmi_device *rmi_dev, bool clear_wake);
+void rmi_disable_irq(struct rmi_device *rmi_dev, bool enable_wake);
 int rmi_init_functions(struct rmi_driver_data *data);
 int rmi_initial_reset(struct rmi_device *rmi_dev, void *ctx,
 		      const struct pdt_entry *pdt);
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 0b118ab..621f098 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -356,6 +356,7 @@ struct rmi_driver_data {
 	u8 num_tx_electrodes;
 
 	bool enabled;
+	struct mutex enabled_mutex;
 };
 
 int rmi_register_transport_device(struct rmi_transport_dev *xport);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/13] Input: synaptics-rmi4 - remove mutex calls while updating the firmware
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
                   ` (2 preceding siblings ...)
  2016-11-29 10:08 ` [PATCH 03/13] Input: synaptics-rmi4 - add rmi_enable/disable_irq Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 05/13] Input: synaptics-rmi4 - remove EXPORT_SYMBOL_GPL for internal functions Benjamin Tissoires
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

This partially reverts commit 29fd0ec2bdbe ("Input: synaptics-rmi4 -
add support for F34 device reflash")

irq_mutex should be used only to protect data->current_irq_mask, not
preventing incoming input to be processed while the upgrade of the
firmware is happening. We can simply disable the irqs when we don't
want them to interfere with the upgrade process.

Tested on S7300 and S7800 (with F34 v7 patch added)

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Nick Dyer <nick@shmanahar.org>
---
 drivers/input/rmi4/rmi_driver.c | 40 ++++++++--------------------------------
 drivers/input/rmi4/rmi_f34.c    | 19 ++++++++++++++-----
 2 files changed, 22 insertions(+), 37 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index f04fc41..27c731a 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -42,8 +42,6 @@ void rmi_free_function_list(struct rmi_device *rmi_dev)
 
 	rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev, "Freeing function list\n");
 
-	mutex_lock(&data->irq_mutex);
-
 	devm_kfree(&rmi_dev->dev, data->irq_memory);
 	data->irq_memory = NULL;
 	data->irq_status = NULL;
@@ -60,8 +58,6 @@ void rmi_free_function_list(struct rmi_device *rmi_dev)
 		list_del(&fn->node);
 		rmi_unregister_function(fn);
 	}
-
-	mutex_unlock(&data->irq_mutex);
 }
 EXPORT_SYMBOL_GPL(rmi_free_function_list);
 
@@ -160,25 +156,24 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 	if (!data)
 		return 0;
 
-	mutex_lock(&data->irq_mutex);
-	if (!data->irq_status || !data->f01_container) {
-		mutex_unlock(&data->irq_mutex);
-		return 0;
-	}
-
 	if (!rmi_dev->xport->attn_data) {
 		error = rmi_read_block(rmi_dev,
 				data->f01_container->fd.data_base_addr + 1,
 				data->irq_status, data->num_of_irq_regs);
 		if (error < 0) {
 			dev_err(dev, "Failed to read irqs, code=%d\n", error);
-			mutex_unlock(&data->irq_mutex);
 			return error;
 		}
 	}
 
+	mutex_lock(&data->irq_mutex);
 	bitmap_and(data->irq_status, data->irq_status, data->current_irq_mask,
 	       data->irq_count);
+	/*
+	 * At this point, irq_status has all bits that are set in the
+	 * interrupt status register and are enabled.
+	 */
+	mutex_unlock(&data->irq_mutex);
 
 	/*
 	 * It would be nice to be able to use irq_chip to handle these
@@ -194,8 +189,6 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 	if (data->input)
 		input_sync(data->input);
 
-	mutex_unlock(&data->irq_mutex);
-
 	return 0;
 }
 
@@ -263,18 +256,12 @@ static int rmi_suspend_functions(struct rmi_device *rmi_dev)
 	struct rmi_function *entry;
 	int retval;
 
-	mutex_lock(&data->irq_mutex);
-
 	list_for_each_entry(entry, &data->function_list, node) {
 		retval = suspend_one_function(entry);
-		if (retval < 0) {
-			mutex_unlock(&data->irq_mutex);
+		if (retval < 0)
 			return retval;
-		}
 	}
 
-	mutex_unlock(&data->irq_mutex);
-
 	return 0;
 }
 
@@ -303,18 +290,12 @@ static int rmi_resume_functions(struct rmi_device *rmi_dev)
 	struct rmi_function *entry;
 	int retval;
 
-	mutex_lock(&data->irq_mutex);
-
 	list_for_each_entry(entry, &data->function_list, node) {
 		retval = resume_one_function(entry);
-		if (retval < 0) {
-			mutex_unlock(&data->irq_mutex);
+		if (retval < 0)
 			return retval;
-		}
 	}
 
-	mutex_unlock(&data->irq_mutex);
-
 	return 0;
 }
 
@@ -1043,8 +1024,6 @@ int rmi_init_functions(struct rmi_driver_data *data)
 	int irq_count;
 	int retval;
 
-	mutex_lock(&data->irq_mutex);
-
 	irq_count = 0;
 	rmi_dbg(RMI_DEBUG_CORE, dev, "%s: Creating functions.\n", __func__);
 	retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_create_function);
@@ -1069,13 +1048,10 @@ int rmi_init_functions(struct rmi_driver_data *data)
 		goto err_destroy_functions;
 	}
 
-	mutex_unlock(&data->irq_mutex);
-
 	return 0;
 
 err_destroy_functions:
 	rmi_free_function_list(rmi_dev);
-	mutex_unlock(&data->irq_mutex);
 	return retval;
 }
 EXPORT_SYMBOL_GPL(rmi_init_functions);
diff --git a/drivers/input/rmi4/rmi_f34.c b/drivers/input/rmi4/rmi_f34.c
index 03df85a..01936a4 100644
--- a/drivers/input/rmi4/rmi_f34.c
+++ b/drivers/input/rmi4/rmi_f34.c
@@ -282,7 +282,8 @@ int rmi_f34_update_firmware(struct f34_data *f34, const struct firmware *fw)
 static int rmi_firmware_update(struct rmi_driver_data *data,
 			       const struct firmware *fw)
 {
-	struct device *dev = &data->rmi_dev->dev;
+	struct rmi_device *rmi_dev = data->rmi_dev;
+	struct device *dev = &rmi_dev->dev;
 	struct f34_data *f34;
 	int ret;
 
@@ -305,8 +306,10 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
 	if (ret)
 		return ret;
 
+	rmi_disable_irq(rmi_dev, false);
+
 	/* Tear down functions and re-probe */
-	rmi_free_function_list(data->rmi_dev);
+	rmi_free_function_list(rmi_dev);
 
 	ret = rmi_probe_interrupts(data);
 	if (ret)
@@ -322,6 +325,8 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
 		return -EINVAL;
 	}
 
+	rmi_enable_irq(rmi_dev, false);
+
 	f34 = dev_get_drvdata(&data->f34_container->dev);
 
 	/* Perform firmware update */
@@ -329,11 +334,13 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
 
 	dev_info(&f34->fn->dev, "Firmware update complete, status:%d\n", ret);
 
+	rmi_disable_irq(rmi_dev, false);
+
 	/* Re-probe */
 	rmi_dbg(RMI_DEBUG_FN, dev, "Re-probing device\n");
-	rmi_free_function_list(data->rmi_dev);
+	rmi_free_function_list(rmi_dev);
 
-	ret = rmi_scan_pdt(data->rmi_dev, NULL, rmi_initial_reset);
+	ret = rmi_scan_pdt(rmi_dev, NULL, rmi_initial_reset);
 	if (ret < 0)
 		dev_warn(dev, "RMI reset failed!\n");
 
@@ -345,9 +352,11 @@ static int rmi_firmware_update(struct rmi_driver_data *data,
 	if (ret)
 		return ret;
 
+	rmi_enable_irq(rmi_dev, false);
+
 	if (data->f01_container->dev.driver)
 		/* Driver already bound, so enable ATTN now. */
-		return rmi_enable_sensor(data->rmi_dev);
+		return rmi_enable_sensor(rmi_dev);
 
 	rmi_dbg(RMI_DEBUG_FN, dev, "%s complete\n", __func__);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/13] Input: synaptics-rmi4 - remove EXPORT_SYMBOL_GPL for internal functions
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
                   ` (3 preceding siblings ...)
  2016-11-29 10:08 ` [PATCH 04/13] Input: synaptics-rmi4 - remove mutex calls while updating the firmware Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 06/13] Input: synaptics-rmi4 - have only one struct platform data Benjamin Tissoires
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

those functions should not be used outside of rmi_core.ko.
There is no point in exporting them to the world.

It looks like rmi_read_pdt_entry() should be static too.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_driver.c | 15 ++-------------
 drivers/input/rmi4/rmi_driver.h |  3 ---
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 27c731a..a718e51 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -59,7 +59,6 @@ void rmi_free_function_list(struct rmi_device *rmi_dev)
 		rmi_unregister_function(fn);
 	}
 }
-EXPORT_SYMBOL_GPL(rmi_free_function_list);
 
 static int reset_one_function(struct rmi_function *fn)
 {
@@ -309,7 +308,6 @@ int rmi_enable_sensor(struct rmi_device *rmi_dev)
 
 	return rmi_process_interrupt_requests(rmi_dev);
 }
-EXPORT_SYMBOL_GPL(rmi_enable_sensor);
 
 /**
  * rmi_driver_set_input_params - set input device id and other data.
@@ -431,8 +429,8 @@ static int rmi_driver_reset_handler(struct rmi_device *rmi_dev)
 	return 0;
 }
 
-int rmi_read_pdt_entry(struct rmi_device *rmi_dev, struct pdt_entry *entry,
-			u16 pdt_address)
+static int rmi_read_pdt_entry(struct rmi_device *rmi_dev,
+			      struct pdt_entry *entry, u16 pdt_address)
 {
 	u8 buf[RMI_PDT_ENTRY_SIZE];
 	int error;
@@ -455,7 +453,6 @@ int rmi_read_pdt_entry(struct rmi_device *rmi_dev, struct pdt_entry *entry,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(rmi_read_pdt_entry);
 
 static void rmi_driver_copy_pdt_to_fd(const struct pdt_entry *pdt,
 				      struct rmi_function_descriptor *fd)
@@ -532,7 +529,6 @@ int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
 
 	return retval < 0 ? retval : 0;
 }
-EXPORT_SYMBOL_GPL(rmi_scan_pdt);
 
 int rmi_read_register_desc(struct rmi_device *d, u16 addr,
 				struct rmi_register_descriptor *rdesc)
@@ -664,7 +660,6 @@ int rmi_read_register_desc(struct rmi_device *d, u16 addr,
 	kfree(struct_buf);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(rmi_read_register_desc);
 
 const struct rmi_register_desc_item *rmi_get_register_desc_item(
 				struct rmi_register_descriptor *rdesc, u16 reg)
@@ -680,7 +675,6 @@ const struct rmi_register_desc_item *rmi_get_register_desc_item(
 
 	return NULL;
 }
-EXPORT_SYMBOL_GPL(rmi_get_register_desc_item);
 
 size_t rmi_register_desc_calc_size(struct rmi_register_descriptor *rdesc)
 {
@@ -694,7 +688,6 @@ size_t rmi_register_desc_calc_size(struct rmi_register_descriptor *rdesc)
 	}
 	return size;
 }
-EXPORT_SYMBOL_GPL(rmi_register_desc_calc_size);
 
 /* Compute the register offset relative to the base address */
 int rmi_register_desc_calc_reg_offset(
@@ -712,7 +705,6 @@ int rmi_register_desc_calc_reg_offset(
 	}
 	return -1;
 }
-EXPORT_SYMBOL_GPL(rmi_register_desc_calc_reg_offset);
 
 bool rmi_register_desc_has_subpacket(const struct rmi_register_desc_item *item,
 	u8 subpacket)
@@ -796,7 +788,6 @@ int rmi_initial_reset(struct rmi_device *rmi_dev, void *ctx,
 	/* F01 should always be on page 0. If we don't find it there, fail. */
 	return pdt->page_start == 0 ? RMI_SCAN_CONTINUE : -ENODEV;
 }
-EXPORT_SYMBOL_GPL(rmi_initial_reset);
 
 static int rmi_create_function(struct rmi_device *rmi_dev,
 			       void *ctx, const struct pdt_entry *pdt)
@@ -1015,7 +1006,6 @@ int rmi_probe_interrupts(struct rmi_driver_data *data)
 
 	return retval;
 }
-EXPORT_SYMBOL_GPL(rmi_probe_interrupts);
 
 int rmi_init_functions(struct rmi_driver_data *data)
 {
@@ -1054,7 +1044,6 @@ int rmi_init_functions(struct rmi_driver_data *data)
 	rmi_free_function_list(rmi_dev);
 	return retval;
 }
-EXPORT_SYMBOL_GPL(rmi_init_functions);
 
 static int rmi_driver_probe(struct device *dev)
 {
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index c9fe3d3..cc94585 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -51,9 +51,6 @@ struct pdt_entry {
 	u8 function_number;
 };
 
-int rmi_read_pdt_entry(struct rmi_device *rmi_dev, struct pdt_entry *entry,
-			u16 pdt_address);
-
 #define RMI_REG_DESC_PRESENSE_BITS	(32 * BITS_PER_BYTE)
 #define RMI_REG_DESC_SUBPACKET_BITS	(37 * BITS_PER_BYTE)
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/13] Input: synaptics-rmi4 - have only one struct platform data
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
                   ` (4 preceding siblings ...)
  2016-11-29 10:08 ` [PATCH 05/13] Input: synaptics-rmi4 - remove EXPORT_SYMBOL_GPL for internal functions Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 07/13] Input: synaptics-rmi4 - add support for F03 Benjamin Tissoires
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

If struct rmi_device_platform_data contains pointers to other struct,
it gets difficult to allocate a fixed size struct and copy it over between
drivers.

Change the pointers into a struct and change the code in rmi4 accordingly.

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---
 drivers/input/rmi4/rmi_f11.c | 4 ++--
 drivers/input/rmi4/rmi_f12.c | 4 ++--
 drivers/input/rmi4/rmi_f30.c | 9 ++++-----
 include/linux/rmi.h          | 4 ++--
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 3401aac..44fc275 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -1079,8 +1079,8 @@ static int rmi_f11_initialize(struct rmi_function *fn)
 		rc = rmi_2d_sensor_of_probe(&fn->dev, &f11->sensor_pdata);
 		if (rc)
 			return rc;
-	} else if (pdata->sensor_pdata) {
-		f11->sensor_pdata = *pdata->sensor_pdata;
+	} else {
+		f11->sensor_pdata = pdata->sensor_pdata;
 	}
 
 	f11->rezero_wait_ms = f11->sensor_pdata.rezero_wait;
diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index d7255f1..82a4964 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -352,8 +352,8 @@ static int rmi_f12_probe(struct rmi_function *fn)
 		ret = rmi_2d_sensor_of_probe(&fn->dev, &f12->sensor_pdata);
 		if (ret)
 			return ret;
-	} else if (pdata->sensor_pdata) {
-		f12->sensor_pdata = *pdata->sensor_pdata;
+	} else {
+		f12->sensor_pdata = pdata->sensor_pdata;
 	}
 
 	ret = rmi_read_register_desc(rmi_dev, query_addr,
diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
index 485907f..f696137 100644
--- a/drivers/input/rmi4/rmi_f30.c
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -196,7 +196,7 @@ static int rmi_f30_config(struct rmi_function *fn)
 				rmi_get_platform_data(fn->rmi_dev);
 	int error;
 
-	if (pdata->f30_data && pdata->f30_data->disable) {
+	if (pdata->f30_data.disable) {
 		drv->clear_irq_bits(fn->rmi_dev, fn->irq_mask);
 	} else {
 		/* Write Control Register values back to device */
@@ -355,7 +355,7 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
 	f30->gpioled_key_map = (u16 *)map_memory;
 
 	pdata = rmi_get_platform_data(rmi_dev);
-	if (pdata && f30->has_gpio) {
+	if (f30->has_gpio) {
 		button = BTN_LEFT;
 		for (i = 0; i < f30->gpioled_count; i++) {
 			if (rmi_f30_is_valid_button(i, f30->ctrl)) {
@@ -366,8 +366,7 @@ static inline int rmi_f30_initialize(struct rmi_function *fn)
 				 * f30->has_mech_mouse_btns, but I am
 				 * not sure, so use only the pdata info
 				 */
-				if (pdata->f30_data &&
-				    pdata->f30_data->buttonpad)
+				if (pdata->f30_data.buttonpad)
 					break;
 			}
 		}
@@ -382,7 +381,7 @@ static int rmi_f30_probe(struct rmi_function *fn)
 	const struct rmi_device_platform_data *pdata =
 				rmi_get_platform_data(fn->rmi_dev);
 
-	if (pdata->f30_data && pdata->f30_data->disable)
+	if (pdata->f30_data.disable)
 		return 0;
 
 	rc = rmi_f30_initialize(fn);
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 621f098..7780e40 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -218,9 +218,9 @@ struct rmi_device_platform_data {
 	struct rmi_device_platform_data_spi spi_data;
 
 	/* function handler pdata */
-	struct rmi_2d_sensor_platform_data *sensor_pdata;
+	struct rmi_2d_sensor_platform_data sensor_pdata;
 	struct rmi_f01_power_management power_management;
-	struct rmi_f30_data *f30_data;
+	struct rmi_f30_data f30_data;
 };
 
 /**
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/13] Input: synaptics-rmi4 - add support for F03
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
                   ` (5 preceding siblings ...)
  2016-11-29 10:08 ` [PATCH 06/13] Input: synaptics-rmi4 - have only one struct platform data Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-12-01  1:36   ` Dmitry Torokhov
  2016-11-29 10:08 ` [PATCH 08/13] Input: synaptics-rmi4 - f03: grab data passed by transport device Benjamin Tissoires
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

From: Lyude Paul <thatslyude@gmail.com>

This adds basic functionality for PS/2 passthrough on Synaptics
Touchpads using RMI4 through smbus.

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>
Signed-off-by: Lyude Paul <thatslyude@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---
 drivers/input/mouse/psmouse-base.c |   6 +
 drivers/input/rmi4/Kconfig         |   9 ++
 drivers/input/rmi4/Makefile        |   1 +
 drivers/input/rmi4/rmi_bus.c       |   3 +
 drivers/input/rmi4/rmi_driver.h    |   1 +
 drivers/input/rmi4/rmi_f03.c       | 227 +++++++++++++++++++++++++++++++++++++
 include/uapi/linux/serio.h         |   1 +
 7 files changed, 248 insertions(+)
 create mode 100644 drivers/input/rmi4/rmi_f03.c

diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index fb4b185..691dd74 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -1663,6 +1663,12 @@ static struct serio_device_id psmouse_serio_ids[] = {
 		.id	= SERIO_ANY,
 		.extra	= SERIO_ANY,
 	},
+	{
+		.type	= SERIO_RMI_PSTHRU,
+		.proto	= SERIO_ANY,
+		.id	= SERIO_ANY,
+		.extra	= SERIO_ANY,
+	},
 	{ 0 }
 };
 
diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index a9c36a5..b8189a3 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -39,6 +39,15 @@ config RMI4_SMB
 	  To compile this driver as a module, choose M here: the module will be
 	  called rmi_smbus.
 
+config RMI4_F03
+        bool "RMI4 Function 03 (PS2 Guest)"
+        depends on RMI4_CORE
+        help
+          Say Y here if you want to add support for RMI4 function 03.
+
+          Function 03 provides PS2 guest support for RMI4 devices. This
+          includes support for TrackPoints on TouchPads.
+
 config RMI4_2D_SENSOR
 	bool
 	depends on RMI4_CORE
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index e7f4ca6..a199cbe 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -4,6 +4,7 @@ rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
 rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
 
 # Function drivers
+rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o
 rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
 rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
 rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index 81be9c1..1c40d94 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -305,6 +305,9 @@ struct bus_type rmi_bus_type = {
 
 static struct rmi_function_handler *fn_handlers[] = {
 	&rmi_f01_handler,
+#ifdef CONFIG_RMI4_F03
+	&rmi_f03_handler,
+#endif
 #ifdef CONFIG_RMI4_F11
 	&rmi_f11_handler,
 #endif
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index cc94585..24f8f76 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -121,6 +121,7 @@ static inline void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev)
 #endif /* CONFIG_RMI_F34 */
 
 extern struct rmi_function_handler rmi_f01_handler;
+extern struct rmi_function_handler rmi_f03_handler;
 extern struct rmi_function_handler rmi_f11_handler;
 extern struct rmi_function_handler rmi_f12_handler;
 extern struct rmi_function_handler rmi_f30_handler;
diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
new file mode 100644
index 0000000..a7e1b98
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f03.c
@@ -0,0 +1,227 @@
+/*
+ * Copyright (C) 2015-2016 Red Hat
+ * Copyright (C) 2015 Lyude Paul <thatslyude@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/serio.h>
+#include <linux/notifier.h>
+#include "rmi_driver.h"
+
+#define RMI_F03_RX_DATA_OFB		0x01
+#define RMI_F03_OB_SIZE			2
+
+#define RMI_F03_OB_OFFSET		2
+#define RMI_F03_OB_DATA_OFFSET		1
+#define RMI_F03_OB_FLAG_TIMEOUT		(1 << 6)
+#define RMI_F03_OB_FLAG_PARITY		(1 << 7)
+
+#define RMI_F03_DEVICE_COUNT		0x07
+#define RMI_F03_BYTES_PER_DEVICE_MASK	0x70
+#define RMI_F03_BYTES_PER_DEVICE_SHIFT	4
+#define RMI_F03_QUEUE_LENGTH		0x0F
+
+struct f03_data {
+	struct rmi_function *fn;
+
+	struct serio *serio;
+
+	unsigned int overwrite_buttons;
+
+	u8 device_count;
+	u8 rx_queue_length;
+};
+
+static int rmi_f03_pt_write(struct serio *id, unsigned char val)
+{
+	struct f03_data *f03 = id->port_data;
+	int rc;
+
+	rmi_dbg(RMI_DEBUG_FN, &f03->fn->dev,
+		"%s: Wrote %.2hhx to PS/2 passthrough address",
+		__func__, val);
+
+	rc = rmi_write(f03->fn->rmi_dev, f03->fn->fd.data_base_addr, val);
+	if (rc) {
+		dev_err(&f03->fn->dev,
+			"%s: Failed to write to F03 TX register.\n", __func__);
+		return rc;
+	}
+
+	return 0;
+}
+
+static inline int rmi_f03_initialize(struct rmi_function *fn)
+{
+	struct f03_data *f03;
+	struct device *dev = &fn->dev;
+	int rc;
+	u8 bytes_per_device;
+	u8 query1;
+	size_t query2_len;
+
+	rc = rmi_read(fn->rmi_dev, fn->fd.query_base_addr, &query1);
+	if (rc) {
+		dev_err(dev, "Failed to read query register.\n");
+		return rc;
+	}
+
+	f03 = devm_kzalloc(dev, sizeof(struct f03_data), GFP_KERNEL);
+	if (!f03)
+		return -ENOMEM;
+
+	f03->device_count = query1 & RMI_F03_DEVICE_COUNT;
+	bytes_per_device = (query1 & RMI_F03_BYTES_PER_DEVICE_MASK) >>
+		RMI_F03_BYTES_PER_DEVICE_SHIFT;
+
+	query2_len = f03->device_count * bytes_per_device;
+
+	/*
+	 * The first generation of image sensors don't have a second part to
+	 * their f03 query, as such we have to set some of these values manually
+	 */
+	if (query2_len < 1) {
+		f03->device_count = 1;
+		f03->rx_queue_length = 7;
+	} else {
+		u8 query2[query2_len];
+
+		rc = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr + 1,
+				    query2, query2_len);
+		if (rc) {
+			dev_err(dev, "Failed to read second set of query registers.\n");
+			return rc;
+		}
+
+		f03->rx_queue_length = query2[0] & RMI_F03_QUEUE_LENGTH;
+	}
+
+	f03->fn = fn;
+
+	dev_set_drvdata(dev, f03);
+
+	return f03->device_count;
+}
+
+static inline int rmi_f03_register_pt(struct rmi_function *fn)
+{
+	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
+	struct serio *serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
+
+	if (!serio)
+		return -ENOMEM;
+
+	serio->id.type = SERIO_RMI_PSTHRU;
+	serio->write = rmi_f03_pt_write;
+	serio->port_data = f03;
+
+	strlcpy(serio->name, "Synaptics RMI4 PS2 pass-through",
+		sizeof(serio->name));
+	strlcpy(serio->phys, "synaptics-rmi4-pt/serio1",
+		sizeof(serio->phys));
+	 serio->dev.parent = &fn->dev;
+
+	f03->serio = serio;
+
+	serio_register_port(serio);
+
+	return 0;
+}
+
+static int rmi_f03_probe(struct rmi_function *fn)
+{
+	int rc;
+
+	rc = rmi_f03_initialize(fn);
+	if (rc < 0)
+		return rc;
+
+	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%d devices on PS/2 passthrough", rc);
+
+	rc = rmi_f03_register_pt(fn);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int rmi_f03_config(struct rmi_function *fn)
+{
+	fn->rmi_dev->driver->set_irq_bits(fn->rmi_dev, fn->irq_mask);
+
+	return 0;
+}
+
+static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
+{
+	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
+	u16 data_addr = fn->fd.data_base_addr;
+	const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE;
+	u8 obs[ob_len];
+	u8 ob_status;
+	u8 ob_data;
+	unsigned int serio_flags;
+	int i;
+	int retval;
+
+	/* Grab all of the data registers, and check them for data */
+	retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
+				&obs, ob_len);
+	if (retval) {
+		dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
+			__func__);
+		serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
+		return retval;
+	}
+
+	for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) {
+		ob_status = obs[i];
+		ob_data = obs[i + RMI_F03_OB_DATA_OFFSET];
+		serio_flags = 0;
+
+		if (!(ob_status & RMI_F03_RX_DATA_OFB))
+			continue;
+
+		if (ob_status & RMI_F03_OB_FLAG_TIMEOUT)
+			serio_flags |= SERIO_TIMEOUT;
+		if (ob_status & RMI_F03_OB_FLAG_PARITY)
+			serio_flags |= SERIO_PARITY;
+
+		rmi_dbg(RMI_DEBUG_FN, &fn->dev,
+			"%s: Received %.2hhx from PS2 guest T: %c P: %c\n",
+			__func__, ob_data,
+			serio_flags & SERIO_TIMEOUT ?  'Y' : 'N',
+			serio_flags & SERIO_PARITY ? 'Y' : 'N');
+
+		serio_interrupt(f03->serio, ob_data, serio_flags);
+	}
+
+	return 0;
+}
+
+static void rmi_f03_remove(struct rmi_function *fn)
+{
+	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
+
+	serio_unregister_port(f03->serio);
+}
+
+struct rmi_function_handler rmi_f03_handler = {
+	.driver = {
+		.name = "rmi4_f03",
+	},
+	.func = 0x03,
+	.probe = rmi_f03_probe,
+	.config = rmi_f03_config,
+	.attention = rmi_f03_attention,
+	.remove = rmi_f03_remove,
+};
+
+MODULE_AUTHOR("Lyude Paul <thatslyude@gmail.com>");
+MODULE_DESCRIPTION("RMI F03 module");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/serio.h b/include/uapi/linux/serio.h
index f2447a8..7012178 100644
--- a/include/uapi/linux/serio.h
+++ b/include/uapi/linux/serio.h
@@ -30,6 +30,7 @@
 #define SERIO_HIL_MLC	0x03
 #define SERIO_PS_PSTHRU	0x05
 #define SERIO_8042_XL	0x06
+#define SERIO_RMI_PSTHRU	0x07
 
 /*
  * Serio protocols
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/13] Input: synaptics-rmi4 - f03: grab data passed by transport device
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
                   ` (6 preceding siblings ...)
  2016-11-29 10:08 ` [PATCH 07/13] Input: synaptics-rmi4 - add support for F03 Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 09/13] Input: synaptics-rmi4 - allow to add attention data Benjamin Tissoires
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

From: Dennis Wassenberg <dennis.wassenberg@secunet.com>

First check if there are data available passed by the transport device.
If data available use these data. If there are no data available
try to read the rmi block if dsata are passed this way.

This is the way the other rmi function handlers will do this.

This patch is needed on HID devices because the firmware reads F03 data
registers and adds them to the HID attention report. Reading those
registers from the driver after the firmware read them will result in
invalid data.

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>
Signed-off-by: Dennis Wassenberg <dennis.wassenberg@secunet.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---
 drivers/input/rmi4/rmi_f03.c | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
index a7e1b98..a124b33 100644
--- a/drivers/input/rmi4/rmi_f03.c
+++ b/drivers/input/rmi4/rmi_f03.c
@@ -159,6 +159,7 @@ static int rmi_f03_config(struct rmi_function *fn)
 
 static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
 {
+	struct rmi_device *rmi_dev = fn->rmi_dev;
 	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
 	u16 data_addr = fn->fd.data_base_addr;
 	const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE;
@@ -167,16 +168,32 @@ static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
 	u8 ob_data;
 	unsigned int serio_flags;
 	int i;
-	int retval;
-
-	/* Grab all of the data registers, and check them for data */
-	retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
-				&obs, ob_len);
-	if (retval) {
-		dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
-			__func__);
-		serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
-		return retval;
+	int ret;
+
+	if (!rmi_dev || !rmi_dev->xport)
+		return -ENODEV;
+
+	if (rmi_dev->xport->attn_data) {
+		/* First grab the data passed by the transport device */
+		if (rmi_dev->xport->attn_size < ob_len) {
+			dev_warn(&fn->dev, "F03 interrupted, but data is missing!\n");
+			return 0;
+		}
+
+		memcpy(obs, rmi_dev->xport->attn_data, ob_len);
+
+		rmi_dev->xport->attn_data += ob_len;
+		rmi_dev->xport->attn_size -= ob_len;
+	} else {
+		/* Grab all of the data registers, and check them for data */
+		ret = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
+				     &obs, ob_len);
+		if (ret) {
+			dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
+				__func__);
+			serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
+			return ret;
+		}
 	}
 
 	for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/13] Input: synaptics-rmi4 - allow to add attention data
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
                   ` (7 preceding siblings ...)
  2016-11-29 10:08 ` [PATCH 08/13] Input: synaptics-rmi4 - f03: grab data passed by transport device Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 10/13] Input: synaptics-rmi4 - store the attn data in the driver Benjamin Tissoires
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

The HID implementation of RMI4 provides the data during
the interrupt (in the input report). We need to provide
a way for this transport driver to provide the attention
data while calling an IRQ.

We use a fifo in rmi_core to not lose any incoming event.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_driver.c | 49 +++++++++++++++++++++++++++++++++++++++--
 include/linux/rmi.h             | 11 +++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index a718e51..85062e4 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -191,16 +191,53 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 	return 0;
 }
 
+void rmi_set_attn_data(struct rmi_device *rmi_dev, unsigned long irq_status,
+		       void *data, size_t size)
+{
+	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
+	struct rmi4_attn_data attn_data;
+	void *fifo_data;
+
+	if (!drvdata->enabled)
+		return;
+
+	fifo_data = kmemdup(data, size, GFP_ATOMIC);
+	if (!fifo_data)
+		return;
+
+	attn_data.irq_status = irq_status;
+	attn_data.size = size;
+	attn_data.data = fifo_data;
+
+	kfifo_put(&drvdata->attn_fifo, attn_data);
+}
+EXPORT_SYMBOL_GPL(rmi_set_attn_data);
+
 static irqreturn_t rmi_irq_fn(int irq, void *dev_id)
 {
 	struct rmi_device *rmi_dev = dev_id;
-	int ret;
+	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
+	struct rmi4_attn_data attn_data = {0};
+	int ret, count;
+
+	count = kfifo_get(&drvdata->attn_fifo, &attn_data);
+	if (count) {
+		*(drvdata->irq_status) = attn_data.irq_status;
+		rmi_dev->xport->attn_data = attn_data.data;
+		rmi_dev->xport->attn_size = attn_data.size;
+	}
 
 	ret = rmi_process_interrupt_requests(rmi_dev);
 	if (ret)
 		rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev,
 			"Failed to process interrupt request: %d\n", ret);
 
+	if (count)
+		kfree(attn_data.data);
+
+	if (!kfifo_is_empty(&drvdata->attn_fifo))
+		return rmi_irq_fn(irq, dev_id);
+
 	return IRQ_HANDLED;
 }
 
@@ -880,8 +917,9 @@ void rmi_disable_irq(struct rmi_device *rmi_dev, bool enable_wake)
 {
 	struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
 	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
+	struct rmi4_attn_data attn_data = {0};
 	int irq = pdata->irq;
-	int retval;
+	int retval, count;
 
 	mutex_lock(&data->enabled_mutex);
 
@@ -898,6 +936,13 @@ void rmi_disable_irq(struct rmi_device *rmi_dev, bool enable_wake)
 				 retval);
 	}
 
+	/* make sure the fifo is clean */
+	while (!kfifo_is_empty(&data->attn_fifo)) {
+		count = kfifo_get(&data->attn_fifo, &attn_data);
+		if (count)
+			kfree(attn_data.data);
+	}
+
 out:
 	mutex_unlock(&data->enabled_mutex);
 }
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 7780e40..1d48656 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -13,6 +13,7 @@
 #include <linux/device.h>
 #include <linux/interrupt.h>
 #include <linux/input.h>
+#include <linux/kfifo.h>
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/types.h>
@@ -331,6 +332,12 @@ struct rmi_device {
 
 };
 
+struct rmi4_attn_data {
+	unsigned long irq_status;
+	size_t size;
+	void *data;
+};
+
 struct rmi_driver_data {
 	struct list_head function_list;
 
@@ -357,11 +364,15 @@ struct rmi_driver_data {
 
 	bool enabled;
 	struct mutex enabled_mutex;
+	DECLARE_KFIFO(attn_fifo, struct rmi4_attn_data, 16);
 };
 
 int rmi_register_transport_device(struct rmi_transport_dev *xport);
 void rmi_unregister_transport_device(struct rmi_transport_dev *xport);
 
+void rmi_set_attn_data(struct rmi_device *rmi_dev, unsigned long irq_status,
+		       void *data, size_t size);
+
 int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake);
 int rmi_driver_resume(struct rmi_device *rmi_dev, bool clear_wake);
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/13] Input: synaptics-rmi4 - store the attn data in the driver
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
                   ` (8 preceding siblings ...)
  2016-11-29 10:08 ` [PATCH 09/13] Input: synaptics-rmi4 - allow to add attention data Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 11/13] HID: rmi: Make hid-rmi a transport driver for synaptics-rmi4 Benjamin Tissoires
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

Now that we have a proper API to set the attention data, there is
no point in keeping it in the transport driver.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_driver.c |  5 ++---
 drivers/input/rmi4/rmi_f03.c    | 13 +++++++------
 drivers/input/rmi4/rmi_f11.c    | 12 ++++++------
 drivers/input/rmi4/rmi_f12.c    | 43 +++++++++++++++++++++--------------------
 drivers/input/rmi4/rmi_f30.c    | 11 ++++++-----
 include/linux/rmi.h             |  5 ++---
 6 files changed, 45 insertions(+), 44 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 85062e4..05a3c4b 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -155,7 +155,7 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 	if (!data)
 		return 0;
 
-	if (!rmi_dev->xport->attn_data) {
+	if (!data->attn_data.data) {
 		error = rmi_read_block(rmi_dev,
 				data->f01_container->fd.data_base_addr + 1,
 				data->irq_status, data->num_of_irq_regs);
@@ -223,8 +223,7 @@ static irqreturn_t rmi_irq_fn(int irq, void *dev_id)
 	count = kfifo_get(&drvdata->attn_fifo, &attn_data);
 	if (count) {
 		*(drvdata->irq_status) = attn_data.irq_status;
-		rmi_dev->xport->attn_data = attn_data.data;
-		rmi_dev->xport->attn_size = attn_data.size;
+		drvdata->attn_data = attn_data;
 	}
 
 	ret = rmi_process_interrupt_requests(rmi_dev);
diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
index a124b33..b5454d3 100644
--- a/drivers/input/rmi4/rmi_f03.c
+++ b/drivers/input/rmi4/rmi_f03.c
@@ -160,6 +160,7 @@ static int rmi_f03_config(struct rmi_function *fn)
 static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
 {
 	struct rmi_device *rmi_dev = fn->rmi_dev;
+	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
 	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
 	u16 data_addr = fn->fd.data_base_addr;
 	const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE;
@@ -170,20 +171,20 @@ static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
 	int i;
 	int ret;
 
-	if (!rmi_dev || !rmi_dev->xport)
+	if (!rmi_dev)
 		return -ENODEV;
 
-	if (rmi_dev->xport->attn_data) {
+	if (drvdata->attn_data.data) {
 		/* First grab the data passed by the transport device */
-		if (rmi_dev->xport->attn_size < ob_len) {
+		if (drvdata->attn_data.size < ob_len) {
 			dev_warn(&fn->dev, "F03 interrupted, but data is missing!\n");
 			return 0;
 		}
 
-		memcpy(obs, rmi_dev->xport->attn_data, ob_len);
+		memcpy(obs, drvdata->attn_data.data, ob_len);
 
-		rmi_dev->xport->attn_data += ob_len;
-		rmi_dev->xport->attn_size -= ob_len;
+		drvdata->attn_data.data += ob_len;
+		drvdata->attn_data.size -= ob_len;
 	} else {
 		/* Grab all of the data registers, and check them for data */
 		ret = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 44fc275..bc5e37f 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -1284,19 +1284,19 @@ static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
 	int error;
 	int valid_bytes = f11->sensor.pkt_size;
 
-	if (rmi_dev->xport->attn_data) {
+	if (drvdata->attn_data.data) {
 		/*
 		 * The valid data in the attention report is less then
 		 * expected. Only process the complete fingers.
 		 */
-		if (f11->sensor.attn_size > rmi_dev->xport->attn_size)
-			valid_bytes = rmi_dev->xport->attn_size;
+		if (f11->sensor.attn_size > drvdata->attn_data.size)
+			valid_bytes = drvdata->attn_data.size;
 		else
 			valid_bytes = f11->sensor.attn_size;
-		memcpy(f11->sensor.data_pkt, rmi_dev->xport->attn_data,
+		memcpy(f11->sensor.data_pkt, drvdata->attn_data.data,
 			valid_bytes);
-		rmi_dev->xport->attn_data += f11->sensor.attn_size;
-		rmi_dev->xport->attn_size -= f11->sensor.attn_size;
+		drvdata->attn_data.data += f11->sensor.attn_size;
+		drvdata->attn_data.size -= f11->sensor.attn_size;
 	} else {
 		error = rmi_read_block(rmi_dev,
 				data_base_addr, f11->sensor.data_pkt,
diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 82a4964..8c5360c 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -208,19 +208,20 @@ static int rmi_f12_attention(struct rmi_function *fn,
 {
 	int retval;
 	struct rmi_device *rmi_dev = fn->rmi_dev;
+	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
 	struct f12_data *f12 = dev_get_drvdata(&fn->dev);
 	struct rmi_2d_sensor *sensor = &f12->sensor;
 	int valid_bytes = sensor->pkt_size;
 
-	if (rmi_dev->xport->attn_data) {
-		if (sensor->attn_size > rmi_dev->xport->attn_size)
-			valid_bytes = rmi_dev->xport->attn_size;
+	if (drvdata->attn_data.data) {
+		if (sensor->attn_size > drvdata->attn_data.size)
+			valid_bytes = drvdata->attn_data.size;
 		else
 			valid_bytes = sensor->attn_size;
-		memcpy(sensor->data_pkt, rmi_dev->xport->attn_data,
+		memcpy(sensor->data_pkt, drvdata->attn_data.data,
 			valid_bytes);
-		rmi_dev->xport->attn_data += sensor->attn_size;
-		rmi_dev->xport->attn_size -= sensor->attn_size;
+		drvdata->attn_data.data += sensor->attn_size;
+		drvdata->attn_data.size -= sensor->attn_size;
 	} else {
 		retval = rmi_read_block(rmi_dev, f12->data_addr,
 					sensor->data_pkt, sensor->pkt_size);
@@ -323,7 +324,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
 	const struct rmi_register_desc_item *item;
 	struct rmi_2d_sensor *sensor;
 	struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
-	struct rmi_transport_dev *xport = rmi_dev->xport;
+	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
 	u16 data_offset = 0;
 
 	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s\n", __func__);
@@ -422,7 +423,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
 	 * HID attention reports.
 	 */
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 0);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 1);
@@ -436,15 +437,15 @@ static int rmi_f12_probe(struct rmi_function *fn)
 	}
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 2);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 3);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 4);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 5);
@@ -456,22 +457,22 @@ static int rmi_f12_probe(struct rmi_function *fn)
 	}
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 6);
-	if (item && !xport->attn_data) {
+	if (item && !drvdata->attn_data.data) {
 		f12->data6 = item;
 		f12->data6_offset = data_offset;
 		data_offset += item->reg_size;
 	}
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 7);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 8);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 9);
-	if (item && !xport->attn_data) {
+	if (item && !drvdata->attn_data.data) {
 		f12->data9 = item;
 		f12->data9_offset = data_offset;
 		data_offset += item->reg_size;
@@ -480,27 +481,27 @@ static int rmi_f12_probe(struct rmi_function *fn)
 	}
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 10);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 11);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 12);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 13);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 14);
-	if (item && !xport->attn_data)
+	if (item && !drvdata->attn_data.data)
 		data_offset += item->reg_size;
 
 	item = rmi_get_register_desc_item(&f12->data_reg_desc, 15);
-	if (item && !xport->attn_data) {
+	if (item && !drvdata->attn_data.data) {
 		f12->data15 = item;
 		f12->data15_offset = data_offset;
 		data_offset += item->reg_size;
diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
index f696137..f4b491e 100644
--- a/drivers/input/rmi4/rmi_f30.c
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -99,6 +99,7 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
 {
 	struct f30_data *f30 = dev_get_drvdata(&fn->dev);
 	struct rmi_device *rmi_dev = fn->rmi_dev;
+	struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
 	int retval;
 	int gpiled = 0;
 	int value = 0;
@@ -109,15 +110,15 @@ static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
 		return 0;
 
 	/* Read the gpi led data. */
-	if (rmi_dev->xport->attn_data) {
-		if (rmi_dev->xport->attn_size < f30->register_count) {
+	if (drvdata->attn_data.data) {
+		if (drvdata->attn_data.size < f30->register_count) {
 			dev_warn(&fn->dev, "F30 interrupted, but data is missing\n");
 			return 0;
 		}
-		memcpy(f30->data_regs, rmi_dev->xport->attn_data,
+		memcpy(f30->data_regs, drvdata->attn_data.data,
 			f30->register_count);
-		rmi_dev->xport->attn_data += f30->register_count;
-		rmi_dev->xport->attn_size -= f30->register_count;
+		drvdata->attn_data.data += f30->register_count;
+		drvdata->attn_data.size -= f30->register_count;
 	} else {
 		retval = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
 			f30->data_regs, f30->register_count);
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 1d48656..ac910f7 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -272,9 +272,6 @@ struct rmi_transport_dev {
 	struct rmi_device_platform_data pdata;
 
 	struct input_dev *input;
-
-	void *attn_data;
-	int attn_size;
 };
 
 /**
@@ -364,6 +361,8 @@ struct rmi_driver_data {
 
 	bool enabled;
 	struct mutex enabled_mutex;
+
+	struct rmi4_attn_data attn_data;
 	DECLARE_KFIFO(attn_fifo, struct rmi4_attn_data, 16);
 };
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 11/13] HID: rmi: Make hid-rmi a transport driver for synaptics-rmi4
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
                   ` (9 preceding siblings ...)
  2016-11-29 10:08 ` [PATCH 10/13] Input: synaptics-rmi4 - store the attn data in the driver Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 12/13] HID: rmi: Handle all Synaptics touchpads using hid-rmi Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 13/13] HID: rmi: Support the Lenovo Thinkpad X1 Tablet dock " Benjamin Tissoires
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

From: Andrew Duggan <aduggan@synaptics.com>

The Synaptics RMI4 driver provides support for RMI4 devices. Instead of
duplicating the RMI4 processing code, make hid-rmi a transport driver
and register it with the Synaptics RMI4 core.

Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/Kconfig   |   4 +
 drivers/hid/hid-rmi.c | 973 ++++++++------------------------------------------
 2 files changed, 144 insertions(+), 833 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 3a4e6ed..26719e0 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -786,6 +786,10 @@ config HID_SUNPLUS
 config HID_RMI
 	tristate "Synaptics RMI4 device support"
 	depends on HID
+	select RMI4_CORE
+	select RMI4_F11
+	select RMI4_F12
+	select RMI4_F30
 	---help---
 	Support for Synaptics RMI4 touchpads.
 	Say Y here if you have a Synaptics RMI4 touchpads over i2c-hid or usbhid
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index be89bcb..57ed592 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -14,11 +14,13 @@
 #include <linux/hid.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
+#include <linux/irqdomain.h>
 #include <linux/module.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
 #include <linux/wait.h>
 #include <linux/sched.h>
+#include <linux/rmi.h>
 #include "hid-ids.h"
 
 #define RMI_MOUSE_REPORT_ID		0x01 /* Mouse emulation Report */
@@ -33,9 +35,6 @@
 #define RMI_READ_DATA_PENDING		1
 #define RMI_STARTED			2
 
-#define RMI_SLEEP_NORMAL		0x0
-#define RMI_SLEEP_DEEP_SLEEP		0x1
-
 /* device flags */
 #define RMI_DEVICE			BIT(0)
 #define RMI_DEVICE_HAS_PHYS_BUTTONS	BIT(1)
@@ -54,25 +53,12 @@ enum rmi_mode_type {
 	RMI_MODE_NO_PACKED_ATTN_REPORTS	= 2,
 };
 
-struct rmi_function {
-	unsigned page;			/* page of the function */
-	u16 query_base_addr;		/* base address for queries */
-	u16 command_base_addr;		/* base address for commands */
-	u16 control_base_addr;		/* base address for controls */
-	u16 data_base_addr;		/* base address for datas */
-	unsigned int interrupt_base;	/* cross-function interrupt number
-					 * (uniq in the device)*/
-	unsigned int interrupt_count;	/* number of interrupts */
-	unsigned int report_size;	/* size of a report */
-	unsigned long irq_mask;		/* mask of the interrupts
-					 * (to be applied against ATTN IRQ) */
-};
-
 /**
  * struct rmi_data - stores information for hid communication
  *
  * @page_mutex: Locks current page to avoid changing pages in unexpected ways.
  * @page: Keeps track of the current virtual page
+ * @xport: transport device to be registered with the RMI4 core.
  *
  * @wait: Used for waiting for read data
  *
@@ -84,26 +70,18 @@ struct rmi_function {
  *
  * @flags: flags for the current device (started, reading, etc...)
  *
- * @f11: placeholder of internal RMI function F11 description
- * @f30: placeholder of internal RMI function F30 description
- *
- * @max_fingers: maximum finger count reported by the device
- * @max_x: maximum x value reported by the device
- * @max_y: maximum y value reported by the device
- *
- * @gpio_led_count: count of GPIOs + LEDs reported by F30
- * @button_count: actual physical buttons count
- * @button_mask: button mask used to decode GPIO ATTN reports
- * @button_state_mask: pull state of the buttons
- *
- * @input: pointer to the kernel input device
- *
  * @reset_work: worker which will be called in case of a mouse report
  * @hdev: pointer to the struct hid_device
+ *
+ * @device_flags: flags which describe the device
+ *
+ * @domain: the IRQ domain allocated for this RMI4 device
+ * @rmi_irq: the irq that will be used to generate events to rmi-core
  */
 struct rmi_data {
 	struct mutex page_mutex;
 	int page;
+	struct rmi_transport_dev xport;
 
 	wait_queue_head_t wait;
 
@@ -115,34 +93,13 @@ struct rmi_data {
 
 	unsigned long flags;
 
-	struct rmi_function f01;
-	struct rmi_function f11;
-	struct rmi_function f30;
-
-	unsigned int max_fingers;
-	unsigned int max_x;
-	unsigned int max_y;
-	unsigned int x_size_mm;
-	unsigned int y_size_mm;
-	bool read_f11_ctrl_regs;
-	u8 f11_ctrl_regs[RMI_F11_CTRL_REG_COUNT];
-
-	unsigned int gpio_led_count;
-	unsigned int button_count;
-	unsigned long button_mask;
-	unsigned long button_state_mask;
-
-	struct input_dev *input;
-
 	struct work_struct reset_work;
 	struct hid_device *hdev;
 
 	unsigned long device_flags;
-	unsigned long firmware_id;
 
-	u8 f01_ctrl0;
-	u8 interrupt_enable_mask;
-	bool restore_interrupt_mask;
+	struct irq_domain *domain;
+	int rmi_irq;
 };
 
 #define RMI_PAGE(addr) (((addr) >> 8) & 0xff)
@@ -220,10 +177,11 @@ static int rmi_write_report(struct hid_device *hdev, u8 *report, int len)
 	return ret;
 }
 
-static int rmi_read_block(struct hid_device *hdev, u16 addr, void *buf,
-		const int len)
+static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
+		void *buf, size_t len)
 {
-	struct rmi_data *data = hid_get_drvdata(hdev);
+	struct rmi_data *data = container_of(xport, struct rmi_data, xport);
+	struct hid_device *hdev = data->hdev;
 	int ret;
 	int bytes_read;
 	int bytes_needed;
@@ -292,15 +250,11 @@ static int rmi_read_block(struct hid_device *hdev, u16 addr, void *buf,
 	return ret;
 }
 
-static inline int rmi_read(struct hid_device *hdev, u16 addr, void *buf)
-{
-	return rmi_read_block(hdev, addr, buf, 1);
-}
-
-static int rmi_write_block(struct hid_device *hdev, u16 addr, void *buf,
-		const int len)
+static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
+		const void *buf, size_t len)
 {
-	struct rmi_data *data = hid_get_drvdata(hdev);
+	struct rmi_data *data = container_of(xport, struct rmi_data, xport);
+	struct hid_device *hdev = data->hdev;
 	int ret;
 
 	mutex_lock(&data->page_mutex);
@@ -332,62 +286,20 @@ static int rmi_write_block(struct hid_device *hdev, u16 addr, void *buf,
 	return ret;
 }
 
-static inline int rmi_write(struct hid_device *hdev, u16 addr, void *buf)
-{
-	return rmi_write_block(hdev, addr, buf, 1);
-}
-
-static void rmi_f11_process_touch(struct rmi_data *hdata, int slot,
-		u8 finger_state, u8 *touch_data)
-{
-	int x, y, wx, wy;
-	int wide, major, minor;
-	int z;
-
-	input_mt_slot(hdata->input, slot);
-	input_mt_report_slot_state(hdata->input, MT_TOOL_FINGER,
-			finger_state == 0x01);
-	if (finger_state == 0x01) {
-		x = (touch_data[0] << 4) | (touch_data[2] & 0x0F);
-		y = (touch_data[1] << 4) | (touch_data[2] >> 4);
-		wx = touch_data[3] & 0x0F;
-		wy = touch_data[3] >> 4;
-		wide = (wx > wy);
-		major = max(wx, wy);
-		minor = min(wx, wy);
-		z = touch_data[4];
-
-		/* y is inverted */
-		y = hdata->max_y - y;
-
-		input_event(hdata->input, EV_ABS, ABS_MT_POSITION_X, x);
-		input_event(hdata->input, EV_ABS, ABS_MT_POSITION_Y, y);
-		input_event(hdata->input, EV_ABS, ABS_MT_ORIENTATION, wide);
-		input_event(hdata->input, EV_ABS, ABS_MT_PRESSURE, z);
-		input_event(hdata->input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
-		input_event(hdata->input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
-	}
-}
-
 static int rmi_reset_attn_mode(struct hid_device *hdev)
 {
 	struct rmi_data *data = hid_get_drvdata(hdev);
+	struct rmi_device *rmi_dev = data->xport.rmi_dev;
 	int ret;
 
 	ret = rmi_set_mode(hdev, RMI_MODE_ATTN_REPORTS);
 	if (ret)
 		return ret;
 
-	if (data->restore_interrupt_mask) {
-		ret = rmi_write(hdev, data->f01.control_base_addr + 1,
-				&data->interrupt_enable_mask);
-		if (ret) {
-			hid_err(hdev, "can not write F01 control register\n");
-			return ret;
-		}
-	}
+	if (test_bit(RMI_STARTED, &data->flags))
+		ret = rmi_dev->driver->reset_handler(rmi_dev);
 
-	return 0;
+	return ret;
 }
 
 static void rmi_reset_work(struct work_struct *work)
@@ -399,102 +311,22 @@ static void rmi_reset_work(struct work_struct *work)
 	rmi_reset_attn_mode(hdata->hdev);
 }
 
-static inline int rmi_schedule_reset(struct hid_device *hdev)
-{
-	struct rmi_data *hdata = hid_get_drvdata(hdev);
-	return schedule_work(&hdata->reset_work);
-}
-
-static int rmi_f11_input_event(struct hid_device *hdev, u8 irq, u8 *data,
-		int size)
-{
-	struct rmi_data *hdata = hid_get_drvdata(hdev);
-	int offset;
-	int i;
-
-	if (!(irq & hdata->f11.irq_mask) || size <= 0)
-		return 0;
-
-	offset = (hdata->max_fingers >> 2) + 1;
-	for (i = 0; i < hdata->max_fingers; i++) {
-		int fs_byte_position = i >> 2;
-		int fs_bit_position = (i & 0x3) << 1;
-		int finger_state = (data[fs_byte_position] >> fs_bit_position) &
-					0x03;
-		int position = offset + 5 * i;
-
-		if (position + 5 > size) {
-			/* partial report, go on with what we received */
-			printk_once(KERN_WARNING
-				"%s %s: Detected incomplete finger report. Finger reports may occasionally get dropped on this platform.\n",
-				 dev_driver_string(&hdev->dev),
-				 dev_name(&hdev->dev));
-			hid_dbg(hdev, "Incomplete finger report\n");
-			break;
-		}
-
-		rmi_f11_process_touch(hdata, i, finger_state, &data[position]);
-	}
-	input_mt_sync_frame(hdata->input);
-	input_sync(hdata->input);
-	return hdata->f11.report_size;
-}
-
-static int rmi_f30_input_event(struct hid_device *hdev, u8 irq, u8 *data,
-		int size)
+static int rmi_input_event(struct hid_device *hdev, u8 *data, int size)
 {
 	struct rmi_data *hdata = hid_get_drvdata(hdev);
-	int i;
-	int button = 0;
-	bool value;
+	struct rmi_device *rmi_dev = hdata->xport.rmi_dev;
+	unsigned long flags;
 
-	if (!(irq & hdata->f30.irq_mask))
+	if (!(test_bit(RMI_STARTED, &hdata->flags)))
 		return 0;
 
-	if (size < (int)hdata->f30.report_size) {
-		hid_warn(hdev, "Click Button pressed, but the click data is missing\n");
-		return 0;
-	}
+	local_irq_save(flags);
 
-	for (i = 0; i < hdata->gpio_led_count; i++) {
-		if (test_bit(i, &hdata->button_mask)) {
-			value = (data[i / 8] >> (i & 0x07)) & BIT(0);
-			if (test_bit(i, &hdata->button_state_mask))
-				value = !value;
-			input_event(hdata->input, EV_KEY, BTN_LEFT + button++,
-					value);
-		}
-	}
-	return hdata->f30.report_size;
-}
-
-static int rmi_input_event(struct hid_device *hdev, u8 *data, int size)
-{
-	struct rmi_data *hdata = hid_get_drvdata(hdev);
-	unsigned long irq_mask = 0;
-	unsigned index = 2;
+	rmi_set_attn_data(rmi_dev, data[1], &data[2], size - 2);
 
-	if (!(test_bit(RMI_STARTED, &hdata->flags)))
-		return 0;
+	generic_handle_irq(hdata->rmi_irq);
 
-	irq_mask |= hdata->f11.irq_mask;
-	irq_mask |= hdata->f30.irq_mask;
-
-	if (data[1] & ~irq_mask)
-		hid_dbg(hdev, "unknown intr source:%02lx %s:%d\n",
-			data[1] & ~irq_mask, __FILE__, __LINE__);
-
-	if (hdata->f11.interrupt_base < hdata->f30.interrupt_base) {
-		index += rmi_f11_input_event(hdev, data[1], &data[index],
-				size - index);
-		index += rmi_f30_input_event(hdev, data[1], &data[index],
-				size - index);
-	} else {
-		index += rmi_f30_input_event(hdev, data[1], &data[index],
-				size - index);
-		index += rmi_f11_input_event(hdev, data[1], &data[index],
-				size - index);
-	}
+	local_irq_restore(flags);
 
 	return 1;
 }
@@ -568,7 +400,7 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field,
 				return 1;
 		}
 
-		rmi_schedule_reset(hdev);
+		schedule_work(&data->reset_work);
 		return 1;
 	}
 
@@ -576,637 +408,71 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field,
 }
 
 #ifdef CONFIG_PM
-static int rmi_set_sleep_mode(struct hid_device *hdev, int sleep_mode)
-{
-	struct rmi_data *data = hid_get_drvdata(hdev);
-	int ret;
-	u8 f01_ctrl0;
-
-	f01_ctrl0 = (data->f01_ctrl0 & ~0x3) | sleep_mode;
-
-	ret = rmi_write(hdev, data->f01.control_base_addr,
-			&f01_ctrl0);
-	if (ret) {
-		hid_err(hdev, "can not write sleep mode\n");
-		return ret;
-	}
-
-	return 0;
-}
-
 static int rmi_suspend(struct hid_device *hdev, pm_message_t message)
 {
 	struct rmi_data *data = hid_get_drvdata(hdev);
-	int ret;
-	u8 buf[RMI_F11_CTRL_REG_COUNT];
-
-	if (!(data->device_flags & RMI_DEVICE))
-		return 0;
-
-	ret = rmi_read_block(hdev, data->f11.control_base_addr, buf,
-				RMI_F11_CTRL_REG_COUNT);
-	if (ret)
-		hid_warn(hdev, "can not read F11 control registers\n");
-	else
-		memcpy(data->f11_ctrl_regs, buf, RMI_F11_CTRL_REG_COUNT);
-
-
-	if (!device_may_wakeup(hdev->dev.parent))
-		return rmi_set_sleep_mode(hdev, RMI_SLEEP_DEEP_SLEEP);
-
-	return 0;
-}
-
-static int rmi_post_reset(struct hid_device *hdev)
-{
-	struct rmi_data *data = hid_get_drvdata(hdev);
+	struct rmi_device *rmi_dev = data->xport.rmi_dev;
 	int ret;
 
 	if (!(data->device_flags & RMI_DEVICE))
 		return 0;
 
-	ret = rmi_reset_attn_mode(hdev);
+	ret = rmi_driver_suspend(rmi_dev, false);
 	if (ret) {
-		hid_err(hdev, "can not set rmi mode\n");
+		hid_warn(hdev, "Failed to suspend device: %d\n", ret);
 		return ret;
 	}
 
-	if (data->read_f11_ctrl_regs) {
-		ret = rmi_write_block(hdev, data->f11.control_base_addr,
-				data->f11_ctrl_regs, RMI_F11_CTRL_REG_COUNT);
-		if (ret)
-			hid_warn(hdev,
-				"can not write F11 control registers after reset\n");
-	}
-
-	if (!device_may_wakeup(hdev->dev.parent)) {
-		ret = rmi_set_sleep_mode(hdev, RMI_SLEEP_NORMAL);
-		if (ret) {
-			hid_err(hdev, "can not write sleep mode\n");
-			return ret;
-		}
-	}
-
-	return ret;
+	return 0;
 }
 
 static int rmi_post_resume(struct hid_device *hdev)
 {
 	struct rmi_data *data = hid_get_drvdata(hdev);
+	struct rmi_device *rmi_dev = data->xport.rmi_dev;
+	int ret;
 
 	if (!(data->device_flags & RMI_DEVICE))
 		return 0;
 
-	return rmi_reset_attn_mode(hdev);
-}
-#endif /* CONFIG_PM */
-
-#define RMI4_MAX_PAGE 0xff
-#define RMI4_PAGE_SIZE 0x0100
-
-#define PDT_START_SCAN_LOCATION 0x00e9
-#define PDT_END_SCAN_LOCATION	0x0005
-#define RMI4_END_OF_PDT(id) ((id) == 0x00 || (id) == 0xff)
-
-struct pdt_entry {
-	u8 query_base_addr:8;
-	u8 command_base_addr:8;
-	u8 control_base_addr:8;
-	u8 data_base_addr:8;
-	u8 interrupt_source_count:3;
-	u8 bits3and4:2;
-	u8 function_version:2;
-	u8 bit7:1;
-	u8 function_number:8;
-} __attribute__((__packed__));
-
-static inline unsigned long rmi_gen_mask(unsigned irq_base, unsigned irq_count)
-{
-	return GENMASK(irq_count + irq_base - 1, irq_base);
-}
-
-static void rmi_register_function(struct rmi_data *data,
-	struct pdt_entry *pdt_entry, int page, unsigned interrupt_count)
-{
-	struct rmi_function *f = NULL;
-	u16 page_base = page << 8;
-
-	switch (pdt_entry->function_number) {
-	case 0x01:
-		f = &data->f01;
-		break;
-	case 0x11:
-		f = &data->f11;
-		break;
-	case 0x30:
-		f = &data->f30;
-		break;
-	}
-
-	if (f) {
-		f->page = page;
-		f->query_base_addr = page_base | pdt_entry->query_base_addr;
-		f->command_base_addr = page_base | pdt_entry->command_base_addr;
-		f->control_base_addr = page_base | pdt_entry->control_base_addr;
-		f->data_base_addr = page_base | pdt_entry->data_base_addr;
-		f->interrupt_base = interrupt_count;
-		f->interrupt_count = pdt_entry->interrupt_source_count;
-		f->irq_mask = rmi_gen_mask(f->interrupt_base,
-						f->interrupt_count);
-		data->interrupt_enable_mask |= f->irq_mask;
-	}
-}
-
-static int rmi_scan_pdt(struct hid_device *hdev)
-{
-	struct rmi_data *data = hid_get_drvdata(hdev);
-	struct pdt_entry entry;
-	int page;
-	bool page_has_function;
-	int i;
-	int retval;
-	int interrupt = 0;
-	u16 page_start, pdt_start , pdt_end;
-
-	hid_info(hdev, "Scanning PDT...\n");
-
-	for (page = 0; (page <= RMI4_MAX_PAGE); page++) {
-		page_start = RMI4_PAGE_SIZE * page;
-		pdt_start = page_start + PDT_START_SCAN_LOCATION;
-		pdt_end = page_start + PDT_END_SCAN_LOCATION;
-
-		page_has_function = false;
-		for (i = pdt_start; i >= pdt_end; i -= sizeof(entry)) {
-			retval = rmi_read_block(hdev, i, &entry, sizeof(entry));
-			if (retval) {
-				hid_err(hdev,
-					"Read of PDT entry at %#06x failed.\n",
-					i);
-				goto error_exit;
-			}
-
-			if (RMI4_END_OF_PDT(entry.function_number))
-				break;
-
-			page_has_function = true;
-
-			hid_info(hdev, "Found F%02X on page %#04x\n",
-					entry.function_number, page);
-
-			rmi_register_function(data, &entry, page, interrupt);
-			interrupt += entry.interrupt_source_count;
-		}
-
-		if (!page_has_function)
-			break;
-	}
-
-	hid_info(hdev, "%s: Done with PDT scan.\n", __func__);
-	retval = 0;
-
-error_exit:
-	return retval;
-}
-
-#define RMI_DEVICE_F01_BASIC_QUERY_LEN	11
-
-static int rmi_populate_f01(struct hid_device *hdev)
-{
-	struct rmi_data *data = hid_get_drvdata(hdev);
-	u8 basic_queries[RMI_DEVICE_F01_BASIC_QUERY_LEN];
-	u8 info[3];
-	int ret;
-	bool has_query42;
-	bool has_lts;
-	bool has_sensor_id;
-	bool has_ds4_queries = false;
-	bool has_build_id_query = false;
-	bool has_package_id_query = false;
-	u16 query_offset = data->f01.query_base_addr;
-	u16 prod_info_addr;
-	u8 ds4_query_len;
-
-	ret = rmi_read_block(hdev, query_offset, basic_queries,
-				RMI_DEVICE_F01_BASIC_QUERY_LEN);
-	if (ret) {
-		hid_err(hdev, "Can not read basic queries from Function 0x1.\n");
-		return ret;
-	}
-
-	has_lts = !!(basic_queries[0] & BIT(2));
-	has_sensor_id = !!(basic_queries[1] & BIT(3));
-	has_query42 = !!(basic_queries[1] & BIT(7));
-
-	query_offset += 11;
-	prod_info_addr = query_offset + 6;
-	query_offset += 10;
-
-	if (has_lts)
-		query_offset += 20;
-
-	if (has_sensor_id)
-		query_offset++;
-
-	if (has_query42) {
-		ret = rmi_read(hdev, query_offset, info);
-		if (ret) {
-			hid_err(hdev, "Can not read query42.\n");
-			return ret;
-		}
-		has_ds4_queries = !!(info[0] & BIT(0));
-		query_offset++;
-	}
-
-	if (has_ds4_queries) {
-		ret = rmi_read(hdev, query_offset, &ds4_query_len);
-		if (ret) {
-			hid_err(hdev, "Can not read DS4 Query length.\n");
-			return ret;
-		}
-		query_offset++;
-
-		if (ds4_query_len > 0) {
-			ret = rmi_read(hdev, query_offset, info);
-			if (ret) {
-				hid_err(hdev, "Can not read DS4 query.\n");
-				return ret;
-			}
-
-			has_package_id_query = !!(info[0] & BIT(0));
-			has_build_id_query = !!(info[0] & BIT(1));
-		}
-	}
-
-	if (has_package_id_query)
-		prod_info_addr++;
-
-	if (has_build_id_query) {
-		ret = rmi_read_block(hdev, prod_info_addr, info, 3);
-		if (ret) {
-			hid_err(hdev, "Can not read product info.\n");
-			return ret;
-		}
-
-		data->firmware_id = info[1] << 8 | info[0];
-		data->firmware_id += info[2] * 65536;
-	}
-
-	ret = rmi_read_block(hdev, data->f01.control_base_addr, info,
-				2);
-
-	if (ret) {
-		hid_err(hdev, "can not read f01 ctrl registers\n");
-		return ret;
-	}
-
-	data->f01_ctrl0 = info[0];
-
-	if (!info[1]) {
-		/*
-		 * Do to a firmware bug in some touchpads the F01 interrupt
-		 * enable control register will be cleared on reset.
-		 * This will stop the touchpad from reporting data, so
-		 * if F01 CTRL1 is 0 then we need to explicitly enable
-		 * interrupts for the functions we want data for.
-		 */
-		data->restore_interrupt_mask = true;
-
-		ret = rmi_write(hdev, data->f01.control_base_addr + 1,
-				&data->interrupt_enable_mask);
-		if (ret) {
-			hid_err(hdev, "can not write to control reg 1: %d.\n",
-				ret);
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
-static int rmi_populate_f11(struct hid_device *hdev)
-{
-	struct rmi_data *data = hid_get_drvdata(hdev);
-	u8 buf[20];
-	int ret;
-	bool has_query9;
-	bool has_query10 = false;
-	bool has_query11;
-	bool has_query12;
-	bool has_query27;
-	bool has_query28;
-	bool has_query36 = false;
-	bool has_physical_props;
-	bool has_gestures;
-	bool has_rel;
-	bool has_data40 = false;
-	bool has_dribble = false;
-	bool has_palm_detect = false;
-	unsigned x_size, y_size;
-	u16 query_offset;
-
-	if (!data->f11.query_base_addr) {
-		hid_err(hdev, "No 2D sensor found, giving up.\n");
-		return -ENODEV;
-	}
-
-	/* query 0 contains some useful information */
-	ret = rmi_read(hdev, data->f11.query_base_addr, buf);
-	if (ret) {
-		hid_err(hdev, "can not get query 0: %d.\n", ret);
-		return ret;
-	}
-	has_query9 = !!(buf[0] & BIT(3));
-	has_query11 = !!(buf[0] & BIT(4));
-	has_query12 = !!(buf[0] & BIT(5));
-	has_query27 = !!(buf[0] & BIT(6));
-	has_query28 = !!(buf[0] & BIT(7));
-
-	/* query 1 to get the max number of fingers */
-	ret = rmi_read(hdev, data->f11.query_base_addr + 1, buf);
-	if (ret) {
-		hid_err(hdev, "can not get NumberOfFingers: %d.\n", ret);
-		return ret;
-	}
-	data->max_fingers = (buf[0] & 0x07) + 1;
-	if (data->max_fingers > 5)
-		data->max_fingers = 10;
-
-	data->f11.report_size = data->max_fingers * 5 +
-				DIV_ROUND_UP(data->max_fingers, 4);
-
-	if (!(buf[0] & BIT(4))) {
-		hid_err(hdev, "No absolute events, giving up.\n");
-		return -ENODEV;
-	}
-
-	has_rel = !!(buf[0] & BIT(3));
-	has_gestures = !!(buf[0] & BIT(5));
-
-	ret = rmi_read(hdev, data->f11.query_base_addr + 5, buf);
-	if (ret) {
-		hid_err(hdev, "can not get absolute data sources: %d.\n", ret);
+	ret = rmi_reset_attn_mode(hdev);
+	if (ret)
 		return ret;
-	}
-
-	has_dribble = !!(buf[0] & BIT(4));
-
-	/*
-	 * At least 4 queries are guaranteed to be present in F11
-	 * +1 for query 5 which is present since absolute events are
-	 * reported and +1 for query 12.
-	 */
-	query_offset = 6;
-
-	if (has_rel)
-		++query_offset; /* query 6 is present */
-
-	if (has_gestures) {
-		/* query 8 to find out if query 10 exists */
-		ret = rmi_read(hdev,
-			data->f11.query_base_addr + query_offset + 1, buf);
-		if (ret) {
-			hid_err(hdev, "can not read gesture information: %d.\n",
-				ret);
-			return ret;
-		}
-		has_palm_detect = !!(buf[0] & BIT(0));
-		has_query10 = !!(buf[0] & BIT(2));
-
-		query_offset += 2; /* query 7 and 8 are present */
-	}
-
-	if (has_query9)
-		++query_offset;
-
-	if (has_query10)
-		++query_offset;
-
-	if (has_query11)
-		++query_offset;
-
-	/* query 12 to know if the physical properties are reported */
-	if (has_query12) {
-		ret = rmi_read(hdev, data->f11.query_base_addr
-				+ query_offset, buf);
-		if (ret) {
-			hid_err(hdev, "can not get query 12: %d.\n", ret);
-			return ret;
-		}
-		has_physical_props = !!(buf[0] & BIT(5));
-
-		if (has_physical_props) {
-			query_offset += 1;
-			ret = rmi_read_block(hdev,
-					data->f11.query_base_addr
-						+ query_offset, buf, 4);
-			if (ret) {
-				hid_err(hdev, "can not read query 15-18: %d.\n",
-					ret);
-				return ret;
-			}
-
-			x_size = buf[0] | (buf[1] << 8);
-			y_size = buf[2] | (buf[3] << 8);
-
-			data->x_size_mm = DIV_ROUND_CLOSEST(x_size, 10);
-			data->y_size_mm = DIV_ROUND_CLOSEST(y_size, 10);
-
-			hid_info(hdev, "%s: size in mm: %d x %d\n",
-				 __func__, data->x_size_mm, data->y_size_mm);
-
-			/*
-			 * query 15 - 18 contain the size of the sensor
-			 * and query 19 - 26 contain bezel dimensions
-			 */
-			query_offset += 12;
-		}
-	}
-
-	if (has_query27)
-		++query_offset;
 
-	if (has_query28) {
-		ret = rmi_read(hdev, data->f11.query_base_addr
-				+ query_offset, buf);
-		if (ret) {
-			hid_err(hdev, "can not get query 28: %d.\n", ret);
-			return ret;
-		}
-
-		has_query36 = !!(buf[0] & BIT(6));
-	}
-
-	if (has_query36) {
-		query_offset += 2;
-		ret = rmi_read(hdev, data->f11.query_base_addr
-				+ query_offset, buf);
-		if (ret) {
-			hid_err(hdev, "can not get query 36: %d.\n", ret);
-			return ret;
-		}
-
-		has_data40 = !!(buf[0] & BIT(5));
-	}
-
-
-	if (has_data40)
-		data->f11.report_size += data->max_fingers * 2;
-
-	ret = rmi_read_block(hdev, data->f11.control_base_addr,
-			data->f11_ctrl_regs, RMI_F11_CTRL_REG_COUNT);
+	ret = rmi_driver_resume(rmi_dev, false);
 	if (ret) {
-		hid_err(hdev, "can not read ctrl block of size 11: %d.\n", ret);
+		hid_warn(hdev, "Failed to resume device: %d\n", ret);
 		return ret;
 	}
 
-	/* data->f11_ctrl_regs now contains valid register data */
-	data->read_f11_ctrl_regs = true;
-
-	data->max_x = data->f11_ctrl_regs[6] | (data->f11_ctrl_regs[7] << 8);
-	data->max_y = data->f11_ctrl_regs[8] | (data->f11_ctrl_regs[9] << 8);
-
-	if (has_dribble) {
-		data->f11_ctrl_regs[0] = data->f11_ctrl_regs[0] & ~BIT(6);
-		ret = rmi_write(hdev, data->f11.control_base_addr,
-				data->f11_ctrl_regs);
-		if (ret) {
-			hid_err(hdev, "can not write to control reg 0: %d.\n",
-				ret);
-			return ret;
-		}
-	}
-
-	if (has_palm_detect) {
-		data->f11_ctrl_regs[11] = data->f11_ctrl_regs[11] & ~BIT(0);
-		ret = rmi_write(hdev, data->f11.control_base_addr + 11,
-				&data->f11_ctrl_regs[11]);
-		if (ret) {
-			hid_err(hdev, "can not write to control reg 11: %d.\n",
-				ret);
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
-static int rmi_populate_f30(struct hid_device *hdev)
-{
-	struct rmi_data *data = hid_get_drvdata(hdev);
-	u8 buf[20];
-	int ret;
-	bool has_gpio, has_led;
-	unsigned bytes_per_ctrl;
-	u8 ctrl2_addr;
-	int ctrl2_3_length;
-	int i;
-
-	/* function F30 is for physical buttons */
-	if (!data->f30.query_base_addr) {
-		hid_err(hdev, "No GPIO/LEDs found, giving up.\n");
-		return -ENODEV;
-	}
-
-	ret = rmi_read_block(hdev, data->f30.query_base_addr, buf, 2);
-	if (ret) {
-		hid_err(hdev, "can not get F30 query registers: %d.\n", ret);
-		return ret;
-	}
-
-	has_gpio = !!(buf[0] & BIT(3));
-	has_led = !!(buf[0] & BIT(2));
-	data->gpio_led_count = buf[1] & 0x1f;
-
-	/* retrieve ctrl 2 & 3 registers */
-	bytes_per_ctrl = (data->gpio_led_count + 7) / 8;
-	/* Ctrl0 is present only if both has_gpio and has_led are set*/
-	ctrl2_addr = (has_gpio && has_led) ? bytes_per_ctrl : 0;
-	/* Ctrl1 is always be present */
-	ctrl2_addr += bytes_per_ctrl;
-	ctrl2_3_length = 2 * bytes_per_ctrl;
-
-	data->f30.report_size = bytes_per_ctrl;
-
-	ret = rmi_read_block(hdev, data->f30.control_base_addr + ctrl2_addr,
-				buf, ctrl2_3_length);
-	if (ret) {
-		hid_err(hdev, "can not read ctrl 2&3 block of size %d: %d.\n",
-			ctrl2_3_length, ret);
-		return ret;
-	}
-
-	for (i = 0; i < data->gpio_led_count; i++) {
-		int byte_position = i >> 3;
-		int bit_position = i & 0x07;
-		u8 dir_byte = buf[byte_position];
-		u8 data_byte = buf[byte_position + bytes_per_ctrl];
-		bool dir = (dir_byte >> bit_position) & BIT(0);
-		bool dat = (data_byte >> bit_position) & BIT(0);
-
-		if (dir == 0) {
-			/* input mode */
-			if (dat) {
-				/* actual buttons have pull up resistor */
-				data->button_count++;
-				set_bit(i, &data->button_mask);
-				set_bit(i, &data->button_state_mask);
-			}
-		}
-
-	}
-
 	return 0;
 }
+#endif /* CONFIG_PM */
 
-static int rmi_populate(struct hid_device *hdev)
+static int rmi_hid_reset(struct rmi_transport_dev *xport, u16 reset_addr)
 {
-	struct rmi_data *data = hid_get_drvdata(hdev);
-	int ret;
-
-	ret = rmi_scan_pdt(hdev);
-	if (ret) {
-		hid_err(hdev, "PDT scan failed with code %d.\n", ret);
-		return ret;
-	}
-
-	ret = rmi_populate_f01(hdev);
-	if (ret) {
-		hid_err(hdev, "Error while initializing F01 (%d).\n", ret);
-		return ret;
-	}
-
-	ret = rmi_populate_f11(hdev);
-	if (ret) {
-		hid_err(hdev, "Error while initializing F11 (%d).\n", ret);
-		return ret;
-	}
-
-	if (!(data->device_flags & RMI_DEVICE_HAS_PHYS_BUTTONS)) {
-		ret = rmi_populate_f30(hdev);
-		if (ret)
-			hid_warn(hdev, "Error while initializing F30 (%d).\n", ret);
-	}
+	struct rmi_data *data = container_of(xport, struct rmi_data, xport);
+	struct hid_device *hdev = data->hdev;
 
-	return 0;
+	return rmi_reset_attn_mode(hdev);
 }
 
 static int rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
 {
 	struct rmi_data *data = hid_get_drvdata(hdev);
 	struct input_dev *input = hi->input;
-	int ret;
-	int res_x, res_y, i;
+	int ret = 0;
+
+	if (!(data->device_flags & RMI_DEVICE))
+		return 0;
 
-	data->input = input;
+	data->xport.input = input;
 
 	hid_dbg(hdev, "Opening low level driver\n");
 	ret = hid_hw_open(hdev);
 	if (ret)
 		return ret;
 
-	if (!(data->device_flags & RMI_DEVICE))
-		return 0;
-
 	/* Allow incoming hid reports */
 	hid_device_io_start(hdev);
 
@@ -1222,40 +488,10 @@ static int rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
 		goto exit;
 	}
 
-	ret = rmi_populate(hdev);
-	if (ret)
-		goto exit;
-
-	hid_info(hdev, "firmware id: %ld\n", data->firmware_id);
-
-	__set_bit(EV_ABS, input->evbit);
-	input_set_abs_params(input, ABS_MT_POSITION_X, 1, data->max_x, 0, 0);
-	input_set_abs_params(input, ABS_MT_POSITION_Y, 1, data->max_y, 0, 0);
-
-	if (data->x_size_mm && data->y_size_mm) {
-		res_x = (data->max_x - 1) / data->x_size_mm;
-		res_y = (data->max_y - 1) / data->y_size_mm;
-
-		input_abs_set_res(input, ABS_MT_POSITION_X, res_x);
-		input_abs_set_res(input, ABS_MT_POSITION_Y, res_y);
-	}
-
-	input_set_abs_params(input, ABS_MT_ORIENTATION, 0, 1, 0, 0);
-	input_set_abs_params(input, ABS_MT_PRESSURE, 0, 0xff, 0, 0);
-	input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 0x0f, 0, 0);
-	input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 0x0f, 0, 0);
-
-	ret = input_mt_init_slots(input, data->max_fingers, INPUT_MT_POINTER);
-	if (ret < 0)
+	ret = rmi_register_transport_device(&data->xport);
+	if (ret < 0) {
+		dev_err(&hdev->dev, "failed to register transport driver\n");
 		goto exit;
-
-	if (data->button_count) {
-		__set_bit(EV_KEY, input->evbit);
-		for (i = 0; i < data->button_count; i++)
-			__set_bit(BTN_LEFT + i, input->keybit);
-
-		if (data->button_count == 1)
-			__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
 	}
 
 	set_bit(RMI_STARTED, &data->flags);
@@ -1304,6 +540,71 @@ static int rmi_check_valid_report_id(struct hid_device *hdev, unsigned type,
 	return 0;
 }
 
+static struct rmi_device_platform_data rmi_hid_pdata = {
+	.sensor_pdata = {
+		.sensor_type = rmi_sensor_touchpad,
+		.axis_align.flip_y = true,
+		.dribble = RMI_REG_STATE_ON,
+		.palm_detect = RMI_REG_STATE_OFF,
+	},
+};
+
+static const struct rmi_transport_ops hid_rmi_ops = {
+	.write_block	= rmi_hid_write_block,
+	.read_block	= rmi_hid_read_block,
+	.reset		= rmi_hid_reset,
+};
+
+static void rmi_irq_teardown(void *data)
+{
+	struct rmi_data *hdata = data;
+	struct irq_domain *domain = hdata->domain;
+
+	if (!domain)
+		return;
+
+	irq_dispose_mapping(irq_find_mapping(domain, 0));
+
+	irq_domain_remove(domain);
+	hdata->domain = NULL;
+	hdata->rmi_irq = 0;
+}
+
+static int rmi_irq_map(struct irq_domain *h, unsigned int virq,
+		       irq_hw_number_t hw_irq_num)
+{
+	irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops rmi_irq_ops = {
+	.map = rmi_irq_map,
+};
+
+static int rmi_setup_irq_domain(struct hid_device *hdev)
+{
+	struct rmi_data *hdata = hid_get_drvdata(hdev);
+	int ret;
+
+	hdata->domain = irq_domain_create_linear(hdev->dev.fwnode, 1,
+						 &rmi_irq_ops, hdata);
+	if (!hdata->domain)
+		return -ENOMEM;
+
+	ret = devm_add_action_or_reset(&hdev->dev, &rmi_irq_teardown, hdata);
+	if (ret)
+		return ret;
+
+	hdata->rmi_irq = irq_create_mapping(hdata->domain, 0);
+	if (hdata->rmi_irq <= 0) {
+		hid_err(hdev, "Can't allocate an IRQ\n");
+		return hdata->rmi_irq < 0 ? hdata->rmi_irq : -ENXIO;
+	}
+
+	return 0;
+}
+
 static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	struct rmi_data *data = NULL;
@@ -1365,8 +666,8 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	data->writeReport = devm_kzalloc(&hdev->dev, alloc_size, GFP_KERNEL);
 	if (!data->writeReport) {
-		ret = -ENOMEM;
-		return ret;
+		hid_err(hdev, "failed to allocate buffer for HID reports\n");
+		return -ENOMEM;
 	}
 
 	data->readReport = data->writeReport + data->output_report_size;
@@ -1375,6 +676,21 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	mutex_init(&data->page_mutex);
 
+	ret = rmi_setup_irq_domain(hdev);
+	if (ret) {
+		hid_err(hdev, "failed to allocate IRQ domain\n");
+		return ret;
+	}
+
+	if (data->device_flags & RMI_DEVICE_HAS_PHYS_BUTTONS)
+		rmi_hid_pdata.f30_data.disable = true;
+
+	data->xport.dev = hdev->dev.parent;
+	data->xport.pdata = rmi_hid_pdata;
+	data->xport.pdata.irq = data->rmi_irq;
+	data->xport.proto_name = "hid";
+	data->xport.ops = &hid_rmi_ops;
+
 start:
 	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
 	if (ret) {
@@ -1382,17 +698,6 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return ret;
 	}
 
-	if ((data->device_flags & RMI_DEVICE) &&
-	    !test_bit(RMI_STARTED, &data->flags))
-		/*
-		 * The device maybe in the bootloader if rmi_input_configured
-		 * failed to find F11 in the PDT. Print an error, but don't
-		 * return an error from rmi_probe so that hidraw will be
-		 * accessible from userspace. That way a userspace tool
-		 * can be used to reload working firmware on the touchpad.
-		 */
-		hid_err(hdev, "Device failed to be properly configured\n");
-
 	return 0;
 }
 
@@ -1401,6 +706,8 @@ static void rmi_remove(struct hid_device *hdev)
 	struct rmi_data *hdata = hid_get_drvdata(hdev);
 
 	clear_bit(RMI_STARTED, &hdata->flags);
+	cancel_work_sync(&hdata->reset_work);
+	rmi_unregister_transport_device(&hdata->xport);
 
 	hid_hw_stop(hdev);
 }
@@ -1425,7 +732,7 @@ static struct hid_driver rmi_driver = {
 #ifdef CONFIG_PM
 	.suspend		= rmi_suspend,
 	.resume			= rmi_post_resume,
-	.reset_resume		= rmi_post_reset,
+	.reset_resume		= rmi_post_resume,
 #endif
 };
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 12/13] HID: rmi: Handle all Synaptics touchpads using hid-rmi
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
                   ` (10 preceding siblings ...)
  2016-11-29 10:08 ` [PATCH 11/13] HID: rmi: Make hid-rmi a transport driver for synaptics-rmi4 Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  2016-11-29 10:08 ` [PATCH 13/13] HID: rmi: Support the Lenovo Thinkpad X1 Tablet dock " Benjamin Tissoires
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

From: Andrew Duggan <aduggan@synaptics.com>

With the addition of HID and F12 support in the synaptics-rmi4 driver
touchpads which had been using the hid-multitouch driver can now
be support by the synaptics-rmi4 via hid-rmi. The advantage is that
additional data can be reported from the RMI registers which is not
available in the Microsoft Precision Touchpad collection.

Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b43df2d..2da75d4 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -826,7 +826,8 @@ static int hid_scan_report(struct hid_device *hid)
 		hid->group = HID_GROUP_WACOM;
 		break;
 	case USB_VENDOR_ID_SYNAPTICS:
-		if (hid->group == HID_GROUP_GENERIC)
+		if (hid->group == HID_GROUP_GENERIC ||
+		    hid->group == HID_GROUP_MULTITOUCH_WIN_8)
 			if ((parser->scan_flags & HID_SCAN_FLAG_VENDOR_SPECIFIC)
 			    && (parser->scan_flags & HID_SCAN_FLAG_GD_POINTER))
 				/*
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 13/13] HID: rmi: Support the Lenovo Thinkpad X1 Tablet dock using hid-rmi
  2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
                   ` (11 preceding siblings ...)
  2016-11-29 10:08 ` [PATCH 12/13] HID: rmi: Handle all Synaptics touchpads using hid-rmi Benjamin Tissoires
@ 2016-11-29 10:08 ` Benjamin Tissoires
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-11-29 10:08 UTC (permalink / raw)
  To: Dmitry Torokhov, Jiri Kosina, Andrew Duggan, Lyude Paul,
	Nick Dyer, Dennis Wassenberg
  Cc: linux-input, linux-kernel

From: Andrew Duggan <aduggan@synaptics.com>

Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-core.c | 1 +
 drivers/hid/hid-ids.h  | 1 +
 drivers/hid/hid-rmi.c  | 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 2da75d4..e5356dc 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2125,6 +2125,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_WIIMOTE2) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_RAZER, USB_DEVICE_ID_RAZER_BLADE_14) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CMEDIA, USB_DEVICE_ID_CM6533) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X1_COVER) },
 	{ }
 };
 
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 222b966..a27a26c 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -622,6 +622,7 @@
 #define USB_DEVICE_ID_LENOVO_CUSBKBD	0x6047
 #define USB_DEVICE_ID_LENOVO_CBTKBD	0x6048
 #define USB_DEVICE_ID_LENOVO_TPPRODOCK	0x6067
+#define USB_DEVICE_ID_LENOVO_X1_COVER	0x6085
 
 #define USB_VENDOR_ID_LG		0x1fd2
 #define USB_DEVICE_ID_LG_MULTITOUCH	0x0064
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 57ed592..11825ad 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -715,6 +715,7 @@ static void rmi_remove(struct hid_device *hdev)
 static const struct hid_device_id rmi_id[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_RAZER, USB_DEVICE_ID_RAZER_BLADE_14),
 		.driver_data = RMI_DEVICE_HAS_PHYS_BUTTONS },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_X1_COVER) },
 	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_RMI, HID_ANY_ID, HID_ANY_ID) },
 	{ }
 };
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 07/13] Input: synaptics-rmi4 - add support for F03
  2016-11-29 10:08 ` [PATCH 07/13] Input: synaptics-rmi4 - add support for F03 Benjamin Tissoires
@ 2016-12-01  1:36   ` Dmitry Torokhov
  2016-12-01 14:54     ` Benjamin Tissoires
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2016-12-01  1:36 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Andrew Duggan, Lyude Paul, Nick Dyer,
	Dennis Wassenberg, linux-input, linux-kernel

Hi Benjamin,

On Tue, Nov 29, 2016 at 11:08:18AM +0100, Benjamin Tissoires wrote:
> From: Lyude Paul <thatslyude@gmail.com>
> 
> This adds basic functionality for PS/2 passthrough on Synaptics
> Touchpads using RMI4 through smbus.
> 
> Reviewed-by: Andrew Duggan <aduggan@synaptics.com>
> Signed-off-by: Lyude Paul <thatslyude@gmail.com>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> ---
>  drivers/input/mouse/psmouse-base.c |   6 +
>  drivers/input/rmi4/Kconfig         |   9 ++
>  drivers/input/rmi4/Makefile        |   1 +
>  drivers/input/rmi4/rmi_bus.c       |   3 +
>  drivers/input/rmi4/rmi_driver.h    |   1 +
>  drivers/input/rmi4/rmi_f03.c       | 227 +++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/serio.h         |   1 +
>  7 files changed, 248 insertions(+)
>  create mode 100644 drivers/input/rmi4/rmi_f03.c
> 
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index fb4b185..691dd74 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -1663,6 +1663,12 @@ static struct serio_device_id psmouse_serio_ids[] = {
>  		.id	= SERIO_ANY,
>  		.extra	= SERIO_ANY,
>  	},
> +	{
> +		.type	= SERIO_RMI_PSTHRU,

Why do we need new serio type here? We had SERIO_PS_PSTHRU because we
needed some quirks in how it interacted with the parent PS/2 port, but
here it seems SERIO_I8042 (which could be called SERIO_STANDARD_PS2)
would suffice?

> +		.proto	= SERIO_ANY,
> +		.id	= SERIO_ANY,
> +		.extra	= SERIO_ANY,
> +	},
>  	{ 0 }
>  };
>  
> diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
> index a9c36a5..b8189a3 100644
> --- a/drivers/input/rmi4/Kconfig
> +++ b/drivers/input/rmi4/Kconfig
> @@ -39,6 +39,15 @@ config RMI4_SMB
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called rmi_smbus.
>  
> +config RMI4_F03
> +        bool "RMI4 Function 03 (PS2 Guest)"
> +        depends on RMI4_CORE
> +        help
> +          Say Y here if you want to add support for RMI4 function 03.
> +
> +          Function 03 provides PS2 guest support for RMI4 devices. This
> +          includes support for TrackPoints on TouchPads.
> +
>  config RMI4_2D_SENSOR
>  	bool
>  	depends on RMI4_CORE
> diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
> index e7f4ca6..a199cbe 100644
> --- a/drivers/input/rmi4/Makefile
> +++ b/drivers/input/rmi4/Makefile
> @@ -4,6 +4,7 @@ rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
>  rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
>  
>  # Function drivers
> +rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o
>  rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
>  rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
>  rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> index 81be9c1..1c40d94 100644
> --- a/drivers/input/rmi4/rmi_bus.c
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -305,6 +305,9 @@ struct bus_type rmi_bus_type = {
>  
>  static struct rmi_function_handler *fn_handlers[] = {
>  	&rmi_f01_handler,
> +#ifdef CONFIG_RMI4_F03
> +	&rmi_f03_handler,
> +#endif
>  #ifdef CONFIG_RMI4_F11
>  	&rmi_f11_handler,
>  #endif
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index cc94585..24f8f76 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -121,6 +121,7 @@ static inline void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev)
>  #endif /* CONFIG_RMI_F34 */
>  
>  extern struct rmi_function_handler rmi_f01_handler;
> +extern struct rmi_function_handler rmi_f03_handler;
>  extern struct rmi_function_handler rmi_f11_handler;
>  extern struct rmi_function_handler rmi_f12_handler;
>  extern struct rmi_function_handler rmi_f30_handler;
> diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
> new file mode 100644
> index 0000000..a7e1b98
> --- /dev/null
> +++ b/drivers/input/rmi4/rmi_f03.c
> @@ -0,0 +1,227 @@
> +/*
> + * Copyright (C) 2015-2016 Red Hat
> + * Copyright (C) 2015 Lyude Paul <thatslyude@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/serio.h>
> +#include <linux/notifier.h>
> +#include "rmi_driver.h"
> +
> +#define RMI_F03_RX_DATA_OFB		0x01
> +#define RMI_F03_OB_SIZE			2
> +
> +#define RMI_F03_OB_OFFSET		2
> +#define RMI_F03_OB_DATA_OFFSET		1
> +#define RMI_F03_OB_FLAG_TIMEOUT		(1 << 6)
> +#define RMI_F03_OB_FLAG_PARITY		(1 << 7)

BIT()?

> +
> +#define RMI_F03_DEVICE_COUNT		0x07
> +#define RMI_F03_BYTES_PER_DEVICE_MASK	0x70
> +#define RMI_F03_BYTES_PER_DEVICE_SHIFT	4
> +#define RMI_F03_QUEUE_LENGTH		0x0F
> +
> +struct f03_data {
> +	struct rmi_function *fn;
> +
> +	struct serio *serio;
> +
> +	unsigned int overwrite_buttons;

Unused?

> +
> +	u8 device_count;
> +	u8 rx_queue_length;
> +};
> +
> +static int rmi_f03_pt_write(struct serio *id, unsigned char val)
> +{
> +	struct f03_data *f03 = id->port_data;
> +	int rc;
> +
> +	rmi_dbg(RMI_DEBUG_FN, &f03->fn->dev,
> +		"%s: Wrote %.2hhx to PS/2 passthrough address",
> +		__func__, val);
> +
> +	rc = rmi_write(f03->fn->rmi_dev, f03->fn->fd.data_base_addr, val);

error = rmi_write()?

> +	if (rc) {
> +		dev_err(&f03->fn->dev,
> +			"%s: Failed to write to F03 TX register.\n", __func__);

Please report error code as well.

> +		return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static inline int rmi_f03_initialize(struct rmi_function *fn)

Why inline?

> +{
> +	struct f03_data *f03;
> +	struct device *dev = &fn->dev;
> +	int rc;
> +	u8 bytes_per_device;
> +	u8 query1;
> +	size_t query2_len;
> +
> +	rc = rmi_read(fn->rmi_dev, fn->fd.query_base_addr, &query1);

error = rmi_read()?

> +	if (rc) {
> +		dev_err(dev, "Failed to read query register.\n");
> +		return rc;
> +	}
> +
> +	f03 = devm_kzalloc(dev, sizeof(struct f03_data), GFP_KERNEL);
> +	if (!f03)
> +		return -ENOMEM;
> +
> +	f03->device_count = query1 & RMI_F03_DEVICE_COUNT;
> +	bytes_per_device = (query1 & RMI_F03_BYTES_PER_DEVICE_MASK) >>
> +		RMI_F03_BYTES_PER_DEVICE_SHIFT;
> +
> +	query2_len = f03->device_count * bytes_per_device;
> +
> +	/*
> +	 * The first generation of image sensors don't have a second part to
> +	 * their f03 query, as such we have to set some of these values manually
> +	 */
> +	if (query2_len < 1) {
> +		f03->device_count = 1;
> +		f03->rx_queue_length = 7;
> +	} else {
> +		u8 query2[query2_len];
> +
> +		rc = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr + 1,
> +				    query2, query2_len);
> +		if (rc) {
> +			dev_err(dev, "Failed to read second set of query registers.\n");
> +			return rc;
> +		}
> +
> +		f03->rx_queue_length = query2[0] & RMI_F03_QUEUE_LENGTH;
> +	}
> +
> +	f03->fn = fn;
> +
> +	dev_set_drvdata(dev, f03);
> +
> +	return f03->device_count;

I'd rather we returned customary 0 here.

> +}
> +
> +static inline int rmi_f03_register_pt(struct rmi_function *fn)
> +{
> +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> +	struct serio *serio = kzalloc(sizeof(struct serio), GFP_KERNEL);

Please do not do allocations at declaration; limit to declarations with
initialization to operations without side effect. So:

	struct serio *serio;

	serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
	if (!serio)
		return -ENOMEM;

> +
> +	if (!serio)
> +		return -ENOMEM;
> +
> +	serio->id.type = SERIO_RMI_PSTHRU;
> +	serio->write = rmi_f03_pt_write;
> +	serio->port_data = f03;
> +
> +	strlcpy(serio->name, "Synaptics RMI4 PS2 pass-through",
> +		sizeof(serio->name));
> +	strlcpy(serio->phys, "synaptics-rmi4-pt/serio1",
> +		sizeof(serio->phys));
> +	 serio->dev.parent = &fn->dev;

Extra space after tab in indentation.

> +
> +	f03->serio = serio;
> +
> +	serio_register_port(serio);
> +
> +	return 0;
> +}
> +
> +static int rmi_f03_probe(struct rmi_function *fn)
> +{
> +	int rc;

int error;

Maybe allocate the memory here...

> +
> +	rc = rmi_f03_initialize(fn);
> +	if (rc < 0)
> +		return rc;
> +
> +	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%d devices on PS/2 passthrough", rc);

so you can use f03->device_count here.

Do we need to warn if we see device count greater than 1?

> +
> +	rc = rmi_f03_register_pt(fn);
> +	if (rc)
> +		return rc;
> +
> +	return 0;
> +}
> +
> +static int rmi_f03_config(struct rmi_function *fn)
> +{
> +	fn->rmi_dev->driver->set_irq_bits(fn->rmi_dev, fn->irq_mask);
> +
> +	return 0;
> +}
> +
> +static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
> +{
> +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> +	u16 data_addr = fn->fd.data_base_addr;
> +	const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE;
> +	u8 obs[ob_len];
> +	u8 ob_status;
> +	u8 ob_data;
> +	unsigned int serio_flags;
> +	int i;
> +	int retval;
> +
> +	/* Grab all of the data registers, and check them for data */
> +	retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
> +				&obs, ob_len);
> +	if (retval) {
> +		dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
> +			__func__);
> +		serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
> +		return retval;
> +	}
> +
> +	for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) {
> +		ob_status = obs[i];
> +		ob_data = obs[i + RMI_F03_OB_DATA_OFFSET];
> +		serio_flags = 0;
> +
> +		if (!(ob_status & RMI_F03_RX_DATA_OFB))
> +			continue;
> +
> +		if (ob_status & RMI_F03_OB_FLAG_TIMEOUT)
> +			serio_flags |= SERIO_TIMEOUT;
> +		if (ob_status & RMI_F03_OB_FLAG_PARITY)
> +			serio_flags |= SERIO_PARITY;
> +
> +		rmi_dbg(RMI_DEBUG_FN, &fn->dev,
> +			"%s: Received %.2hhx from PS2 guest T: %c P: %c\n",
> +			__func__, ob_data,
> +			serio_flags & SERIO_TIMEOUT ?  'Y' : 'N',
> +			serio_flags & SERIO_PARITY ? 'Y' : 'N');
> +
> +		serio_interrupt(f03->serio, ob_data, serio_flags);
> +	}
> +
> +	return 0;
> +}
> +
> +static void rmi_f03_remove(struct rmi_function *fn)
> +{
> +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> +
> +	serio_unregister_port(f03->serio);
> +}
> +
> +struct rmi_function_handler rmi_f03_handler = {
> +	.driver = {
> +		.name = "rmi4_f03",
> +	},
> +	.func = 0x03,
> +	.probe = rmi_f03_probe,
> +	.config = rmi_f03_config,
> +	.attention = rmi_f03_attention,
> +	.remove = rmi_f03_remove,
> +};
> +
> +MODULE_AUTHOR("Lyude Paul <thatslyude@gmail.com>");
> +MODULE_DESCRIPTION("RMI F03 module");
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/serio.h b/include/uapi/linux/serio.h
> index f2447a8..7012178 100644
> --- a/include/uapi/linux/serio.h
> +++ b/include/uapi/linux/serio.h
> @@ -30,6 +30,7 @@
>  #define SERIO_HIL_MLC	0x03
>  #define SERIO_PS_PSTHRU	0x05
>  #define SERIO_8042_XL	0x06
> +#define SERIO_RMI_PSTHRU	0x07
>  
>  /*
>   * Serio protocols
> -- 
> 2.7.4
> 

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 07/13] Input: synaptics-rmi4 - add support for F03
  2016-12-01  1:36   ` Dmitry Torokhov
@ 2016-12-01 14:54     ` Benjamin Tissoires
  0 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2016-12-01 14:54 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jiri Kosina, Andrew Duggan, Lyude Paul, Nick Dyer,
	Dennis Wassenberg, linux-input, linux-kernel

Hi Dmitry,

On Nov 30 2016 or thereabouts, Dmitry Torokhov wrote:
> Hi Benjamin,
> 
> On Tue, Nov 29, 2016 at 11:08:18AM +0100, Benjamin Tissoires wrote:
> > From: Lyude Paul <thatslyude@gmail.com>
> > 
> > This adds basic functionality for PS/2 passthrough on Synaptics
> > Touchpads using RMI4 through smbus.
> > 
> > Reviewed-by: Andrew Duggan <aduggan@synaptics.com>
> > Signed-off-by: Lyude Paul <thatslyude@gmail.com>
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > 
> > ---
> >  drivers/input/mouse/psmouse-base.c |   6 +
> >  drivers/input/rmi4/Kconfig         |   9 ++
> >  drivers/input/rmi4/Makefile        |   1 +
> >  drivers/input/rmi4/rmi_bus.c       |   3 +
> >  drivers/input/rmi4/rmi_driver.h    |   1 +
> >  drivers/input/rmi4/rmi_f03.c       | 227 +++++++++++++++++++++++++++++++++++++
> >  include/uapi/linux/serio.h         |   1 +
> >  7 files changed, 248 insertions(+)
> >  create mode 100644 drivers/input/rmi4/rmi_f03.c
> > 
> > diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> > index fb4b185..691dd74 100644
> > --- a/drivers/input/mouse/psmouse-base.c
> > +++ b/drivers/input/mouse/psmouse-base.c
> > @@ -1663,6 +1663,12 @@ static struct serio_device_id psmouse_serio_ids[] = {
> >  		.id	= SERIO_ANY,
> >  		.extra	= SERIO_ANY,
> >  	},
> > +	{
> > +		.type	= SERIO_RMI_PSTHRU,
> 
> Why do we need new serio type here? We had SERIO_PS_PSTHRU because we
> needed some quirks in how it interacted with the parent PS/2 port, but
> here it seems SERIO_I8042 (which could be called SERIO_STANDARD_PS2)
> would suffice?

I guess this must be some kind of left over from the time we were trying
to fix the suspend/resume interactions between this driver and the PS/2
connection of the rmi-smbus devices.

Given the rest of the code which seems to be indeed not dependent of
SERIO_RMI_PSTHRU, I'll switch over to SERIO_I8042.

> 
> > +		.proto	= SERIO_ANY,
> > +		.id	= SERIO_ANY,
> > +		.extra	= SERIO_ANY,
> > +	},
> >  	{ 0 }
> >  };
> >  
> > diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
> > index a9c36a5..b8189a3 100644
> > --- a/drivers/input/rmi4/Kconfig
> > +++ b/drivers/input/rmi4/Kconfig
> > @@ -39,6 +39,15 @@ config RMI4_SMB
> >  	  To compile this driver as a module, choose M here: the module will be
> >  	  called rmi_smbus.
> >  
> > +config RMI4_F03
> > +        bool "RMI4 Function 03 (PS2 Guest)"
> > +        depends on RMI4_CORE
> > +        help
> > +          Say Y here if you want to add support for RMI4 function 03.
> > +
> > +          Function 03 provides PS2 guest support for RMI4 devices. This
> > +          includes support for TrackPoints on TouchPads.
> > +
> >  config RMI4_2D_SENSOR
> >  	bool
> >  	depends on RMI4_CORE
> > diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
> > index e7f4ca6..a199cbe 100644
> > --- a/drivers/input/rmi4/Makefile
> > +++ b/drivers/input/rmi4/Makefile
> > @@ -4,6 +4,7 @@ rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
> >  rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o
> >  
> >  # Function drivers
> > +rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o
> >  rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
> >  rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o
> >  rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o
> > diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> > index 81be9c1..1c40d94 100644
> > --- a/drivers/input/rmi4/rmi_bus.c
> > +++ b/drivers/input/rmi4/rmi_bus.c
> > @@ -305,6 +305,9 @@ struct bus_type rmi_bus_type = {
> >  
> >  static struct rmi_function_handler *fn_handlers[] = {
> >  	&rmi_f01_handler,
> > +#ifdef CONFIG_RMI4_F03
> > +	&rmi_f03_handler,
> > +#endif
> >  #ifdef CONFIG_RMI4_F11
> >  	&rmi_f11_handler,
> >  #endif
> > diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> > index cc94585..24f8f76 100644
> > --- a/drivers/input/rmi4/rmi_driver.h
> > +++ b/drivers/input/rmi4/rmi_driver.h
> > @@ -121,6 +121,7 @@ static inline void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev)
> >  #endif /* CONFIG_RMI_F34 */
> >  
> >  extern struct rmi_function_handler rmi_f01_handler;
> > +extern struct rmi_function_handler rmi_f03_handler;
> >  extern struct rmi_function_handler rmi_f11_handler;
> >  extern struct rmi_function_handler rmi_f12_handler;
> >  extern struct rmi_function_handler rmi_f30_handler;
> > diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c
> > new file mode 100644
> > index 0000000..a7e1b98
> > --- /dev/null
> > +++ b/drivers/input/rmi4/rmi_f03.c
> > @@ -0,0 +1,227 @@
> > +/*
> > + * Copyright (C) 2015-2016 Red Hat
> > + * Copyright (C) 2015 Lyude Paul <thatslyude@gmail.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as published by
> > + * the Free Software Foundation.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/serio.h>
> > +#include <linux/notifier.h>
> > +#include "rmi_driver.h"
> > +
> > +#define RMI_F03_RX_DATA_OFB		0x01
> > +#define RMI_F03_OB_SIZE			2
> > +
> > +#define RMI_F03_OB_OFFSET		2
> > +#define RMI_F03_OB_DATA_OFFSET		1
> > +#define RMI_F03_OB_FLAG_TIMEOUT		(1 << 6)
> > +#define RMI_F03_OB_FLAG_PARITY		(1 << 7)
> 
> BIT()?
> 
> > +
> > +#define RMI_F03_DEVICE_COUNT		0x07
> > +#define RMI_F03_BYTES_PER_DEVICE_MASK	0x70
> > +#define RMI_F03_BYTES_PER_DEVICE_SHIFT	4
> > +#define RMI_F03_QUEUE_LENGTH		0x0F
> > +
> > +struct f03_data {
> > +	struct rmi_function *fn;
> > +
> > +	struct serio *serio;
> > +
> > +	unsigned int overwrite_buttons;
> 
> Unused?

Looks like it's a bad split between the F03 core implementation and the
tweaks we have to do for the trackstick "buttons-that-are-wired-into-the-
touchpad-instead-of-the-trackstick".

> 
> > +
> > +	u8 device_count;
> > +	u8 rx_queue_length;
> > +};
> > +
> > +static int rmi_f03_pt_write(struct serio *id, unsigned char val)
> > +{
> > +	struct f03_data *f03 = id->port_data;
> > +	int rc;
> > +
> > +	rmi_dbg(RMI_DEBUG_FN, &f03->fn->dev,
> > +		"%s: Wrote %.2hhx to PS/2 passthrough address",
> > +		__func__, val);
> > +
> > +	rc = rmi_write(f03->fn->rmi_dev, f03->fn->fd.data_base_addr, val);
> 
> error = rmi_write()?
> 
> > +	if (rc) {
> > +		dev_err(&f03->fn->dev,
> > +			"%s: Failed to write to F03 TX register.\n", __func__);
> 
> Please report error code as well.
> 
> > +		return rc;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static inline int rmi_f03_initialize(struct rmi_function *fn)
> 
> Why inline?
> 
> > +{
> > +	struct f03_data *f03;
> > +	struct device *dev = &fn->dev;
> > +	int rc;
> > +	u8 bytes_per_device;
> > +	u8 query1;
> > +	size_t query2_len;
> > +
> > +	rc = rmi_read(fn->rmi_dev, fn->fd.query_base_addr, &query1);
> 
> error = rmi_read()?
> 
> > +	if (rc) {
> > +		dev_err(dev, "Failed to read query register.\n");
> > +		return rc;
> > +	}
> > +
> > +	f03 = devm_kzalloc(dev, sizeof(struct f03_data), GFP_KERNEL);
> > +	if (!f03)
> > +		return -ENOMEM;
> > +
> > +	f03->device_count = query1 & RMI_F03_DEVICE_COUNT;
> > +	bytes_per_device = (query1 & RMI_F03_BYTES_PER_DEVICE_MASK) >>
> > +		RMI_F03_BYTES_PER_DEVICE_SHIFT;
> > +
> > +	query2_len = f03->device_count * bytes_per_device;
> > +
> > +	/*
> > +	 * The first generation of image sensors don't have a second part to
> > +	 * their f03 query, as such we have to set some of these values manually
> > +	 */
> > +	if (query2_len < 1) {
> > +		f03->device_count = 1;
> > +		f03->rx_queue_length = 7;
> > +	} else {
> > +		u8 query2[query2_len];
> > +
> > +		rc = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr + 1,
> > +				    query2, query2_len);
> > +		if (rc) {
> > +			dev_err(dev, "Failed to read second set of query registers.\n");
> > +			return rc;
> > +		}
> > +
> > +		f03->rx_queue_length = query2[0] & RMI_F03_QUEUE_LENGTH;
> > +	}
> > +
> > +	f03->fn = fn;
> > +
> > +	dev_set_drvdata(dev, f03);
> > +
> > +	return f03->device_count;
> 
> I'd rather we returned customary 0 here.
> 
> > +}
> > +
> > +static inline int rmi_f03_register_pt(struct rmi_function *fn)
> > +{
> > +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> > +	struct serio *serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
> 
> Please do not do allocations at declaration; limit to declarations with
> initialization to operations without side effect. So:
> 
> 	struct serio *serio;
> 
> 	serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
> 	if (!serio)
> 		return -ENOMEM;
> 
> > +
> > +	if (!serio)
> > +		return -ENOMEM;
> > +
> > +	serio->id.type = SERIO_RMI_PSTHRU;
> > +	serio->write = rmi_f03_pt_write;
> > +	serio->port_data = f03;
> > +
> > +	strlcpy(serio->name, "Synaptics RMI4 PS2 pass-through",
> > +		sizeof(serio->name));
> > +	strlcpy(serio->phys, "synaptics-rmi4-pt/serio1",
> > +		sizeof(serio->phys));
> > +	 serio->dev.parent = &fn->dev;
> 
> Extra space after tab in indentation.
> 
> > +
> > +	f03->serio = serio;
> > +
> > +	serio_register_port(serio);
> > +
> > +	return 0;
> > +}
> > +
> > +static int rmi_f03_probe(struct rmi_function *fn)
> > +{
> > +	int rc;
> 
> int error;
> 
> Maybe allocate the memory here...
> 
> > +
> > +	rc = rmi_f03_initialize(fn);
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%d devices on PS/2 passthrough", rc);
> 
> so you can use f03->device_count here.
> 
> Do we need to warn if we see device count greater than 1?

Good quesstion. I can't recall the exact protocol, and we certainly
never seen anything with more than one device. So probably yes, we
should shout something if it's not what we expect.

> 
> > +
> > +	rc = rmi_f03_register_pt(fn);
> > +	if (rc)
> > +		return rc;
> > +
> > +	return 0;
> > +}
> > +
> > +static int rmi_f03_config(struct rmi_function *fn)
> > +{
> > +	fn->rmi_dev->driver->set_irq_bits(fn->rmi_dev, fn->irq_mask);
> > +
> > +	return 0;
> > +}
> > +
> > +static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits)
> > +{
> > +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> > +	u16 data_addr = fn->fd.data_base_addr;
> > +	const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE;
> > +	u8 obs[ob_len];
> > +	u8 ob_status;
> > +	u8 ob_data;
> > +	unsigned int serio_flags;
> > +	int i;
> > +	int retval;
> > +
> > +	/* Grab all of the data registers, and check them for data */
> > +	retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET,
> > +				&obs, ob_len);
> > +	if (retval) {
> > +		dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n",
> > +			__func__);
> > +		serio_interrupt(f03->serio, 0, SERIO_TIMEOUT);
> > +		return retval;
> > +	}
> > +
> > +	for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) {
> > +		ob_status = obs[i];
> > +		ob_data = obs[i + RMI_F03_OB_DATA_OFFSET];
> > +		serio_flags = 0;
> > +
> > +		if (!(ob_status & RMI_F03_RX_DATA_OFB))
> > +			continue;
> > +
> > +		if (ob_status & RMI_F03_OB_FLAG_TIMEOUT)
> > +			serio_flags |= SERIO_TIMEOUT;
> > +		if (ob_status & RMI_F03_OB_FLAG_PARITY)
> > +			serio_flags |= SERIO_PARITY;
> > +
> > +		rmi_dbg(RMI_DEBUG_FN, &fn->dev,
> > +			"%s: Received %.2hhx from PS2 guest T: %c P: %c\n",
> > +			__func__, ob_data,
> > +			serio_flags & SERIO_TIMEOUT ?  'Y' : 'N',
> > +			serio_flags & SERIO_PARITY ? 'Y' : 'N');
> > +
> > +		serio_interrupt(f03->serio, ob_data, serio_flags);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void rmi_f03_remove(struct rmi_function *fn)
> > +{
> > +	struct f03_data *f03 = dev_get_drvdata(&fn->dev);
> > +
> > +	serio_unregister_port(f03->serio);
> > +}
> > +
> > +struct rmi_function_handler rmi_f03_handler = {
> > +	.driver = {
> > +		.name = "rmi4_f03",
> > +	},
> > +	.func = 0x03,
> > +	.probe = rmi_f03_probe,
> > +	.config = rmi_f03_config,
> > +	.attention = rmi_f03_attention,
> > +	.remove = rmi_f03_remove,
> > +};
> > +
> > +MODULE_AUTHOR("Lyude Paul <thatslyude@gmail.com>");
> > +MODULE_DESCRIPTION("RMI F03 module");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/uapi/linux/serio.h b/include/uapi/linux/serio.h
> > index f2447a8..7012178 100644
> > --- a/include/uapi/linux/serio.h
> > +++ b/include/uapi/linux/serio.h
> > @@ -30,6 +30,7 @@
> >  #define SERIO_HIL_MLC	0x03
> >  #define SERIO_PS_PSTHRU	0x05
> >  #define SERIO_8042_XL	0x06
> > +#define SERIO_RMI_PSTHRU	0x07
> >  
> >  /*
> >   * Serio protocols
> > -- 
> > 2.7.4
> > 
> 
> Thanks.
> 

I saw that you pushed part of the series. Many thanks for that. I'll
work on the fixes for this one and submit v2 ASAP.

Cheers,
Benjamin

> -- 
> Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-12-01 14:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-29 10:08 [PATCH 00/13] RMI4 cleanups and switch hid-rmi to rmi_core Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 01/13] Input: synaptics-rmi4 - fix documentation of rmi_2d_sensor_platform_data Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 02/13] Input: synaptics-rmi4 - remove unused fields in struct rmi_driver_data Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 03/13] Input: synaptics-rmi4 - add rmi_enable/disable_irq Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 04/13] Input: synaptics-rmi4 - remove mutex calls while updating the firmware Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 05/13] Input: synaptics-rmi4 - remove EXPORT_SYMBOL_GPL for internal functions Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 06/13] Input: synaptics-rmi4 - have only one struct platform data Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 07/13] Input: synaptics-rmi4 - add support for F03 Benjamin Tissoires
2016-12-01  1:36   ` Dmitry Torokhov
2016-12-01 14:54     ` Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 08/13] Input: synaptics-rmi4 - f03: grab data passed by transport device Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 09/13] Input: synaptics-rmi4 - allow to add attention data Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 10/13] Input: synaptics-rmi4 - store the attn data in the driver Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 11/13] HID: rmi: Make hid-rmi a transport driver for synaptics-rmi4 Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 12/13] HID: rmi: Handle all Synaptics touchpads using hid-rmi Benjamin Tissoires
2016-11-29 10:08 ` [PATCH 13/13] HID: rmi: Support the Lenovo Thinkpad X1 Tablet dock " Benjamin Tissoires

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).