Re: [PATCH v2] input: mouse: add qci touchpad driver

Previous thread: [RFC] the right way to use gpiolib hooks to automate power management? by Gregory Bean on Thursday, August 12, 2010 - 9:39 am. (3 messages)

Next thread: [PATCH/RFC 0/5] sched: add new 'book' scheduling domain by Heiko Carstens on Thursday, August 12, 2010 - 10:25 am. (17 messages)
From: Neil Leeder
Date: Thursday, August 12, 2010 - 9:49 am

This driver is for the QCI trackpad used on Quanta smartbooks

Signed-off-by: Horace Fu <horace.fu@quantatw.com>
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
[nleeder@codeaurora.org: cleanup i2c calls, address review comments etc]
Signed-off-by: Neil Leeder <nleeder@codeaurora.org>
---
 drivers/input/mouse/Kconfig        |   12 ++
 drivers/input/mouse/Makefile       |    1 +
 drivers/input/mouse/qci_touchpad.c |  270 ++++++++++++++++++++++++++++++++++++
 include/linux/input/qci_touchpad.h |   25 ++++
 4 files changed, 308 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/mouse/qci_touchpad.c
 create mode 100644 include/linux/input/qci_touchpad.h

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index c714ca2..32a88b4 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -303,6 +303,18 @@ config MOUSE_MAPLE
 	  To compile this driver as a module choose M here: the module will be
 	  called maplemouse.
 
+config MOUSE_QCITP
+	tristate "Quanta Computer Inc. Touchpad"
+	depends on I2C
+	help
+	  This driver supports the touchpad on Quanta smartbook devices.
+
+	  Say Y here if you have a Quanta-based smartboot or notepad
+	  device and want to use the Quanta touchpad driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called qci_touchpad.
+
 config MOUSE_SYNAPTICS_I2C
 	tristate "Synaptics I2C Touchpad support"
 	depends on I2C
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 570c84a..6eda35d 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_MOUSE_MAPLE)		+= maplemouse.o
 obj-$(CONFIG_MOUSE_PC110PAD)		+= pc110pad.o
 obj-$(CONFIG_MOUSE_PS2)			+= psmouse.o
 obj-$(CONFIG_MOUSE_PXA930_TRKBALL)	+= pxa930_trkball.o
+obj-$(CONFIG_MOUSE_QCITP)		+= qci_touchpad.o
 obj-$(CONFIG_MOUSE_RISCPC)		+= rpcmouse.o
 obj-$(CONFIG_MOUSE_SERIAL)		+= sermouse.o
 ...
From: Stepan Moskovchenko
Date: Thursday, August 12, 2010 - 10:25 am

Sorry, very minor point - do you mean "smartbook" here?

A lot of machines are made by Quanta. Should this be given a description 
/ option name that is more specific, ie, "Quanta [chip name] Touchpad" 
or even "Quanta I2C Touchpad" ?  What you have might be fine, however...

Steve

--

From: Neil Leeder
Date: Thursday, August 12, 2010 - 10:58 am

Good catch on the typo. I can update the Kconfig item to include I2C as 
you suggested.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

From: Dmitry Torokhov
Date: Thursday, August 12, 2010 - 7:49 pm

Actually, since this is not a new touchpad but simply a PS/2 interface
it should be implemented as a serio driver, not input device driver.

Could you please tell me if the following works for you? Note that it
expects IRQ to be set up properly (edge vs. level trigger) by the
platform code

Thanks.

-- 
Dmitry

Input: wpce775x-serio - add driver for WPCE775x PS/2 interface

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

This is a driver for PS/2 interface part of Nuvoton WPCE775x family of
Advanced Embedded Controllers.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/serio/Kconfig          |   12 +
 drivers/input/serio/Makefile         |    1 
 drivers/input/serio/wpce775x_serio.c |  377 ++++++++++++++++++++++++++++++++++
 3 files changed, 389 insertions(+), 1 deletions(-)
 create mode 100644 drivers/input/serio/wpce775x_serio.c


diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 3bfe8fa..259e550 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -215,7 +215,7 @@ config SERIO_AMS_DELTA
 	depends on MACH_AMS_DELTA
 	default y
 	select AMS_DELTA_FIQ
-	---help---
+	help
 	  Say Y here if you have an E3 and want to use its mailboard,
 	  or any standard AT keyboard connected to the mailboard port.
 
@@ -226,4 +226,14 @@ config SERIO_AMS_DELTA
 	  To compile this driver as a module, choose M here;
 	  the module will be called ams_delta_serio.
 
+config SERIO_WPCE775X
+	tristate "WPCE775x EC PS/2 interface support"
+	depends on I2C
+	help
+	  Say Y here if you have a system with Nuvoton WPCE775x Advanced
+	  Embedded Controller chip and want to use its PS/2 interface part.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called wpce775x_serio.
+
 endif
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 84c80bf..e442550 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -24,3 +24,4 @@ ...
From: Neil Leeder
Date: Friday, August 13, 2010 - 2:56 pm

Dmitri,

Thanks for supplying that serio driver. I just have a couple of questions.

Even though the interface on the wpce775x EC happens to be a PS/2 
interface, this is completely hidden by the firmware on that device. To 
the linux driver it looks like a dedicated i2c connection directly to 
the touchpad. You can't substitute any other device on that PS/2 
interface without rewriting the firmware in the EC - it's not a generic 
interface.  A manufacturer could even move the touchpad from the PS/2 
interface to say GPIOs, re-write the firmware and the linux driver 
couldn't tell the difference. Does that change the rationale for using a 
serio driver?

If the request to use a serio driver is still valid, then it seems that 
the workqueue from the interrupt handler sends each byte of data 
received over i2c in a separate serio_interrupt() call to the touchpad 
driver. Touchpad data comes in 3-byte packets, so the touchpad driver 
will have to re-assemble the packet from 3 separate interrupts. Is that 

The i2c_board_info that supplies the irq # from platform code doesn't 
have a way to set i2c flags, and can't set_irq_type() until after 
irq_request(). This may require another platform_data struct to pass the 
trigger level in.

Thanks.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

From: Dmitry Torokhov
Date: Friday, August 13, 2010 - 5:54 pm

Hmm, according to the following diagram from  Nuvoton trhe chip does in
fact has a distinct PS/2 interface (3 of them actually):

http://www.nuvoton.com/NuvotonMOSS/Community/ProductInfo.aspx?tp_GUID=d2a1e761-c93d-4b...

Also it is not a simple coincidence that to enable device you send 0xf4
to it (which is PSMOUSE_CMD_ENABLE - standard PS/2 command). This tends
to suggest that interface is not actually hidden and that the device
(touchpad) could be replaced with other kinds of devices.

Anyway, please try the driver (you  may need to hardcode the IRQ trigger
type for now) and see if psmouse is able to talk to the touchpad. If it

Yes, serio ports are byte-oriented devices. psmouse and atkbd drivers should
be able to work with such data.

Thanks.

-- 
Dmitry
--

From: Neil Leeder
Date: Tuesday, August 17, 2010 - 10:14 am

Dmitri,
I fixed one line in the wpce775x_serio driver which looked like a typo - 
hope I got that right:

	for (i = 0; i < count; i++)
-		serio_interrupt(ps2if->serio, ps2if->data_in[1], 0);
+		serio_interrupt(ps2if->serio, ps2if->data_in[i], 0);
	}

I tried running with psmouse but it didn't work. The touchpad was never 
detected as a synaptics touchpad. I looked at the dataflow from the 
device and it wasn't responding to the commands that synaptics_detect() 
was sending it. It eventually showed up as an unknown logitech mouse. I 
can see data being passed through the serio driver but the logitech 
driver can't process it.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

From: Dmitry Torokhov
Date: Tuesday, August 17, 2010 - 11:05 am

Hi Neil,

On Aug 17, 2010, at 10:14 AM, Neil Leeder <nleeder@codeaurora.org>  




What about loading psmouse module with proto=bare?

Could you also dump the data received from touchpad during probing?


-- 
Dmitry

--

From: Neil Leeder
Date: Wednesday, August 18, 2010 - 2:38 pm

Hi Dmitry,


It appears the controller does somewhat respond as a PS/2 device. The 
first problem is it only responds to CMD_GETID correctly about 50% of 
the time. Sometimes it responds well with a 0x00, other times with 0xfa, 
which causes psmouse_probe to not recognize it as a pointing device.

[    1.037223] wpce775x_send len=1 data=f2
[    1.054412] wpce775x_recv ... data_in=fa fa 64 0 0 0 0 0 fa 0 0 0 0 0 
0 0

It also doesn't respond well to CMD_GETINFO, which is why it gets 
mis-detected as the wrong device type. As an aside, I'm not sure if 
combining separate commands into one i2c packet is correct, but 
separating them didn't seem to produce better results either.

[    1.723943] wpce775x_send len=3 data=e8 3 e6
[    1.735666] wpce775x_recv ... data_in=fa fa 64 0 0 0 0 0 fa 0 0 0 0 0 
0 0
[    1.735892] wpce775x_send len=3 data=e6 e6 e9
[    1.748281] wpce775x_recv ... data_in=fe fa 64 0 0 0 0 0 fa 0 0 0 0 0 
0 0
[    1.748500] wpce775x_send len=3 data=e8 0 e6
[    1.760233] wpce775x_recv ... data_in=fa fa 64 0 0 0 0 0 fa 0 0 0 0 0 
0 0
[    1.760457] wpce775x_send len=3 data=e6 e6 e9
[    1.772876] wpce775x_recv ... data_in=fe fa 64 0 0 0 0 0 fa 0 0 0 0 0 
0 0

Using proto=bare gets around the GETINFO failure, but doesn't help with 
the more important GETID failure.

The other issue is that when the serio driver requests up to 16 bytes 
over i2c, the controller responds with 16 bytes of data, which appear to 
be the 3 flags/X/Y regs plus 13 bytes of irrelevant data. This all gets 
passed to psmouse which interprets it 3 bytes at a time as movement 
data. Not only does the extra data get interpreted as movement info, but 
it causes subsequent real data to be mis-aligned and wrongly interpreted.

[  115.915558] wpce775x_recv ... data_in=28 1 ff 0 0 0 0 0 9c 0 0 0 0 0 0 0
[  115.915649] psmouse_process_byte data=28 01 ff rel_x=1 rel_y=1 
<--- OK
[  115.915688] psmouse_process_byte data=00 00 00 rel_x=0 rel_y=0
[  115.915714] psmouse_process_byte data=00 00 ...
From: Dmitry Torokhov
Date: Wednesday, August 18, 2010 - 10:33 pm

One way would be to look for PSMOUSE_CMD_ENABLE/PSMOUSE_CMD_DISABLE
(0xf4/0xf5) in ->write() method to switch between 1 and 3-byte transfers.

Thanks.

-- 
Dmitry
--

From: Neil Leeder
Date: Thursday, August 19, 2010 - 3:19 pm

Hi Dmitry,


Thanks for the suggestions. Nothing I change seems able to get a 
consistent valid response to GETID. I think this is the first time 
anyone has written anything other than 0xF4 to the firmware in the EC 
and it's just not passing through the other PS/2 commands and responses 

If I force my way past GETID and set the receive length to 3 based on 
seeing F4/F5 being written then the touchpad does work. However the 
serio driver is now a touchpad-specific driver rather than a generic 
PS/2 one.  I have a keyboard on the same device. I'm not sure if it will 
work with serio, but if so it will probably have its own requirements 
for the length of data sent on responses, probably not 3.

BTW, using 1 as the receive length for commands has its own problems as 
some commands require more than one byte response (GETID, GETINFO). I 
can work around it, but it's not particularly clean and getting further 
away from a generic driver.

At this point I'm thinking that the interface is close to being able to 
work with serio/psmouse, but just not close enough. The unreliability of 
responding to basic commands as well as the length of data problems 
indicates some custom driver is going to be needed. That could either be 
Quanta's original touchpad driver I posted, or a modification of your 
serio driver. I'd lean towards not having a serio driver which includes 
workarounds for a specific device, but I'd appreciate hearing your opinion.

Thanks.
--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

From: Matthew Garrett
Date: Thursday, August 19, 2010 - 4:35 pm

By any incredible stroke of luck, does it have a unique PNPID? 
/sys/bus/pnp/devices/*/id should give you some insight.

-- 
Matthew Garrett | mjg59@srcf.ucam.org
--

From: Neil Leeder
Date: Friday, August 20, 2010 - 12:16 pm

Thanks but I'm running this on an ARM netbook which doesn't have 
/sys/bus/pnp.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

From: Neil Leeder
Date: Wednesday, August 25, 2010 - 11:26 am

Hi Dmitry,

Any comment on the above alternatives?

A third suggestion might be to have a fairly general serio driver, and 
rewrite the original Quanta driver to use serio rather than using i2c 
directly. I think that has its own set of problems, but thought I'd 
mention it anyway. The nuvoTon EC doesn't present a fully-working and 
reliable PS/2 interface to the driver so I'd lean toward using the 
Quanta touchpad driver by itself.

Thanks.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

From: Dmitry Torokhov
Date: Thursday, August 26, 2010 - 7:49 am

Hi Neil,


I concur that since EC firmware does not seem to support full PS/2
protocol we should got with the original driver creating an input device
rather than serio.

Please send me the latest version of your driver and I will apply it to
my .37 queue.

Thanks.

-- 
Dmitry
--

From: Neil Leeder
Date: Thursday, August 26, 2010 - 2:00 pm

Thanks - I'll incorporate review comments and post an updated patch.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

From: Datta, Shubhrajyoti
Date: Friday, August 13, 2010 - 1:36 am

From: Trilok Soni
Date: Friday, August 13, 2010 - 2:34 am

That looks OK, because qcitp_dev is set to NULL before this call.

---Trilok Soni

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

From: Neil Leeder
Date: Thursday, August 26, 2010 - 11:45 am

I'll check the result.

--
Neil
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

Previous thread: [RFC] the right way to use gpiolib hooks to automate power management? by Gregory Bean on Thursday, August 12, 2010 - 9:39 am. (3 messages)

Next thread: [PATCH/RFC 0/5] sched: add new 'book' scheduling domain by Heiko Carstens on Thursday, August 12, 2010 - 10:25 am. (17 messages)