Re: [RESEND PATCH 3/4] clk: mediatek: support COMMON_CLK_MT6779 module build

[Miles Chen <miles.chen@mediatek.com>]

On Sat, 2021-08-28 at 21:14 -0700, Stephen Boyd wrote:
> Quoting Miles Chen (2021-08-12 20:24:28)
> > diff --git a/drivers/clk/mediatek/clk-mt6779-aud.c b/drivers/clk/mediatek/clk-mt6779-aud.c
> > index 11b209f95e25..439c0bc94b73 100644
> > --- a/drivers/clk/mediatek/clk-mt6779-aud.c
> > +++ b/drivers/clk/mediatek/clk-mt6779-aud.c
> > @@ -4,6 +4,7 @@
> >   * Author: Wendell Lin <wendell.lin@mediatek.com>
> >   */
> >  
> > +#include <linux/module.h>
> >  #include <linux/clk-provider.h>
> >  #include <linux/of.h>
> >  #include <linux/of_address.h>
> > @@ -115,3 +116,4 @@ static struct platform_driver clk_mt6779_aud_drv = {
> >  };
> >  
> >  builtin_platform_driver(clk_mt6779_aud_drv);
> > +MODULE_LICENSE("GPL");
> 
> How does this work? builtin_platform_driver() means that it's not
> modular code. Shouldn't that be module_platform_driver()? Have you tried
> loading and unloading the module?

sorry for my late response. 

Thanks for pointing this out. I have the same question when I was
building this patch. That time I found some examples where
they are using builtin_platform_driver and can be built as 
kernel modules:

config CLK_IMX8QXP (tristate) && drivers/clk/imx/clk-imx8qxp-lpcg.c
config CLK_RK3399 (tristate) && drivers/clk/rockchip/clk-rk3399.c

my load test:
load these moduless and do 'lsmod' on v5.14-rc1/mt6779 environment:

clk_mt6779_aud 16384 0 [permanent], Live 0xffff800008fd8000
clk_mt6779_mfg 16384 0 [permanent], Live 0xffff800008fd0000
clk_mt6779_venc 16384 0 [permanent], Live 0xffff800008fc8000
clk_mt6779_vdec 16384 0 [permanent], Live 0xffff800008fc0000
clk_mt6779_cam 16384 0 [permanent], Live 0xffff800008fb8000
clk_mt6779_ipe 16384 0 [permanent], Live 0xffff800008fb0000
clk_mt6779_img 16384 0 [permanent], Live 0xffff800008fa8000
clk_mt6779_mm 16384 0 [permanent], Live 0xffff800008fa0000

I did not test 'unload' kernel modules because I did not
define module_exit in this patch.

But as you pointed out, it should be module_platform_driver().
I will use module_platform_driver() in the next patch.


Miles