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
