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

[Luca Ceresoli <luca.ceresoli@bootlin.com>]

Hi Maxime,

On Thu, 11 Sep 2025 08:44:34 +0200
Maxime Ripard <mripard@kernel.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