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

[Aradhya Bhatia <aradhya.bhatia@linux.dev>]

Hi Tomi,

On 15/01/25 13:47, Tomi Valkeinen wrote:
> 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().

Right, yes!

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

Yeah, it doesn't do anything... until patch-12 comes back in picture.
So, I shall drop patch-4 too as there's no point in getting that patch
backported. And I will let patch-12 take care of the correct ordering.

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

Yes, this sounds good.


Regards
Aradhya