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

["DebBarma, Tarun Kanti" <tarun.kanti@xxxxxx>]

Hi Paul,

On Tue, Apr 17, 2012 at 2:21 AM, Cousson, Benoit <b-cousson@xxxxxx> wrote:
> 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.
That' right, I confirmed that clk_get(NULL, "fck") does not work.
For this to work we need the dev pointer in place of NULL.
What is your alternate suggestion? Thanks.
--
Tarun

>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html