Re: [PATCH 4/5] clocksource: rockchip: add support for rk3399 SoC

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 06/01/2016 03:58 AM, Huang, Tao wrote:
> Hi Daniel:
> On 2016年05月31日 22:06, Daniel Lezcano wrote:

[ ... ]

>>> -CLOCKSOURCE_OF_DECLARE(rk_timer, "rockchip,rk3288-timer", rk_timer_init);
>>> +static void __init rk3288_timer_init(struct device_node *np)
>>> +{
>>> +	bc_timer.ctrl = TIMER_CONTROL_REG3288;
>>> +	rk_timer_init(np);
>>
>> 	rk_timer_init(np);
>> 	bc_timer.ctrl = bc_timer.base + TIMER_CONTROL_REG3288;
>
> No. It's not such simple. You will access null pointer when
> rk_timer_init, if we keep rk_timer_disable call in init or after
> request_irq/clockevents_config_and_register and interrupt happen
> immediately.
>
> So the code maybe:
> static void __init rk3288_timer_init(struct device_node *np)
> {
> 	bc_timer.base = of_iomap(np, 0);
> 	if (!bc_timer.base) {
> 		pr_err("Failed to get base address for '%s'\n", TIMER_NAME);
> 		return;
> 	}
> 	bc_timer.ctrl = bc_timer.base + TIMER_CONTROL_REG3288;
> 	rk_imter_init(np); // of course remove of_iomap from init.
>
> Is this what you want?

Not necessarily. There are plenty of variants.

eg. rk_timer_init(np, TIMER_CONTROL_REG3288);



-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog