linux-kernel - Re: [PATCH v3 3/3] drm: bridge: ti-sn65dsi83: Add error recovery mechanism

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250108184442.64f38fbc@bootlin.com>
Date: Wed, 8 Jan 2025 18:44:42 +0100
From: Herve Codina <herve.codina@...tlin.com>
To: Alexander Stein <alexander.stein@...tq-group.com>
Cc: Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong
 <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, Laurent
 Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman
 <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, David Airlie
 <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Maarten Lankhorst
 <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
 Thomas Zimmermann <tzimmermann@...e.de>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Marek Vasut <marex@...x.de>,
 dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Louis Chauvet <louis.chauvet@...tlin.com>,
 Luca Ceresoli <luca.ceresoli@...tlin.com>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v3 3/3] drm: bridge: ti-sn65dsi83: Add error recovery
 mechanism

Hi Alexander,

On Wed, 08 Jan 2025 11:54:49 +0100
Alexander Stein <alexander.stein@...tq-group.com> wrote:

[...]
> >  #include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_bridge.h>
> > +#include <drm/drm_drv.h> /* DRM_MODESET_LOCK_ALL_BEGIN() needs drm_drv_uses_atomic_modeset() */  
> 
> Shouldn't this include be added to include/drm/drm_modeset_lock.h instead?

Yes indeed. I will change that in the next iteration.

> 
> >  #include <drm/drm_mipi_dsi.h>
> >  #include <drm/drm_of.h>
> >  #include <drm/drm_panel.h>
> > @@ -147,6 +150,9 @@ struct sn65dsi83 {
> >  	struct regulator		*vcc;
> >  	bool				lvds_dual_link;
> >  	bool				lvds_dual_link_even_odd_swap;
> > +	bool				use_irq;
> > +	struct delayed_work		monitor_work;
> > +	struct work_struct		reset_work;  
> 
> Can you please rebase? You are missing commit d2b8c6d549570
> ("drm/bridge: ti-sn65dsi83: Add ti,lvds-vod-swing optional properties")

Sure, I will rebase.

[...]
> > +static void sn65dsi83_handle_errors(struct sn65dsi83 *ctx)
> > +{
> > +	unsigned int irq_stat;
> > +	int ret;
> > +
> > +	/*
> > +	 * Schedule a reset in case of:
> > +	 *  - the bridge doesn't answer
> > +	 *  - the bridge signals an error
> > +	 */
> > +
> > +	ret = regmap_read(ctx->regmap, REG_IRQ_STAT, &irq_stat);
> > +	if (ret || irq_stat)
> > +		schedule_work(&ctx->reset_work);  
> 
> Shouldn't you clear the error bits as well?

Thanks for pointing that.

I can clear the error bit but further more, I probably need to simply
disable the interrupt.

In some cases, we observed i2c access failure. In that cases clearing error
bits is simply not possible.

To avoid some possible interrupt storms (the chip detect a failure, set its
interrupt line but could be not accessible anymore), the best thing to do
is to disable the interrupt line here, let the reset work to do its job
performing a full reset of the device and re-enabling the interrupt line
when needed, probably in sn65dsi83_atomic_enable().

What do you think about that?

Best regards,
Hervé