2021-10-27 21:33:07

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

This series adds support for the touch-keys that can be present on some
touchscreen configurations, adds the compatible for bt532 and fixes a
small race condition bug in the driver probe function.

I also pick up the series that converts the dt bindings to yaml
initially submitted by Linus Walleij in [1].
I made some minor changes to those patches:
- Fixed dt_schema_check error
- Adressed the review comments from Dmitry on the original series

[1] https://lore.kernel.org/linux-input/[email protected]/

Linus Walleij (2):
dt-bindings: input/ts/zinitix: Convert to YAML, fix and extend
Input: zinitix - Handle proper supply names

Nikita Travkin (4):
input: touchscreen: zinitix: Make sure the IRQ is allocated before it
gets enabled
input: touchscreen: zinitix: Add compatible for bt532
dt-bindings: input: zinitix: Document touch-keys support
input: touchscreen: zinitix: Add touchkey support

.../input/touchscreen/zinitix,bt400.yaml | 123 ++++++++++++++++++
.../bindings/input/touchscreen/zinitix.txt | 40 ------
drivers/input/touchscreen/zinitix.c | 101 +++++++++++---
3 files changed, 207 insertions(+), 57 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix.txt

--
2.30.2


2021-10-27 21:35:07

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH 6/6] input: touchscreen: zinitix: Add touchkey support

Zinitix touch controllers can use some of the sense lines for virtual
keys (like those found on many phones). Add support for those keys.

Signed-off-by: Nikita Travkin <[email protected]>
---
drivers/input/touchscreen/zinitix.c | 61 +++++++++++++++++++++++++++--
1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index c67ff8be0df1..3928cb743a1c 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -119,6 +119,7 @@

#define DEFAULT_TOUCH_POINT_MODE 2
#define MAX_SUPPORTED_FINGER_NUM 5
+#define MAX_SUPPORTED_BUTTON_NUM 8

#define CHIP_ON_DELAY 15 // ms
#define FIRMWARE_ON_DELAY 40 // ms
@@ -146,6 +147,8 @@ struct bt541_ts_data {
struct touchscreen_properties prop;
struct regulator_bulk_data supplies[2];
u32 zinitix_mode;
+ u32 keycodes[MAX_SUPPORTED_BUTTON_NUM];
+ int num_keycodes;
};

static int zinitix_read_data(struct i2c_client *client,
@@ -195,6 +198,7 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
struct i2c_client *client = bt541->client;
int i;
int error;
+ u16 int_flags = 0;

error = zinitix_write_cmd(client, BT541_SWRESET_CMD);
if (error) {
@@ -225,6 +229,11 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
if (error)
return error;

+ error = zinitix_write_u16(client, BT541_BUTTON_SUPPORTED_NUM,
+ bt541->num_keycodes);
+ if (error)
+ return error;
+
error = zinitix_write_u16(client, BT541_INITIAL_TOUCH_MODE,
bt541->zinitix_mode);
if (error)
@@ -235,9 +244,12 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
if (error)
return error;

- error = zinitix_write_u16(client, BT541_INT_ENABLE_FLAG,
- BIT_PT_CNT_CHANGE | BIT_DOWN | BIT_MOVE |
- BIT_UP);
+ int_flags = BIT_PT_CNT_CHANGE | BIT_DOWN | BIT_MOVE | BIT_UP;
+
+ if (bt541->num_keycodes)
+ int_flags |= BIT_ICON_EVENT;
+
+ error = zinitix_write_u16(client, BT541_INT_ENABLE_FLAG, int_flags);
if (error)
return error;

@@ -329,6 +341,15 @@ static void zinitix_report_finger(struct bt541_ts_data *bt541, int slot,
input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, p->width);
}

+static void zinitix_report_keys(struct bt541_ts_data *bt541, __le16 icon_events)
+{
+ int i;
+
+ for (i = 0; i < bt541->num_keycodes; i++)
+ input_report_key(bt541->input_dev,
+ bt541->keycodes[i], !!(icon_events & BIT(i)));
+}
+
static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
{
struct bt541_ts_data *bt541 = bt541_handler;
@@ -336,6 +357,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
struct touch_event touch_event;
int error;
int i;
+ __le16 icon_events = 0;

memset(&touch_event, 0, sizeof(struct touch_event));

@@ -346,6 +368,17 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
goto out;
}

+ if (touch_event.status & BIT_ICON_EVENT) {
+ error = zinitix_read_data(bt541->client, BT541_ICON_STATUS_REG,
+ &icon_events, sizeof(icon_events));
+ if (error) {
+ dev_err(&client->dev, "Failed to read icon events\n");
+ goto out;
+ }
+
+ zinitix_report_keys(bt541, icon_events);
+ }
+
for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++)
if (touch_event.point_coord[i].sub_status & SUB_BIT_EXIST)
zinitix_report_finger(bt541, i,
@@ -427,6 +460,7 @@ static int zinitix_init_input_dev(struct bt541_ts_data *bt541)
{
struct input_dev *input_dev;
int error;
+ int i;

input_dev = devm_input_allocate_device(&bt541->client->dev);
if (!input_dev) {
@@ -444,6 +478,14 @@ static int zinitix_init_input_dev(struct bt541_ts_data *bt541)
input_dev->open = zinitix_input_open;
input_dev->close = zinitix_input_close;

+ if (bt541->num_keycodes) {
+ input_dev->keycode = bt541->keycodes;
+ input_dev->keycodemax = bt541->num_keycodes;
+ input_dev->keycodesize = sizeof(bt541->keycodes[0]);
+ for (i = 0; i < bt541->num_keycodes; i++)
+ input_set_capability(input_dev, EV_KEY, bt541->keycodes[i]);
+ }
+
input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
input_set_abs_params(input_dev, ABS_MT_WIDTH_MAJOR, 0, 255, 0, 0);
@@ -508,6 +550,19 @@ static int zinitix_ts_probe(struct i2c_client *client)
return error;
}

+ bt541->num_keycodes = of_property_read_variable_u32_array(
+ client->dev.of_node, "linux,keycodes",
+ bt541->keycodes, 0,
+ ARRAY_SIZE(bt541->keycodes));
+ if (bt541->num_keycodes == -EINVAL) {
+ bt541->num_keycodes = 0;
+ } else if (bt541->num_keycodes < 0) {
+ dev_err(&client->dev,
+ "Unable to parse \"linux,keycodes\" property: %d\n",
+ bt541->num_keycodes);
+ return bt541->num_keycodes;
+ }
+
error = zinitix_init_input_dev(bt541);
if (error) {
dev_err(&client->dev,
--
2.30.2

2021-10-27 21:36:13

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH 2/6] dt-bindings: input/ts/zinitix: Convert to YAML, fix and extend

From: Linus Walleij <[email protected]>

This converts the Zinitix BT4xx and BT5xx touchscreen bindings to YAML, fix
them up a bit and extends them.

We list all the existing BT4xx and BT5xx components with compatible strings.
These are all similar, use the same bindings and work in similar ways.

We rename the supplies from the erroneous vdd/vddo to the actual supply
names vcca/vdd as specified on the actual component. It is long established
that supplies shall be named after the supply pin names of a component.
The confusion probably stems from that in a certain product the rails to the
component were named vdd/vddo. Drop some notes on how OS implementations should
avoid confusion by first looking for vddo, and if that exists assume the
legacy binding pair and otherwise use vcca/vdd.

Add reset-gpios as sometimes manufacturers pulls a GPIO line to the reset
line on the chip.

Add optional touchscreen-fuzz-x and touchscreen-fuzz-y properties.

Cc: Mark Brown <[email protected]>
Cc: Michael Srba <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Linus Walleij <[email protected]>
[Fixed dt_schema_check error]
Signed-off-by: Nikita Travkin <[email protected]>
---
This patch was previously submited here:
https://lore.kernel.org/linux-input/[email protected]/

Changes since the original patch:
- Use enum for compatible list instead of oneOf + const
---
.../input/touchscreen/zinitix,bt400.yaml | 115 ++++++++++++++++++
.../bindings/input/touchscreen/zinitix.txt | 40 ------
2 files changed, 115 insertions(+), 40 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
new file mode 100644
index 000000000000..b4e5ba7c0b49
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/zinitix,bt400.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Zinitix BT4xx and BT5xx series touchscreen controller bindings
+
+description: The Zinitix BT4xx and BT5xx series of touchscreen controllers
+ are Korea-produced touchscreens with embedded microcontrollers. The
+ BT4xx series was produced 2010-2013 and the BT5xx series 2013-2014.
+
+maintainers:
+ - Michael Srba <[email protected]>
+ - Linus Walleij <[email protected]>
+
+allOf:
+ - $ref: touchscreen.yaml#
+
+properties:
+ $nodename:
+ pattern: "^touchscreen(@.*)?$"
+
+ compatible:
+ enum:
+ - zinitix,bt402
+ - zinitix,bt403
+ - zinitix,bt404
+ - zinitix,bt412
+ - zinitix,bt413
+ - zinitix,bt431
+ - zinitix,bt432
+ - zinitix,bt531
+ - zinitix,bt532
+ - zinitix,bt538
+ - zinitix,bt541
+ - zinitix,bt548
+ - zinitix,bt554
+ - zinitix,at100
+
+ reg:
+ description: I2C address on the I2C bus
+
+ clock-frequency:
+ description: I2C client clock frequency, defined for host when using
+ the device on the I2C bus
+ minimum: 0
+ maximum: 400000
+
+ interrupts:
+ description: Interrupt to host
+ maxItems: 1
+
+ vcca-supply:
+ description: Analog power supply regulator on the VCCA pin
+
+ vdd-supply:
+ description: Digital power supply regulator on the VDD pin.
+ In older device trees this can be the accidental name for the analog
+ supply on the VCCA pin, and in that case the deprecated vddo-supply is
+ used for the digital power supply.
+
+ vddo-supply:
+ description: Deprecated name for the digital power supply, use vdd-supply
+ as this reflects the real name of the pin. If this supply is present,
+ the vdd-supply represents VCCA instead of VDD. Implementers should first
+ check for this property, and if it is present assume that the vdd-supply
+ represents the analog supply.
+ deprecated: true
+
+ reset-gpios:
+ description: Reset line for the touchscreen, should be tagged
+ as GPIO_ACTIVE_LOW
+
+ zinitix,mode:
+ description: Mode of reporting touch points. Some modes may not work
+ with a particular ts firmware for unknown reasons. Available modes are
+ 1 and 2. Mode 2 is the default and preferred.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [1, 2]
+
+ touchscreen-size-x: true
+ touchscreen-size-y: true
+ touchscreen-fuzz-x: true
+ touchscreen-fuzz-y: true
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - touchscreen-size-x
+ - touchscreen-size-y
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@20 {
+ compatible = "zinitix,bt541";
+ reg = <0x20>;
+ interrupt-parent = <&gpio>;
+ interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
+ vcca-supply = <&reg_vcca_tsp>;
+ vdd-supply = <&reg_vdd_tsp>;
+ touchscreen-size-x = <540>;
+ touchscreen-size-y = <960>;
+ zinitix,mode = <2>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/input/touchscreen/zinitix.txt b/Documentation/devicetree/bindings/input/touchscreen/zinitix.txt
deleted file mode 100644
index 446efb9f5f55..000000000000
--- a/Documentation/devicetree/bindings/input/touchscreen/zinitix.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-Device tree bindings for Zinitx BT541 touchscreen controller
-
-Required properties:
-
- - compatible : Should be "zinitix,bt541"
- - reg : I2C address of the chip. Should be 0x20
- - interrupts : Interrupt to which the chip is connected
-
-Optional properties:
-
- - vdd-supply : Analog power supply regulator on VCCA pin
- - vddo-supply : Digital power supply regulator on VDD pin
- - zinitix,mode : Mode of reporting touch points. Some modes may not work
- with a particular ts firmware for unknown reasons. Available
- modes are 1 and 2. Mode 2 is the default and preferred.
-
-The touchscreen-* properties are documented in touchscreen.txt in this
-directory.
-
-Example:
-
- i2c@00000000 {
- /* ... */
-
- bt541@20 {
- compatible = "zinitix,bt541";
- reg = <0x20>;
- interrupt-parent = <&msmgpio>;
- interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
- pinctrl-names = "default";
- pinctrl-0 = <&tsp_default>;
- vdd-supply = <&reg_vdd_tsp>;
- vddo-supply = <&pm8916_l6>;
- touchscreen-size-x = <540>;
- touchscreen-size-y = <960>;
- zinitix,mode = <2>;
- };
-
- /* ... */
- };
--
2.30.2

2021-10-27 21:36:01

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH 1/6] input: touchscreen: zinitix: Make sure the IRQ is allocated before it gets enabled

Since irq request is the last thing in the driver probe, it happens
later than the input device registration. This means that there is a
small time window where if the open method is called the driver will
attempt to enable not yet available irq.

Fix that by moving the irq request before the input device registration.

Fixes: 26822652c85e ("Input: add zinitix touchscreen driver")
Signed-off-by: Nikita Travkin <[email protected]>
---
drivers/input/touchscreen/zinitix.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index b8d901099378..1e70b8d2a8d7 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -488,6 +488,15 @@ static int zinitix_ts_probe(struct i2c_client *client)
return error;
}

+ error = devm_request_threaded_irq(&client->dev, client->irq,
+ NULL, zinitix_ts_irq_handler,
+ IRQF_ONESHOT | IRQF_NO_AUTOEN,
+ client->name, bt541);
+ if (error) {
+ dev_err(&client->dev, "Failed to request IRQ: %d\n", error);
+ return error;
+ }
+
error = zinitix_init_input_dev(bt541);
if (error) {
dev_err(&client->dev,
@@ -513,15 +522,6 @@ static int zinitix_ts_probe(struct i2c_client *client)
return -EINVAL;
}

- error = devm_request_threaded_irq(&client->dev, client->irq,
- NULL, zinitix_ts_irq_handler,
- IRQF_ONESHOT | IRQF_NO_AUTOEN,
- client->name, bt541);
- if (error) {
- dev_err(&client->dev, "Failed to request IRQ: %d\n", error);
- return error;
- }
-
return 0;
}

--
2.30.2

2021-10-27 22:09:45

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH 4/6] input: touchscreen: zinitix: Add compatible for bt532

Zinitix BT532 is another touch controller that seem to implement the
same interface as an already supported BT541. Add it to the driver.

Signed-off-by: Nikita Travkin <[email protected]>
---
drivers/input/touchscreen/zinitix.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index d4e06a88a883..c67ff8be0df1 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -571,6 +571,7 @@ static SIMPLE_DEV_PM_OPS(zinitix_pm_ops, zinitix_suspend, zinitix_resume);

#ifdef CONFIG_OF
static const struct of_device_id zinitix_of_match[] = {
+ { .compatible = "zinitix,bt532" },
{ .compatible = "zinitix,bt541" },
{ }
};
--
2.30.2

2021-11-01 21:40:38

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 2/6] dt-bindings: input/ts/zinitix: Convert to YAML, fix and extend

On Wed, 27 Oct 2021 23:13:46 +0500, Nikita Travkin wrote:
> From: Linus Walleij <[email protected]>
>
> This converts the Zinitix BT4xx and BT5xx touchscreen bindings to YAML, fix
> them up a bit and extends them.
>
> We list all the existing BT4xx and BT5xx components with compatible strings.
> These are all similar, use the same bindings and work in similar ways.
>
> We rename the supplies from the erroneous vdd/vddo to the actual supply
> names vcca/vdd as specified on the actual component. It is long established
> that supplies shall be named after the supply pin names of a component.
> The confusion probably stems from that in a certain product the rails to the
> component were named vdd/vddo. Drop some notes on how OS implementations should
> avoid confusion by first looking for vddo, and if that exists assume the
> legacy binding pair and otherwise use vcca/vdd.
>
> Add reset-gpios as sometimes manufacturers pulls a GPIO line to the reset
> line on the chip.
>
> Add optional touchscreen-fuzz-x and touchscreen-fuzz-y properties.
>
> Cc: Mark Brown <[email protected]>
> Cc: Michael Srba <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Linus Walleij <[email protected]>
> [Fixed dt_schema_check error]
> Signed-off-by: Nikita Travkin <[email protected]>
> ---
> This patch was previously submited here:
> https://lore.kernel.org/linux-input/[email protected]/
>
> Changes since the original patch:
> - Use enum for compatible list instead of oneOf + const
> ---
> .../input/touchscreen/zinitix,bt400.yaml | 115 ++++++++++++++++++
> .../bindings/input/touchscreen/zinitix.txt | 40 ------
> 2 files changed, 115 insertions(+), 40 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
> delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix.txt
>

Reviewed-by: Rob Herring <[email protected]>

2021-11-09 12:54:06

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 4/6] input: touchscreen: zinitix: Add compatible for bt532

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <[email protected]> wrote:

> Zinitix BT532 is another touch controller that seem to implement the
> same interface as an already supported BT541. Add it to the driver.
>
> Signed-off-by: Nikita Travkin <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-11-09 13:40:55

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 2/6] dt-bindings: input/ts/zinitix: Convert to YAML, fix and extend

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <[email protected]> wrote:

> This patch was previously submited here:
> https://lore.kernel.org/linux-input/[email protected]/
>
> Changes since the original patch:
> - Use enum for compatible list instead of oneOf + const

Thanks for picking this up. I was meaning to get back to fixing up the
Zinitix driver but haven't had time.

Yours,
Linus Walleij

2021-11-09 13:49:11

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 6/6] input: touchscreen: zinitix: Add touchkey support

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <[email protected]> wrote:

> Zinitix touch controllers can use some of the sense lines for virtual
> keys (like those found on many phones). Add support for those keys.
>
> Signed-off-by: Nikita Travkin <[email protected]>

Nice!
Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-11-09 13:49:11

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

Hi Nikita,

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <[email protected]> wrote:

> This series adds support for the touch-keys that can be present on some
> touchscreen configurations, adds the compatible for bt532 and fixes a
> small race condition bug in the driver probe function.
>
> I also pick up the series that converts the dt bindings to yaml
> initially submitted by Linus Walleij in [1].
> I made some minor changes to those patches:
> - Fixed dt_schema_check error
> - Adressed the review comments from Dmitry on the original series

Thanks for picking this up!

Have you notices some behaviour like surplus touch events
(like many press/release events fall through to the UI)
when using this driver? I think it might need some z fuzzing
but I am not sure.

Yours,
Linus Walleij

2021-11-09 13:49:44

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/6] input: touchscreen: zinitix: Make sure the IRQ is allocated before it gets enabled

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <[email protected]> wrote:

> Since irq request is the last thing in the driver probe, it happens
> later than the input device registration. This means that there is a
> small time window where if the open method is called the driver will
> attempt to enable not yet available irq.
>
> Fix that by moving the irq request before the input device registration.
>
> Fixes: 26822652c85e ("Input: add zinitix touchscreen driver")
> Signed-off-by: Nikita Travkin <[email protected]>

Good catch!
Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-11-10 00:00:31

by Nikita Travkin

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

Hi Linus,

Linus Walleij писал(а) 09.11.2021 09:45:
> Hi Nikita,
>
> On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <[email protected]> wrote:
>
>> This series adds support for the touch-keys that can be present on
>> some
>> touchscreen configurations, adds the compatible for bt532 and fixes a
>> small race condition bug in the driver probe function.
>>
>> I also pick up the series that converts the dt bindings to yaml
>> initially submitted by Linus Walleij in [1].
>> I made some minor changes to those patches:
>> - Fixed dt_schema_check error
>> - Adressed the review comments from Dmitry on the original series
>
> Thanks for picking this up!
>
> Have you notices some behaviour like surplus touch events
> (like many press/release events fall through to the UI)
> when using this driver? I think it might need some z fuzzing
> but I am not sure.
>

On my device (8 inch tablet with BT532) I saw no problems with touch
so far. However another person with a different tablet (10 inch with
ZT7554)
indeed says that they notice "multiplied" touches that make typing hard
so maybe that depends on controller model/firmware...

And speaking of that ZT7554: Seems like it's works with the driver
and I'd like to add the compatible for it in v2 but I'd also have to add
it
to the bindings. Looking at how you add all other similar names for BT*
there
does it make sense to add ZT* as well? Maybe you have some hints where
to look
for a list of the models?

I was planning to send a v2 with all the review fixes near the end of
the week
but I've noticed a yet another quirky issue with the touch controller:
At least on my device, for some reason enabling touchkeys changes the
way the
controller reports the finger touch events which breaks multi-touch...
Assuming that *not* enabling the touchkeys leads to calibration being
wrong
(controller assigns the touchkey sense lines to the touch area in that
case)
I now have to resolve this quirk as well...

Thanks,
Nikita

> Yours,
> Linus Walleij

2021-11-10 06:05:12

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 6/6] input: touchscreen: zinitix: Add touchkey support

Hi Nikita,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on robh/for-next hid/for-next linus/master v5.15 next-20211109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Nikita-Travkin/Add-touch-keys-support-to-the-Zinitix-touch-driver/20211028-031652
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: i386-randconfig-s001-20211027 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/4d4045dad42ed26b0dba61827ffddcd453601895
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Nikita-Travkin/Add-touch-keys-support-to-the-Zinitix-touch-driver/20211028-031652
git checkout 4d4045dad42ed26b0dba61827ffddcd453601895
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/input/touchscreen/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>


sparse warnings: (new ones prefixed by >>)
>> drivers/input/touchscreen/zinitix.c:350:57: sparse: sparse: restricted __le16 degrades to integer
drivers/input/touchscreen/zinitix.c:371:24: sparse: sparse: restricted __le16 degrades to integer

vim +350 drivers/input/touchscreen/zinitix.c

343
344 static void zinitix_report_keys(struct bt541_ts_data *bt541, __le16 icon_events)
345 {
346 int i;
347
348 for (i = 0; i < bt541->num_keycodes; i++)
349 input_report_key(bt541->input_dev,
> 350 bt541->keycodes[i], !!(icon_events & BIT(i)));
351 }
352

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (2.12 kB)
.config.gz (35.19 kB)
Download all attachments

2021-11-11 10:40:21

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

On Tue, Nov 9, 2021 at 4:23 PM Nikita Travkin <[email protected]> wrote:
> [Me]
> > Have you notices some behaviour like surplus touch events
> > (like many press/release events fall through to the UI)
> > when using this driver? I think it might need some z fuzzing
> > but I am not sure.
> >
>
> On my device (8 inch tablet with BT532) I saw no problems with touch
> so far. However another person with a different tablet (10 inch with
> ZT7554)
> indeed says that they notice "multiplied" touches that make typing hard
> so maybe that depends on controller model/firmware...

It may even be depending on specimen. I saw that the vendor driver
does contain some debouncing code.

> And speaking of that ZT7554: Seems like it's works with the driver
> and I'd like to add the compatible for it in v2 but I'd also have to add
> it to the bindings. Looking at how you add all other similar names for BT*
> there does it make sense to add ZT* as well?

Yeah probably, if they are electrically very similar.

> Maybe you have some hints where
> to look for a list of the models?

I usually google ... try to find things like powerpoints with roadmaps
from the vendor and things like that. Best thing is if they answer
to mail but I don't know if Zinitix are even around anymore.

> I've noticed a yet another quirky issue with the touch controller:
> At least on my device, for some reason enabling touchkeys changes the
> way the
> controller reports the finger touch events which breaks multi-touch...
> Assuming that *not* enabling the touchkeys leads to calibration being
> wrong
> (controller assigns the touchkey sense lines to the touch area in that
> case)
> I now have to resolve this quirk as well...

Hm yeah I guess refer to the (messy) vendor driver for hints.

Yours,
Linus Walleij

2022-01-04 21:04:55

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

Hi Nikita,

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <[email protected]> wrote:

> This series adds support for the touch-keys that can be present on some
> touchscreen configurations, adds the compatible for bt532 and fixes a
> small race condition bug in the driver probe function.

This appears unaddressed since October?
I see there are just some small nits in patch 5 & 6 to fix, then
it is finished.

Do you have time to pick it up for kernel v5.17 instead?
Make sure to collect all Reviewed-by on this series.

Yours,
Linus Walleij

2022-01-05 05:10:48

by Nikita Travkin

[permalink] [raw]
Subject: Re: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

Linus Walleij писал(а) 05.01.2022 02:04:
> Hi Nikita,
>
> On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <[email protected]> wrote:
>
>> This series adds support for the touch-keys that can be present on some
>> touchscreen configurations, adds the compatible for bt532 and fixes a
>> small race condition bug in the driver probe function.
>
> This appears unaddressed since October?
> I see there are just some small nits in patch 5 & 6 to fix, then
> it is finished.
>

Hi, I was planning to include the fix for the message reporting
to the next version as well but then I got rather low on time
and could never finish that bit. As it seem to only affect my
device, there was not really much stopping me from submitting
a next version without that fix other than my "irrational
perfectionism" which I should probably learn to recognize better...

> Do you have time to pick it up for kernel v5.17 instead?
> Make sure to collect all Reviewed-by on this series.
>

I will try to submit a new version with review fixes and
tags shortly.

Thanks,
Nikita

> Yours,
> Linus Walleij