| 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: <20250624114630.303058-1-mike.looijmans@topic.nl>
Date: Tue, 24 Jun 2025 13:45:15 +0200
From: Mike Looijmans <mike.looijmans@...ic.nl>
To: dri-devel@...ts.freedesktop.org
CC: Mike Looijmans <mike.looijmans@...ic.nl>,
Andrzej Hajda <andrzej.hajda@...el.com>,
David Airlie <airlied@...il.com>,
Herve Codina <herve.codina@...tlin.com>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Jonas Karlman <jonas@...boo.se>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Neil Armstrong <neil.armstrong@...aro.org>,
Robert Foss <rfoss@...nel.org>,
Simona Vetter <simona@...ll.ch>,
Thomas Zimmermann <tzimmermann@...e.de>,
linux-kernel@...r.kernel.org
Subject: [PATCH] drm/bridge: ti-sn65dsi83: Improve error reporting and handling
The datasheet advises to wait 5ms after starting the video stream before
resetting the error registers. The driver only waits 1ms. Change the
sequence to match the datasheet:
- Turn on the DSI
- Wait 5ms
- Write 0xFF to CSR 0xE5 to clear the error registers
Don't read the error register (which may fail), just write 0xff as the
datasheet suggests.
The driver creates a timer or IRQ handler that reads the error register,
which implements the "wait some time and read the register" part.
When using a timer to poll the status register, the timer did not stop
when the error handler triggers a reset. This has been observed to cause
a series of multiple resets. Let handle_errors return a bool indicating
whether all is fine, and only extend the time when it returns true. That
also allows the IRQ disable call to move to the interrupt routine.
When the error handler does trigger, log a message that explains the
reset cause.
Fixes: ad5c6ecef27e ("drm: bridge: ti-sn65dsi83: Add error recovery mechanism")
Signed-off-by: Mike Looijmans <mike.looijmans@...ic.nl>
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 50 +++++++++++++++------------
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 033c44326552..6240a9997cc2 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -417,7 +417,7 @@ static void sn65dsi83_reset_work(struct work_struct *ws)
enable_irq(ctx->irq);
}
-static void sn65dsi83_handle_errors(struct sn65dsi83 *ctx)
+static bool sn65dsi83_handle_errors(struct sn65dsi83 *ctx)
{
unsigned int irq_stat;
int ret;
@@ -430,17 +430,20 @@ static void sn65dsi83_handle_errors(struct sn65dsi83 *ctx)
ret = regmap_read(ctx->regmap, REG_IRQ_STAT, &irq_stat);
if (ret || irq_stat) {
- /*
- * IRQ acknowledged is not always possible (the bridge can be in
- * a state where it doesn't answer anymore). To prevent an
- * interrupt storm, disable interrupt. The interrupt will be
- * after the reset.
- */
- if (ctx->irq)
- disable_irq_nosync(ctx->irq);
+ if (ret) {
+ dev_err(ctx->dev, "Communication failure\n");
+ } else {
+ dev_err(ctx->dev, "Error status: 0x%02x\n", irq_stat);
+ /* Clear errors if the chip was still responding */
+ regmap_write(ctx->regmap, REG_IRQ_STAT, irq_stat);
+ }
schedule_work(&ctx->reset_work);
+
+ return false;
}
+
+ return true;
}
static void sn65dsi83_monitor_work(struct work_struct *work)
@@ -448,9 +451,8 @@ static void sn65dsi83_monitor_work(struct work_struct *work)
struct sn65dsi83 *ctx = container_of(to_delayed_work(work),
struct sn65dsi83, monitor_work);
- sn65dsi83_handle_errors(ctx);
-
- schedule_delayed_work(&ctx->monitor_work, msecs_to_jiffies(1000));
+ if (sn65dsi83_handle_errors(ctx))
+ schedule_delayed_work(&ctx->monitor_work, msecs_to_jiffies(1000));
}
static void sn65dsi83_monitor_start(struct sn65dsi83 *ctx)
@@ -639,18 +641,13 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
struct drm_atomic_state *state)
{
struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
- unsigned int pval;
+ /* Wait 5 ms after starting DSI stream */
+ usleep_range(5000, 5500);
/* Clear all errors that got asserted during initialization. */
- regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);
- regmap_write(ctx->regmap, REG_IRQ_STAT, pval);
-
- /* Wait for 1ms and check for errors in status register */
- usleep_range(1000, 1100);
- regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);
- if (pval)
- dev_err(ctx->dev, "Unexpected link status 0x%02x\n", pval);
+ regmap_write(ctx->regmap, REG_IRQ_STAT, 0xff);
+ /* Start checking for errors in status register */
if (ctx->irq) {
/* Enable irq to detect errors */
regmap_write(ctx->regmap, REG_IRQ_GLOBAL, REG_IRQ_GLOBAL_IRQ_EN);
@@ -929,7 +926,16 @@ static irqreturn_t sn65dsi83_irq(int irq, void *data)
{
struct sn65dsi83 *ctx = data;
- sn65dsi83_handle_errors(ctx);
+ if (!sn65dsi83_handle_errors(ctx)) {
+ /*
+ * IRQ acknowledged is not always possible (the bridge can be in
+ * a state where it doesn't answer anymore). To prevent an
+ * interrupt storm, disable interrupt. The interrupt will be
+ * after the reset.
+ */
+ disable_irq_nosync(ctx->irq);
+ }
+
return IRQ_HANDLED;
}
--
2.43.0
base-commit: 78f4e737a53e1163ded2687a922fce138aee73f5
branch: linux-master-sn65dsi83-errorhandling
Met vriendelijke groet / kind regards,
Mike Looijmans
System Expert
TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands
T: +31 (0) 499 33 69 69
E: mike.looijmans@...ic.nl
W: www.topic.nl
Please consider the environment before printing this e-mail
Powered by blists - more mailing lists