hi Jeremy,
It's nice to be close :).
And it seems that many people in arm kernel list are also intested in
the clock debug information, maybe we can post next version to that
list and let them know how things are going.
On Thu, Dec 9, 2010 at 10:02 AM, Jeremy Kerr <jeremy.k...@canonical.com> wrote:
> Hi Yong,
>
> We're getting pretty close - just a couple of changes:
>
>> + } else {
>> + struct preinit_clk *p;
>> + mutex_lock(&preinit_lock);
>> + p = kmalloc(sizeof(*p), GFP_KERNEL);
>> + if (!p)
>> + goto unlock;
>> + p->clk = clk;
>> + list_add(&p->list, &preinit_clks);
>> +unlock:
>> + mutex_unlock(&preinit_lock);
>> + }
>
> Might be best to avoid the goto:
Acked.
>
> } else {
> struct preinit_clk *p;
> mutex_lock(&preinit_lock);
> p = kmalloc(sizeof(*p), GFP_KERNEL);
> if (p) {
> p->clk = clk;
> list_add(&p->list, &preinit_clks);
> }
> mutex_unlock(&preinit_lock);
> }
>
>> +static int __init clk_debugfs_init(void)
>> +{
>> + struct preinit_clk *pclk, *tmp;
>> +
>> + if (debugfs_initialized())
>> + init_done = 1;
>
> No need to check debugfs_initialised() here; if it's not initialised we're in
> trouble anyway.
IMO, it is still needed. If debugfs is not initialised, we should
return directly, otherwise, it will redo clk_debug_register() again.
How about:
if (!debugfs_initialized())
return 0;
init_done = 1;
>
>> +
>> + list_for_each_entry(pclk, &preinit_clks, list) {
>> + clk_debug_register(pclk->clk);
>> + }
>
> You should hold preinit_lock while traversing the list here.
>
> Cheers,
>
>
> Jeremy
>
Thanks
Yong
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
