linux-kernel - Re: [PATCH] drm/bridge: ti-sn65dsi83: Improve error reporting and handling

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250624140345.77e8780d@bootlin.com>
Date: Tue, 24 Jun 2025 14:03:45 +0200
From: Herve Codina <herve.codina@...tlin.com>
To: Mike Looijmans <mike.looijmans@...ic.nl>
Cc: dri-devel@...ts.freedesktop.org, Andrzej Hajda
 <andrzej.hajda@...el.com>, David Airlie <airlied@...il.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: Re: [PATCH] drm/bridge: ti-sn65dsi83: Improve error reporting and
 handling

Hi Mike,

On Tue, 24 Jun 2025 13:45:15 +0200
Mike Looijmans <mike.looijmans@...ic.nl> wrote:

> 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
...

> @@ -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.

I think I have missed a verb in the last sentence when I previously wrote
the comment.
  "The interrupt will be re-enabled after the reset."
                         ^^^^^^^^^^
Can you fix it ?

Other than that,

Reviewed-by: Herve Codina <herve.codina@...tlin.com>

Thanks,
Hervé