linux-kernel - Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: protect device resources on unplug

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: <20250911150933.52a0416d@booty>
Date: Thu, 11 Sep 2025 15:09:33 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Maxime Ripard <mripard@...nel.org>
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>, Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann
 <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter
 <simona@...ll.ch>, Hui Pu <Hui.Pu@...ealthcare.com>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, Dmitry Baryshkov <lumag@...nel.org>
Subject: Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: protect device resources
 on unplug

Hi Maxime,

On Thu, 11 Sep 2025 08:44:34 +0200
Maxime Ripard <mripard@...nel.org> wrote:

> > > > Option 2: have a function to unconditionally do the cleanups:
> > > > 
> > > > sn65dsi83_cleanups()
> > > > {
> > > > 	/* the same cleanups (no if) */
> > > > }
> > > > 
> > > > And then:
> > > >  * in atomic_pre_enable, instead of setting the flag
> > > >    add a devres action to call sn65dsi83_cleanups()
> > > >  * in atomic_disable, instead of clearing the flag
> > > >    remove the devres action
> > > > 
> > > > Even this option looks like more complicated and less readable code
> > > > to do the same thing.
> > > > 
> > > > Do you have in mind a better option that I haven't figured out?    
> > > 
> > > Would using devm_add_action in atomic_pre_enable, and
> > > devm_release_action in atomic_post_disable work?
> > > 
> > > That way, if you have a typical enable / disable cycle, the action will
> > > get registered and executed properly, and if you only have an enable but
> > > no matching disable, it will be collected after remove.  
> > 
> > So you're OK with option 2. I just implemented it, works well and the
> > resulting code is a bit cleaner. Queued for v2.  
> 
> Kind of, but you shouldn't remove but release it, and it doesn't have to
> be a single action / function.

Released instead of removed: yes.

Doesn't have to be a single function: I currently implemented a single
function with the 3 actions that are currently done in atomic_disable.
I think you propose to add 3 different devres actions, one for each,
but it would be more code and a little more resources used and I don't
see the advantage.

I think it makes sense that I send my current version and we can
continue discussion based on the code. I'm waiting a bit before
sending it, in case you have feedback about the other branch of this
discussion (placement of drm_bridge_unplug()).

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com