Initial driver for Synaptics touchscreens using RMI4 protocol. Signed-off-by: William Manson <WManson@synaptics.com> Signed-off-by: Allie Xiong <axiong@synaptics.com> Signed-off-by: Christopher Heiny <cheiny@synaptics.com> --- drivers/input/touchscreen/Kconfig | 13 + drivers/input/touchscreen/Makefile | 1 + drivers/input/touchscreen/rmi.h | 196 ++++++++ drivers/input/touchscreen/rmi_app_touchpad.c | 355 +++++++++++++++ drivers/input/touchscreen/rmi_core.c | 632 ++++++++++++++++++++++++++ drivers/input/touchscreen/rmi_core.h | 57 +++ drivers/input/touchscreen/rmi_function_11.c | 352 ++++++++++++++ drivers/input/touchscreen/rmi_function_11.h | 39 ++ drivers/input/touchscreen/rmi_functions.h | 109 +++++ drivers/input/touchscreen/rmi_i2c.h | 54 +++ drivers/input/touchscreen/rmi_i2c_gta01.c | 122 +++++ drivers/input/touchscreen/rmi_phys_i2c.c | 545 ++++++++++++++++++++++ 12 files changed, 2475 insertions(+), 0 deletions(-) diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index 8a8fa4d..ebd3abf 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig @@ -307,6 +307,19 @@ config TOUCHSCREEN_MIGOR To compile this driver as a module, choose M here: the module will be called migor_ts. +config TOUCHSCREEN_SYNAPTICS_RMI4_I2C + tristate "Synaptics RMI4 I2C touchscreens" + depends on I2C + help + Say Y here if you have a Synaptics RMI4 I2C touchscreen connected to + your system. This enables support for Synaptics RMI4 over I2C based + touchscreens. + + If unsure, say N. + + To compile this driver as a set of modules, choose M here: the + modules will be called rmi, rmi_app_touchpad, rmi_phys_i2c. + config TOUCHSCREEN_TOUCHRIGHT tristate "Touchright serial touchscreen" select SERIO diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile index ...
Hi Christopher, Sorry for the delay. Here finally come my comments on your code. I'm only commenting on the i2c side of things. This name is unfortunate. "client data" usually refers to run-time state data of an i2c client. For configuration data, we rather use the This is already included in the i2c client struct itself, so you This is incorrect. What goes in device ID tables is _device_ names, nor _driver_ names. It might be the same string, but you don't want to use You don't need a copy of the i2c client in your own structure. It is allocated by the i2c core, if you need a reference to it, then just store a pointer thereto. This is the only blocker point in your driver as far as I am concerned. This works, but I would encourage you to use i2c_smbus_write_byte_data Using i2c_smbus_read_byte_data() would look better, be more portable, Likewise, i2c_smbus_read_i2c_block_data() would be better, if you never need to read more than 32 bytes at once. If you need to, then i2c_transfer() would still be better than i2c_master_send + i2c_master_recv, as you avoid the downtime between the 2 messages on Whould be <= 15, if I'm not mistaken. Using <= sizeof(txbuf_most) + 1 If you were using i2c_smbus_write_i2c_block_data(), you wouldn't have to. Unless you must write more than 32 bytes sometimes? In that case, If you keep that code, please just define "i" earlier in your function That's what you get when hard-coding function names in log messages ;) I am curious why your read functions have a retry mechanism and your Now I am confused. Instead of attaching the proper platform data to each I2C device, you have a single object with all the platform data for all devices in it, and you pass that big object as the platform data of all devices, and they have to find out which part is meant for them? This is odd. What's the point of doing that? Why don't you just set the platform data pointer to the right subset of data for every What if no matching ...
Hi Jean, Thanks for the input! It's quite helpful and clears up some things we were a bit puzzled about. We'll fold most of your suggestions into the next patch. Answers to some questions you raised will be coming shortly. Thanks again, Chris --
On Mon, Mar 22, 2010 at 7:07 PM, Christopher Heiny <cheiny@synaptics.com> wrote: Do you plan to add platform data to align the reported touchscreen data with the screen behind it, or do the new hardware allow the the firmware handle this? In the past we even needed separate parameters for different firmware versions (seen in drivers/staging/dream/synaptics_i2c_rmi.h). -- Arve Hjønnevåg --
Hi Arve, RMI4 touchscreens allow adjustment of the reported coordinate range (see the F11_2D_Ctrl6..9 registers, page 48 of the current version of the spec at http://www.synaptics.com/developers/manuals). Using this feature, the device can be configured to report the same number of positions on each axis as there are pixels on the display. We plan to make these settings accessible via sysfs, so that it can be dynamically tweaked if the user changes the display resolution (not likely on a phone, probable on a tablet/slate/ereader type device). Assuming there are no significant issues with our current patch, we plan to include that in the next one. We're holding off that implementation because we're still finding our feet on the submission process, and wanted to keep the initial submits small so changes would be more manageable. Coordinate rotation/reflection settings will be handled at the driver level, again via sysfs. These features should be independent of the touchscreen firmware level. Initial settings for these features should probably be done at the platform level. Chris --
This does not help aligning the touchscreen values with the screen behind it. It just moves the linear scaling from userspace (which can use fixed or floating point values to preserve subpixel precision) to Do you also have a plan to let the userspace know that touchscreen coordinate x1,y1 correspond to screen coordinate 0,0 and x2,y2 correspond to screen coordinate xmax,ymax? The android driver sets absmin/max to the values reported when touching the display edges (not the actual min and max that the touchscreen can report), but other In the past they have depended on the firmware version for two reasons. On one product, firmware changes to improve the edges of the screen completely changed the relationship between values reported and the physical touch location. On another product, the physical size of the sensor changed, and the firmware version was used to detect this. If all RMI4 based product allow field updates of the firmware the first case it less important, but we still need to cover the second -- Arve Hjønnevåg --
Hi Arve, It sounds like your concern is for cases when the origin of the touchscreen coordinates does not correspond to a corner of the pixel area. Is that correct? In any case, it's a perfectly valid issue - not all manufacturers take care to map the touchscreen to the display screen that way (though most do). Adding a translation control to the driver would be easy - we'll It's more the other direction - we were concerned (validly, it turned out) that some extensive changes might be required as a result of feedback on the initial submissions, and wanted to keep the codebase we'd have to refactor small. As the codebase grows, we'll switch to using patch series. Probably We are not planning on that, since it would require the driver to know the orientation (standard? rot 90? rot -90? rot 180?) and resolution of the display and track whenever that changes. It is better to handle that information at a higher level, which can then tell the touchscreen Hmmmm. I can see a lot of other cases where it might be desirable to know the size of the touchscreen in a platform independent manner. Certainly the firmware version is not a reliable way to do this going forward. I will contact the spec maintainer and see if we can have the device report the relative information in a query. Thanks, Chris --
