From: Samuel Holland <samuel@sholland.org> To: Alistair Francis <alistair@alistair23.me>, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Benjamin Tissoires <benjamin.tissoires@redhat.com> Cc: dmitry.torokhov@gmail.com, alistair23@gmail.com, robh+dt@kernel.org, linus.walleij@linaro.org, rydberg@bitmath.org, andreas@kemnade.info Subject: Re: [PATCH v4 0/4] Add support for the Cypress cyttsp5 Date: Thu, 6 Jan 2022 23:49:50 -0600 [thread overview] Message-ID: <37291fe7-7028-cd9e-7fde-01d13f85f0d9@sholland.org> (raw) In-Reply-To: <20211222124603.326920-1-alistair@alistair23.me> 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 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. 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. 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. 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. 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. Regards, Samuel > 1: https://lwn.net/ml/linux-kernel/20180703094309.18514-1-mylene.josserand@bootlin.com/ > 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 >
WARNING: multiple messages have this Message-ID (diff)
From: Samuel Holland <samuel@sholland.org> To: Alistair Francis <alistair@alistair23.me>, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Benjamin Tissoires <benjamin.tissoires@redhat.com> Cc: dmitry.torokhov@gmail.com, alistair23@gmail.com, robh+dt@kernel.org, linus.walleij@linaro.org, rydberg@bitmath.org, andreas@kemnade.info Subject: Re: [PATCH v4 0/4] Add support for the Cypress cyttsp5 Date: Thu, 6 Jan 2022 23:49:50 -0600 [thread overview] Message-ID: <37291fe7-7028-cd9e-7fde-01d13f85f0d9@sholland.org> (raw) In-Reply-To: <20211222124603.326920-1-alistair@alistair23.me> 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 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. 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. 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. 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. 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. Regards, Samuel > 1: https://lwn.net/ml/linux-kernel/20180703094309.18514-1-mylene.josserand@bootlin.com/ > 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-01-07 5:49 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-22 12:45 [PATCH v4 0/4] Add support for the Cypress cyttsp5 Alistair Francis 2021-12-22 12:45 ` Alistair Francis 2021-12-22 12:46 ` [PATCH v4 1/4] Input: Add driver for Cypress Generation 5 touchscreen Alistair Francis 2021-12-22 12:46 ` Alistair Francis 2021-12-23 9:07 ` Andreas Kemnade 2021-12-23 9:07 ` Andreas Kemnade 2021-12-22 12:46 ` [PATCH v4 2/4] dt-bindings: input: Add Cypress TT2100 touchscreen controller Alistair Francis 2021-12-22 12:46 ` Alistair Francis 2022-01-04 19:30 ` Rob Herring 2022-01-04 19:30 ` Rob Herring 2021-12-22 12:46 ` [PATCH v4 3/4] ARM: imx_v6_v7_defconfig: Enable the cyttsp5 touchscreen Alistair Francis 2021-12-22 12:46 ` Alistair Francis 2021-12-22 12:46 ` [PATCH v4 4/4] ARM: dts: imx7d-remarkable2: Enable the cyttsp5 Alistair Francis 2021-12-22 12:46 ` Alistair Francis 2022-01-07 5:49 ` Samuel Holland [this message] 2022-01-07 5:49 ` [PATCH v4 0/4] Add support for the Cypress cyttsp5 Samuel Holland 2022-01-09 12:00 ` Alistair Francis 2022-01-09 12:00 ` Alistair Francis
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=37291fe7-7028-cd9e-7fde-01d13f85f0d9@sholland.org \ --to=samuel@sholland.org \ --cc=alistair23@gmail.com \ --cc=alistair@alistair23.me \ --cc=andreas@kemnade.info \ --cc=benjamin.tissoires@redhat.com \ --cc=devicetree@vger.kernel.org \ --cc=dmitry.torokhov@gmail.com \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-input@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=robh+dt@kernel.org \ --cc=rydberg@bitmath.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.