Re: [ARM:PATCH v2 1/1] Pass the timer clock-frequency to DOM0

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

Hello Suryian,

On 03/27/2014 10:16 PM, Suriyan Ramasami wrote:
> On Thu, Mar 27, 2014 at 2:48 PM, Julien Grall <julien.grall@linaro.org> wrote:
>> Hello Suriyan,
>> Thank you for the patch. Xen-devel starts to have a big traffic, we use to
>> cc maintainers on patches. You can find them via script/get_maintainers.pl
>>
> Thank you for the comments Julien. Next time around, I shall make sure
> I use get_maintainers.pl and get the list of the maintainers that I
> need to cc.
> 
>>
>> On 27/03/14 03:54, Suriyan Ramasami wrote:
>>>
>>>      xen/arm: Pass the timer "clock-frequency" to DOM0 in make_timer_node
>>
>>
>> You guess you are copying git show in an email? If so, I advise you to read
>> this website:
>> http://alblue.bandlem.com/2011/12/git-tip-of-week-patches-by-email.html
>>
> I actually used git send-email as mentioned in the wiki.

Do you also use git format-patch?

>>
>>>      If the DT representing the ARM generic timer mentions a
>>> clock-frequency,
>>>      propragate it to the DT that is built for DOM0.
>>>
>>>      This is necessary as a workaround for boards (Odroid-XU) where CNTFRQ
>>> is
>>>      not set or returns a wrong value.
>>
>>
>> Do you have any plan to send a similar patch for domU?
>>
> I am exploring other options for domU - possibly a parameter that can
> be introduced in the cfg file for the domU. I am still exploring. Any
> suggestions would be deeply appreciated!

I'm not in favor to add a new config option. The user may not be aware
that the clock frequency has to be set up in xl config file for his board.

That made me think that this is the only board with this issue. I would
definitely prefer to fix the clock frequency in U-boot. The bootloader
should respect the ARM ARM (see B.8.1.1):

"The CNTFRQ register is UNKNOWN at reset, and therefore the counter
frequency must written to CNTFRQ as part
of the system boot process."

> I also am in parallel working on a patch for the odroid-xu (exynos5410).

Great, thanks!

>>>
>>> @@ -639,6 +641,9 @@ static int make_timer_node(const struct domain *d,
>>> void *fdt,
>>>           return -FDT_ERR_XEN(ENOENT);
>>>       }
>>>
>>> +    clock_valid = dt_property_read_u32(dev, "clock-frequency",
>>> +                                       &clock_frequency);
>>> +
>>
>>
>> Can you gather all your changes in one place? I don't see why you need to
>> read the clock-frequency earlier.
>>
> I could gather them in one place, but I mimicked what make_cpus_node()
> does, so that make_timer_node() too has similar code. Please let me
> know if you would really like it in one place.

It has been done earlier because we also retrieve the compatible string
at the same time. The former is used to check if we can create the node
or not.

Regards,

-- 
Julien Grall