Re: [PATCH v2 2/3] xen/arm: Add new driver for R-Car Gen2 UART

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

On 22/01/15 17:43, Oleksandr Tyshchenko wrote:
> On Thu, Jan 22, 2015 at 7:27 PM, Oleksandr Tyshchenko
> <oleksandr.tyshchenko@globallogic.com> wrote:
>> On Thu, Jan 22, 2015 at 7:02 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>> On 22/01/15 16:55, Oleksandr Tyshchenko wrote:
>>>> On Thu, Jan 22, 2015 at 6:49 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>>>> On 22/01/15 16:44, Oleksandr Tyshchenko wrote:
>>>>>> I understand, then I will implement local delay func in uart driver
>>>>>> based on READ_SYSREG64(CNTPCT_EL0).
>>>>>
>>>>> Unless I miss something, udelay should work in your case even if the
>>>>> xen_init_time is not called.
>>>> Unfortunately, no. If I understand correctly the var "cpu_khz" (used
>>>> in ticks_to_ns()) is initialized in init_xen_time().
>>>
>>> Hrm, right. I looked too quickly to the function.
>>>
>>> I don't like the idea to use READ_SYSREG64(CNTPCT_EL0) in the UART drivers.
>>>
>>> Does the udelay necessary here? If yes, maybe we should either split the
>>> xen_init_time in 2 parts or create a udelay_tick function to use when
>>> timer is not set.
>>>
>>> I'm not sure what is the best one.
>>>
>>> Regards,
>>>
>>> --
>>> Julien Grall
>>
>> According to the TRM for this family we need to add delay after
>> changing baudrate before continuing init sequence.
>> I tried without udelay and it is worked fine. But, if the TRM says we
>> need to follow this.
>>
>> --
>>
>> Oleksandr Tyshchenko | Embedded Dev
>> GlobalLogic
>> www.globallogic.com
> 
> I would prefer a first variant. In xen_init_time_1() initialize
> "cpu_khz" and "boot_count" only.
> 
>  cpu_khz = READ_SYSREG32(CNTFRQ_EL0) / 1000;
>  boot_count = READ_SYSREG64(CNTPCT_EL0);
> 
> In xen_init_time_2() perform all initialization as it was done for
> this moment and correct "cpu_khz" if node is present.
> 
> What do you think?
> 

The clock-frequency property is usually present when CNTFRQ_EL0 is
invalid. So the udelay won't work correctly for those board.

Also, some platform may need to have specific initialization for timer
before been able to access it. (see platform_init_time).

So we need to move at least to move preinit_xen_time:
	platform_init()
        read property clock-frequency
	cpu_khz = ...
	boot_count = ...

init_xen_time would contain:
	retrieve IRQs
	print messages

Regards,

-- 
Julien Grall