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

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 27/01/15 17:52, Oleksandr Tyshchenko wrote:
> On Tue, Jan 27, 2015 at 7:09 PM, Julien Grall <julien.grall@xxxxxxxxxx> 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel