RE: [PATCH 1/3] reset: imx7: Support module build

[Anson Huang]

> Subject: Re: [PATCH 1/3] reset: imx7: Support module build
> 
> On Mon, Jun 29, 2020 at 12:25 PM Anson Huang <Anson.Huang@xxxxxxx>
> wrote:
> >
> > Add module device table, module license to support module build.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@xxxxxxx>
> > ---
> >  drivers/reset/Kconfig      | 4 ++--
> >  drivers/reset/reset-imx7.c | 4 +++-
> >  2 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index
> > d9efbfd..033ab60 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -65,9 +65,9 @@ config RESET_HSDK
> >           This enables the reset controller driver for HSDK board.
> >
> >  config RESET_IMX7
> > -       bool "i.MX7/8 Reset Driver" if COMPILE_TEST
> > +       tristate "i.MX7/8 Reset Driver"
> >         depends on HAS_IOMEM
> > -       default SOC_IMX7D || (ARM64 && ARCH_MXC)
> > +       depends on SOC_IMX7D || (ARM64 && ARCH_MXC) ||
> COMPILE_TEST
> >         select MFD_SYSCON
> 
> You are dropping the 'default' line, so the driver is now disabled in a defconfig
> build, which is not mentioned in the patch description.
> 
> Maybe make it 'default m'?
> 
> config RESET_IMX7
>        tristate "i.MX7/8 Reset Driver"
>        depends on HAS_IOMEM
>        depends on SOC_IMX7D || (ARM64 && ARCH_MXC) ||
> COMPILE_TEST
>        default m if (SOC_IMX7D || (ARM64 && ARCH_MXC))
>        select MFD_SYSCON
> 
> > @@ -395,3 +396,4 @@ static struct platform_driver imx7_reset_driver = {
> >         },
> >  };
> >  builtin_platform_driver(imx7_reset_driver);
> > +MODULE_LICENSE("GPL v2");
> 
> Generally speaking: when you add a MODULE_LICENSE tag, please also add
> MODULE_AUTHOR and MODULE_DESCRIPTION.
> 

OK, will add them in V2.

> The 'builtin_platform_driver()' should work correctly but prevent unloading
> the module. Ideally please changed to 'module_platform_driver()' and add
> a .remove function for the platform_driver.
> 

The reset driver normally won't be removed since it is necessary for drivers which
need it, it is just for Android GKI support, in this case, do we need to change it to
module_platform_driver()? 

Thanks,
Anson