Re: [Xen-devel] [PATCH v2 21/41] arm : acpi Initialize serial port from ACPI SPCR table

[Parth Dixit <parth.dixit@xxxxxxxxxx>]

+shannon

On 26 May 2015 at 20:34, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Hi Parth,
>
> On 17/05/2015 22:03, Parth Dixit wrote:
>>
>> @@ -307,6 +308,54 @@ DT_DEVICE_START(pl011, "PL011 UART", DEVICE_SERIAL)
>>           .init = dt_pl011_uart_init,
>>   DT_DEVICE_END
>>
>> +#ifdef CONFIG_ACPI
>> +static int __init acpi_pl011_uart_init(const void *data)
>> +{
>> +    struct pl011 *uart;
>> +    acpi_status status;
>> +    struct acpi_table_spcr *spcr=NULL;
>> +    int res;
>> +
>> +    status = acpi_get_table(ACPI_SIG_SPCR, 0,
>> +            (struct acpi_table_header **)&spcr);
>
>
> Indentation.
>
> And I think this could have been done in the generic UART code. Every UART
> driver will likely need to get the SPCR table.
>
>> +
>> +    if ( ACPI_FAILURE(status) )
>> +    {
>> +        printk("\nFailed to get SPCR table \n");
>
>
> No need of the first newline and the last space.
>
>> +        return 1;
>> +    }
>> +
>> +    uart = &pl011_com;
>> +
>> +    if ( spcr->interrupt < 0 )
>
>
> No need of the check, the field interrupt is an u32. Is there any other way
> to find check if the interrupt is valid?
>
>> +    {
>> +        printk("pl011: Unable to retrieve the IRQ\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    uart->irq = spcr->interrupt;
>> +    /* trigger/polarity information is not available in spcr */
>
>
> If so, how did you find/device that the interrupt is edge? Shouldn't we just
> avoid to configure it?
>
> Regards,
>
> --
> Julien Grall

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