Re: [PATCH v2 36/47] clk: mediatek: mt2712: Change Kconfig options to allow module build

[AngeloGioacchino Del Regno]

Il 17/02/23 16:19, Chen-Yu Tsai ha scritto:
> On Fri, Feb 17, 2023 at 7:25 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:
> > Il 17/02/23 05:24, Chen-Yu Tsai ha scritto:
> > > On Tue, Feb 14, 2023 at 9:42 PM AngeloGioacchino Del Regno
> > > <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:
> > > > All of the mt2712 drivers have been converted to platform drivers!
> > > > Change the Kconfig options for all MT2712 clocks to tristate to allow
> > > > building all clock drivers as modules.
> > > >
> > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> > > > ---
> > > >    drivers/clk/mediatek/Kconfig | 16 ++++++++--------
> > > >    1 file changed, 8 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig
> > > > index b9c0a9e21cf1..45b7aea7648d 100644
> > > > --- a/drivers/clk/mediatek/Kconfig
> > > > +++ b/drivers/clk/mediatek/Kconfig
> > > > @@ -75,7 +75,7 @@ config COMMON_CLK_MT2701_G3DSYS
> > > >             This driver supports MediaTek MT2701 g3dsys clocks.
> > > >
> > > >    config COMMON_CLK_MT2712
> > > > -       bool "Clock driver for MediaTek MT2712"
> > > > +       tristate "Clock driver for MediaTek MT2712"
> > >
> > > Hmm... How does that work out if mt2712-apmixedsys is a
> > > builtin_platform_driver?
> > >
> > > ChenYu
> >
> > That doesn't. Thanks for catching that, I've added a .remove() callback
> > and changed it to module_platform_driver() for v3!
>
> Actually, I thought that if it were built as a module, then the
> builtin_platform_driver would then expand to a module_init() without
> module_exit(). So it would become a loadable module that cannot be
> unloaded.
>
> That was just looking at the header files, so I could be mistaken.
>
> Side note: IIRC a missing .remove() driver callback doesn't actually
> block driver removal or unbinding.

It doesn't, but we'd leak memory because of the kzalloc() that happened
at the beginning of the probe function... so adding the .remove() callback
was pretty much necessary :-)

Thanks,
Angelo

> ChenYu