On Fri, Dec 16, 2016 at 7:15 AM, Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
Hi Stephen,
can you elaborate on the last comment?
Please don't do top posting.
devm_kasprintf()
Please no.
That's why I used modal verb "might" instead of "would".
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.
Giving more thoughts about design and use of this I would propose to do the following.
1. Create under clock framework something like clk-pmc-atom clock driver (see, for example, clk-fractional-divider, though this one should indeed go under x86 folder). 2. In real provider, i.e. pmc_atom, create the necessary clock tree with *names*.
Scheme with ID is fragile, imagine another version of PMC where ordering would be mixed up? It's not hypothetical since we used to have this already in pmc_atom for some registers and bits.