Re: [RFC PATCH v2] xen/arm: split the init_xen_time() in 2 parts

[Julien Grall <julien.grall@linaro.org>]

On 27/01/15 17:52, Oleksandr Tyshchenko wrote:
> On Tue, Jan 27, 2015 at 7:09 PM, Julien Grall <julien.grall@linaro.org> wrote:
>> Hi Oleksandr,
> Hi Julien
> 
>>
>> On 27/01/15 13:39, Oleksandr Tyshchenko wrote:
>>> -/* Set up the timer on the boot CPU */
>>> -int __init init_xen_time(void)
>>> +static const struct dt_device_match timer_ids[] __initconst =
>>> +{
>>> +     DT_MATCH_TIMER,
>>> +     { /* sentinel */ },
>>> +};
>>> +
>>> +/* Set up the timer on the boot CPU (early init function) */
>>> +void __init preinit_xen_time(void)
>>>  {
>>> -    static const struct dt_device_match timer_ids[] __initconst =
>>> -    {
>>> -        DT_MATCH_TIMER,
>>> -        { /* sentinel */ },
>>> -    };
>>
>> I guess this is a left-over from the previous version?
>> I would keep the definition of the variable here.
> Sorry, I am not sure that I fully understand you.
> 
> in v1 I created separate find_timer_node() a move to it timer_ids,
> where the reason is to not duplicate it in init_xen_time() and
> preinit_xen_time().
> 
> +static __init struct dt_device_node *find_timer_node(void)
> +{
> +       static const struct dt_device_match timer_ids[] __initconst =
> +       {
> +           DT_MATCH_TIMER,
> +           { /* sentinel */ },
> +       };
> +
> +       return dt_find_matching_node(NULL, timer_ids);
> +}
> 
> in v2 taking into account your comment about "static" I decided to not
> introduce additional func whose purpose is just to return pointer and
> drop it.

I think you misunderstood my comment on the V1. You don't need to look
twice for the device node. You can store it outside the function.

What I asked was something like:

static __initdata struct dt_device_node *timer;

void __init preinit_xen_time(void)
{
	static const struct dt_device_match timer_ids[] ... =
		...
	
	timer = dt_find_matching_node(NULL, timer_ids);
	if (!timer)
		panic("...");

}

init __init init_xen_time(void)
{

	/* Use timer without checking */
}

Regards,

-- 
Julien Grall