[U-Boot] [PATCH v3 2/4] mips: ath79: add serial driver for ar933x SOC
Thomas Chou
thomas at wytron.com.tw
Fri Dec 25 03:39:04 CET 2015
Hi Wills,
Is there any reason that you need to support pre-DM serial driver? It
should be safe to support DM only. You may add debug_uart support which
might be helpful during debug. The ops for DM serial is different to
pre-DM though the name look similar. Please see the comments below.
I worked on mips before I moved to nios very long ago (20 yr). And nios2
mostly followed mips.
I understand there is not device tree control for u-boot on mips yet.
But I added it to nios2 recently. It might be worthy if you could add it
to mips. You may find the dts binding on Linux kernel. And please add
dts binding to doc/ and you might copy them from Linux kernel.
> +
> +#include <common.h>
> +#include <serial.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <asm/io.h>
> +#include <asm/addrspace.h>
> +#include <asm/types.h>
> +#include <asm/arch/ar71xx_regs.h>
> +#include <asm/arch/ar933x_uart.h>
Please sort the sequence of header files inclusion.
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct ar933x_serial_baudrate{
> + u32 baudrate;
> + u32 scale;
> + u32 step;
> +};
> +
> +const struct ar933x_serial_baudrate baudrate_table_40mhz[] = {
> +/* baudrate, scale, step */
> + {600, 255, 503},
> + {1200, 249, 983},
> + {2400, 167, 1321},
> + {4800, 87, 1384},
> + {9600, 45, 1447},
> + {14400, 53, 2548},
> + {19200, 22, 1447},
> + {28800, 26, 2548},
> + {38400, 28, 3649},
> + {56000, 7, 1468},
> + {57600, 34, 6606},
> + {115200, 28, 10947},
> + {128000, 6, 2936},
> + {153600, 18, 9563},
> + {230400, 16, 12834},
> + {250000, 4, 4096},
> + {256000, 6, 5872},
> + {460800, 7, 12079},
> + {576000, 4, 9437},
> + {921600, 3, 12079},
> + {1000000, 2, 9830},
> + {1152000, 2, 11324},
> + {1500000, 0, 4915},
> + {2000000, 0, 6553},
> + };
> +
> +const struct ar933x_serial_baudrate baudrate_table_25mhz[] = {
> +/* baudrate, scale, step */
> + {600, 255, 805},
> + {1200, 209, 1321},
> + {2400, 104, 1321},
> + {4800, 54, 1384},
> + {9600, 78, 3976},
> + {14400, 98, 7474},
> + {19200, 55, 5637},
> + {28800, 77, 11777},
> + {38400, 36, 7449},
> + {56000, 4, 1468},
> + {57600, 35, 10871},
> + {115200, 20, 12683},
> + {128000, 11, 8053},
> + {153600, 9, 8053},
> + {230400, 9, 12079},
> + {250000, 6, 9175},
> + {256000, 5, 8053},
> + {460800, 4, 12079},
> + {576000, 3, 12079},
> + {921600, 1, 9663},
> + {1000000, 1, 10485},
> + {1152000, 1, 12079},
> + {1500000, 0, 7864},
> + {2000000, 0, 10485},
> +};
The requirement of u-boot is much simpler than Linux kernel. For the
uart header, you should include only the macros that the driver really
used. Or you can add it to the driver directly without the additional
header file. The same rule applies to the baudrate tables. You need only
some practical rates, not every possible one.
> +
> +static inline u32 ar933x_read(u32 base, u32 offset)
> +{
> + return readl(KSEG1ADDR(base + offset));
> +}
> +
> +static inline void ar933x_write(u32 base, u32 offset, u32 val)
> +{
> + writel(val, KSEG1ADDR(base + offset));
> +}
> +
For the KSEG1ADDR mapping, which Marek also mentioned, I would suggest
rework map_physmem() in asm/io.h on mips to map K1 kernel space, which
is very similar to ioremap() on Linux kernel. So that you don't need to
map it for every IO.
plat->regs = map_physmem(dev_get_addr(dev),
sizeof(...),
MAP_NOCACHE);
> +static int ar933x_serial_init(void)
> +{
> + u32 val;
> +
> + /*
> + * Set GPIO10 (UART_SO) as output and enable UART,
> + * BIT(15) in GPIO_FUNCTION_1 register must be written with 1
> + */
> + val = ar933x_read(AR71XX_GPIO_BASE, AR71XX_GPIO_REG_OE);
> + val |= BIT(10);
> + ar933x_write(AR71XX_GPIO_BASE, AR71XX_GPIO_REG_OE, val);
> +
> + val = ar933x_read(AR71XX_GPIO_BASE, AR71XX_GPIO_REG_FUNC);
> + val |= (AR933X_GPIO_FUNC_UART_EN | BIT(15));
> + ar933x_write(AR71XX_GPIO_BASE, AR71XX_GPIO_REG_FUNC, val);
These might go to the pinctrl driver or board initialization.
> +
> + /*
> + * UART controller configuration:
> + * - no DMA
> + * - no interrupt
> + * - DCE mode
> + * - no flow control
> + * - set RX ready oride
> + * - set TX ready oride
> + */
> + val = AR933X_UART_CS_TX_READY_ORIDE | AR933X_UART_CS_RX_READY_ORIDE
> + | (AR933X_UART_CS_IF_MODE_DCE << AR933X_UART_CS_IF_MODE_S);
> + ar933x_write(AR933X_UART_BASE, AR933X_UART_CS_REG, val);
> + return 0;
> +}
> +
> +#ifdef CONFIG_DM_SERIAL
> +static int ar933x_serial_setbrg(struct udevice *dev, int baudrate)
> +{
> +#else
> +static void ar933x_serial_setbrg(void)
> +{
> + int baudrate = gd->baudrate;
> +#endif
> + u32 val, scale, step;
> + const struct ar933x_serial_baudrate *baudrate_table;
> + int i, baudrate_table_size;
> +
> + val = ar933x_read(AR71XX_RESET_BASE, AR933X_RESET_REG_BOOTSTRAP);
This can go to some clock rate in board info during board initialization.
> + if (val & AR933X_BOOTSTRAP_REF_CLK_40) {
> + baudrate_table = baudrate_table_40mhz;
> + baudrate_table_size = ARRAY_SIZE(baudrate_table_40mhz);
> + scale = (40000000 / (16 * baudrate)) - 1;
> + step = 8192;
> + } else {
> + baudrate_table = baudrate_table_25mhz;
> + baudrate_table_size = ARRAY_SIZE(baudrate_table_25mhz);
> + scale = (25000000 / (16 * baudrate)) - 1;
> + step = 8192;
> + }
> +
> + for (i = 0; i < baudrate_table_size; i++) {
> + if (baudrate_table[i].baudrate == gd->baudrate) {
> + scale = baudrate_table[i].scale;
> + step = baudrate_table[i].step;
> + }
> + }
What happens if not found?
> +
> + val = ((scale & AR933X_UART_CLOCK_SCALE_M)
> + << AR933X_UART_CLOCK_SCALE_S);
The outer parenthesis is not needed.
> + val |= ((step & AR933X_UART_CLOCK_STEP_M)
> + << AR933X_UART_CLOCK_STEP_S);
> + ar933x_write(AR933X_UART_BASE, AR933X_UART_CLOCK_REG, val);
> +#ifdef CONFIG_DM_SERIAL
> + return 0;
> +#endif
> +}
> +
> +#ifdef CONFIG_DM_SERIAL
> +static int ar933x_serial_putc(struct udevice *dev, const char c)
> +#else
> +static void ar933x_serial_putc(const char c)
> +#endif
> +{
> + u32 data;
> +
> + if (c == '\n')
> +#ifdef CONFIG_DM_SERIAL
> + ar933x_serial_putc(dev, '\r');
Not needed for DM serial.
> +#else
> + ar933x_serial_putc('\r');
> +#endif
> + do {
> + data = ar933x_read(AR933X_UART_BASE, AR933X_UART_DATA_REG);
> + } while (!(data & AR933X_UART_DATA_TX_CSR));
Return -EAGAIN if not ready for DM serial.
> +
> + data = (u32)c | AR933X_UART_DATA_TX_CSR;
> + ar933x_write(AR933X_UART_BASE, AR933X_UART_DATA_REG, data);
> +#ifdef CONFIG_DM_SERIAL
> + return 0;
> +#endif
> +}
> +
> +#ifdef CONFIG_DM_SERIAL
> +static int ar933x_serial_getc(struct udevice *dev)
> +#else
> +static int ar933x_serial_getc(void)
> +#endif
> +{
> + u32 data;
> +
> + do {
> + data = ar933x_read(AR933X_UART_BASE, AR933X_UART_DATA_REG);
> + } while (!(data & AR933X_UART_DATA_RX_CSR));
Return -EAGAIN if no data available for DM serial.
> +
> + data = ar933x_read(AR933X_UART_BASE, AR933X_UART_DATA_REG);
> + ar933x_write(AR933X_UART_BASE, AR933X_UART_DATA_REG, AR933X_UART_DATA_RX_CSR);
> + return data & AR933X_UART_DATA_TX_RX_MASK;
> +}
> +
> +#ifdef CONFIG_DM_SERIAL
> +static int ar933x_serial_pending(struct udevice *dev, bool input)
> +{
> + u32 data;
> +
> + data = ar933x_read(AR933X_UART_BASE, AR933X_UART_DATA_REG);
> + if (input)
> + return (data & AR933X_UART_DATA_RX_CSR) ? 1 : 0;
> + else
> + return (data & AR933X_UART_DATA_TX_CSR) ? 0 : 1;
> +}
> +
> +static int ar933x_serial_probe(struct udevice *dev)
> +{
> + ar933x_serial_init();
> + return 0;
> +}
> +
> +static const struct dm_serial_ops ar933x_serial_ops = {
> + .putc = ar933x_serial_putc,
> + .pending = ar933x_serial_pending,
> + .getc = ar933x_serial_getc,
> + .setbrg = ar933x_serial_setbrg,
> +};
> +
> +static const struct udevice_id ar933x_serial_ids[] = {
> + { .compatible = "ath79,ar933x-uart" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(serial_ar933x) = {
> + .name = "serial_ar933x",
> + .id = UCLASS_SERIAL,
> + .of_match = ar933x_serial_ids,
> + .probe = ar933x_serial_probe,
> + .ops = &ar933x_serial_ops,
> + .flags = DM_FLAG_PRE_RELOC,
> +};
I understand there might be only a single UART on the ar9331. But it
might be better to bind the reg address with U_BOOT_DEVICE() and
platform data, or better dts.
Thanks.
Best regards,
Thomas
More information about the U-Boot
mailing list