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
