|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v6 4/6] xen/arm: Add the new OMAP UART driver.
On 13 August 2013 03:35, Chen Baozi <baozich@xxxxxxxxx> wrote:
>
> On Aug 13, 2013, at 6:21 AM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
>
>> On 9 August 2013 03:20, Chen Baozi <baozich@xxxxxxxxx> wrote:
>>> TI OMAP UART introduces some features such as register access modes, which
>>> makes its configuration and interrupt handling differs from 8250 compatible
>>> UART. Thus, we seperate this driver from ns16550's implementation.
>>>
>>> Signed-off-by: Chen Baozi <baozich@xxxxxxxxx>
>>> ---
>>> config/arm32.mk | 1 +
>>> xen/drivers/char/Makefile | 1 +
>>> xen/drivers/char/omap-uart.c | 354
>>> +++++++++++++++++++++++++++++++++++++++++++
>>> xen/include/xen/8250-uart.h | 51 +++++++
>>> 4 files changed, 407 insertions(+)
>>> create mode 100644 xen/drivers/char/omap-uart.c
>>>
>>> diff --git a/config/arm32.mk b/config/arm32.mk
>>> index 8b9899e..ef31cff 100644
>>> --- a/config/arm32.mk
>>> +++ b/config/arm32.mk
>>> @@ -11,6 +11,7 @@ CFLAGS += -marm
>>>
>>> HAS_PL011 := y
>>> HAS_EXYNOS4210 := y
>>> +HAS_OMAP := y
>>>
>>> # Use only if calling $(LD) directly.
>>> LDFLAGS_DIRECT += -EL
>>> diff --git a/xen/drivers/char/Makefile b/xen/drivers/char/Makefile
>>> index 37543f0..911b788 100644
>>> --- a/xen/drivers/char/Makefile
>>> +++ b/xen/drivers/char/Makefile
>>> @@ -2,6 +2,7 @@ obj-y += console.o
>>> obj-$(HAS_NS16550) += ns16550.o
>>> obj-$(HAS_PL011) += pl011.o
>>> obj-$(HAS_EXYNOS4210) += exynos4210-uart.o
>>> +obj-$(HAS_OMAP) += omap-uart.o
>>> obj-$(HAS_EHCI) += ehci-dbgp.o
>>> obj-$(CONFIG_ARM) += dt-uart.o
>>> obj-y += serial.o
>>> diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
>>> new file mode 100644
>>> index 0000000..11cb74b
>>> --- /dev/null
>>> +++ b/xen/drivers/char/omap-uart.c
>>> @@ -0,0 +1,354 @@
>>> +/*
>>> + * omap-uart.c
>>> + * Based on drivers/char/ns16550.c
>>> + *
>>> + * Driver for OMAP-UART controller
>>> + *
>>> + * Copyright (C) 2013, Chen Baozi <baozich@xxxxxxxxx>
>>> + *
>>> + * Note: This driver is made separate from 16550-series UART driver as
>>> + * omap platform has some specific configurations
>>> + */
>>> +
>>> +#include <xen/config.h>
>>> +#include <xen/console.h>
>>> +#include <xen/serial.h>
>>> +#include <xen/init.h>
>>> +#include <xen/irq.h>
>>> +#include <asm/early_printk.h>
>>> +#include <xen/device_tree.h>
>>> +#include <asm/device.h>
>>> +#include <xen/errno.h>
>>> +#include <xen/mm.h>
>>> +#include <xen/vmap.h>
>>> +#include <xen/8250-uart.h>
>>> +
>>> +#define REG_SHIFT 2
>>> +
>>> +#define omap_read(uart, off) ioreadl((uart)->regs + (off<<REG_SHIFT))
>>> +#define omap_write(uart, off, val) iowritel((uart)->regs +
>>> (off<<REG_SHIFT), (val))
>>> +
>>> +static struct omap_uart {
>>> + u32 baud, clock_hz, data_bits, parity, stop_bits, fifo_size;
>>> + struct dt_irq irq;
>>> + void __iomem *regs;
>>
>> I have noticed that I also use void * on the other drivers, but it's a
>> gcc extension
>> (http://gcc.gnu.org/onlinedocs/gcc-4.8.0/gcc/Pointer-Arith.html).
>> It's better to use unsigned char *.
>>
>>> + struct irqaction irqaction;
>>> +} omap_com = {0};
>>> +
>>> +static void omap_uart_interrupt(int irq, void *data, struct cpu_user_regs
>>> *regs)
>>> +{
>>> + struct serial_port *port = data;
>>> + struct omap_uart *uart = port->uart;
>>> + u32 lsr;
>>> +
>>> + while ( !(omap_read(uart, UART_IIR) & UART_IIR_NOINT) )
>>> + {
>>> + lsr = omap_read(uart, UART_LSR) & 0xff;
>>> + if ( lsr & UART_LSR_THRE )
>>> + serial_tx_interrupt(port, regs);
>>> + if ( lsr & UART_LSR_DR )
>>> + serial_rx_interrupt(port, regs);
>>> +
>>> + if ( port->txbufc == port->txbufp )
>>> + omap_write(uart, UART_IER, UART_IER_ERDAI|UART_IER_ELSI);
>>> + };
>>> +}
>>> +
>>> +static void baud_protocol_setup(struct omap_uart *uart)
>>> +{
>>> + u32 dll, dlh, efr;
>>> + unsigned int divisor;
>>> +
>>> + divisor = uart->clock_hz / (uart->baud << 4);
>>> + dll = divisor & 0xff;
>>> + dlh = divisor >> 8;
>>> +
>>> + /*
>>> + * Switch to register configuration mode B to access the UART_OMAP_EFR
>>> + * register.
>>> + */
>>> + omap_write(uart, UART_LCR, UART_LCR_CONF_MODE_B);
>>> + /*
>>> + * Enable access to the UART_IER[7:4] bit field.
>>> + */
>>> + efr = omap_read(uart, UART_OMAP_EFR);
>>> + omap_write(uart, UART_OMAP_EFR, efr|UART_OMAP_EFR_ECB);
>>> + /*
>>> + * Switch to register operation mode to access the UART_IER register.
>>> + */
>>> + omap_write(uart, UART_LCR, 0);
>>> + /*
>>> + * Clear the UART_IER register (set the UART_IER[4] SLEEP_MODE bit
>>> + * to 0 to change the UART_DLL and UART_DLM register). Set the
->>> + * UART_IER register value to 0x0000.
>>> + */
>>> + omap_write(uart, UART_IER, 0);
>>> + /*
>>> + * Switch to register configuartion mode B to access the UART_DLL and
>>> + * UART_DLM registers.
>>> + */
>>> + omap_write(uart, UART_LCR, UART_LCR_CONF_MODE_B);
>>> + /*
>>> + * Load divisor value.
>>> + */
>>> + omap_write(uart, UART_DLL, dll);
>>> + omap_write(uart, UART_DLM, dlh);
>>> + /*
>>> + * Restore the UART_OMAP_EFR
>>> + */
>>> + omap_write(uart, UART_OMAP_EFR, efr);
>>> + /*
>>> + * Load the new protocol formatting (parity, stop-bit, character
>>> length)
>>> + * and switch to register operational mode.
>>> + */
>>> + omap_write(uart, UART_LCR, (uart->data_bits - 5) |
>>> + ((uart->stop_bits - 1) << 2) | uart->parity);
>>> +}
>>> +
>>> +static void fifo_setup(struct omap_uart *uart)
>>> +{
>>> + u32 lcr, efr, mcr;
>>> + /*
>>> + * Switch to register configuration mode B to access the UART_OMAP_EFR
>>> + * register.
>>> + */
>>> + lcr = omap_read(uart, UART_LCR);
>>> + omap_write(uart, UART_LCR, UART_LCR_CONF_MODE_B);
>>> + /*
>>> + * Enable register submode TCR_TLR to access the UART_OMAP_TLR
>>> register.
>>> + */
>>> + efr = omap_read(uart, UART_OMAP_EFR);
>>> + omap_write(uart, UART_OMAP_EFR, efr|UART_OMAP_EFR_ECB);
>>> + /*
>>> + * Switch to register configuration mode A to access the UART_MCR
>>> + * register.
>>> + */
>>> + omap_write(uart, UART_LCR, UART_LCR_CONF_MODE_A);
>>> + /*
>>> + * Enable register submode TCR_TLR to access the UART_OMAP_TLR register
>>> + */
>>> + mcr = omap_read(uart, UART_MCR);
>>> + omap_write(uart, UART_MCR, mcr|UART_MCR_TCRTLR);
>>> + /*
>>> + * Enable the FIFO; load the new FIFO trigger and the new DMA mode.
>>> + */
>>> + omap_write(uart, UART_FCR, UART_FCR_R_TRIG_01|
>>> + UART_FCR_T_TRIG_10|UART_FCR_ENABLE);
>>> + /*
>>> + * Switch to register configuration mode B to access the UART_EFR
>>> + * register.
>>> + */
>>> + omap_write(uart, UART_LCR, UART_LCR_CONF_MODE_B);
>>> + /*
>>> + * Load the new FIFO triggers and the new DMA mode bit.
>>> + */
>>> + omap_write(uart, UART_OMAP_SCR, OMAP_UART_SCR_RX_TRIG_GRANU1_MASK);
>>> + /*
>>> + * Restore the UART_OMAP_EFR[4] value.
>>> + */
>>> + omap_write(uart, UART_OMAP_EFR, efr);
>>> + /*
>>> + * Switch to register configuration mode A to access the UART_MCR
>>> + * register.
>>> + */
>>> + omap_write(uart, UART_LCR, UART_LCR_CONF_MODE_A);
>>> + /*
>>> + * Restore UART_MCR[6] value.
>>> + */
>>> + omap_write(uart, UART_MCR, mcr);
>>> + /*
>>> + * Restore UART_LCR value.
>>> + */
>>> + omap_write(uart, UART_LCR, lcr);
>>> +
>>> + uart->fifo_size = 64;
>>> +}
>>> +
>>> +static void __init omap_uart_init_preirq(struct serial_port *port)
>>> +{
>>> + struct omap_uart *uart = port->uart;
>>> +
>>> + /*
>>> + * Clear the FIFO buffers.
>>> + */
>>> + omap_write(uart, UART_FCR, UART_FCR_ENABLE);
>>> + omap_write(uart, UART_FCR,
>>> UART_FCR_ENABLE|UART_FCR_CLRX|UART_FCR_CLTX);
>>> + omap_write(uart, UART_FCR, 0);
>>> +
>>> + /*
>>> + * The TRM says the mode should be disabled while UART_DLL and UART_DHL
>>> + * are being changed so we disable before setup, then enable.
>>> + */
>>> + omap_write(uart, UART_OMAP_MDR1, UART_OMAP_MDR1_DISABLE);
>>> +
>>> + /* Baud rate & protocol format setup */
>>> + baud_protocol_setup(uart);
>>> +
>>> + /* FIFO setup */
>>> + fifo_setup(uart);
>>> +
>>> + /* No flow control */
>>> + omap_write(uart, UART_MCR, UART_MCR_DTR|UART_MCR_RTS);
>>> +
>>> + omap_write(uart, UART_OMAP_MDR1, UART_OMAP_MDR1_16X_MODE);
>>> +}
>>> +
>>> +static void __init omap_uart_init_postirq(struct serial_port *port)
>>> +{
>>> + struct omap_uart *uart = port->uart;
>>> +
>>> + uart->irqaction.handler = omap_uart_interrupt;
>>> + uart->irqaction.name = "omap_uart";
>>> + uart->irqaction.dev_id = port;
>>> +
>>> + if ( setup_dt_irq(&uart->irq, &uart->irqaction) != 0 )
>>> + {
>>> + dprintk(XENLOG_ERR, "Failed to allocated omap_uart IRQ %d\n",
>>> + uart->irq.irq);
>>> + return;
>>> + }
>>> +
>>> + /* Enable interrupts */
>>> + omap_write(uart, UART_IER,
>>> UART_IER_ERDAI|UART_IER_ETHREI|UART_IER_ELSI);
>>> +}
>>> +
>>> +static void omap_uart_suspend(struct serial_port *port)
>>> +{
>>> + BUG();
>>> +}
>>> +
>>> +static void omap_uart_resume(struct serial_port *port)
>>> +{
>>> + BUG();
>>> +}
>>> +
>>> +static unsigned int omap_uart_tx_ready(struct serial_port *port)
>>> +{
>>> + struct omap_uart *uart = port->uart;
>>> +
>>> + omap_write(uart, UART_IER,
>>> UART_IER_ERDAI|UART_IER_ETHREI|UART_IER_ELSI);
>>
>> Why do you need to enable unmask interrupt here? Xen is just asking if
>> the UART is ready to transmit another character.
>
> Well, it is because we need to mask THRE interrupt after tx completes and
> unmake it
> once it starts to tx. This is one of main differences between OMAP UART and
> common
> 8250, since the THRE interrupt won't be cleared by writing to THR or reading
> IIR
> in OMAP UART.
>
> In Linux, there are .start_tx and .stop_tx callbacks for UART driver. Since we
> don't have that mechanism on Xen, I added it here.
If you need to only enable THRE interrupt, please only enable THRE interrupt.
You can't assume that *_ERDAI, *_ELSI are already set.
I think something like that is better:
reg = omap_read(uart, UART_IER);
omap_write(uart, UART_IER, reg | UART_IER);
This comment is the same for every place you play with the interrupt mask.
>>> +static const char const *omap_uart_dt_compat[] __initdata =
>>
>> const char * const.
>
> Hmmm, I just followed exynos4210-uart.c and pl011.c here...
Actually, this is an "issue" in the other drivers. const char * has the
same meaning as char const *. So one of the const was useless.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |