On 2015年12月25日 14:05, Wills Wang wrote:
On 12/25/2015 10:39 AM, Thomas Chou wrote: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 will remove the code about pre-DM serial driver.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.I has taken a long time to add base device tree for this patch, but i can't get the driver serial number if don't specify explicitly "aliases" section in dts file. but It seems to not need for ARM device.+
You will need to add "stdout-path=..." property with your serial path to the chosen
node, like this,
chosen {
stdout-path = &uart;
};
+#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);In MIPS, function "map_physmem" is empty.
Yes. But you may improve it to do the real work. For example, not tested,
#define MAP_NOCACHE 1
#define MAP_WRCOMBINE 0
#define MAP_WRBACK 0
#define MAP_WRTHROUGH 0
static inline void *
map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
{
if (flags)
return (void *)KSEG1ADDR(paddr);
else
return (void *)KSEG0ADDR(paddr);
}
+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?"scale" and "step" was initialized by "if...else" statement above.+ + 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.The pinctrl and clock drvier is not ready, there is no way to operate clock and gpio resister if use address from device tree.
You may add a legacy get_serial_clock() temporarily. And put the reset/pin configuration under mach-ath79. You may improve them with pinctrl and clock drivers.
Best regards, Thomas _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
