Re: [PATCH 1/1] mtd:nand:fix memory leak

[Boris Brezillon <boris.brezillon@bootlin.com>]

On Thu, 5 Apr 2018 13:26:31 +0200
Marc Gonzalez <marc.w.gonzalez@free.fr> wrote:

> On 05/04/2018 11:54, Boris Brezillon wrote:
> 
> > On Thu, 5 Apr 2018 11:12:11 +0200, Marc Gonzalez wrote:
> >   
> >> I was not aware that clk_get() allocated memory, and required clk_put()
> >> for cleanup. IIRC, I looked at Documentation/clk.txt
> >>
> >> On tango, clocks are configured by the boot loader. The existing clk driver
> >> provides only read access to various clocks -- except the CPU clock, which
> >> can be changed by tweaking a post-divider. Tweaking the PLLs requires much
> >> more complex code. The boot loader enables every clock, and Linux has no
> >> way to gate any of them.  
> > 
> > Well, even if that's not supported today, it's always a good practice
> > to retain reference and prepare/enable clks your HW depends on. This
> > change should be harmless and when/if you someday decide to provide a
> > way to gate clks, it will work out of the box.  
> 
> IIUC, you're saying:
> 1) use devm_clk_get() instead of clk_get() to solve the memory leak
> 2) call clk_prepare_enable() before clk_get_rate() even if the former is a no-op today

+ a disable_unprepare() in the remove path and a another one in the
error path (in case something fails after prepare_enable() has been
called).

> 
> The following patch implements these suggestions.
> (Only compile-tested)
> 
> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> index c5bee00b7f5e..39d190b7521f 100644
> --- a/drivers/mtd/nand/tango_nand.c
> +++ b/drivers/mtd/nand/tango_nand.c
> @@ -643,7 +643,7 @@ static int tango_nand_probe(struct platform_device *pdev)
>  
>         writel_relaxed(MODE_RAW, nfc->pbus_base + PBUS_PAD_MODE);
>  
> -       clk = clk_get(&pdev->dev, NULL);
> +       clk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(clk))
>                 return PTR_ERR(clk);
>  
> @@ -651,6 +651,10 @@ static int tango_nand_probe(struct platform_device *pdev)
>         if (IS_ERR(nfc->chan))
>                 return PTR_ERR(nfc->chan);
>  
> +       err = clk_prepare_enable(clk);
> +       if (err)
> +               return err;
> +
>         platform_set_drvdata(pdev, nfc);
>         nand_hw_control_init(&nfc->hw);
>         nfc->freq_kHz = clk_get_rate(clk) / 1000;