Re: [PATCH v7 03/12] drm/bridge: cdns-dsi: Fix phy de-init and flag it so

[Tomi Valkeinen]

Hi,

On 14/01/2025 18:32, Aradhya Bhatia wrote:

> > But generally speaking, yes, it's good to keep fixes simple, and do
> > cleanups later on top. Keeping that in mind, maybe this current patch is
> > fine as it is. Although... if the init is done in pre_enable, shouldn't
> > the deinit be done in post_disable?
>
> Yes, I will move the deinit to _bridge_post_disable().
>
>
> So, if we keep the fix limited to deinit in _bridge_post_disable(), then
> the cleanup would involve dropping the init calls from _bridge_enable().
> And then the patch-12 would do 3 things -
>
> 	1. Drop older _bridge_pre_enable()
> 	2. Rename old _bridge_enable() to _bridge_pre_enable()
> 	3. Since the _old_ _bridge_enable() has the calls dropped in the
> 	   cleanup patch, add those calls again in the _new_
> 	   _bridge_pre_enable() (which are really the same function
> 	   bodies).

I would think patch-12 differently: it doesn't do what you list above, 
but rather combines the current pre_enable() and enable() into a new 
pre_enable().

> Do you think we can instead skip the cleanup patch, as well as #3 from
> patch-12?

Yes, I think the cleanup patch can just be dropped. It's not really 
relevant.

> Fun fact: We already have patch-4 which fixes the order of init calls in
> _bridge_enable()! =)

Right. And I guess that fix doesn't fix anything in practice, as those 
init calls are no-ops in the bridge_enable()...

It's a bit difficult to make meaningful fixes when things are so badly 
messed up =).

So, maybe try to arrange the series so that the obvious "makes-sense" 
fixes for stable are in the beginning. So... patches 1, 3, 5? And then 
work towards the patch 12.

And I'll try to not nit-pick too much, so that we can actually get this 
series merged, and then later do the cleanups on top =).

 Tomi