On Fri, Jan 7, 2022 at 3:49 PM Samuel Holland <samuel@xxxxxxxxxxxx> wrote: > > Hello! > > On 12/22/21 6:45 AM, Alistair Francis wrote: > > This patch series builds on top of [1] and adds support for the cyttsp5 > > touchscreen controller for the reMarkable 2. > > > > I first tried to add an I2C HID device. Although the cyttsp5 has some HID > > looking aspects it is not HID compatible. Just in trying to probe the device > > I found: > > - The HID descriptor has extra padding > > - The HID descriptor sets the high bytes of the descriptor length > > - The HID descriptor has extra unrecognised tags > > - The HID reset command doesn't appear to work > > > > I don't think there is a way to use the I2C HID framework with the cyttsp5. > > For anyone interested you can see the work here [2]. In that branch though I > > can only obtain a HID descriptor, nothing else works without more core > > changes. > > > > So instead I rebased the series from [1]. Converted to the new yaml DTS > > documentation, added regulator support and fixed a x/y miscalculation bug. > > I am working on the PineNote, which also uses a cyttsp5 touchscreen. I attempted Hey! That's great! PineNote support would be awesome > to use the driver from this series, but I could not get it to work. I found that > the cyttsp5_sensing_conf_data was filled with all zeroes, so every touch failed > the max_tch check. I noticed that cmd_done was being completed by empty > responses (which explains why the response buffer was zeroes), but I got stuck > after that. Hmm... That's annoying. This series now works with the rM2 and a kobo device, so I don't understand why it wouldn't work on the PineNote. I just sent a v5 (sorry, I forgot to CC you) that has some code cleanups, but I don't expect it to fix the issues you are seeing. > > So I looked back at the thread you linked below, and tried to implement the > workarounds described there, and those above, plus some others, and I was able > to get the touchscreen working with i2c_hid. Here are the changes I made to i2c_hid: > https://github.com/smaeul/linux/commit/a1e07425a6c4 > > In summary: > - Perform a 2-byte dummy read before reading the HID descriptor. > This is required to clear the two-byte empty message. > - Split command/response into multiple I2C transactions. > This is probably some sort of timing issue. > Without these first two, HID descriptor reads return "02 00". > - Chop 2 bytes out of the HID descriptor, as per the thread below. > - Similarly, chop 3 bytes out of the report descriptor. > - Skip the reset command, as above. Otherwise, the touchscreen > sends a different, short, unusable (partial?) report descriptor. Cool! I'm impressed you got this working! > > I reused the the existing i2c_hid_of_goodix driver to handle toggling the reset > line, which is required. That existing binding is almost identical to the one in > this series. Here's the glue I added: > https://github.com/smaeul/linux/commit/65d9250d3899 > > And here is the result, from dmesg and debugfs: > https://gist.github.com/smaeul/60b4b0f784bfff8bb8ce3ee3b4483be9 > > So far, the quirks only appear to affect probing the device. The touchscreen > works normally after that. If the PineNote and eInk setups are different (they probably are if this series isn't working for you) then there might be other changes as well. > > What do you think of this approach? It certainly seems cleaner than parsing the > HID reports/responses by hand. But I don't know if all of the quirks are > acceptable for i2c_hid. I'll leave that for an I2C HID maintainer to answer. > > One additional quirk that I haven't handled yet is the missing min/max for ABS_* > axes in the report descriptor. This prevents libinput from working, but other > evdev users appear to work fine. The driver in this series appears to get that > information from some vendor-specific command, and I am not sure where to hook > that up. We will need that won't we? So then i2c_hid would have to implement that as well? Alistair > > Regards, > Samuel > > > 1: https://lwn.net/ml/linux-kernel/20180703094309.18514-1-mylene.josserand@xxxxxxxxxxx/ > > 2: https://github.com/alistair23/linux/commits/rM2-mainline-cyttsp5-hid > > > > Alistair Francis (2): > > ARM: imx_v6_v7_defconfig: Enable the cyttsp5 touchscreen > > ARM: dts: imx7d-remarkable2: Enable the cyttsp5 > > > > Mylène Josserand (2): > > Input: Add driver for Cypress Generation 5 touchscreen > > dt-bindings: input: Add Cypress TT2100 touchscreen controller > > > > .../input/touchscreen/cypress,tt21000.yaml | 92 ++ > > arch/arm/boot/dts/imx7d-remarkable2.dts | 89 ++ > > arch/arm/configs/imx_v6_v7_defconfig | 1 + > > drivers/input/touchscreen/Kconfig | 14 + > > drivers/input/touchscreen/Makefile | 1 + > > drivers/input/touchscreen/cyttsp5.c | 922 ++++++++++++++++++ > > 6 files changed, 1119 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml > > create mode 100644 drivers/input/touchscreen/cyttsp5.c > > >
