linux-kernel - Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks

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:   Tue, 13 Dec 2016 15:25:24 -0800
From:   Stephen Boyd <sboyd@...eaurora.org>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Irina Tirdea <irina.tirdea@...el.com>, linux-clk@...r.kernel.org,
        "x86@...nel.org" <x86@...nel.org>,
        platform-driver-x86@...r.kernel.org,
        Darren Hart <dvhart@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Michael Turquette <mturquette@...libre.com>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        ALSA Development Mailing List <alsa-devel@...a-project.org>,
        Mark Brown <broonie@...nel.org>, Takashi Iwai <tiwai@...e.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...el.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
Subject: Re: [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks

On 12/13, Andy Shevchenko wrote:
> On Fri, Dec 9, 2016 at 8:01 PM, Irina Tirdea <irina.tirdea@...el.com> wrote:
> 
> > --- a/drivers/clk/x86/Makefile
> > +++ b/drivers/clk/x86/Makefile
> > @@ -1,2 +1,5 @@
> >  clk-x86-lpss-objs              := clk-lpt.o
> >  obj-$(CONFIG_X86_INTEL_LPSS)   += clk-x86-lpss.o
> 
> > +ifeq ($(CONFIG_COMMON_CLK), y)
> 
> Hmm... I think (I didn't check) we don't go here otherwise.

We should move this statement to drivers/clk/Makefile around the
x86 line.

> > +                                       void __iomem *base,
> > +                                       const char **parent_names,
> > +                                       int num_parents)
> > +{
> > +       struct clk_plt *pclk;
> > +       struct clk_init_data init;
> > +       int ret;
> > +
> > +       pclk = devm_kzalloc(&pdev->dev, sizeof(*pclk), GFP_KERNEL);
> > +       if (!pclk)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       init.name =  kasprintf(GFP_KERNEL, "%s%d", PLT_CLK_NAME_BASE, id);
> 
> devm_kasprintf()

Please no.

> 
> > +       init.ops = &plt_clk_ops;
> > +       init.flags = 0;
> > +       init.parent_names = parent_names;
> > +       init.num_parents = num_parents;
> > +
> > +       pclk->hw.init = &init;
> > +       pclk->reg = base + id * PMC_CLK_CTL_SIZE;
> > +       spin_lock_init(&pclk->lock);
> > +
> > +       ret = devm_clk_hw_register(&pdev->dev, &pclk->hw);
> > +       if (ret)
> > +               goto err_free_init;
> > +
> > +       pclk->lookup = clkdev_hw_create(&pclk->hw, init.name, NULL);
> > +       if (!pclk->lookup) {
> > +               ret = -ENOMEM;
> > +               goto err_free_init;
> > +       }
> > +
> 
> > +       kfree(init.name);
> 
> devm_kfree();

It's all local to this function, devm isn't helping anything.
Having one kfree() would be good though. And using init.name for
the clkdev lookup is probably wrong and should be replaced with
something more generic along with an associated device name.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project