Re: [PATCH 3/3] xen/arm: drivers: scif: Add clock auto detection

[Julien Grall <julien.grall@arm.com>]

On 21/06/16 13:30, Dirk Behme wrote:
>>> diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
>>> index bc157fe..678f46b 100644
>>> --- a/xen/drivers/char/scif-uart.c
>>> +++ b/xen/drivers/char/scif-uart.c
>>> @@ -107,8 +107,19 @@ static void __init scif_uart_init_preirq(struct
>>> serial_port *port)
>>>       scif_readw(uart, SCIF_SCLSR);
>>>       scif_writew(uart, SCIF_SCLSR, 0);
>>>
>>> -    /* Select Baud rate generator output as a clock source */
>>> -    scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10);
>>> +    /*
>>> +     * Select Baud rate generator output as a clock source
>>> +     * The clock source can be an internal or external clock.
>>> +     * Depending on this the DL register is either 0 or contains
>>> +     * the divisor. I.e. we can use this to detect the clock
>>> +     * source and based on this can configure the CKE[1:0] bits
>>> +     * of the SCSCR register.
>>> +     */
>>> +    if ( scif_readw(uart, SCIF_DL) )
>>> +        scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); /* External clk */
>>> +    else
>>> +        scif_writew(uart, SCIF_SCSCR, SCSCR_CKE00); /* Internal clk */
>>
>> Why would we need to select the baud rate generator if the baud has been
>> configured by the firmware?
>
>
> Just to get the correct understanding: The proposal is to just remove
> the code which (wrongly) overwrites the correct settings done by the
> firmware?
>
> I.e. instead of doing the same thing the firmware is already doing,
> again (the if .. else ), the proposal is simply dropping the
>
>
>   -    /* Select Baud rate generator output as a clock source */
>   -    scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10);
>
> ?

Yes. However I don't have any spec in hand so I am not sure if it is 
correct.

Regards,

-- 
Julien Grall

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