[U-Boot] [PATCH v1 05/18] drivers: serial: add driver for Microchip PIC32 UART controller.

[Purna Chandra Mandal]

On 12/17/2015 11:22 PM, Marek Vasut wrote:
> On Thursday, December 17, 2015 at 06:29:32 PM, Purna Chandra Mandal wrote:
>
> Hi!
>
> Minor nits below.
>
> btw do we expect MIPS to become maintained in U-Boot? That's nice :)
>
> [...]
>
>> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
>> index dd87147..57cd38b 100644
>> --- a/drivers/serial/Makefile
>> +++ b/drivers/serial/Makefile
>> @@ -41,6 +41,7 @@ obj-$(CONFIG_MXS_AUART) += mxs_auart.o
>>  obj-$(CONFIG_ARC_SERIAL) += serial_arc.o
>>  obj-$(CONFIG_UNIPHIER_SERIAL) += serial_uniphier.o
>>  obj-$(CONFIG_STM32_SERIAL) += serial_stm32.o
>> +obj-$(CONFIG_PIC32_SERIAL) += serial_pic32.o
>>
>>  ifndef CONFIG_SPL_BUILD
>>  obj-$(CONFIG_USB_TTY) += usbtty.o
>> diff --git a/drivers/serial/serial_pic32.c b/drivers/serial/serial_pic32.c
>> new file mode 100644
>> index 0000000..01c62e7
>> --- /dev/null
>> +++ b/drivers/serial/serial_pic32.c
>> @@ -0,0 +1,220 @@
>> +/*
>> + * (c) 2015 Paul Thacker <paul.thacker at microchip.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + *
>> + */
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <clk.h>
>> +#include <errno.h>
>> +#include <config.h>
>> +#include <serial.h>
>> +#include <linux/bitops.h>
>> +#include <common.h>
>> +#include <asm/io.h>
>> +#include <asm/arch-pic32/pic32.h>
>> +#include <asm/arch-pic32/clock.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#define UART_ENABLE        BIT(15)
>> +#define UART_ENABLE_RX        BIT(12)
>> +#define UART_ENABLE_TX        BIT(10)
>> +#define UART_RX_DATA_AVAIL    BIT(0)
>> +#define UART_RX_OERR        BIT(1)
>> +#define UART_TX_FULL        BIT(9)
>> +
>> +/* UART Control */
>> +#define U_BASE(x)    (x)
>> +#define U_MODE(x)       U_BASE(x)
>> +#define U_MODECLR(x)    (U_MODE(x) + _CLR_OFFSET)
>> +#define U_MODESET(x)    (U_MODE(x) + _SET_OFFSET)
>> +#define U_STA(x)        (U_BASE(x) + 0x10)
>> +#define U_STACLR(x)     (U_STA(x) + _CLR_OFFSET)
>> +#define U_STASET(x)     (U_STA(x) + _SET_OFFSET)
>> +#define U_TXREG(x)      (U_BASE(x) + 0x20)
>> +#define U_RXREG(x)      (U_BASE(x) + 0x30)
>> +#define U_BRG(x)        (U_BASE(x) + 0x40)
> Why don't you just use uart_priv->regs + PIC32_REGISTER_OFFSET in the code?
> The U_BASE is redundant and so is UMODE.

ack. Will update.

>> +struct pic32_uart_priv {
>> +    void __iomem *regs;
>> +    ulong uartclk;
>> +};
>> +
>> +static void pic32_serial_setbrg(void __iomem *regs, ulong uart_clk, u32
>> baud) +{
>> +    writel(0, U_BRG(regs));
>> +    writel((uart_clk / baud / 16) - 1, U_BRG(regs));
>> +    udelay(100);
>> +}
>> +
>> +/*
>> + * Initialize the serial port with the given baudrate.
>> + * The settings are always 8 data bits, no parity, 1 stop bit, no start
>> bits. + */
>> +static int pic32_serial_init(void __iomem *regs, ulong clk, u32 baudrate)
>> +{
>> +    /* disable and clear mode */
>> +    writel(0, U_MODE(regs));
>> +    writel(0, U_STA(regs));
>> +
>> +    /* set baud rate generator */
>> +    pic32_serial_setbrg(regs, clk, baudrate);
>> +
>> +    /* enable the UART for TX and RX */
>> +    writel(UART_ENABLE_TX | UART_ENABLE_RX, U_STASET(regs));
>> +
>> +    /* enable the UART */
>> +    writel(UART_ENABLE, U_MODESET(regs));
>> +    return 0;
>> +}
>> +
>> +/* Output a single byte to the serial port */
>> +static void pic32_serial_putc(void __iomem *regs, const char c)
>> +{
>> +    /* if \n, then add a \r */
>> +    if (c == '\n')
>> +        pic32_serial_putc(regs, '\r');
>> +
>> +    /* Wait for Tx FIFO not full */
>> +    while (readl(U_STA(regs)) & UART_TX_FULL)
>> +        ;
>> +
>> +    /* stuff the tx buffer with the character */
>> +    writel(c, U_TXREG(regs));
>> +}
>> +
>> +/* Test whether a character is in the RX buffer */
>> +static int pic32_serial_tstc(void __iomem *regs)
>> +{
>> +    /* check if rcv buf overrun error has occurred */
>> +    if (readl(U_STA(regs)) & UART_RX_OERR) {
>> +        readl(U_RXREG(regs));
>> +
>> +        /* clear OERR to keep receiving */
>> +        writel(UART_RX_OERR, U_STACLR(regs));
>> +    }
>> +
>> +    if (readl(U_STA(regs)) & UART_RX_DATA_AVAIL)
>> +        return 1;    /* yes, there is data in rcv buffer */
>> +    else
>> +        return 0;    /* no data in rcv buffer */
> return readl() & UART_RX_DATA_AVAIL; is sufficient here.

ack.

>> +}
>> +
>> +/*
>> + * Read a single byte from the rx buffer.
>> + * Blocking: waits until a character is received, then returns.
>> + * Return the character read directly from the UART's receive register.
>> + *
>> + */
>> +static int pic32_serial_getc(void __iomem *regs)
>> +{
>> +    /* wait here until data is available */
>> +    while (!pic32_serial_tstc(regs))
>> +        ;
>> +
>> +    /* read the character from the rcv buffer */
>> +    return readl(U_RXREG(regs));
> return readl() & 0xff, since the return value is a signed integer.

ack.

>> +}
> [...]
>
>> +U_BOOT_DRIVER(pic32_serial) = {
>> +    .name        = "pic32-uart",
>> +    .id        = UCLASS_SERIAL,
>> +    .of_match    = pic32_uart_ids,
>> +    .probe        = pic32_uart_probe,
>> +    .ops        = &pic32_uart_ops,
>> +    .flags        = DM_FLAG_PRE_RELOC,
>> +    .ofdata_to_platdata = pic32_uart_ofdata_to_platdata,
>> +    .platdata_auto_alloc_size = sizeof(struct pic32_uart_priv),
> Is there some problem with tab/space conversion going on in here?

ack. will fix.

>> +};
>> +
> [...]