Hello Shanker,
On 01/06/16 16:14, Shanker Donthineni wrote:
On 06/01/2016 08:49 AM, Julien Grall wrote:
On 31/05/16 15:02, Shanker Donthineni wrote:
The ARM Server Base System Architecture describes a generic UART
interface. It doesn't support clock control registers, modem
control, DMA and hardware flow control features. So, extend the
driver probe() to handle SBSA interface and skip the accessing
PL011 registers that are not described in SBSA document.
Please mention the version of the spec in the commit message.
Sure, I'll do.
Signed-off-by: Shanker Donthineni <shank...@codeaurora.org>
---
Changes since v1:
Don't access UART registers that are not part of SBSA document.
Move setting baudrate function to a separate function.
xen/drivers/char/pl011.c | 56
+++++++++++++++++++++++++++++++++++-------------
1 file changed, 41 insertions(+), 15 deletions(-)
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 1212d5c..b57f3b0 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -41,6 +41,7 @@ static struct pl011 {
/* struct timer timer; */
/* unsigned int timeout_ms; */
/* bool_t probing, intr_works; */
+ bool sbsa; /* ARM SBSA generic interface */
} pl011_com = {0};
/* These parity settings can be ORed directly into the LCR. */
@@ -81,17 +82,10 @@ static void pl011_interrupt(int irq, void *data,
struct cpu_user_regs *regs)
}
}
-static void __init pl011_init_preirq(struct serial_port *port)
+static void __init pl011_init_baudrate(struct serial_port *port)
{
struct pl011 *uart = port->uart;
unsigned int divisor;
- unsigned int cr;
-
- /* No interrupts, please. */
- pl011_write(uart, IMSC, 0);
-
- /* Definitely no DMA */
- pl011_write(uart, DMACR, 0x0);
/* Line control and baud-rate generator. */
if ( uart->baud != BAUD_AUTO )
@@ -114,6 +108,24 @@ static void __init pl011_init_preirq(struct
serial_port *port)
| FEN
| ((uart->stop_bits - 1) << 3)
| uart->parity);
+}
As mentioned on the previous version, the code to set/read the baudrate is just
wrong. The clock frequency is hardcoded rather than read from the firmware.
However, the baudrate is always set to BAUD_AUTO for this driver, and never
used after. So all this code should be dropped.
SPCR (non-SBSA) spec has baudrate, stop and parity information, I think we
should support setting the correct baudrate according to SPCR table instead of
removing the code.
I need your opinion on this.
Which is not useful because we don't have the clock frequency in hand to
compute the divisor.
For ACPI, it just not available in the SPCR. For Device-Tree, we would
need to parse the list of clocks which could be cumbersome.
I think it is fine to expect the firmware to configure the UART baudrate
properly if required.
@@ -315,6 +331,7 @@ static int __init pl011_acpi_uart_init(const void
*data)
{
acpi_status status;
struct acpi_table_spcr *spcr = NULL;
+ bool sbsa;
int res;
status = acpi_get_table(ACPI_SIG_SPCR, 0,
@@ -326,17 +343,21 @@ static int __init pl011_acpi_uart_init(const void
*data)
return -EINVAL;
}
+ sbsa = (spcr->interface_type == ACPI_DBG2_SBSA_32) ? true : false;
sbsa = (spcr->interface_type == ACPI_DBG2_SBSA_32);
However, can you explain why you kept ACPI_DBG2_SBSA_32 and not ACPI_DBG2_SBSA?
The former is deprecated whilst the latter is the official one.
Qualcomm Technologies QDF2XXX ARM SBSA serial port hardware require registers
access should be 32-bit, so our firmware sets interface type to
ACPI_DBG2_SBSA_32.
I would like to support both the interfaces (ACPI_DBG2_SBSA_32/ACPI_DBG2_SBSA)
If you are okay.
I am a bit puzzled, so your UART is only supporting 32-bit access (i.e
no 16-bit and 8-bit access)?
If so your platform is based on SBSA v2.3, and therefore the PL011
driver needs more modification to support properly your platform. For
instance, the register UARTMIS is not present in v2.3 but used by the
driver.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
