Commit 63f92f11 authored by Dmitry Torokhov's avatar Dmitry Torokhov

Input: tsc2004/5 - use guard notation when acquiring mutexes/locks

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

Link: https://lore.kernel.org/r/20240711172719.1248373-6-dmitry.torokhov@gmail.comSigned-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
parent f46b18f3
...@@ -136,7 +136,6 @@ static void tsc200x_update_pen_state(struct tsc200x *ts, ...@@ -136,7 +136,6 @@ static void tsc200x_update_pen_state(struct tsc200x *ts,
static irqreturn_t tsc200x_irq_thread(int irq, void *_ts) static irqreturn_t tsc200x_irq_thread(int irq, void *_ts)
{ {
struct tsc200x *ts = _ts; struct tsc200x *ts = _ts;
unsigned long flags;
unsigned int pressure; unsigned int pressure;
struct tsc200x_data tsdata; struct tsc200x_data tsdata;
int error; int error;
...@@ -182,13 +181,11 @@ static irqreturn_t tsc200x_irq_thread(int irq, void *_ts) ...@@ -182,13 +181,11 @@ static irqreturn_t tsc200x_irq_thread(int irq, void *_ts)
if (unlikely(pressure > MAX_12BIT)) if (unlikely(pressure > MAX_12BIT))
goto out; goto out;
spin_lock_irqsave(&ts->lock, flags); scoped_guard(spinlock_irqsave, &ts->lock) {
tsc200x_update_pen_state(ts, tsdata.x, tsdata.y, pressure);
tsc200x_update_pen_state(ts, tsdata.x, tsdata.y, pressure); mod_timer(&ts->penup_timer,
mod_timer(&ts->penup_timer, jiffies + msecs_to_jiffies(TSC200X_PENUP_TIME_MS));
jiffies + msecs_to_jiffies(TSC200X_PENUP_TIME_MS)); }
spin_unlock_irqrestore(&ts->lock, flags);
ts->last_valid_interrupt = jiffies; ts->last_valid_interrupt = jiffies;
out: out:
...@@ -198,11 +195,9 @@ static irqreturn_t tsc200x_irq_thread(int irq, void *_ts) ...@@ -198,11 +195,9 @@ static irqreturn_t tsc200x_irq_thread(int irq, void *_ts)
static void tsc200x_penup_timer(struct timer_list *t) static void tsc200x_penup_timer(struct timer_list *t)
{ {
struct tsc200x *ts = from_timer(ts, t, penup_timer); 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); tsc200x_update_pen_state(ts, 0, 0, 0);
spin_unlock_irqrestore(&ts->lock, flags);
} }
static void tsc200x_start_scan(struct tsc200x *ts) static void tsc200x_start_scan(struct tsc200x *ts)
...@@ -232,12 +227,10 @@ static void __tsc200x_disable(struct tsc200x *ts) ...@@ -232,12 +227,10 @@ static void __tsc200x_disable(struct tsc200x *ts)
{ {
tsc200x_stop_scan(ts); tsc200x_stop_scan(ts);
disable_irq(ts->irq); guard(disable_irq)(&ts->irq);
del_timer_sync(&ts->penup_timer);
del_timer_sync(&ts->penup_timer);
cancel_delayed_work_sync(&ts->esd_work); cancel_delayed_work_sync(&ts->esd_work);
enable_irq(ts->irq);
} }
/* must be called with ts->mutex held */ /* must be called with ts->mutex held */
...@@ -253,80 +246,79 @@ static void __tsc200x_enable(struct tsc200x *ts) ...@@ -253,80 +246,79 @@ static void __tsc200x_enable(struct tsc200x *ts)
} }
} }
static ssize_t tsc200x_selftest_show(struct device *dev, /*
struct device_attribute *attr, * Test TSC200X communications via temp high register.
char *buf) */
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_orig;
unsigned int temp_high_test; unsigned int temp_high_test;
bool success = true; unsigned int temp_high;
int error; 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); error = regmap_read(ts->regmap, TSC200X_REG_TEMP_HIGH, &temp_high_orig);
if (error) { if (error) {
dev_warn(dev, "selftest failed: read error %d\n", error); dev_warn(ts->dev, "selftest failed: read error %d\n", error);
success = false; return error;
goto out;
} }
temp_high_test = (temp_high_orig - 1) & MAX_12BIT; temp_high_test = (temp_high_orig - 1) & MAX_12BIT;
error = regmap_write(ts->regmap, TSC200X_REG_TEMP_HIGH, temp_high_test); error = regmap_write(ts->regmap, TSC200X_REG_TEMP_HIGH, temp_high_test);
if (error) { if (error) {
dev_warn(dev, "selftest failed: write error %d\n", error); dev_warn(ts->dev, "selftest failed: write error %d\n", error);
success = false; return error;
goto out;
} }
error = regmap_read(ts->regmap, TSC200X_REG_TEMP_HIGH, &temp_high); error = regmap_read(ts->regmap, TSC200X_REG_TEMP_HIGH, &temp_high);
if (error) { if (error) {
dev_warn(dev, "selftest failed: read error %d after write\n", dev_warn(ts->dev,
error); "selftest failed: read error %d after write\n", error);
success = false; return error;
goto out;
}
if (temp_high != temp_high_test) {
dev_warn(dev, "selftest failed: %d != %d\n",
temp_high, temp_high_test);
success = false;
} }
/* hardware reset */ /* hardware reset */
tsc200x_reset(ts); tsc200x_reset(ts);
if (!success) if (temp_high != temp_high_test) {
goto out; dev_warn(ts->dev, "selftest failed: %d != %d\n",
temp_high, temp_high_test);
return -EINVAL;
}
/* test that the reset really happened */ /* test that the reset really happened */
error = regmap_read(ts->regmap, TSC200X_REG_TEMP_HIGH, &temp_high); error = regmap_read(ts->regmap, TSC200X_REG_TEMP_HIGH, &temp_high);
if (error) { if (error) {
dev_warn(dev, "selftest failed: read error %d after reset\n", dev_warn(ts->dev,
error); "selftest failed: read error %d after reset\n", error);
success = false; return error;
goto out;
} }
if (temp_high != temp_high_orig) { 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); temp_high, temp_high_orig);
success = false; return -EINVAL;
} }
out: return 0;
__tsc200x_enable(ts); }
mutex_unlock(&ts->mutex);
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); static DEVICE_ATTR(selftest, S_IRUGO, tsc200x_selftest_show, NULL);
...@@ -368,46 +360,42 @@ static void tsc200x_esd_work(struct work_struct *work) ...@@ -368,46 +360,42 @@ static void tsc200x_esd_work(struct work_struct *work)
int error; int error;
unsigned int r; 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 * If the mutex is taken, it means that disable or enable is in
* then we should reset the controller as if from power-up and start * progress. In that case just reschedule the work. If the work
* scanning again. * 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 */ /* re-arm the watchdog */
schedule_delayed_work(&ts->esd_work, schedule_delayed_work(&ts->esd_work,
round_jiffies_relative( round_jiffies_relative(
...@@ -418,15 +406,13 @@ static int tsc200x_open(struct input_dev *input) ...@@ -418,15 +406,13 @@ static int tsc200x_open(struct input_dev *input)
{ {
struct tsc200x *ts = input_get_drvdata(input); struct tsc200x *ts = input_get_drvdata(input);
mutex_lock(&ts->mutex); guard(mutex)(&ts->mutex);
if (!ts->suspended) if (!ts->suspended)
__tsc200x_enable(ts); __tsc200x_enable(ts);
ts->opened = true; ts->opened = true;
mutex_unlock(&ts->mutex);
return 0; return 0;
} }
...@@ -434,14 +420,12 @@ static void tsc200x_close(struct input_dev *input) ...@@ -434,14 +420,12 @@ static void tsc200x_close(struct input_dev *input)
{ {
struct tsc200x *ts = input_get_drvdata(input); struct tsc200x *ts = input_get_drvdata(input);
mutex_lock(&ts->mutex); guard(mutex)(&ts->mutex);
if (!ts->suspended) if (!ts->suspended)
__tsc200x_disable(ts); __tsc200x_disable(ts);
ts->opened = false; ts->opened = false;
mutex_unlock(&ts->mutex);
} }
int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id, 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) ...@@ -573,7 +557,7 @@ static int tsc200x_suspend(struct device *dev)
{ {
struct tsc200x *ts = dev_get_drvdata(dev); struct tsc200x *ts = dev_get_drvdata(dev);
mutex_lock(&ts->mutex); guard(mutex)(&ts->mutex);
if (!ts->suspended && ts->opened) if (!ts->suspended && ts->opened)
__tsc200x_disable(ts); __tsc200x_disable(ts);
...@@ -583,8 +567,6 @@ static int tsc200x_suspend(struct device *dev) ...@@ -583,8 +567,6 @@ static int tsc200x_suspend(struct device *dev)
if (device_may_wakeup(dev)) if (device_may_wakeup(dev))
ts->wake_irq_enabled = enable_irq_wake(ts->irq) == 0; ts->wake_irq_enabled = enable_irq_wake(ts->irq) == 0;
mutex_unlock(&ts->mutex);
return 0; return 0;
} }
...@@ -592,7 +574,7 @@ static int tsc200x_resume(struct device *dev) ...@@ -592,7 +574,7 @@ static int tsc200x_resume(struct device *dev)
{ {
struct tsc200x *ts = dev_get_drvdata(dev); struct tsc200x *ts = dev_get_drvdata(dev);
mutex_lock(&ts->mutex); guard(mutex)(&ts->mutex);
if (ts->wake_irq_enabled) { if (ts->wake_irq_enabled) {
disable_irq_wake(ts->irq); disable_irq_wake(ts->irq);
...@@ -604,8 +586,6 @@ static int tsc200x_resume(struct device *dev) ...@@ -604,8 +586,6 @@ static int tsc200x_resume(struct device *dev)
ts->suspended = false; ts->suspended = false;
mutex_unlock(&ts->mutex);
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment