Messages in this thread | | | From | Anson Huang <> | Subject | RE: [PATCH V4 5/5] clk: imx8qxp: Support building i.MX8QXP clock driver as module | Date | Thu, 2 Jul 2020 05:50:46 +0000 |
| |
> Subject: Re: [PATCH V4 5/5] clk: imx8qxp: Support building i.MX8QXP clock > driver as module > > On Thu, Jul 2, 2020 at 11:55 AM Anson Huang <anson.huang@nxp.com> > wrote: > [...] > > > > +{ > > > > + return platform_driver_register(&imx8qxp_lpcg_clk_driver); > > > > +} > > > > +device_initcall(imx8qxp_lpcg_clk_init); > > > > > > Any reason to change to device_initcall which looks a bit strange? > > > Is it because the following line? > > > +obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o > > > But it looks to me they're still two modules. Aren't they? > > > > It is suggested by Arnd to NOT use builtin_platform_driver(), in order > > to support module unload, although the clock driver normally does NOT > > support remove, but it is better to follow the right way. > > > > By expanding builtin_platform_driver() marcro, you will find your patch is > exactly doing the same thing as buildin_platform_driver() which obivously is > unneccesary. > > #define builtin_platform_driver(__platform_driver) \ > builtin_driver(__platform_driver, platform_driver_register) #define > builtin_driver(__driver, __register, ...) \ > > static int __init __driver##_init(void) \ { \ > return __register(&(__driver) , ##__VA_ARGS__); \ } \ > device_initcall(__driver##_init); > > If we want to support unload, we need a .remove() callback as current clocks > are not allocated by devm_(). > If don't support, we probably can use builtin_platform_driver() first and > switch to module_platform_driver() in the future once the driver supports > release resource properly. >
Yes, that is why I use the device_initcall() to make it exactly same as builtin_driver(), and also yes that i.MX clock driver does NOT support module unload, so .remove() is NOT implemented, I am fine with either way, just try to address Arnd's comment.
Hi, Arnd What do you think? Do you agree to keep using the builtin_driver()?
Thanks, Anson
| |