| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20210625065922.8310-2-o.rempel@pengutronix.de>
Date: Fri, 25 Jun 2021 08:59:22 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Rob Herring <robh+dt@...nel.org>,
Jonathan Cameron <jic23@...nel.org>
Cc: Oleksij Rempel <o.rempel@...gutronix.de>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Pengutronix Kernel Team <kernel@...gutronix.de>,
David Jander <david@...tonic.nl>,
Robin van der Gracht <robin@...tonic.nl>,
linux-iio@...r.kernel.org, Lars-Peter Clausen <lars@...afoo.de>,
Peter Meerwald-Stadler <pmeerw@...erw.net>,
Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: [PATCH v1 2/2] iio: adc: tsc2046: fix sleeping in atomic context warning and a deadlock after iio_trigger_poll() call
If iio_trigger_poll() is called after IRQ was disabled, we will call
reenable_trigger() directly from hard IRQ or hrtimer context instead of
IRQ thread. In this case we will run in to multiple issue as sleeping in atomic
context and a deadlock.
To avoid this issue, rework the trigger to use state machine. All state
changes are done over the hrtimer, so it allows us to drop fsleep() and
avoid the deadlock.
Fixes: 9374e8f5a38d ("iio: adc: add ADC driver for the TI TSC2046 controller")
Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
---
drivers/iio/adc/ti-tsc2046.c | 102 ++++++++++++++++++++---------------
1 file changed, 58 insertions(+), 44 deletions(-)
diff --git a/drivers/iio/adc/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c
index d84ae6b008c1..91f6bd5effe7 100644
--- a/drivers/iio/adc/ti-tsc2046.c
+++ b/drivers/iio/adc/ti-tsc2046.c
@@ -123,14 +123,21 @@ struct tsc2046_adc_ch_cfg {
unsigned int oversampling_ratio;
};
+enum tsc2046_state {
+ TSC2046_STATE_STANDBY,
+ TSC2046_STATE_ENABLE_IRQ_POLL,
+ TSC2046_STATE_POLL,
+ TSC2046_STATE_ENABLE_IRQ,
+};
+
struct tsc2046_adc_priv {
struct spi_device *spi;
const struct tsc2046_adc_dcfg *dcfg;
struct iio_trigger *trig;
struct hrtimer trig_timer;
- spinlock_t trig_lock;
- unsigned int trig_more_count;
+ enum tsc2046_state state;
+ spinlock_t state_lock;
struct spi_transfer xfer;
struct spi_message msg;
@@ -411,21 +418,47 @@ static const struct iio_info tsc2046_adc_info = {
.update_scan_mode = tsc2046_adc_update_scan_mode,
};
-static enum hrtimer_restart tsc2046_adc_trig_more(struct hrtimer *hrtimer)
+static enum hrtimer_restart tsc2046_adc_timer(struct hrtimer *hrtimer)
{
struct tsc2046_adc_priv *priv = container_of(hrtimer,
struct tsc2046_adc_priv,
trig_timer);
unsigned long flags;
- spin_lock_irqsave(&priv->trig_lock, flags);
-
- disable_irq_nosync(priv->spi->irq);
-
- priv->trig_more_count++;
- iio_trigger_poll(priv->trig);
-
- spin_unlock_irqrestore(&priv->trig_lock, flags);
+ spin_lock_irqsave(&priv->state_lock, flags);
+ switch (priv->state) {
+ case TSC2046_STATE_ENABLE_IRQ_POLL:
+ /*
+ * IRQ handler called iio_trigger_poll() to sample ADC.
+ * Here we
+ * - re-enable IRQs
+ * - start hrtimer for timeout if no IRQ will occur
+ */
+ priv->state = TSC2046_STATE_POLL;
+ enable_irq(priv->spi->irq);
+ hrtimer_start(&priv->trig_timer,
+ ns_to_ktime(priv->scan_interval_us *
+ NSEC_PER_USEC),
+ HRTIMER_MODE_REL_SOFT);
+ break;
+ case TSC2046_STATE_POLL:
+ disable_irq_nosync(priv->spi->irq);
+ priv->state = TSC2046_STATE_ENABLE_IRQ;
+ /* iio_trigger_poll() starts hrtimer */
+ iio_trigger_poll(priv->trig);
+ break;
+ case TSC2046_STATE_ENABLE_IRQ:
+ priv->state = TSC2046_STATE_STANDBY;
+ enable_irq(priv->spi->irq);
+ break;
+ case TSC2046_STATE_STANDBY:
+ fallthrough;
+ default:
+ dev_warn(&priv->spi->dev, "Got unexpected state: %i\n",
+ priv->state);
+ break;
+ }
+ spin_unlock_irqrestore(&priv->state_lock, flags);
return HRTIMER_NORESTART;
}
@@ -434,16 +467,17 @@ static irqreturn_t tsc2046_adc_irq(int irq, void *dev_id)
{
struct iio_dev *indio_dev = dev_id;
struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
-
- spin_lock(&priv->trig_lock);
+ unsigned long flags;
hrtimer_try_to_cancel(&priv->trig_timer);
- priv->trig_more_count = 0;
+ spin_lock_irqsave(&priv->state_lock, flags);
disable_irq_nosync(priv->spi->irq);
- iio_trigger_poll(priv->trig);
+ priv->state = TSC2046_STATE_ENABLE_IRQ_POLL;
- spin_unlock(&priv->trig_lock);
+ /* iio_trigger_poll() starts hrtimer */
+ iio_trigger_poll(priv->trig);
+ spin_unlock_irqrestore(&priv->state_lock, flags);
return IRQ_HANDLED;
}
@@ -452,37 +486,16 @@ static void tsc2046_adc_reenable_trigger(struct iio_trigger *trig)
{
struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
- unsigned long flags;
- int delta;
+ ktime_t tim;
/*
* We can sample it as fast as we can, but usually we do not need so
* many samples. Reduce the sample rate for default (touchscreen) use
* case.
- * Currently we do not need a highly precise sample rate. It is enough
- * to have calculated numbers.
- */
- delta = priv->scan_interval_us - priv->time_per_scan_us;
- if (delta > 0)
- fsleep(delta);
-
- spin_lock_irqsave(&priv->trig_lock, flags);
-
- /*
- * We need to trigger at least one extra sample to detect state
- * difference on ADC side.
*/
- if (!priv->trig_more_count) {
- int timeout_ms = DIV_ROUND_UP(priv->scan_interval_us,
- USEC_PER_MSEC);
-
- hrtimer_start(&priv->trig_timer, ms_to_ktime(timeout_ms),
- HRTIMER_MODE_REL_SOFT);
- }
-
- enable_irq(priv->spi->irq);
-
- spin_unlock_irqrestore(&priv->trig_lock, flags);
+ tim = ns_to_ktime((priv->scan_interval_us - priv->time_per_scan_us) *
+ NSEC_PER_USEC);
+ hrtimer_start(&priv->trig_timer, tim, HRTIMER_MODE_REL_SOFT);
}
static int tsc2046_adc_set_trigger_state(struct iio_trigger *trig, bool enable)
@@ -493,8 +506,8 @@ static int tsc2046_adc_set_trigger_state(struct iio_trigger *trig, bool enable)
if (enable) {
enable_irq(priv->spi->irq);
} else {
+ hrtimer_cancel(&priv->trig_timer);
disable_irq(priv->spi->irq);
- hrtimer_try_to_cancel(&priv->trig_timer);
}
return 0;
@@ -668,10 +681,11 @@ static int tsc2046_adc_probe(struct spi_device *spi)
iio_trigger_set_drvdata(trig, indio_dev);
trig->ops = &tsc2046_adc_trigger_ops;
- spin_lock_init(&priv->trig_lock);
+ spin_lock_init(&priv->state_lock);
+ priv->state = TSC2046_STATE_STANDBY;
hrtimer_init(&priv->trig_timer, CLOCK_MONOTONIC,
HRTIMER_MODE_REL_SOFT);
- priv->trig_timer.function = tsc2046_adc_trig_more;
+ priv->trig_timer.function = tsc2046_adc_timer;
ret = devm_iio_trigger_register(dev, trig);
if (ret) {
--
2.29.2
Powered by blists - more mailing lists