Re: [Xen-devel] [PATCH] arm/acpi: Add Server Base System Architecture UART support

[Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>]

On 05/27/2016 08:04 AM, Julien Grall wrote:
> Hello Shanker,
>
> On 27/05/16 01:28, Shanker Donthineni wrote:
>> The ARM Server Base System Architecture describes a generic UART
>> interface. It doesn't support clock control registers to set
>> baudrate. So, extend the driver probe() to handle SBSA interface
>> types and set the baudrate to 115200 for SBSA interfaces.
>
> I cannot find any mention of the baudrate in the SBSA. Where does it come 
> from?
>
Yes, no where mentioned about the baudrate in SBSA document. I used 115200 
based on the  the Linux  PL011 driver.

> Also the driver is using registers which should not be touch for the SBSA 
> UART (see appendix B in the SBSA ARM-DEN-0029 v3.0). So this need to be 
> address to get a proper support.
>
Yes, I agree with you, Which registers it is touching other than baudrate 
control registers?

>> Signed-off-by: Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>
>> ---
>> https://silver.arm.com/download/download.tm?pv=2950177
>>   xen/drivers/char/pl011.c | 25 +++++++++++++++++++++----
>>   1 file changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
>> index 1212d5c..81d095f 100644
>> --- a/xen/drivers/char/pl011.c
>> +++ b/xen/drivers/char/pl011.c
>> @@ -226,14 +226,14 @@ static struct uart_driver __read_mostly pl011_driver = 
>> {
>>       .vuart_info   = pl011_vuart,
>>   };
>>
>> -static int __init pl011_uart_init(int irq, u64 addr, u64 size)
>> +static int __init pl011_uart_init(int irq, u64 addr, u64 size, int baud)
>>   {
>>       struct pl011 *uart;
>>
>>       uart = &pl011_com;
>>       uart->irq       = irq;
>>       uart->clock_hz  = 0x16e3600;
>> -    uart->baud      = BAUD_AUTO;
>> +    uart->baud      = baud;
>>       uart->data_bits = 8;
>>       uart->parity    = PARITY_NONE;
>>       uart->stop_bits = 1;
>> @@ -285,7 +285,7 @@ static int __init pl011_dt_uart_init(struct 
>> dt_device_node *dev,
>>           return -EINVAL;
>>       }
>>
>> -    res = pl011_uart_init(res, addr, size);
>> +    res = pl011_uart_init(res, addr, size, BAUD_AUTO);
>>       if ( res < 0 )
>>       {
>>           printk("pl011: Unable to initialize\n");
>> @@ -315,6 +315,7 @@ static int __init pl011_acpi_uart_init(const void *data)
>>   {
>>       acpi_status status;
>>       struct acpi_table_spcr *spcr = NULL;
>> +    int baud = BAUD_AUTO;
>>       int res;
>>
>>       status = acpi_get_table(ACPI_SIG_SPCR, 0,
>> @@ -326,17 +327,23 @@ static int __init pl011_acpi_uart_init(const void 
>> *data)
>>           return -EINVAL;
>>       }
>>
>> +    if ( (spcr->interface_type == ACPI_DBG2_SBSA_32) ||
>
> Based on the DBG2 spec, this value is deprecated. So I do not think we should 
> support it.
>

I'll fix.

>> +         (spcr->interface_type == ACPI_DBG2_SBSA) )
>> +        baud = 115200;
>> +
>>       /* trigger/polarity information is not available in spcr */
>>       irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_HIGH);
>>
>>       res = pl011_uart_init(spcr->interrupt, spcr->serial_port.address,
>> -                          PAGE_SIZE);
>> +                          PAGE_SIZE, baud);
>>       if ( res < 0 )
>>       {
>>           printk("pl011: Unable to initialize\n");
>>           return res;
>>       }
>>
>> +
>> +
>
> Spurious changes.
>
>>       return 0;
>>   }
>>
>> @@ -344,6 +351,16 @@ ACPI_DEVICE_START(apl011, "PL011 UART", DEVICE_SERIAL)
>>           .class_type = ACPI_DBG2_PL011,
>>           .init = pl011_acpi_uart_init,
>>   ACPI_DEVICE_END
>> +
>> +ACPI_DEVICE_START(asbsa_uart, "SBSA UART", DEVICE_SERIAL)
>> +        .class_type = ACPI_DBG2_SBSA,
>> +        .init = pl011_acpi_uart_init,
>> +ACPI_DEVICE_END
>> +
>> +ACPI_DEVICE_START(asbsa32_uart, "SBSA32 UART", DEVICE_SERIAL)
>> +        .class_type = ACPI_DBG2_SBSA_32,
>> +        .init = pl011_acpi_uart_init,
>> +ACPI_DEVICE_END
>>   #endif
>>
>>   /*
>>
>
> Regards,
>

-- 
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project


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