diff mbox

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

Message ID 1431893048-5214-22-git-send-email-parth.dixit@linaro.org
State New
Headers show

Commit Message

Parth Dixit May 17, 2015, 8:03 p.m. UTC 
Parse ACPI SPCR (Serial Port Console Redirection table) table and
initialize the serial port pl011.

Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
---
 xen/drivers/char/pl011.c  | 49 +++++++++++++++++++++++++++++++++++++++++++++++
 xen/include/acpi/actbl2.h |  5 +++++
 2 files changed, 54 insertions(+)

Comments

Parth Dixit July 5, 2015, 1:14 p.m. UTC | #1 
+shannon

On 26 May 2015 at 20:34, Julien Grall <julien.grall@citrix.com> 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
diff mbox

Patch

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index f0c3daf..bc34555 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -27,6 +27,7 @@ 
 #include <asm/device.h>
 #include <xen/mm.h>
 #include <xen/vmap.h>
+#include <xen/acpi.h>
 #include <asm/pl011-uart.h>
 #include <asm/io.h>
 
@@ -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);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        printk("\nFailed to get SPCR table \n");
+        return 1;
+    }
+
+    uart = &pl011_com;
+
+    if ( spcr->interrupt < 0 )
+    {
+        printk("pl011: Unable to retrieve the IRQ\n");
+        return -EINVAL;
+    }
+
+    uart->irq = spcr->interrupt;
+    /* trigger/polarity information is not available in spcr */
+    set_irq_type(uart->irq, ACPI_IRQ_TYPE_EDGE_BOTH);
+
+    res = pl011_uart_init(uart, spcr->serial_port.address, PAGE_SIZE);
+    if ( res < 0 )
+    {
+        printk("pl011: Unable to initialize\n");
+        return res;
+    }
+
+    /* Register with generic serial driver. */
+    serial_register_uart(SERHND_UART, &pl011_driver, uart);
+
+    return 0;
+}
+
+ACPI_DEVICE_START(apl011, "PL011 UART", DEVICE_SERIAL)
+        .class_type = ACPI_SPCR_TYPE_PL011,
+        .init = acpi_pl011_uart_init,
+ACPI_DEVICE_END
+#endif
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/acpi/actbl2.h b/xen/include/acpi/actbl2.h
index 87bc6b3..25be429 100644
--- a/xen/include/acpi/actbl2.h
+++ b/xen/include/acpi/actbl2.h
@@ -815,6 +815,11 @@  struct acpi_table_spcr {
 
 #define ACPI_SPCR_DO_NOT_DISABLE    (1)
 
+/* UART Interface type */
+#define ACPI_SPCR_TYPE_16550 0
+#define ACPI_SPCR_TYPE_16550_SUB 1
+#define ACPI_SPCR_TYPE_PL011 3
+
 /*******************************************************************************
  *
  * SPMI - Server Platform Management Interface table