* [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek
@ 2026-01-17 0:12 Marek Vasut
2026-01-17 0:12 ` [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe() Marek Vasut
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Marek Vasut @ 2026-01-17 0:12 UTC (permalink / raw)
To: linux-input
Cc: Marek Vasut, Frank Li, Conor Dooley, Dmitry Torokhov, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
The old Ilitek touch controllers V3 and V6 can operate without
interrupt line, in polling mode. Drop the 'interrupts' property
requirement for those four controllers. To avoid overloading the
trivial-touch, fork the old Ilitek V3/V6 touch controller binding
into separate document.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Frank Li <Frank.Li@nxp.com>
Cc: Job Noorman <job@noorman.info>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: Fork the Ilitek V3/V6 bindings into separate document
V3: Add RB from Frank
V4: No change
---
.../input/touchscreen/ilitek,ili210x.yaml | 51 +++++++++++++++++++
.../input/touchscreen/trivial-touch.yaml | 4 --
2 files changed, 51 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ilitek,ili210x.yaml
diff --git a/Documentation/devicetree/bindings/input/touchscreen/ilitek,ili210x.yaml b/Documentation/devicetree/bindings/input/touchscreen/ilitek,ili210x.yaml
new file mode 100644
index 0000000000000..1d02aaba64f97
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ilitek,ili210x.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/ilitek,ili210x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ilitek ILI21xx/ILI251x V3/V6 touch screen controller with i2c interface
+
+maintainers:
+ - Frank Li <Frank.Li@nxp.com>
+ - Marek Vasut <marek.vasut+renesas@mailbox.org>
+
+properties:
+ compatible:
+ enum:
+ - ilitek,ili210x
+ - ilitek,ili2117
+ - ilitek,ili2120
+ - ilitek,ili251x
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ wakeup-source: true
+
+allOf:
+ - $ref: touchscreen.yaml
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@41 {
+ compatible = "ilitek,ili2120";
+ reg = <0x41>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/input/touchscreen/trivial-touch.yaml b/Documentation/devicetree/bindings/input/touchscreen/trivial-touch.yaml
index fa27c6754ca4e..6441d21223caf 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/trivial-touch.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/trivial-touch.yaml
@@ -23,9 +23,6 @@ properties:
# Hynitron cstxxx series touchscreen controller
- hynitron,cst340
# Ilitek I2C Touchscreen Controller
- - ilitek,ili210x
- - ilitek,ili2117
- - ilitek,ili2120
- ilitek,ili2130
- ilitek,ili2131
- ilitek,ili2132
@@ -33,7 +30,6 @@ properties:
- ilitek,ili2322
- ilitek,ili2323
- ilitek,ili2326
- - ilitek,ili251x
- ilitek,ili2520
- ilitek,ili2521
# MAXI MAX11801 Resistive touch screen controller with i2c interface
--
2.51.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe()
2026-01-17 0:12 [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek Marek Vasut
@ 2026-01-17 0:12 ` Marek Vasut
2026-01-17 11:22 ` Krzysztof Kozlowski
` (2 more replies)
2026-01-17 0:12 ` [PATCH v4 3/3] Input: ili210x - add support for polling mode Marek Vasut
2026-01-17 11:22 ` [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek Krzysztof Kozlowski
2 siblings, 3 replies; 17+ messages in thread
From: Marek Vasut @ 2026-01-17 0:12 UTC (permalink / raw)
To: linux-input
Cc: Marek Vasut, Conor Dooley, Dmitry Torokhov, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
Simplify error return handling, use dev_err_probe() where possible.
No functional change.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Frank Li <Frank.Li@nxp.com>
Cc: Job Noorman <job@noorman.info>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V3: New patch
V4: Add missing return to dev_err_probe(dev, -EINVAL, "No IRQ!\n");
---
drivers/input/touchscreen/ili210x.c | 31 ++++++++++-------------------
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index fa38d70aded7b..264eee3e61d0a 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -942,15 +942,11 @@ static int ili210x_i2c_probe(struct i2c_client *client)
chip = device_get_match_data(dev);
if (!chip && id)
chip = (const struct ili2xxx_chip *)id->driver_data;
- if (!chip) {
- dev_err(&client->dev, "unknown device model\n");
- return -ENODEV;
- }
+ if (!chip)
+ return dev_err_probe(&client->dev, -ENODEV, "unknown device model\n");
- if (client->irq <= 0) {
- dev_err(dev, "No IRQ!\n");
- return -EINVAL;
- }
+ if (client->irq <= 0)
+ return dev_err_probe(dev, -EINVAL, "No IRQ!\n");
reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(reset_gpio))
@@ -998,28 +994,21 @@ static int ili210x_i2c_probe(struct i2c_client *client)
error = input_mt_init_slots(input, priv->chip->max_touches,
INPUT_MT_DIRECT);
- if (error) {
- dev_err(dev, "Unable to set up slots, err: %d\n", error);
- return error;
- }
+ if (error)
+ return dev_err_probe(dev, error, "Unable to set up slots\n");
error = devm_request_threaded_irq(dev, client->irq, NULL, ili210x_irq,
IRQF_ONESHOT, client->name, priv);
- if (error) {
- dev_err(dev, "Unable to request touchscreen IRQ, err: %d\n",
- error);
- return error;
- }
+ if (error)
+ return dev_err_probe(dev, error, "Unable to request touchscreen IRQ\n");
error = devm_add_action_or_reset(dev, ili210x_stop, priv);
if (error)
return error;
error = input_register_device(priv->input);
- if (error) {
- dev_err(dev, "Cannot register input device, err: %d\n", error);
- return error;
- }
+ if (error)
+ return dev_err_probe(dev, error, "Cannot register input device\n");
return 0;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 3/3] Input: ili210x - add support for polling mode
2026-01-17 0:12 [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek Marek Vasut
2026-01-17 0:12 ` [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe() Marek Vasut
@ 2026-01-17 0:12 ` Marek Vasut
2026-01-18 17:19 ` Frank Li
2026-01-20 18:31 ` Dmitry Torokhov
2026-01-17 11:22 ` [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek Krzysztof Kozlowski
2 siblings, 2 replies; 17+ messages in thread
From: Marek Vasut @ 2026-01-17 0:12 UTC (permalink / raw)
To: linux-input
Cc: Marek Vasut, Conor Dooley, Dmitry Torokhov, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
There are designs incorporating Ilitek ILI2xxx touch controller that
do not connect interrupt pin, for example Waveshare 13.3" DSI display.
To support such systems use polling mode for the input device when I2C
client does not have interrupt assigned to it.
Factor out ili210x_firmware_update_noirq() to allow conditional scoped
guard around this code. The scoped guard has to be applied only in case
the IRQ line is connected, and not applied otherwise.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Frank Li <Frank.Li@nxp.com>
Cc: Job Noorman <job@noorman.info>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
V2: Test client->irq > 0 for IRQ presence
V3: - Rebase on dev_err_probe() conversion
- Fix if (client->irq > 0) in ili210x_firmware_update_store()
V4: No change
---
drivers/input/touchscreen/ili210x.c | 76 +++++++++++++++++++++--------
1 file changed, 56 insertions(+), 20 deletions(-)
diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 264eee3e61d0a..22917a5825778 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -327,9 +327,8 @@ static bool ili210x_report_events(struct ili210x *priv, u8 *touchdata)
return contact;
}
-static irqreturn_t ili210x_irq(int irq, void *irq_data)
+static void ili210x_process_events(struct ili210x *priv)
{
- struct ili210x *priv = irq_data;
struct i2c_client *client = priv->client;
const struct ili2xxx_chip *chip = priv->chip;
u8 touchdata[ILI210X_DATA_SIZE] = { 0 };
@@ -356,8 +355,22 @@ static irqreturn_t ili210x_irq(int irq, void *irq_data)
usleep_range(time_delta, time_delta + 1000);
}
} while (!priv->stop && keep_polling);
+}
+
+static irqreturn_t ili210x_irq(int irq, void *irq_data)
+{
+ struct ili210x *priv = irq_data;
+
+ ili210x_process_events(priv);
return IRQ_HANDLED;
+};
+
+static void ili210x_work_i2c_poll(struct input_dev *input)
+{
+ struct ili210x *priv = input_get_drvdata(input);
+
+ ili210x_process_events(priv);
}
static int ili251x_firmware_update_resolution(struct device *dev)
@@ -829,12 +842,32 @@ static int ili210x_do_firmware_update(struct ili210x *priv,
return 0;
}
+static ssize_t ili210x_firmware_update_noirq(struct device *dev,
+ const u8 *fwbuf, u16 ac_end, u16 df_end)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct ili210x *priv = i2c_get_clientdata(client);
+ const char *fwname = ILI251X_FW_FILENAME;
+ int error;
+
+ dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
+
+ ili210x_hardware_reset(priv->reset_gpio);
+
+ error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
+
+ ili210x_hardware_reset(priv->reset_gpio);
+
+ dev_dbg(dev, "Firmware update ended, error=%i\n", error);
+
+ return error;
+}
+
static ssize_t ili210x_firmware_update_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
struct i2c_client *client = to_i2c_client(dev);
- struct ili210x *priv = i2c_get_clientdata(client);
const char *fwname = ILI251X_FW_FILENAME;
u16 ac_end, df_end;
int error;
@@ -860,16 +893,12 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
* the touch controller to disable the IRQs during update, so we have
* to do it this way here.
*/
- scoped_guard(disable_irq, &client->irq) {
- dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
-
- ili210x_hardware_reset(priv->reset_gpio);
-
- error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
-
- ili210x_hardware_reset(priv->reset_gpio);
-
- dev_dbg(dev, "Firmware update ended, error=%i\n", error);
+ if (client->irq > 0) {
+ scoped_guard(disable_irq, &client->irq) {
+ error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
+ }
+ } else {
+ error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
}
return error ?: count;
@@ -945,9 +974,6 @@ static int ili210x_i2c_probe(struct i2c_client *client)
if (!chip)
return dev_err_probe(&client->dev, -ENODEV, "unknown device model\n");
- if (client->irq <= 0)
- return dev_err_probe(dev, -EINVAL, "No IRQ!\n");
-
reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(reset_gpio))
return PTR_ERR(reset_gpio);
@@ -997,10 +1023,20 @@ static int ili210x_i2c_probe(struct i2c_client *client)
if (error)
return dev_err_probe(dev, error, "Unable to set up slots\n");
- error = devm_request_threaded_irq(dev, client->irq, NULL, ili210x_irq,
- IRQF_ONESHOT, client->name, priv);
- if (error)
- return dev_err_probe(dev, error, "Unable to request touchscreen IRQ\n");
+ input_set_drvdata(input, priv);
+
+ if (client->irq > 0) {
+ error = devm_request_threaded_irq(dev, client->irq, NULL, ili210x_irq,
+ IRQF_ONESHOT, client->name, priv);
+ if (error)
+ return dev_err_probe(dev, error, "Unable to request touchscreen IRQ\n");
+ } else {
+ error = input_setup_polling(input, ili210x_work_i2c_poll);
+ if (error)
+ return dev_err_probe(dev, error, "Could not set up polling mode\n");
+
+ input_set_poll_interval(input, ILI2XXX_POLL_PERIOD);
+ }
error = devm_add_action_or_reset(dev, ili210x_stop, priv);
if (error)
--
2.51.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek
2026-01-17 0:12 [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek Marek Vasut
2026-01-17 0:12 ` [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe() Marek Vasut
2026-01-17 0:12 ` [PATCH v4 3/3] Input: ili210x - add support for polling mode Marek Vasut
@ 2026-01-17 11:22 ` Krzysztof Kozlowski
2026-01-17 15:33 ` Marek Vasut
2 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-17 11:22 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Frank Li, Conor Dooley, Dmitry Torokhov, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On Sat, Jan 17, 2026 at 01:12:02AM +0100, Marek Vasut wrote:
> The old Ilitek touch controllers V3 and V6 can operate without
> interrupt line, in polling mode. Drop the 'interrupts' property
> requirement for those four controllers. To avoid overloading the
> trivial-touch, fork the old Ilitek V3/V6 touch controller binding
> into separate document.
One if: block is fine, so IMO, this should stay in original binding
especially that more devices like some azoteq or semtech might have same
rule of not requiring interrupt line. Anyway, no big deal.
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Frank Li <Frank.Li@nxp.com>
> Cc: Job Noorman <job@noorman.info>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V2: Fork the Ilitek V3/V6 bindings into separate document
> V3: Add RB from Frank
> V4: No change
> ---
> .../input/touchscreen/ilitek,ili210x.yaml | 51 +++++++++++++++++++
> .../input/touchscreen/trivial-touch.yaml | 4 --
> 2 files changed, 51 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ilitek,ili210x.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ilitek,ili210x.yaml b/Documentation/devicetree/bindings/input/touchscreen/ilitek,ili210x.yaml
> new file mode 100644
> index 0000000000000..1d02aaba64f97
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ilitek,ili210x.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/ilitek,ili210x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ilitek ILI21xx/ILI251x V3/V6 touch screen controller with i2c interface
> +
> +maintainers:
> + - Frank Li <Frank.Li@nxp.com>
> + - Marek Vasut <marek.vasut+renesas@mailbox.org>
> +
> +properties:
> + compatible:
> + enum:
> + - ilitek,ili210x
> + - ilitek,ili2117
> + - ilitek,ili2120
> + - ilitek,ili251x
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> +
> + wakeup-source: true
> +
> +allOf:
> + - $ref: touchscreen.yaml
allOf: should be placed after required: block.
With this one fixed:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe()
2026-01-17 0:12 ` [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe() Marek Vasut
@ 2026-01-17 11:22 ` Krzysztof Kozlowski
2026-01-20 10:20 ` Geert Uytterhoeven
2026-01-20 18:25 ` Dmitry Torokhov
2 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-17 11:22 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Conor Dooley, Dmitry Torokhov, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On Sat, Jan 17, 2026 at 01:12:03AM +0100, Marek Vasut wrote:
> Simplify error return handling, use dev_err_probe() where possible.
> No functional change.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek
2026-01-17 11:22 ` [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek Krzysztof Kozlowski
@ 2026-01-17 15:33 ` Marek Vasut
2026-01-17 18:34 ` Krzysztof Kozlowski
0 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2026-01-17 15:33 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-input, Frank Li, Conor Dooley, Dmitry Torokhov, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On 1/17/26 12:22 PM, Krzysztof Kozlowski wrote:
> On Sat, Jan 17, 2026 at 01:12:02AM +0100, Marek Vasut wrote:
>> The old Ilitek touch controllers V3 and V6 can operate without
>> interrupt line, in polling mode. Drop the 'interrupts' property
>> requirement for those four controllers. To avoid overloading the
>> trivial-touch, fork the old Ilitek V3/V6 touch controller binding
>> into separate document.
>
> One if: block is fine, so IMO, this should stay in original binding
> especially that more devices like some azoteq or semtech might have same
> rule of not requiring interrupt line. Anyway, no big deal.
I am not sure about the other non-ilitek devices, but the fruitboards do
use at least goodix and etm/edt touch controllers without interrupt line
too, those I have on my desk (those two have separate, more extensive,
binding document). I also suspect we will see more of those touch
controllers with optional interrupt line, so if we do, I think we can
re-combine the binding documents again ?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek
2026-01-17 15:33 ` Marek Vasut
@ 2026-01-17 18:34 ` Krzysztof Kozlowski
2026-01-20 10:23 ` Geert Uytterhoeven
0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-17 18:34 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Frank Li, Conor Dooley, Dmitry Torokhov, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On 17/01/2026 16:33, Marek Vasut wrote:
> On 1/17/26 12:22 PM, Krzysztof Kozlowski wrote:
>> On Sat, Jan 17, 2026 at 01:12:02AM +0100, Marek Vasut wrote:
>>> The old Ilitek touch controllers V3 and V6 can operate without
>>> interrupt line, in polling mode. Drop the 'interrupts' property
>>> requirement for those four controllers. To avoid overloading the
>>> trivial-touch, fork the old Ilitek V3/V6 touch controller binding
>>> into separate document.
>>
>> One if: block is fine, so IMO, this should stay in original binding
>> especially that more devices like some azoteq or semtech might have same
>> rule of not requiring interrupt line. Anyway, no big deal.
> I am not sure about the other non-ilitek devices, but the fruitboards do
> use at least goodix and etm/edt touch controllers without interrupt line
> too, those I have on my desk (those two have separate, more extensive,
> binding document). I also suspect we will see more of those touch
> controllers with optional interrupt line, so if we do, I think we can
> re-combine the binding documents again ?
Yes.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] Input: ili210x - add support for polling mode
2026-01-17 0:12 ` [PATCH v4 3/3] Input: ili210x - add support for polling mode Marek Vasut
@ 2026-01-18 17:19 ` Frank Li
2026-01-20 18:31 ` Dmitry Torokhov
1 sibling, 0 replies; 17+ messages in thread
From: Frank Li @ 2026-01-18 17:19 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Conor Dooley, Dmitry Torokhov, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On Sat, Jan 17, 2026 at 01:12:04AM +0100, Marek Vasut wrote:
> There are designs incorporating Ilitek ILI2xxx touch controller that
> do not connect interrupt pin, for example Waveshare 13.3" DSI display.
> To support such systems use polling mode for the input device when I2C
> client does not have interrupt assigned to it.
>
> Factor out ili210x_firmware_update_noirq() to allow conditional scoped
> guard around this code. The scoped guard has to be applied only in case
> the IRQ line is connected, and not applied otherwise.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Frank Li <Frank.Li@nxp.com>
> Cc: Job Noorman <job@noorman.info>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V2: Test client->irq > 0 for IRQ presence
> V3: - Rebase on dev_err_probe() conversion
> - Fix if (client->irq > 0) in ili210x_firmware_update_store()
> V4: No change
> ---
> drivers/input/touchscreen/ili210x.c | 76 +++++++++++++++++++++--------
> 1 file changed, 56 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> index 264eee3e61d0a..22917a5825778 100644
> --- a/drivers/input/touchscreen/ili210x.c
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -327,9 +327,8 @@ static bool ili210x_report_events(struct ili210x *priv, u8 *touchdata)
> return contact;
> }
>
> -static irqreturn_t ili210x_irq(int irq, void *irq_data)
> +static void ili210x_process_events(struct ili210x *priv)
> {
> - struct ili210x *priv = irq_data;
> struct i2c_client *client = priv->client;
> const struct ili2xxx_chip *chip = priv->chip;
> u8 touchdata[ILI210X_DATA_SIZE] = { 0 };
> @@ -356,8 +355,22 @@ static irqreturn_t ili210x_irq(int irq, void *irq_data)
> usleep_range(time_delta, time_delta + 1000);
> }
> } while (!priv->stop && keep_polling);
> +}
> +
> +static irqreturn_t ili210x_irq(int irq, void *irq_data)
> +{
> + struct ili210x *priv = irq_data;
> +
> + ili210x_process_events(priv);
>
> return IRQ_HANDLED;
> +};
> +
> +static void ili210x_work_i2c_poll(struct input_dev *input)
> +{
> + struct ili210x *priv = input_get_drvdata(input);
> +
> + ili210x_process_events(priv);
> }
>
> static int ili251x_firmware_update_resolution(struct device *dev)
> @@ -829,12 +842,32 @@ static int ili210x_do_firmware_update(struct ili210x *priv,
> return 0;
> }
>
> +static ssize_t ili210x_firmware_update_noirq(struct device *dev,
> + const u8 *fwbuf, u16 ac_end, u16 df_end)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct ili210x *priv = i2c_get_clientdata(client);
> + const char *fwname = ILI251X_FW_FILENAME;
> + int error;
> +
> + dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
> +
> + ili210x_hardware_reset(priv->reset_gpio);
> +
> + error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
> +
> + ili210x_hardware_reset(priv->reset_gpio);
> +
> + dev_dbg(dev, "Firmware update ended, error=%i\n", error);
> +
> + return error;
> +}
> +
> static ssize_t ili210x_firmware_update_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct i2c_client *client = to_i2c_client(dev);
> - struct ili210x *priv = i2c_get_clientdata(client);
> const char *fwname = ILI251X_FW_FILENAME;
> u16 ac_end, df_end;
> int error;
> @@ -860,16 +893,12 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
> * the touch controller to disable the IRQs during update, so we have
> * to do it this way here.
> */
> - scoped_guard(disable_irq, &client->irq) {
> - dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
> -
> - ili210x_hardware_reset(priv->reset_gpio);
> -
> - error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
> -
> - ili210x_hardware_reset(priv->reset_gpio);
> -
> - dev_dbg(dev, "Firmware update ended, error=%i\n", error);
> + if (client->irq > 0) {
> + scoped_guard(disable_irq, &client->irq) {
> + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> + }
> + } else {
> + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> }
>
> return error ?: count;
> @@ -945,9 +974,6 @@ static int ili210x_i2c_probe(struct i2c_client *client)
> if (!chip)
> return dev_err_probe(&client->dev, -ENODEV, "unknown device model\n");
>
> - if (client->irq <= 0)
> - return dev_err_probe(dev, -EINVAL, "No IRQ!\n");
> -
> reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> if (IS_ERR(reset_gpio))
> return PTR_ERR(reset_gpio);
> @@ -997,10 +1023,20 @@ static int ili210x_i2c_probe(struct i2c_client *client)
> if (error)
> return dev_err_probe(dev, error, "Unable to set up slots\n");
>
> - error = devm_request_threaded_irq(dev, client->irq, NULL, ili210x_irq,
> - IRQF_ONESHOT, client->name, priv);
> - if (error)
> - return dev_err_probe(dev, error, "Unable to request touchscreen IRQ\n");
> + input_set_drvdata(input, priv);
> +
> + if (client->irq > 0) {
> + error = devm_request_threaded_irq(dev, client->irq, NULL, ili210x_irq,
> + IRQF_ONESHOT, client->name, priv);
> + if (error)
> + return dev_err_probe(dev, error, "Unable to request touchscreen IRQ\n");
> + } else {
> + error = input_setup_polling(input, ili210x_work_i2c_poll);
> + if (error)
> + return dev_err_probe(dev, error, "Could not set up polling mode\n");
> +
> + input_set_poll_interval(input, ILI2XXX_POLL_PERIOD);
> + }
>
> error = devm_add_action_or_reset(dev, ili210x_stop, priv);
> if (error)
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe()
2026-01-17 0:12 ` [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe() Marek Vasut
2026-01-17 11:22 ` Krzysztof Kozlowski
@ 2026-01-20 10:20 ` Geert Uytterhoeven
2026-01-20 18:25 ` Dmitry Torokhov
2 siblings, 0 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2026-01-20 10:20 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Conor Dooley, Dmitry Torokhov, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On Sat, 17 Jan 2026 at 01:12, Marek Vasut
<marek.vasut+renesas@mailbox.org> wrote:
> Simplify error return handling, use dev_err_probe() where possible.
> No functional change.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek
2026-01-17 18:34 ` Krzysztof Kozlowski
@ 2026-01-20 10:23 ` Geert Uytterhoeven
0 siblings, 0 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2026-01-20 10:23 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Marek Vasut, linux-input, Frank Li, Conor Dooley, Dmitry Torokhov,
Job Noorman, Krzysztof Kozlowski, Rob Herring, devicetree,
linux-kernel, linux-renesas-soc
On Sat, 17 Jan 2026 at 19:34, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 17/01/2026 16:33, Marek Vasut wrote:
> > On 1/17/26 12:22 PM, Krzysztof Kozlowski wrote:
> >> On Sat, Jan 17, 2026 at 01:12:02AM +0100, Marek Vasut wrote:
> >>> The old Ilitek touch controllers V3 and V6 can operate without
> >>> interrupt line, in polling mode. Drop the 'interrupts' property
> >>> requirement for those four controllers. To avoid overloading the
> >>> trivial-touch, fork the old Ilitek V3/V6 touch controller binding
> >>> into separate document.
> >>
> >> One if: block is fine, so IMO, this should stay in original binding
> >> especially that more devices like some azoteq or semtech might have same
> >> rule of not requiring interrupt line. Anyway, no big deal.
> > I am not sure about the other non-ilitek devices, but the fruitboards do
> > use at least goodix and etm/edt touch controllers without interrupt line
> > too, those I have on my desk (those two have separate, more extensive,
> > binding document). I also suspect we will see more of those touch
> > controllers with optional interrupt line, so if we do, I think we can
> > re-combine the binding documents again ?
>
> Yes.
IMHO more churn, and more git blame mismatches...
Why not keep them in a single document in the first place, like in v1?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe()
2026-01-17 0:12 ` [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe() Marek Vasut
2026-01-17 11:22 ` Krzysztof Kozlowski
2026-01-20 10:20 ` Geert Uytterhoeven
@ 2026-01-20 18:25 ` Dmitry Torokhov
2 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2026-01-20 18:25 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Conor Dooley, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On Sat, Jan 17, 2026 at 01:12:03AM +0100, Marek Vasut wrote:
> Simplify error return handling, use dev_err_probe() where possible.
> No functional change.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Applied, thank you.
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] Input: ili210x - add support for polling mode
2026-01-17 0:12 ` [PATCH v4 3/3] Input: ili210x - add support for polling mode Marek Vasut
2026-01-18 17:19 ` Frank Li
@ 2026-01-20 18:31 ` Dmitry Torokhov
2026-01-20 22:50 ` Marek Vasut
1 sibling, 1 reply; 17+ messages in thread
From: Dmitry Torokhov @ 2026-01-20 18:31 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Conor Dooley, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
Hi Marek,
On Sat, Jan 17, 2026 at 01:12:04AM +0100, Marek Vasut wrote:
> @@ -860,16 +893,12 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
> * the touch controller to disable the IRQs during update, so we have
> * to do it this way here.
> */
> - scoped_guard(disable_irq, &client->irq) {
> - dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
> -
> - ili210x_hardware_reset(priv->reset_gpio);
> -
> - error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
> -
> - ili210x_hardware_reset(priv->reset_gpio);
> -
> - dev_dbg(dev, "Firmware update ended, error=%i\n", error);
> + if (client->irq > 0) {
> + scoped_guard(disable_irq, &client->irq) {
> + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> + }
You already have a scope here, no need to establish a new one:
guard(disable_irq)(&client->irq);
error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
BTW, not a fan of the "_noirq" suffix... Maybe drop it and add
lockdep_is_held() there?
> + } else {
> + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> }
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] Input: ili210x - add support for polling mode
2026-01-20 18:31 ` Dmitry Torokhov
@ 2026-01-20 22:50 ` Marek Vasut
2026-01-21 5:23 ` Dmitry Torokhov
0 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2026-01-20 22:50 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Conor Dooley, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On 1/20/26 7:31 PM, Dmitry Torokhov wrote:
> Hi Marek,
>
> On Sat, Jan 17, 2026 at 01:12:04AM +0100, Marek Vasut wrote:
>> @@ -860,16 +893,12 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
>> * the touch controller to disable the IRQs during update, so we have
>> * to do it this way here.
>> */
>> - scoped_guard(disable_irq, &client->irq) {
>> - dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
>> -
>> - ili210x_hardware_reset(priv->reset_gpio);
>> -
>> - error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
>> -
>> - ili210x_hardware_reset(priv->reset_gpio);
>> -
>> - dev_dbg(dev, "Firmware update ended, error=%i\n", error);
>> + if (client->irq > 0) {
>> + scoped_guard(disable_irq, &client->irq) {
>> + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
>> + }
>
> You already have a scope here, no need to establish a new one:
>
> guard(disable_irq)(&client->irq);
> error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
This part ^ I do not understand. If there is no IRQ defined in DT, I
need to call ili210x_firmware_update_noirq() without the guard because I
cannot disable_irq() with client->irq < 0, else I need to call
ili210x_firmware_update_noirq() within the scoped_guard() to disable
IRQs to avoid spurious IRQs that would interfere with the firmware update ?
> BTW, not a fan of the "_noirq" suffix... Maybe drop it and add
> lockdep_is_held() there?
This part I understand even less, how does lockdep play into this ? The
scoped_guard() disables and enables IRQs if they are available.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] Input: ili210x - add support for polling mode
2026-01-20 22:50 ` Marek Vasut
@ 2026-01-21 5:23 ` Dmitry Torokhov
2026-01-21 22:42 ` Marek Vasut
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Torokhov @ 2026-01-21 5:23 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Conor Dooley, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On Tue, Jan 20, 2026 at 11:50:53PM +0100, Marek Vasut wrote:
> On 1/20/26 7:31 PM, Dmitry Torokhov wrote:
> > Hi Marek,
> >
> > On Sat, Jan 17, 2026 at 01:12:04AM +0100, Marek Vasut wrote:
> > > @@ -860,16 +893,12 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
> > > * the touch controller to disable the IRQs during update, so we have
> > > * to do it this way here.
> > > */
> > > - scoped_guard(disable_irq, &client->irq) {
> > > - dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
> > > -
> > > - ili210x_hardware_reset(priv->reset_gpio);
> > > -
> > > - error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
> > > -
> > > - ili210x_hardware_reset(priv->reset_gpio);
> > > -
> > > - dev_dbg(dev, "Firmware update ended, error=%i\n", error);
> > > + if (client->irq > 0) {
> > > + scoped_guard(disable_irq, &client->irq) {
> > > + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> > > + }
> >
> > You already have a scope here, no need to establish a new one:
> >
> > guard(disable_irq)(&client->irq);
> > error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
>
> This part ^ I do not understand. If there is no IRQ defined in DT, I need to
> call ili210x_firmware_update_noirq() without the guard because I cannot
> disable_irq() with client->irq < 0, else I need to call
> ili210x_firmware_update_noirq() within the scoped_guard() to disable IRQs to
> avoid spurious IRQs that would interfere with the firmware update ?
You do not need to use scoped_guard() because you already define a scope
in your if statement:
if (client->irq > 0) {
guard(disable_irq)(&client->irq);
error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
} else {
error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
}
This is sill a bit awkward. Maybe we could add to interrupt.h
void __disable_valid_irq(unsigned int irq)
{
if (irq > 0)
disable_irq(irq);
}
void __enable_valid_irq(unsigned int irq)
{
if (irq > 0)
enable_irq(irq);
}
DEFINE_LOCK_GUARD_1(disable_valid_irq, int,
disable_valid_irq(*_T->lock), enable_valid_irq(*_T->lock))
and then we'd be able to keep the driver as is (just adjust the type of
the original scoped_guard).
>
> > BTW, not a fan of the "_noirq" suffix... Maybe drop it and add
> > lockdep_is_held() there?
>
> This part I understand even less, how does lockdep play into this ? The
> scoped_guard() disables and enables IRQs if they are available.
Ah, sorry, brainfart on my part. I got confused by _noirq suffix.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] Input: ili210x - add support for polling mode
2026-01-21 5:23 ` Dmitry Torokhov
@ 2026-01-21 22:42 ` Marek Vasut
2026-01-21 22:53 ` Dmitry Torokhov
0 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2026-01-21 22:42 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Conor Dooley, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On 1/21/26 6:23 AM, Dmitry Torokhov wrote:
> On Tue, Jan 20, 2026 at 11:50:53PM +0100, Marek Vasut wrote:
>> On 1/20/26 7:31 PM, Dmitry Torokhov wrote:
>>> Hi Marek,
>>>
>>> On Sat, Jan 17, 2026 at 01:12:04AM +0100, Marek Vasut wrote:
>>>> @@ -860,16 +893,12 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
>>>> * the touch controller to disable the IRQs during update, so we have
>>>> * to do it this way here.
>>>> */
>>>> - scoped_guard(disable_irq, &client->irq) {
>>>> - dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
>>>> -
>>>> - ili210x_hardware_reset(priv->reset_gpio);
>>>> -
>>>> - error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
>>>> -
>>>> - ili210x_hardware_reset(priv->reset_gpio);
>>>> -
>>>> - dev_dbg(dev, "Firmware update ended, error=%i\n", error);
>>>> + if (client->irq > 0) {
>>>> + scoped_guard(disable_irq, &client->irq) {
>>>> + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
>>>> + }
>>>
>>> You already have a scope here, no need to establish a new one:
>>>
>>> guard(disable_irq)(&client->irq);
>>> error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
>>
>> This part ^ I do not understand. If there is no IRQ defined in DT, I need to
>> call ili210x_firmware_update_noirq() without the guard because I cannot
>> disable_irq() with client->irq < 0, else I need to call
>> ili210x_firmware_update_noirq() within the scoped_guard() to disable IRQs to
>> avoid spurious IRQs that would interfere with the firmware update ?
>
> You do not need to use scoped_guard() because you already define a scope
> in your if statement:
>
> if (client->irq > 0) {
> guard(disable_irq)(&client->irq);
> error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> } else {
> error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> }
>
> This is sill a bit awkward. Maybe we could add to interrupt.h
Let me do the part above in V5 , and then the part below as a separate
follow up patch/series. I already added the later in tree so it won't be
lost. Does that work for you ?
> void __disable_valid_irq(unsigned int irq)
> {
> if (irq > 0)
> disable_irq(irq);
> }
>
> void __enable_valid_irq(unsigned int irq)
> {
> if (irq > 0)
> enable_irq(irq);
> }
>
> DEFINE_LOCK_GUARD_1(disable_valid_irq, int,
> disable_valid_irq(*_T->lock), enable_valid_irq(*_T->lock))
>
> and then we'd be able to keep the driver as is (just adjust the type of
> the original scoped_guard).
>
>>
>>> BTW, not a fan of the "_noirq" suffix... Maybe drop it and add
>>> lockdep_is_held() there?
>>
>> This part I understand even less, how does lockdep play into this ? The
>> scoped_guard() disables and enables IRQs if they are available.
>
> Ah, sorry, brainfart on my part. I got confused by _noirq suffix.
OK
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] Input: ili210x - add support for polling mode
2026-01-21 22:42 ` Marek Vasut
@ 2026-01-21 22:53 ` Dmitry Torokhov
2026-01-21 23:12 ` Marek Vasut
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Torokhov @ 2026-01-21 22:53 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Conor Dooley, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On Wed, Jan 21, 2026 at 11:42:55PM +0100, Marek Vasut wrote:
> On 1/21/26 6:23 AM, Dmitry Torokhov wrote:
> > On Tue, Jan 20, 2026 at 11:50:53PM +0100, Marek Vasut wrote:
> > > On 1/20/26 7:31 PM, Dmitry Torokhov wrote:
> > > > Hi Marek,
> > > >
> > > > On Sat, Jan 17, 2026 at 01:12:04AM +0100, Marek Vasut wrote:
> > > > > @@ -860,16 +893,12 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
> > > > > * the touch controller to disable the IRQs during update, so we have
> > > > > * to do it this way here.
> > > > > */
> > > > > - scoped_guard(disable_irq, &client->irq) {
> > > > > - dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
> > > > > -
> > > > > - ili210x_hardware_reset(priv->reset_gpio);
> > > > > -
> > > > > - error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
> > > > > -
> > > > > - ili210x_hardware_reset(priv->reset_gpio);
> > > > > -
> > > > > - dev_dbg(dev, "Firmware update ended, error=%i\n", error);
> > > > > + if (client->irq > 0) {
> > > > > + scoped_guard(disable_irq, &client->irq) {
> > > > > + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> > > > > + }
> > > >
> > > > You already have a scope here, no need to establish a new one:
> > > >
> > > > guard(disable_irq)(&client->irq);
> > > > error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> > >
> > > This part ^ I do not understand. If there is no IRQ defined in DT, I need to
> > > call ili210x_firmware_update_noirq() without the guard because I cannot
> > > disable_irq() with client->irq < 0, else I need to call
> > > ili210x_firmware_update_noirq() within the scoped_guard() to disable IRQs to
> > > avoid spurious IRQs that would interfere with the firmware update ?
> >
> > You do not need to use scoped_guard() because you already define a scope
> > in your if statement:
> >
> > if (client->irq > 0) {
> > guard(disable_irq)(&client->irq);
> > error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> > } else {
> > error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> > }
> >
> > This is sill a bit awkward. Maybe we could add to interrupt.h
>
> Let me do the part above in V5 , and then the part below as a separate
> follow up patch/series. I already added the later in tree so it won't be
> lost. Does that work for you ?
It does, thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] Input: ili210x - add support for polling mode
2026-01-21 22:53 ` Dmitry Torokhov
@ 2026-01-21 23:12 ` Marek Vasut
0 siblings, 0 replies; 17+ messages in thread
From: Marek Vasut @ 2026-01-21 23:12 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Conor Dooley, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
On 1/21/26 11:53 PM, Dmitry Torokhov wrote:
> On Wed, Jan 21, 2026 at 11:42:55PM +0100, Marek Vasut wrote:
>> On 1/21/26 6:23 AM, Dmitry Torokhov wrote:
>>> On Tue, Jan 20, 2026 at 11:50:53PM +0100, Marek Vasut wrote:
>>>> On 1/20/26 7:31 PM, Dmitry Torokhov wrote:
>>>>> Hi Marek,
>>>>>
>>>>> On Sat, Jan 17, 2026 at 01:12:04AM +0100, Marek Vasut wrote:
>>>>>> @@ -860,16 +893,12 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
>>>>>> * the touch controller to disable the IRQs during update, so we have
>>>>>> * to do it this way here.
>>>>>> */
>>>>>> - scoped_guard(disable_irq, &client->irq) {
>>>>>> - dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
>>>>>> -
>>>>>> - ili210x_hardware_reset(priv->reset_gpio);
>>>>>> -
>>>>>> - error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
>>>>>> -
>>>>>> - ili210x_hardware_reset(priv->reset_gpio);
>>>>>> -
>>>>>> - dev_dbg(dev, "Firmware update ended, error=%i\n", error);
>>>>>> + if (client->irq > 0) {
>>>>>> + scoped_guard(disable_irq, &client->irq) {
>>>>>> + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
>>>>>> + }
>>>>>
>>>>> You already have a scope here, no need to establish a new one:
>>>>>
>>>>> guard(disable_irq)(&client->irq);
>>>>> error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
>>>>
>>>> This part ^ I do not understand. If there is no IRQ defined in DT, I need to
>>>> call ili210x_firmware_update_noirq() without the guard because I cannot
>>>> disable_irq() with client->irq < 0, else I need to call
>>>> ili210x_firmware_update_noirq() within the scoped_guard() to disable IRQs to
>>>> avoid spurious IRQs that would interfere with the firmware update ?
>>>
>>> You do not need to use scoped_guard() because you already define a scope
>>> in your if statement:
>>>
>>> if (client->irq > 0) {
>>> guard(disable_irq)(&client->irq);
>>> error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
>>> } else {
>>> error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
>>> }
>>>
>>> This is sill a bit awkward. Maybe we could add to interrupt.h
>>
>> Let me do the part above in V5 , and then the part below as a separate
>> follow up patch/series. I already added the later in tree so it won't be
>> lost. Does that work for you ?
>
> It does, thanks.
OK, V5 is out, the disable_valid_interrupt guard part is coming shortly.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-01-21 23:12 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-17 0:12 [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek Marek Vasut
2026-01-17 0:12 ` [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe() Marek Vasut
2026-01-17 11:22 ` Krzysztof Kozlowski
2026-01-20 10:20 ` Geert Uytterhoeven
2026-01-20 18:25 ` Dmitry Torokhov
2026-01-17 0:12 ` [PATCH v4 3/3] Input: ili210x - add support for polling mode Marek Vasut
2026-01-18 17:19 ` Frank Li
2026-01-20 18:31 ` Dmitry Torokhov
2026-01-20 22:50 ` Marek Vasut
2026-01-21 5:23 ` Dmitry Torokhov
2026-01-21 22:42 ` Marek Vasut
2026-01-21 22:53 ` Dmitry Torokhov
2026-01-21 23:12 ` Marek Vasut
2026-01-17 11:22 ` [PATCH v4 1/3] dt-bindings: touchscreen: trivial-touch: Drop 'interrupts' requirement for old Ilitek Krzysztof Kozlowski
2026-01-17 15:33 ` Marek Vasut
2026-01-17 18:34 ` Krzysztof Kozlowski
2026-01-20 10:23 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox