On 3/17/23 10:36, Andy Shevchenko wrote:
On Fri, Mar 17, 2023 at 4:16 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On Mon, Dec 05, 2022 at 10:53:51AM +0200, Tomer Maimon wrote:
Add Nuvoton NPCM BMC sdhci-pltfm controller driver.
Signed-off-by: Tomer Maimon <tmaimon77@xxxxxxxxx>
I still don't see this driver in the upstream kernel, or in linux-next.
Couple of comments:
- devm ordering does not really matter here. The devm resource
is the clock, it does not depend on local data, and it will be
released last, so that is ok.
Not sure. Strictly speaking this is the problem. If you leave a clock
going on in a wrong period of time it (theoretically) might break your
hardware once and forever. Similar discussion about power, clock and
reset signals has been held for camera sensors.
In general I agree, but not here. The remove function (sdhci_pltfm_unregister)
does call clk_disable_unprepare(), so the clock isn't left running.
Also, I think it is worthwhile to point out that exactly the same sequence
(sdhci_pltfm_init followed by devm_clk_get and cleanup/removal with
sdhci_pltfm_unregister) is shared among several sdhci drivers (including
the memory leak I pointed out, but only in the aspeed driver).
On a higher level I do agree that the sdhci platform code is in need of cleanup,
but I don't think it is appropriate to tie such a cleanup to this driver
submission.
Note that I don't really care much, I just realized that this patch is stuck
when I tried to test booting from SD drive with qemu.
Guenter
