Messages in this thread | | | Date | Fri, 20 May 2022 10:42:40 +0200 | Subject | Re: [PATCH 3/6] clk: mediatek: reset: Return reset data pointer on register | From | AngeloGioacchino Del Regno <> |
| |
Il 19/05/22 15:47, Yassine Oudjana ha scritto: > From: Yassine Oudjana <y.oudjana@protonmail.com> > > Return a struct mtk_clk_rst_data * when registering a reset > controller in preparation for adding an unregister helper > that will take it as an argument. Make the necessary changes > in drivers that do not currently discard the return value > of register functions. > > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Hello Yassine,
Thanks for your efforts on helping to make the MediaTek clocks better - I agree (and I'm not the only one..) that there's a lot of work to do on this side.
Though... I don't think that this is the right direction: you're right about properly unregistering (in patch 4/6) the reset controllers on rmmod/failure but I'm not sure that this kind of noise brings any benefit.
Explaining: You definitely saw that there's a new register _with_dev, which uses devm ops and that's going to automatically cleanup in case of removal/failure. This is what we should do.
Hence, my proposal is to drop patch 3/6, 4/6, 5/6 and (slowly, steadily) migrate all of the MediaTek clocks from CLK_OF_DECLARE() to platform drivers (which also means that we can eventually change them to tristate!), so that we slowly remove all users of all functions that are not "_with_dev", and that we finally remove all of these then-unused functions as well.
Making sure that I don't get misunderstood: I'm not implying that this huge migration work is on your shoulders!
P.S.: Chen-Yu, Miles: do you also agree? :-)
Cheers, Angelo
| |