diff mbox

[U-Boot,v2,8/9] dm: exynos: Move serial to driver model

Message ID 1405276062-4560-9-git-send-email-sjg@chromium.org
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Simon Glass July 13, 2014, 6:27 p.m. UTC 
Change the Exynos serial driver to work with driver model and switch over
all Exynos5 boards to use it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 drivers/serial/serial_s5p.c  | 295 ++++++++++++++-----------------------------
 include/configs/exynos5-dt.h |   1 +
 2 files changed, 93 insertions(+), 203 deletions(-)

Comments

Tom Rini July 28, 2014, 6:05 p.m. UTC | #1 
On Sun, Jul 13, 2014 at 12:27:41PM -0600, Simon Glass wrote:

> Change the Exynos serial driver to work with driver model and switch over
> all Exynos5 boards to use it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

So, maybe it's just the way the diff was generated, but did you re-order
the functions in the file?  Trying to see for example how 'set_brg'
changed here isn't easy since it used to be at the top, but is now near
the bottom?  And other functions too perhaps.

IOW, assuming the function locations changed in the driver, can you
re-shuffle things so the function order is the same?  This will help
both with review and future conversions I think.  Thanks!
Simon Glass July 28, 2014, 6:31 p.m. UTC | #2 
Hi Tom,

On 28 July 2014 19:05, Tom Rini <trini@ti.com> wrote:
> On Sun, Jul 13, 2014 at 12:27:41PM -0600, Simon Glass wrote:
>
>> Change the Exynos serial driver to work with driver model and switch over
>> all Exynos5 boards to use it.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> So, maybe it's just the way the diff was generated, but did you re-order
> the functions in the file?  Trying to see for example how 'set_brg'
> changed here isn't easy since it used to be at the top, but is now near
> the bottom?  And other functions too perhaps.
>
> IOW, assuming the function locations changed in the driver, can you
> re-shuffle things so the function order is the same?  This will help
> both with review and future conversions I think.  Thanks!

Yes I think i can improve it as you say.

But however you look at it this is essentially a rewrite. All the old
boilerplate for multi-serial is gone (DECLARE_S5P_SERIAL_FUNCTIONS and
so on). The new driver model structures are added. All the functions
change signature and quite a bit of contents. The old driver was 300
lines, the new one is 190.

Unfortunately I think this is going to be a complete pain to review
whatever I do. For the driver model conversion the serial drivers are
all going to be quite different if they currently support multi-serial
using the #define work-around.

Regards,
Simon
diff mbox

Patch

diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c
index 98c62b4..a2719e2 100644
--- a/drivers/serial/serial_s5p.c
+++ b/drivers/serial/serial_s5p.c
@@ -9,6 +9,8 @@ 
  */
 
 #include <common.h>
+#include <dm.h>
+#include <errno.h>
 #include <fdtdec.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
@@ -18,26 +20,18 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define RX_FIFO_COUNT_MASK	0xff
-#define RX_FIFO_FULL_MASK	(1 << 8)
-#define TX_FIFO_FULL_MASK	(1 << 24)
+#define RX_FIFO_COUNT_SHIFT	0
+#define RX_FIFO_COUNT_MASK	(0xff << RX_FIFO_COUNT_SHIFT)
+#define RX_FIFO_FULL		(1 << 8)
+#define TX_FIFO_COUNT_SHIFT	16
+#define TX_FIFO_COUNT_MASK	(0xff << TX_FIFO_COUNT_SHIFT)
+#define TX_FIFO_FULL		(1 << 24)
 
 /* Information about a serial port */
-struct fdt_serial {
-	u32 base_addr;  /* address of registers in physical memory */
+struct s5p_serial_platdata {
+	struct s5p_uart *reg;  /* address of registers in physical memory */
 	u8 port_id;     /* uart port number */
-	u8 enabled;     /* 1 if enabled, 0 if disabled */
-} config __attribute__ ((section(".data")));
-
-static inline struct s5p_uart *s5p_get_base_uart(int dev_index)
-{
-#ifdef CONFIG_OF_CONTROL
-	return (struct s5p_uart *)(config.base_addr);
-#else
-	u32 offset = dev_index * sizeof(struct s5p_uart);
-	return (struct s5p_uart *)(samsung_get_base_uart() + offset);
-#endif
-}
+};
 
 /*
  * The coefficient, used to calculate the baudrate on S5P UARTs is
@@ -65,57 +59,8 @@  static const int udivslot[] = {
 	0xffdf,
 };
 
-static void serial_setbrg_dev(const int dev_index)
-{
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
-	u32 uclk = get_uart_clk(dev_index);
-	u32 baudrate = gd->baudrate;
-	u32 val;
-
-#if defined(CONFIG_SILENT_CONSOLE) && \
-		defined(CONFIG_OF_CONTROL) && \
-		!defined(CONFIG_SPL_BUILD)
-	if (fdtdec_get_config_int(gd->fdt_blob, "silent_console", 0))
-		gd->flags |= GD_FLG_SILENT;
-#endif
-
-	if (!config.enabled)
-		return;
-
-	val = uclk / baudrate;
-
-	writel(val / 16 - 1, &uart->ubrdiv);
-
-	if (s5p_uart_divslot())
-		writew(udivslot[val % 16], &uart->rest.slot);
-	else
-		writeb(val % 16, &uart->rest.value);
-}
-
-/*
- * Initialise the serial port with the given baudrate. The settings
- * are always 8 data bits, no parity, 1 stop bit, no start bits.
- */
-static int serial_init_dev(const int dev_index)
+static int serial_err_check(const struct s5p_uart *const uart, int op)
 {
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
-
-	/* enable FIFOs, auto clear Rx FIFO */
-	writel(0x3, &uart->ufcon);
-	writel(0, &uart->umcon);
-	/* 8N1 */
-	writel(0x3, &uart->ulcon);
-	/* No interrupts, no DMA, pure polling */
-	writel(0x245, &uart->ucon);
-
-	serial_setbrg_dev(dev_index);
-
-	return 0;
-}
-
-static int serial_err_check(const int dev_index, int op)
-{
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
 	unsigned int mask;
 
 	/*
@@ -133,169 +78,113 @@  static int serial_err_check(const int dev_index, int op)
 	return readl(&uart->uerstat) & mask;
 }
 
-/*
- * Read a single byte from the serial port. Returns 1 on success, 0
- * otherwise. When the function is succesfull, the character read is
- * written into its argument c.
- */
-static int serial_getc_dev(const int dev_index)
+static int s5p_serial_putc(struct udevice *dev, const char ch)
 {
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
 
-	if (!config.enabled)
-		return 0;
+	if (readl(&uart->ufstat) & TX_FIFO_FULL)
+		return -EAGAIN;
 
-	/* wait for character to arrive */
-	while (!(readl(&uart->ufstat) & (RX_FIFO_COUNT_MASK |
-					 RX_FIFO_FULL_MASK))) {
-		if (serial_err_check(dev_index, 0))
-			return 0;
-	}
+	writeb(ch, &uart->utxh);
+	serial_err_check(uart, 1);
 
-	return (int)(readb(&uart->urxh) & 0xff);
+	return 0;
 }
 
-/*
- * Output a single byte to the serial port.
- */
-static void serial_putc_dev(const char c, const int dev_index)
+static int s5p_serial_pending(struct udevice *dev, bool input)
 {
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
+	uint32_t ufstat = readl(&uart->ufstat);
 
-	if (!config.enabled)
-		return;
+	if (input)
+		return (ufstat & RX_FIFO_COUNT_MASK) >> RX_FIFO_COUNT_SHIFT;
+	else
+		return (ufstat & TX_FIFO_COUNT_MASK) >> TX_FIFO_COUNT_SHIFT;
+}
 
-	/* wait for room in the tx FIFO */
-	while ((readl(&uart->ufstat) & TX_FIFO_FULL_MASK)) {
-		if (serial_err_check(dev_index, 1))
-			return;
-	}
+static int s5p_serial_getc(struct udevice *dev)
+{
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
 
-	writeb(c, &uart->utxh);
+	if (!(readl(&uart->ufstat) & RX_FIFO_COUNT_MASK))
+		return -EAGAIN;
 
-	/* If \n, also do \r */
-	if (c == '\n')
-		serial_putc('\r');
+	serial_err_check(uart, 0);
+	return (int)(readb(&uart->urxh) & 0xff);
 }
 
-/*
- * Test whether a character is in the RX buffer
- */
-static int serial_tstc_dev(const int dev_index)
+int s5p_serial_setbrg(struct udevice *dev, int baudrate)
 {
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
+	u32 uclk = get_uart_clk(plat->port_id);
+	u32 val;
 
-	if (!config.enabled)
-		return 0;
+	val = uclk / baudrate;
+
+	writel(val / 16 - 1, &uart->ubrdiv);
 
-	return (int)(readl(&uart->utrstat) & 0x1);
+	if (s5p_uart_divslot())
+		writew(udivslot[val % 16], &uart->rest.slot);
+	else
+		writeb(val % 16, &uart->rest.value);
+
+	return 0;
 }
 
-static void serial_puts_dev(const char *s, const int dev_index)
+static int s5p_serial_probe(struct udevice *dev)
 {
-	while (*s)
-		serial_putc_dev(*s++, dev_index);
-}
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
+
+	/* enable FIFOs, auto clear Rx FIFO */
+	writel(0x3, &uart->ufcon);
+	writel(0, &uart->umcon);
+	/* 8N1 */
+	writel(0x3, &uart->ulcon);
+	/* No interrupts, no DMA, pure polling */
+	writel(0x245, &uart->ucon);
 
-/* Multi serial device functions */
-#define DECLARE_S5P_SERIAL_FUNCTIONS(port) \
-static int s5p_serial##port##_init(void) { return serial_init_dev(port); } \
-static void s5p_serial##port##_setbrg(void) { serial_setbrg_dev(port); } \
-static int s5p_serial##port##_getc(void) { return serial_getc_dev(port); } \
-static int s5p_serial##port##_tstc(void) { return serial_tstc_dev(port); } \
-static void s5p_serial##port##_putc(const char c) { serial_putc_dev(c, port); } \
-static void s5p_serial##port##_puts(const char *s) { serial_puts_dev(s, port); }
-
-#define INIT_S5P_SERIAL_STRUCTURE(port, __name) {	\
-	.name	= __name,				\
-	.start	= s5p_serial##port##_init,		\
-	.stop	= NULL,					\
-	.setbrg	= s5p_serial##port##_setbrg,		\
-	.getc	= s5p_serial##port##_getc,		\
-	.tstc	= s5p_serial##port##_tstc,		\
-	.putc	= s5p_serial##port##_putc,		\
-	.puts	= s5p_serial##port##_puts,		\
+	return 0;
 }
 
-DECLARE_S5P_SERIAL_FUNCTIONS(0);
-struct serial_device s5p_serial0_device =
-	INIT_S5P_SERIAL_STRUCTURE(0, "s5pser0");
-DECLARE_S5P_SERIAL_FUNCTIONS(1);
-struct serial_device s5p_serial1_device =
-	INIT_S5P_SERIAL_STRUCTURE(1, "s5pser1");
-DECLARE_S5P_SERIAL_FUNCTIONS(2);
-struct serial_device s5p_serial2_device =
-	INIT_S5P_SERIAL_STRUCTURE(2, "s5pser2");
-DECLARE_S5P_SERIAL_FUNCTIONS(3);
-struct serial_device s5p_serial3_device =
-	INIT_S5P_SERIAL_STRUCTURE(3, "s5pser3");
-
-#ifdef CONFIG_OF_CONTROL
-int fdtdec_decode_console(int *index, struct fdt_serial *uart)
+static int s5p_serial_ofdata_to_platdata(struct udevice *dev)
 {
-	const void *blob = gd->fdt_blob;
-	int node;
+	struct s5p_serial_platdata *plat = dev->platdata;
+	fdt_addr_t addr;
 
-	node = fdt_path_offset(blob, "console");
-	if (node < 0)
-		return node;
+	addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
 
-	uart->base_addr = fdtdec_get_addr(blob, node, "reg");
-	if (uart->base_addr == FDT_ADDR_T_NONE)
-		return -FDT_ERR_NOTFOUND;
-
-	uart->port_id = fdtdec_get_int(blob, node, "id", -1);
-	uart->enabled = fdtdec_get_is_enabled(blob, node);
+	plat->reg = (struct s5p_uart *)addr;
+	plat->port_id = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "id", -1);
 
 	return 0;
 }
-#endif
 
-__weak struct serial_device *default_serial_console(void)
-{
-#ifdef CONFIG_OF_CONTROL
-	int index = 0;
-
-	if ((!config.base_addr) && (fdtdec_decode_console(&index, &config))) {
-		debug("Cannot decode default console node\n");
-		return NULL;
-	}
-
-	switch (config.port_id) {
-	case 0:
-		return &s5p_serial0_device;
-	case 1:
-		return &s5p_serial1_device;
-	case 2:
-		return &s5p_serial2_device;
-	case 3:
-		return &s5p_serial3_device;
-	default:
-		debug("Unknown config.port_id: %d", config.port_id);
-		break;
-	}
-
-	return NULL;
-#else
-	config.enabled = 1;
-#if defined(CONFIG_SERIAL0)
-	return &s5p_serial0_device;
-#elif defined(CONFIG_SERIAL1)
-	return &s5p_serial1_device;
-#elif defined(CONFIG_SERIAL2)
-	return &s5p_serial2_device;
-#elif defined(CONFIG_SERIAL3)
-	return &s5p_serial3_device;
-#else
-#error "CONFIG_SERIAL? missing."
-#endif
-#endif
-}
+static const struct dm_serial_ops s5p_serial_ops = {
+	.putc = s5p_serial_putc,
+	.pending = s5p_serial_pending,
+	.getc = s5p_serial_getc,
+	.setbrg = s5p_serial_setbrg,
+};
 
-void s5p_serial_initialize(void)
-{
-	serial_register(&s5p_serial0_device);
-	serial_register(&s5p_serial1_device);
-	serial_register(&s5p_serial2_device);
-	serial_register(&s5p_serial3_device);
-}
+static const struct udevice_id s5p_serial_ids[] = {
+	{ .compatible = "samsung,exynos4210-uart" },
+	{ }
+};
+
+U_BOOT_DRIVER(serial_s5p) = {
+	.name	= "serial_s5p",
+	.id	= UCLASS_SERIAL,
+	.of_match = s5p_serial_ids,
+	.ofdata_to_platdata = s5p_serial_ofdata_to_platdata,
+	.platdata_auto_alloc_size = sizeof(struct s5p_serial_platdata),
+	.probe = s5p_serial_probe,
+	.ops	= &s5p_serial_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+};
diff --git a/include/configs/exynos5-dt.h b/include/configs/exynos5-dt.h
index cc3a409..12b086b 100644
--- a/include/configs/exynos5-dt.h
+++ b/include/configs/exynos5-dt.h
@@ -19,6 +19,7 @@ 
 #define CONFIG_DM
 #define CONFIG_CMD_DM
 #define CONFIG_DM_GPIO
+#define CONFIG_DM_SERIAL
 
 #define CONFIG_SYS_GENERIC_BOARD
 #define CONFIG_ARCH_CPU_INIT