Re: [PATCH 3/3] ARM: OMAP2+: dmtimer: cleanup fclk usage

["Cousson, Benoit" <b-cousson@ti.com>]

On 4/16/2012 6:40 PM, Paul Walmsley wrote:
> Hi
>
> a few comments
>
> On Mon, 16 Apr 2012, Tarun Kanti DebBarma wrote:

...

>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>> index 5d7a0ee..9459d70 100644
>> --- a/arch/arm/mach-omap2/timer.c
>> +++ b/arch/arm/mach-omap2/timer.c
>> @@ -164,8 +164,7 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
>>   		return -ENXIO;
>>
>>   	/* After the dmtimer is using hwmod these clocks won't be needed */
>> -	sprintf(name, "gpt%d_fck", gptimer_id);
>> -	timer->fclk = clk_get(NULL, name);
>> +	timer->fclk = clk_get(NULL, oh->main_clk);
>>   	if (IS_ERR(timer->fclk))
>>   		return -ENODEV;
>
> If you find yourself accessing struct omap_hwmod structure fields
> directly, something is almost certainly wrong; and this case is not an
> exception.  Since the omap_device code is defining an "fck" alias to the
> main_clk, all you should need to do is:
>
> -     sprintf(name, "gpt%d_fck", gptimer_id);
> -     timer->fclk = clk_get(NULL, name);
> +     timer->fclk = clk_get(NULL, "fck");

Are you sure that's the same in that case?
"timer1_fck" is an unique entry in the clkdev table whereas there are a 
bunch of clock nodes with the "fck" alias local to a device.
Without the proper dev_id, there is no way to guaranty we will get the 
timer fck clock alias.

Since that code is executed pretty early before there is any device, 
accessing directly the hwmod entry seems to be the only method to get 
the proper clock without having to provide again the full clock name.

AFAIR, the "fck" alias is created during the omap_device creation, so I 
guess it is too early here to have it.

To be honest I'm always a little bit confused with that early code, but 
it looks like there is no device at all in that early part of the code.

Regards,
Benoit