linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply
@ 2024-07-11 17:27 Dmitry Torokhov
  2024-07-11 17:27 ` [PATCH 2/6] Input: tsc2004/5 - do not hard code interrupt trigger Dmitry Torokhov
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2024-07-11 17:27 UTC (permalink / raw)
  To: linux-input, Sebastian Reichel; +Cc: linux-kernel

The chip needs to be powered up before calling tsc200x_stop_scan() which
communicates with it; move the call to enable the regulator earlier in
tsc200x_probe().

At the same time switch to using devm_regulator_get_enable() to simplify
error handling. This also makes sure that regulator is not shut off too
early when unbinding the driver.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/tsc2004.c      |  6 ------
 drivers/input/touchscreen/tsc2005.c      |  6 ------
 drivers/input/touchscreen/tsc200x-core.c | 27 ++++--------------------
 drivers/input/touchscreen/tsc200x-core.h |  1 -
 4 files changed, 4 insertions(+), 36 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c
index b673098535ad..787f2caf4f73 100644
--- a/drivers/input/touchscreen/tsc2004.c
+++ b/drivers/input/touchscreen/tsc2004.c
@@ -42,11 +42,6 @@ static int tsc2004_probe(struct i2c_client *i2c)
 			     tsc2004_cmd);
 }
 
-static void tsc2004_remove(struct i2c_client *i2c)
-{
-	tsc200x_remove(&i2c->dev);
-}
-
 static const struct i2c_device_id tsc2004_idtable[] = {
 	{ "tsc2004" },
 	{ }
@@ -70,7 +65,6 @@ static struct i2c_driver tsc2004_driver = {
 	},
 	.id_table       = tsc2004_idtable,
 	.probe          = tsc2004_probe,
-	.remove         = tsc2004_remove,
 };
 module_i2c_driver(tsc2004_driver);
 
diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 1b40ce0ca1b9..6fe8b41b3ecc 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -64,11 +64,6 @@ static int tsc2005_probe(struct spi_device *spi)
 			     tsc2005_cmd);
 }
 
-static void tsc2005_remove(struct spi_device *spi)
-{
-	tsc200x_remove(&spi->dev);
-}
-
 #ifdef CONFIG_OF
 static const struct of_device_id tsc2005_of_match[] = {
 	{ .compatible = "ti,tsc2005" },
@@ -85,7 +80,6 @@ static struct spi_driver tsc2005_driver = {
 		.pm		= pm_sleep_ptr(&tsc200x_pm_ops),
 	},
 	.probe	= tsc2005_probe,
-	.remove	= tsc2005_remove,
 };
 module_spi_driver(tsc2005_driver);
 
diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
index a4c0e9db9bb9..39789a27f65b 100644
--- a/drivers/input/touchscreen/tsc200x-core.c
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -104,8 +104,6 @@ struct tsc200x {
 
 	bool			pen_down;
 
-	struct regulator	*vio;
-
 	struct gpio_desc	*reset_gpio;
 	int			(*tsc200x_cmd)(struct device *dev, u8 cmd);
 	int			irq;
@@ -495,10 +493,9 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 		return error;
 	}
 
-	ts->vio = devm_regulator_get(dev, "vio");
-	if (IS_ERR(ts->vio)) {
-		error = PTR_ERR(ts->vio);
-		dev_err(dev, "error acquiring vio regulator: %d", error);
+	error = devm_regulator_get_enable(dev, "vio");
+	if (error) {
+		dev_err(dev, "error acquiring vio regulator: %d\n", error);
 		return error;
 	}
 
@@ -554,36 +551,20 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 		return error;
 	}
 
-	error = regulator_enable(ts->vio);
-	if (error)
-		return error;
-
 	dev_set_drvdata(dev, ts);
 
 	error = input_register_device(ts->idev);
 	if (error) {
 		dev_err(dev,
 			"Failed to register input device, err: %d\n", error);
-		goto disable_regulator;
+		return error;
 	}
 
 	irq_set_irq_wake(irq, 1);
 	return 0;
-
-disable_regulator:
-	regulator_disable(ts->vio);
-	return error;
 }
 EXPORT_SYMBOL_GPL(tsc200x_probe);
 
-void tsc200x_remove(struct device *dev)
-{
-	struct tsc200x *ts = dev_get_drvdata(dev);
-
-	regulator_disable(ts->vio);
-}
-EXPORT_SYMBOL_GPL(tsc200x_remove);
-
 static int tsc200x_suspend(struct device *dev)
 {
 	struct tsc200x *ts = dev_get_drvdata(dev);
diff --git a/drivers/input/touchscreen/tsc200x-core.h b/drivers/input/touchscreen/tsc200x-core.h
index 37de91efd78e..e76ba7a889dd 100644
--- a/drivers/input/touchscreen/tsc200x-core.h
+++ b/drivers/input/touchscreen/tsc200x-core.h
@@ -75,6 +75,5 @@ extern const struct attribute_group *tsc200x_groups[];
 int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 		  struct regmap *regmap,
 		  int (*tsc200x_cmd)(struct device *dev, u8 cmd));
-void tsc200x_remove(struct device *dev);
 
 #endif
-- 
2.45.2.993.g49e7a77208-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/6] Input: tsc2004/5 - do not hard code interrupt trigger
  2024-07-11 17:27 [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply Dmitry Torokhov
@ 2024-07-11 17:27 ` Dmitry Torokhov
  2024-07-11 17:27 ` [PATCH 3/6] Input: tsc2004/5 - fix reset handling on probe Dmitry Torokhov
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2024-07-11 17:27 UTC (permalink / raw)
  To: linux-input, Sebastian Reichel; +Cc: linux-kernel

Instead of hard-coding interrupt trigger rely on ACPI/DT/board code
to set it up appropriately.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/tsc200x-core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
index 39789a27f65b..cd539a2a1dd5 100644
--- a/drivers/input/touchscreen/tsc200x-core.c
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -542,10 +542,8 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 	/* Ensure the touchscreen is off */
 	tsc200x_stop_scan(ts);
 
-	error = devm_request_threaded_irq(dev, irq, NULL,
-					  tsc200x_irq_thread,
-					  IRQF_TRIGGER_RISING | IRQF_ONESHOT,
-					  "tsc200x", ts);
+	error = devm_request_threaded_irq(dev, irq, NULL, tsc200x_irq_thread,
+					  IRQF_ONESHOT, "tsc200x", ts);
 	if (error) {
 		dev_err(dev, "Failed to request irq, err: %d\n", error);
 		return error;
-- 
2.45.2.993.g49e7a77208-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/6] Input: tsc2004/5 - fix reset handling on probe
  2024-07-11 17:27 [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply Dmitry Torokhov
  2024-07-11 17:27 ` [PATCH 2/6] Input: tsc2004/5 - do not hard code interrupt trigger Dmitry Torokhov
@ 2024-07-11 17:27 ` Dmitry Torokhov
  2024-07-11 17:27 ` [PATCH 4/6] Input: tsc2004/5 - do not use irq_set_irq_wake() directly Dmitry Torokhov
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2024-07-11 17:27 UTC (permalink / raw)
  To: linux-input, Sebastian Reichel; +Cc: linux-kernel

When the driver has been converted to use gpiod API it was requesting
and asserting the reset on probe, but never deasserted it. However
because of incorrect annotations in device tree marking reset line as
active high whereas in reality it is active low, the end result was
that the chip was never reset on probe. With polarity of the reset line
now corrected this became a problem.

Fix this by calling tsc200x_reset() from tsc200x_probe() to properly
complete the reset sequence and move requesting the reset GPIO and VIO
supply closer to the point where we need to start talking to the
hardware.

Fixes: d257f2980feb ("Input: tsc2005 - convert to gpiod")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/tsc200x-core.c | 28 +++++++++++++-----------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
index cd539a2a1dd5..87e6839c60fa 100644
--- a/drivers/input/touchscreen/tsc200x-core.c
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -486,19 +486,6 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 					 &esd_timeout);
 	ts->esd_timeout = error ? 0 : esd_timeout;
 
-	ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
-	if (IS_ERR(ts->reset_gpio)) {
-		error = PTR_ERR(ts->reset_gpio);
-		dev_err(dev, "error acquiring reset gpio: %d\n", error);
-		return error;
-	}
-
-	error = devm_regulator_get_enable(dev, "vio");
-	if (error) {
-		dev_err(dev, "error acquiring vio regulator: %d\n", error);
-		return error;
-	}
-
 	mutex_init(&ts->mutex);
 
 	spin_lock_init(&ts->lock);
@@ -539,6 +526,21 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 
 	touchscreen_parse_properties(input_dev, false, &ts->prop);
 
+	ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	error = PTR_ERR_OR_ZERO(ts->reset_gpio);
+	if (error) {
+		dev_err(dev, "error acquiring reset gpio: %d\n", error);
+		return error;
+	}
+
+	error = devm_regulator_get_enable(dev, "vio");
+	if (error) {
+		dev_err(dev, "error acquiring vio regulator: %d\n", error);
+		return error;
+	}
+
+	tsc200x_reset(ts);
+
 	/* Ensure the touchscreen is off */
 	tsc200x_stop_scan(ts);
 
-- 
2.45.2.993.g49e7a77208-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/6] Input: tsc2004/5 - do not use irq_set_irq_wake() directly
  2024-07-11 17:27 [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply Dmitry Torokhov
  2024-07-11 17:27 ` [PATCH 2/6] Input: tsc2004/5 - do not hard code interrupt trigger Dmitry Torokhov
  2024-07-11 17:27 ` [PATCH 3/6] Input: tsc2004/5 - fix reset handling on probe Dmitry Torokhov
@ 2024-07-11 17:27 ` Dmitry Torokhov
  2024-07-11 17:27 ` [PATCH 5/6] Input: tsc2004/5 - respect "wakeup-source" property Dmitry Torokhov
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2024-07-11 17:27 UTC (permalink / raw)
  To: linux-input, Sebastian Reichel; +Cc: linux-kernel

Instead of setting irq_set_irq_wake() directly in probe(), mark the device
as wakeup-capable, and use enable_irq_wake() and disable_irq_wake() in
suspend/resume path.

This also allows changing the wakeup setting dynamically at runtime using
/sys/devices/.../tsc2005/power/wakeup.

Reviewed-By: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/tsc200x-core.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
index 87e6839c60fa..cd60bba11c56 100644
--- a/drivers/input/touchscreen/tsc200x-core.c
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -106,7 +106,9 @@ struct tsc200x {
 
 	struct gpio_desc	*reset_gpio;
 	int			(*tsc200x_cmd)(struct device *dev, u8 cmd);
+
 	int			irq;
+	bool			wake_irq_enabled;
 };
 
 static void tsc200x_update_pen_state(struct tsc200x *ts,
@@ -560,7 +562,8 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 		return error;
 	}
 
-	irq_set_irq_wake(irq, 1);
+	device_init_wakeup(dev, true);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(tsc200x_probe);
@@ -576,6 +579,9 @@ static int tsc200x_suspend(struct device *dev)
 
 	ts->suspended = true;
 
+	if (device_may_wakeup(dev))
+		ts->wake_irq_enabled = enable_irq_wake(ts->irq) == 0;
+
 	mutex_unlock(&ts->mutex);
 
 	return 0;
@@ -587,6 +593,11 @@ static int tsc200x_resume(struct device *dev)
 
 	mutex_lock(&ts->mutex);
 
+	if (ts->wake_irq_enabled) {
+		disable_irq_wake(ts->irq);
+		ts->wake_irq_enabled = false;
+	}
+
 	if (ts->suspended && ts->opened)
 		__tsc200x_enable(ts);
 
-- 
2.45.2.993.g49e7a77208-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/6] Input: tsc2004/5 - respect "wakeup-source" property
  2024-07-11 17:27 [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply Dmitry Torokhov
                   ` (2 preceding siblings ...)
  2024-07-11 17:27 ` [PATCH 4/6] Input: tsc2004/5 - do not use irq_set_irq_wake() directly Dmitry Torokhov
@ 2024-07-11 17:27 ` Dmitry Torokhov
  2024-07-11 17:27 ` [PATCH 6/6] Input: tsc2004/5 - use guard notation when acquiring mutexes/locks Dmitry Torokhov
  2024-08-05  1:11 ` [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply Dmitry Torokhov
  5 siblings, 0 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2024-07-11 17:27 UTC (permalink / raw)
  To: linux-input, Sebastian Reichel; +Cc: linux-kernel

Do not mark the device as wakeup-enabled by default, respect the
standard "wakeup-source" property.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/tsc200x-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
index cd60bba11c56..90a4ace22395 100644
--- a/drivers/input/touchscreen/tsc200x-core.c
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -562,7 +562,8 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 		return error;
 	}
 
-	device_init_wakeup(dev, true);
+	device_init_wakeup(dev,
+			   device_property_read_bool(dev, "wakeup-source"));
 
 	return 0;
 }
-- 
2.45.2.993.g49e7a77208-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 6/6] Input: tsc2004/5 - use guard notation when acquiring mutexes/locks
  2024-07-11 17:27 [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply Dmitry Torokhov
                   ` (3 preceding siblings ...)
  2024-07-11 17:27 ` [PATCH 5/6] Input: tsc2004/5 - respect "wakeup-source" property Dmitry Torokhov
@ 2024-07-11 17:27 ` Dmitry Torokhov
  2024-08-05  1:11 ` [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply Dmitry Torokhov
  5 siblings, 0 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2024-07-11 17:27 UTC (permalink / raw)
  To: linux-input, Sebastian Reichel; +Cc: linux-kernel

This makes the code more compact and error handling more robust.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/tsc200x-core.c | 182 ++++++++++-------------
 1 file changed, 81 insertions(+), 101 deletions(-)

diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
index 90a4ace22395..df39dee13e1c 100644
--- a/drivers/input/touchscreen/tsc200x-core.c
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -136,7 +136,6 @@ static void tsc200x_update_pen_state(struct tsc200x *ts,
 static irqreturn_t tsc200x_irq_thread(int irq, void *_ts)
 {
 	struct tsc200x *ts = _ts;
-	unsigned long flags;
 	unsigned int pressure;
 	struct tsc200x_data tsdata;
 	int error;
@@ -182,13 +181,11 @@ static irqreturn_t tsc200x_irq_thread(int irq, void *_ts)
 	if (unlikely(pressure > MAX_12BIT))
 		goto out;
 
-	spin_lock_irqsave(&ts->lock, flags);
-
-	tsc200x_update_pen_state(ts, tsdata.x, tsdata.y, pressure);
-	mod_timer(&ts->penup_timer,
-		  jiffies + msecs_to_jiffies(TSC200X_PENUP_TIME_MS));
-
-	spin_unlock_irqrestore(&ts->lock, flags);
+	scoped_guard(spinlock_irqsave, &ts->lock) {
+		tsc200x_update_pen_state(ts, tsdata.x, tsdata.y, pressure);
+		mod_timer(&ts->penup_timer,
+			  jiffies + msecs_to_jiffies(TSC200X_PENUP_TIME_MS));
+	}
 
 	ts->last_valid_interrupt = jiffies;
 out:
@@ -198,11 +195,9 @@ static irqreturn_t tsc200x_irq_thread(int irq, void *_ts)
 static void tsc200x_penup_timer(struct timer_list *t)
 {
 	struct tsc200x *ts = from_timer(ts, t, penup_timer);
-	unsigned long flags;
 
-	spin_lock_irqsave(&ts->lock, flags);
+	guard(spinlock_irqsave)(&ts->lock);
 	tsc200x_update_pen_state(ts, 0, 0, 0);
-	spin_unlock_irqrestore(&ts->lock, flags);
 }
 
 static void tsc200x_start_scan(struct tsc200x *ts)
@@ -232,12 +227,10 @@ static void __tsc200x_disable(struct tsc200x *ts)
 {
 	tsc200x_stop_scan(ts);
 
-	disable_irq(ts->irq);
-	del_timer_sync(&ts->penup_timer);
+	guard(disable_irq)(&ts->irq);
 
+	del_timer_sync(&ts->penup_timer);
 	cancel_delayed_work_sync(&ts->esd_work);
-
-	enable_irq(ts->irq);
 }
 
 /* must be called with ts->mutex held */
@@ -253,80 +246,79 @@ static void __tsc200x_enable(struct tsc200x *ts)
 	}
 }
 
-static ssize_t tsc200x_selftest_show(struct device *dev,
-				     struct device_attribute *attr,
-				     char *buf)
+/*
+ * Test TSC200X communications via temp high register.
+ */
+static int tsc200x_do_selftest(struct tsc200x *ts)
 {
-	struct tsc200x *ts = dev_get_drvdata(dev);
-	unsigned int temp_high;
 	unsigned int temp_high_orig;
 	unsigned int temp_high_test;
-	bool success = true;
+	unsigned int temp_high;
 	int error;
 
-	mutex_lock(&ts->mutex);
-
-	/*
-	 * Test TSC200X communications via temp high register.
-	 */
-	__tsc200x_disable(ts);
-
 	error = regmap_read(ts->regmap, TSC200X_REG_TEMP_HIGH, &temp_high_orig);
 	if (error) {
-		dev_warn(dev, "selftest failed: read error %d\n", error);
-		success = false;
-		goto out;
+		dev_warn(ts->dev, "selftest failed: read error %d\n", error);
+		return error;
 	}
 
 	temp_high_test = (temp_high_orig - 1) & MAX_12BIT;
 
 	error = regmap_write(ts->regmap, TSC200X_REG_TEMP_HIGH, temp_high_test);
 	if (error) {
-		dev_warn(dev, "selftest failed: write error %d\n", error);
-		success = false;
-		goto out;
+		dev_warn(ts->dev, "selftest failed: write error %d\n", error);
+		return error;
 	}
 
 	error = regmap_read(ts->regmap, TSC200X_REG_TEMP_HIGH, &temp_high);
 	if (error) {
-		dev_warn(dev, "selftest failed: read error %d after write\n",
-			 error);
-		success = false;
-		goto out;
-	}
-
-	if (temp_high != temp_high_test) {
-		dev_warn(dev, "selftest failed: %d != %d\n",
-			 temp_high, temp_high_test);
-		success = false;
+		dev_warn(ts->dev,
+			 "selftest failed: read error %d after write\n", error);
+		return error;
 	}
 
 	/* hardware reset */
 	tsc200x_reset(ts);
 
-	if (!success)
-		goto out;
+	if (temp_high != temp_high_test) {
+		dev_warn(ts->dev, "selftest failed: %d != %d\n",
+			 temp_high, temp_high_test);
+		return -EINVAL;
+	}
 
 	/* test that the reset really happened */
 	error = regmap_read(ts->regmap, TSC200X_REG_TEMP_HIGH, &temp_high);
 	if (error) {
-		dev_warn(dev, "selftest failed: read error %d after reset\n",
-			 error);
-		success = false;
-		goto out;
+		dev_warn(ts->dev,
+			 "selftest failed: read error %d after reset\n", error);
+		return error;
 	}
 
 	if (temp_high != temp_high_orig) {
-		dev_warn(dev, "selftest failed after reset: %d != %d\n",
+		dev_warn(ts->dev, "selftest failed after reset: %d != %d\n",
 			 temp_high, temp_high_orig);
-		success = false;
+		return -EINVAL;
 	}
 
-out:
-	__tsc200x_enable(ts);
-	mutex_unlock(&ts->mutex);
+	return 0;
+}
 
-	return sprintf(buf, "%d\n", success);
+static ssize_t tsc200x_selftest_show(struct device *dev,
+				     struct device_attribute *attr,
+				     char *buf)
+{
+	struct tsc200x *ts = dev_get_drvdata(dev);
+	int error;
+
+	scoped_guard(mutex, &ts->mutex) {
+		__tsc200x_disable(ts);
+
+		error = tsc200x_do_selftest(ts);
+
+		__tsc200x_enable(ts);
+	}
+
+	return sprintf(buf, "%d\n", !error);
 }
 
 static DEVICE_ATTR(selftest, S_IRUGO, tsc200x_selftest_show, NULL);
@@ -368,46 +360,42 @@ static void tsc200x_esd_work(struct work_struct *work)
 	int error;
 	unsigned int r;
 
-	if (!mutex_trylock(&ts->mutex)) {
-		/*
-		 * If the mutex is taken, it means that disable or enable is in
-		 * progress. In that case just reschedule the work. If the work
-		 * is not needed, it will be canceled by disable.
-		 */
-		goto reschedule;
-	}
-
-	if (time_is_after_jiffies(ts->last_valid_interrupt +
-				  msecs_to_jiffies(ts->esd_timeout)))
-		goto out;
-
-	/* We should be able to read register without disabling interrupts. */
-	error = regmap_read(ts->regmap, TSC200X_REG_CFR0, &r);
-	if (!error &&
-	    !((r ^ TSC200X_CFR0_INITVALUE) & TSC200X_CFR0_RW_MASK)) {
-		goto out;
-	}
-
 	/*
-	 * If we could not read our known value from configuration register 0
-	 * then we should reset the controller as if from power-up and start
-	 * scanning again.
+	 * If the mutex is taken, it means that disable or enable is in
+	 * progress. In that case just reschedule the work. If the work
+	 * is not needed, it will be canceled by disable.
 	 */
-	dev_info(ts->dev, "TSC200X not responding - resetting\n");
+	scoped_guard(mutex_try, &ts->mutex) {
+		if (time_is_after_jiffies(ts->last_valid_interrupt +
+					  msecs_to_jiffies(ts->esd_timeout)))
+			break;
 
-	disable_irq(ts->irq);
-	del_timer_sync(&ts->penup_timer);
+		/*
+		 * We should be able to read register without disabling
+		 * interrupts.
+		 */
+		error = regmap_read(ts->regmap, TSC200X_REG_CFR0, &r);
+		if (!error &&
+		    !((r ^ TSC200X_CFR0_INITVALUE) & TSC200X_CFR0_RW_MASK)) {
+			break;
+		}
 
-	tsc200x_update_pen_state(ts, 0, 0, 0);
+		/*
+		 * If we could not read our known value from configuration
+		 * register 0 then we should reset the controller as if from
+		 * power-up and start scanning again.
+		 */
+		dev_info(ts->dev, "TSC200X not responding - resetting\n");
 
-	tsc200x_reset(ts);
+		scoped_guard(disable_irq, &ts->irq) {
+			del_timer_sync(&ts->penup_timer);
+			tsc200x_update_pen_state(ts, 0, 0, 0);
+			tsc200x_reset(ts);
+		}
 
-	enable_irq(ts->irq);
-	tsc200x_start_scan(ts);
+		tsc200x_start_scan(ts);
+	}
 
-out:
-	mutex_unlock(&ts->mutex);
-reschedule:
 	/* re-arm the watchdog */
 	schedule_delayed_work(&ts->esd_work,
 			      round_jiffies_relative(
@@ -418,15 +406,13 @@ static int tsc200x_open(struct input_dev *input)
 {
 	struct tsc200x *ts = input_get_drvdata(input);
 
-	mutex_lock(&ts->mutex);
+	guard(mutex)(&ts->mutex);
 
 	if (!ts->suspended)
 		__tsc200x_enable(ts);
 
 	ts->opened = true;
 
-	mutex_unlock(&ts->mutex);
-
 	return 0;
 }
 
@@ -434,14 +420,12 @@ static void tsc200x_close(struct input_dev *input)
 {
 	struct tsc200x *ts = input_get_drvdata(input);
 
-	mutex_lock(&ts->mutex);
+	guard(mutex)(&ts->mutex);
 
 	if (!ts->suspended)
 		__tsc200x_disable(ts);
 
 	ts->opened = false;
-
-	mutex_unlock(&ts->mutex);
 }
 
 int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
@@ -573,7 +557,7 @@ static int tsc200x_suspend(struct device *dev)
 {
 	struct tsc200x *ts = dev_get_drvdata(dev);
 
-	mutex_lock(&ts->mutex);
+	guard(mutex)(&ts->mutex);
 
 	if (!ts->suspended && ts->opened)
 		__tsc200x_disable(ts);
@@ -583,8 +567,6 @@ static int tsc200x_suspend(struct device *dev)
 	if (device_may_wakeup(dev))
 		ts->wake_irq_enabled = enable_irq_wake(ts->irq) == 0;
 
-	mutex_unlock(&ts->mutex);
-
 	return 0;
 }
 
@@ -592,7 +574,7 @@ static int tsc200x_resume(struct device *dev)
 {
 	struct tsc200x *ts = dev_get_drvdata(dev);
 
-	mutex_lock(&ts->mutex);
+	guard(mutex)(&ts->mutex);
 
 	if (ts->wake_irq_enabled) {
 		disable_irq_wake(ts->irq);
@@ -604,8 +586,6 @@ static int tsc200x_resume(struct device *dev)
 
 	ts->suspended = false;
 
-	mutex_unlock(&ts->mutex);
-
 	return 0;
 }
 
-- 
2.45.2.993.g49e7a77208-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply
  2024-07-11 17:27 [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply Dmitry Torokhov
                   ` (4 preceding siblings ...)
  2024-07-11 17:27 ` [PATCH 6/6] Input: tsc2004/5 - use guard notation when acquiring mutexes/locks Dmitry Torokhov
@ 2024-08-05  1:11 ` Dmitry Torokhov
  5 siblings, 0 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2024-08-05  1:11 UTC (permalink / raw)
  To: linux-input, Sebastian Reichel; +Cc: linux-kernel

On Thu, Jul 11, 2024 at 10:27:12AM -0700, Dmitry Torokhov wrote:
> The chip needs to be powered up before calling tsc200x_stop_scan() which
> communicates with it; move the call to enable the regulator earlier in
> tsc200x_probe().
> 
> At the same time switch to using devm_regulator_get_enable() to simplify
> error handling. This also makes sure that regulator is not shut off too
> early when unbinding the driver.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

OK, since there were no objections I applied the series.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-08-05  1:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-11 17:27 [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply Dmitry Torokhov
2024-07-11 17:27 ` [PATCH 2/6] Input: tsc2004/5 - do not hard code interrupt trigger Dmitry Torokhov
2024-07-11 17:27 ` [PATCH 3/6] Input: tsc2004/5 - fix reset handling on probe Dmitry Torokhov
2024-07-11 17:27 ` [PATCH 4/6] Input: tsc2004/5 - do not use irq_set_irq_wake() directly Dmitry Torokhov
2024-07-11 17:27 ` [PATCH 5/6] Input: tsc2004/5 - respect "wakeup-source" property Dmitry Torokhov
2024-07-11 17:27 ` [PATCH 6/6] Input: tsc2004/5 - use guard notation when acquiring mutexes/locks Dmitry Torokhov
2024-08-05  1:11 ` [PATCH 1/6] Input: tsc2004/5 - fix handling of VIO power supply Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).