| Message ID | 20230410160200.57261-1-joelselvaraj.oss@gmail.com |
|---|---|
| Headers | show |
| Series | Add support for Focaltech FTS Touchscreen | expand |
On 10/04/2023 18:01, Joel Selvaraj wrote: > Add devicetree bindings for the Focaltech FTS touchscreen drivers. > Subject: drop second/last, redundant "bindings for". The "dt-bindings" prefix is already stating that these are bindings. > Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com> > Signed-off-by: Caleb Connolly <caleb@connolly.tech> > --- > .../input/touchscreen/focaltech,fts5452.yaml | 71 +++++++++++++++++++ > 1 file changed, 71 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml b/Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml > new file mode 100644 > index 000000000000..f42868293439 > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml > @@ -0,0 +1,71 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/input/touchscreen/focaltech,fts5452.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Focaltech FTS I2C Touchscreen Controller > + > +maintainers: > + - Joel Selvaraj <joelselvaraj.oss@gmail.com> > + - Caleb Connolly <caleb@connolly.tech> > + > +allOf: > + - $ref: touchscreen.yaml# > + > +properties: > + compatible: > + enum: > + - focaltech,fts5452 > + - focaltech,fts8719 > + > + reg: > + const: 0x38 > + > + interrupts: > + maxItems: 1 > + > + reset-gpios: > + maxItems: 1 > + > + avdd-supply: > + description: regulator supplying analog power (2.6V to 3.3V). > + > + vddio-supply: > + description: regulator supplying IO power (1.8V). > + > +unevaluatedProperties: false > + > +required: > + - compatible > + - reg > + - touchscreen-size-x > + - touchscreen-size-y We always put required: before unevaluatedProperties. Base your schema on example-schema.yaml. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
Hi Krzysztof Kozlowski, Konrad Dybcio suggested to use interrupts-extended instead interrupts. So in my WIP v3, I have updated it in the dts and bindings example. However, I am confused if I should replace the "interrupts" with "interrupts-extended" property in the schema too? I see a lot of schemas specifying "interrupts", with examples using "interrupts" or "interrupts-extended". At the same time, I see some specifying both "interrupts" and "interrupts-extended" (like one of these two) and very few others specify only "interrupts-extended" in the schema. Which is the currently recommended way to do this? In between, the interrupt property should be a required property as the driver will not function without an interrupt. I will fix that in v3. Thanks, Joel Selvaraj
On 14/04/2023 02:32, Joel Selvaraj wrote: > Hi Krzysztof Kozlowski, > > Konrad Dybcio suggested to use interrupts-extended instead interrupts. Sorry, I have no idea what this email is about. There is no context here, no reply, it just appeared alone in my inbox without any reference. Please respond inline to existing messages, keeping necessary context you are replying to. > So in my WIP v3, I have updated it in the dts and bindings example. > However, I am confused if I should replace the "interrupts" with > "interrupts-extended" property in the schema too? I see a lot of schemas No. > specifying "interrupts", with examples using "interrupts" or > "interrupts-extended". At the same time, I see some specifying both > "interrupts" and "interrupts-extended" (like one of these two) and very > few others specify only "interrupts-extended" in the schema. Which is > the currently recommended way to do this? > > In between, the interrupt property should be a required property as the > driver will not function without an interrupt. I will fix that in v3. > > Thanks, > Joel Selvaraj Best regards, Krzysztof
Changes in v2: -------------- 1. dt-bindings changes (Suggested by Krzysztof Kozlowski) - changed file name from focaltech,fts.yaml to focaltech,fts5452.yaml - removed focaltech,max-touch-number property, handled in driver now - removed touchscreen-* properties and used unevaluatedProperties: false instead of additionalProperties: false - fixed the example dts node name to be generic 2. FTS Touchscreen driver changes (Suggested by Markuss Broks and Jeff LaBundy) - removed repeated license terms since SPDX tag is used - includes are now sorted - added the missing input_mt_sync_frame when reporting touch - focaltech,max-touch-number is no longer read from dts and instead specified in the driver as compatible data. - removed redundant __set_bits - input_mt_init_slots is now called after the axes are defined - irq handler now returns IRQ_NONE when there is an i2c error - other minor fixes and refactoring as suggested - renamed driver filename from focaltech_fts.c to focaltech_fts5452.c (Suggested by Krzysztof Kozlowski) 3. dts changes (Suggested by Krzysztof Kozlowski) - use generic touchscreen nodes - removed focaltech,max-touch-number property - irq type was specified wrongly for Poco F1 in v1. Changed the irq type to IRQ_TYPE_EDGE_FALLING as that is correct. Some Clarifications: -------------------- 1. Jeff LaBundy suggested I could read chip id with the following: __be16 val; regmap_raw_read(data->regmap, FTS_REG_CHIP_ID_H, &val, sizeof(val)); But this is not possible because FTS_REG_CHIP_ID_H and FTS_REG_CHIP_ID_L are not continous register, therefore reading it together as 16-bit values will not work. So I went with what Markuss Broks suggested: regmap_read(data->regmap, FTS_REG_CHIP_ID_L, &id); regmap_read(data->regmap, FTS_REG_CHIP_ID_H, &val); id |= val << 8; 2. As Markuss Broks suggested, I tried to cast the buffer to struct, but unfortunately was not able to successfully do it. The buffer layout is weirdly split into 4 bits and 12 bits at someplaces which makes it hard to cast into a struct. For example, we can note type = buf[base + 3] >> 6 x = ((buf[base + 3] & 0x0F) << 8) + (buf[base + 4] & 0xFF); Here at buffer index 3, the first two bits (>>6) are used for denoting event type. The next two bits are not used. But the last 4 bits (&0x0F) of buffer[3] are added with buffer index 4 to get the x position. I don't know how to handle these when casting to a struct. I tried experimenting with bitfields in struct, but to no avail. So I am sticking with my initial implementation for now. Kindly let me know if any further improvements are needed. Thanks. The Focaltech FTS driver supports several variants of focaltech touchscreens found in ~2018 era smartphones including variants found on the PocoPhone F1 and the SHIFT6mq which are already present in mainline. This driver is loosely based on the original driver from Focaltech and the patches submitted by Caleb Connolly previously[1] but has been simplified and largely reworked. [1] https://patchwork.kernel.org/project/linux-input/patch/20220123173650.290349-3-caleb@connolly.tech/ Joel Selvaraj (5): dt-bindings: input: touchscreen: add bindings for focaltech,fts5452 Input: add driver for Focaltech FTS touchscreen arm64: dts: qcom: sdm845-xiaomi-beryllium-common: add touchscreen related nodes arm64: dts: qcom: sdm845-xiaomi-beryllium-ebbg: introduce support for fts touchscreen arm64: dts: qcom: sdm845-shift-axolotl: update focaltech touchscreen properties .../input/touchscreen/focaltech,fts5452.yaml | 71 +++ MAINTAINERS | 8 + .../boot/dts/qcom/sdm845-shift-axolotl.dts | 15 +- .../qcom/sdm845-xiaomi-beryllium-common.dtsi | 39 ++ .../dts/qcom/sdm845-xiaomi-beryllium-ebbg.dts | 26 ++ drivers/input/touchscreen/Kconfig | 12 + drivers/input/touchscreen/Makefile | 1 + drivers/input/touchscreen/focaltech_fts5452.c | 432 ++++++++++++++++++ 8 files changed, 596 insertions(+), 8 deletions(-) create mode 100644 Documentation/devicetree/bindings/input/touchscreen/focaltech,fts5452.yaml create mode 100644 drivers/input/touchscreen/focaltech_fts5452.c