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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 29 Jun 2020 10:45:07 +0000
From:   Anson Huang <anson.huang@....com>
To:     Arnd Bergmann <arnd@...db.de>
CC:     Russell King - ARM Linux <linux@...linux.org.uk>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Sascha Hauer <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Stefan Agner <stefan.agner@...adex.com>,
        "oleksandr.suvorov@...adex.com" <oleksandr.suvorov@...adex.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Leo Li <leoyang.li@....com>, Vinod Koul <vkoul@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Olof Johansson <olof@...om.net>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH 1/3] reset: imx7: Support module build



> Subject: Re: [PATCH 1/3] reset: imx7: Support module build
> 
> On Mon, Jun 29, 2020 at 12:25 PM Anson Huang <Anson.Huang@....com>
> wrote:
> >
> > Add module device table, module license to support module build.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@....com>
> > ---
> >  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