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

[Oleksandr Tyshchenko <oleksandr.tyshchenko@xxxxxxxxxxxxxxx>]

Hi, Julien.

I created a new patch set (v3) and pushed.

Also I created a standalone patch according to your suggestion above:
http://lists.xen.org/archives/html/xen-devel/2015-01/msg02964.html

I checked it on Lager board on current master.

On Thu, Jan 22, 2015 at 8:33 PM, Oleksandr Tyshchenko <oleksandr.tyshchenko@xxxxxxxxxxxxxxx> wrote:

On Thu, Jan 22, 2015 at 8:09 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> On 22/01/15 17:43, Oleksandr Tyshchenko wrote:
>> On Thu, Jan 22, 2015 at 7:27 PM, Oleksandr Tyshchenko
>> <oleksandr.tyshchenko@xxxxxxxxxxxxxxx> wrote:
>>> On Thu, Jan 22, 2015 at 7:02 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
>>>> On 22/01/15 16:55, Oleksandr Tyshchenko wrote:
>>>>> On Thu, Jan 22, 2015 at 6:49 PM, Julien Grall <julien.grall@xxxxxxxxxx> 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

It is clear. I will try.
Thank you

--

Oleksandr Tyshchenko | Embedded Dev
GlobalLogic
www.globallogic.com

--

Oleksandr Tyshchenko | Embedded Dev
GlobalLogic
www.globallogic.com

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