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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Stephen,

can you elaborate on the last comment?

thanks,

-Pierre


On 12/13/2016 05:25 PM, Stephen Boyd wrote:

+                                       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.
I am not sure I understand this last comment.
init.name is not a constant, it's made of the "pmc_plt_clk_" string concatenated with an id which directly maps to which hardware clock is registered. Clients use devm_clk_get() with a "pmc_plt_clk_<n>" argument.


--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux