[PATCH 1/3] serial: uniphier: use register macros instead of structure

After all, I am not a big fan of using a structure to represent the hardware register map.
You do not need to know the entire register map.
Add only necessary register macros.
Use FIELD_PREP() instead of maintaining a pair of shift and mask.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
drivers/serial/serial_uniphier.c | 75 ++++++++++++++------------------ 1 file changed, 32 insertions(+), 43 deletions(-)
diff --git a/drivers/serial/serial_uniphier.c b/drivers/serial/serial_uniphier.c index c7f46e5598..2ffab004bd 100644 --- a/drivers/serial/serial_uniphier.c +++ b/drivers/serial/serial_uniphier.c @@ -7,6 +7,8 @@
#include <common.h> #include <dm.h> +#include <linux/bitfield.h> +#include <linux/bitops.h> #include <linux/bug.h> #include <linux/io.h> #include <linux/serial_reg.h> @@ -15,77 +17,67 @@ #include <serial.h> #include <fdtdec.h>
-/* - * Note: Register map is slightly different from that of 16550. - */ -struct uniphier_serial { - u32 rx; /* In: Receive buffer */ -#define tx rx /* Out: Transmit buffer */ - u32 ier; /* Interrupt Enable Register */ - u32 iir; /* In: Interrupt ID Register */ - u32 char_fcr; /* Charactor / FIFO Control Register */ - u32 lcr_mcr; /* Line/Modem Control Register */ -#define LCR_SHIFT 8 -#define LCR_MASK (0xff << (LCR_SHIFT)) - u32 lsr; /* In: Line Status Register */ - u32 msr; /* In: Modem Status Register */ - u32 __rsv0; - u32 __rsv1; - u32 dlr; /* Divisor Latch Register */ -}; +#define UNIPHIER_UART_REGSHIFT 2 + +#define UNIPHIER_UART_RX (0 << (UNIPHIER_UART_REGSHIFT)) +#define UNIPHIER_UART_TX UNIPHIER_UART_RX +/* bit[15:8] = CHAR, bit[7:0] = FCR */ +#define UNIPHIER_UART_CHAR_FCR (3 << (UNIPHIER_UART_REGSHIFT)) +/* bit[15:8] = LCR, bit[7:0] = MCR */ +#define UNIPHIER_UART_LCR_MCR (4 << (UNIPHIER_UART_REGSHIFT)) +#define UNIPHIER_UART_LCR_MASK GENMASK(15, 8) +#define UNIPHIER_UART_LSR (5 << (UNIPHIER_UART_REGSHIFT)) +/* Divisor Latch Register */ +#define UNIPHIER_UART_DLR (9 << (UNIPHIER_UART_REGSHIFT))
struct uniphier_serial_priv { - struct uniphier_serial __iomem *membase; + void __iomem *membase; unsigned int uartclk; };
-#define uniphier_serial_port(dev) \ - ((struct uniphier_serial_priv *)dev_get_priv(dev))->membase - static int uniphier_serial_setbrg(struct udevice *dev, int baudrate) { struct uniphier_serial_priv *priv = dev_get_priv(dev); - struct uniphier_serial __iomem *port = uniphier_serial_port(dev); - const unsigned int mode_x_div = 16; + static const unsigned int mode_x_div = 16; unsigned int divisor;
divisor = DIV_ROUND_CLOSEST(priv->uartclk, mode_x_div * baudrate);
- writel(divisor, &port->dlr); + writel(divisor, priv->membase + UNIPHIER_UART_DLR);
return 0; }
static int uniphier_serial_getc(struct udevice *dev) { - struct uniphier_serial __iomem *port = uniphier_serial_port(dev); + struct uniphier_serial_priv *priv = dev_get_priv(dev);
- if (!(readl(&port->lsr) & UART_LSR_DR)) + if (!(readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_DR)) return -EAGAIN;
- return readl(&port->rx); + return readl(priv->membase + UNIPHIER_UART_RX); }
static int uniphier_serial_putc(struct udevice *dev, const char c) { - struct uniphier_serial __iomem *port = uniphier_serial_port(dev); + struct uniphier_serial_priv *priv = dev_get_priv(dev);
- if (!(readl(&port->lsr) & UART_LSR_THRE)) + if (!(readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_THRE)) return -EAGAIN;
- writel(c, &port->tx); + writel(c, priv->membase + UNIPHIER_UART_TX);
return 0; }
static int uniphier_serial_pending(struct udevice *dev, bool input) { - struct uniphier_serial __iomem *port = uniphier_serial_port(dev); + struct uniphier_serial_priv *priv = dev_get_priv(dev);
if (input) - return readl(&port->lsr) & UART_LSR_DR; + return readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_DR; else - return !(readl(&port->lsr) & UART_LSR_THRE); + return !(readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_THRE); }
/* @@ -113,7 +105,6 @@ static const struct uniphier_serial_clk_data uniphier_serial_clk_data[] = { static int uniphier_serial_probe(struct udevice *dev) { struct uniphier_serial_priv *priv = dev_get_priv(dev); - struct uniphier_serial __iomem *port; const struct uniphier_serial_clk_data *clk_data; ofnode root_node; fdt_addr_t base; @@ -123,12 +114,10 @@ static int uniphier_serial_probe(struct udevice *dev) if (base == FDT_ADDR_T_NONE) return -EINVAL;
- port = devm_ioremap(dev, base, SZ_64); - if (!port) + priv->membase = devm_ioremap(dev, base, SZ_64); + if (!priv->membase) return -ENOMEM;
- priv->membase = port; - root_node = ofnode_path("/"); clk_data = uniphier_serial_clk_data; while (clk_data->compatible) { @@ -143,10 +132,10 @@ static int uniphier_serial_probe(struct udevice *dev)
priv->uartclk = clk_data->clk_rate;
- tmp = readl(&port->lcr_mcr); - tmp &= ~LCR_MASK; - tmp |= UART_LCR_WLEN8 << LCR_SHIFT; - writel(tmp, &port->lcr_mcr); + tmp = readl(priv->membase + UNIPHIER_UART_LCR_MCR); + tmp &= ~UNIPHIER_UART_LCR_MASK; + tmp |= FIELD_PREP(UNIPHIER_UART_LCR_MASK, UART_LCR_WLEN8); + writel(tmp, priv->membase + UNIPHIER_UART_LCR_MCR);
return 0; }

Ensure the transmitter is empty when chaining the baudrate or any hardware settings. If a character is remaining in the transmitter, the console will be garbled.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
drivers/serial/serial_uniphier.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/serial/serial_uniphier.c b/drivers/serial/serial_uniphier.c index 2ffab004bd..aaf8657ee1 100644 --- a/drivers/serial/serial_uniphier.c +++ b/drivers/serial/serial_uniphier.c @@ -43,6 +43,10 @@ static int uniphier_serial_setbrg(struct udevice *dev, int baudrate)
divisor = DIV_ROUND_CLOSEST(priv->uartclk, mode_x_div * baudrate);
+ /* flush the trasmitter before changing hw setting */ + while (!(readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_TEMT)) + ; + writel(divisor, priv->membase + UNIPHIER_UART_DLR);
return 0; @@ -132,6 +136,10 @@ static int uniphier_serial_probe(struct udevice *dev)
priv->uartclk = clk_data->clk_rate;
+ /* flush the trasmitter empty before changing hw setting */ + while (!(readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_TEMT)) + ; + tmp = readl(priv->membase + UNIPHIER_UART_LCR_MCR); tmp &= ~UNIPHIER_UART_LCR_MASK; tmp |= FIELD_PREP(UNIPHIER_UART_LCR_MASK, UART_LCR_WLEN8);

This UART controller is integrated with a FIFO. Enable it.
You can put the next character into the FIFO while the transmitter is sending out the current character. This works slightly faster.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
drivers/serial/serial_uniphier.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/serial/serial_uniphier.c b/drivers/serial/serial_uniphier.c index aaf8657ee1..ad691b66da 100644 --- a/drivers/serial/serial_uniphier.c +++ b/drivers/serial/serial_uniphier.c @@ -23,6 +23,7 @@ #define UNIPHIER_UART_TX UNIPHIER_UART_RX /* bit[15:8] = CHAR, bit[7:0] = FCR */ #define UNIPHIER_UART_CHAR_FCR (3 << (UNIPHIER_UART_REGSHIFT)) +#define UNIPHIER_UART_FCR_MASK GENMASK(7, 0) /* bit[15:8] = LCR, bit[7:0] = MCR */ #define UNIPHIER_UART_LCR_MCR (4 << (UNIPHIER_UART_REGSHIFT)) #define UNIPHIER_UART_LCR_MASK GENMASK(15, 8) @@ -140,6 +141,12 @@ static int uniphier_serial_probe(struct udevice *dev) while (!(readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_TEMT)) ;
+ /* enable FIFO */ + tmp = readl(priv->membase + UNIPHIER_UART_CHAR_FCR); + tmp &= ~UNIPHIER_UART_FCR_MASK; + tmp |= FIELD_PREP(UNIPHIER_UART_FCR_MASK, UART_FCR_ENABLE_FIFO); + writel(tmp, priv->membase + UNIPHIER_UART_CHAR_FCR); + tmp = readl(priv->membase + UNIPHIER_UART_LCR_MCR); tmp &= ~UNIPHIER_UART_LCR_MASK; tmp |= FIELD_PREP(UNIPHIER_UART_LCR_MASK, UART_LCR_WLEN8);

On Fri, Jul 10, 2020 at 1:13 AM Masahiro Yamada yamada.masahiro@socionext.com wrote:
After all, I am not a big fan of using a structure to represent the hardware register map.
You do not need to know the entire register map.
Add only necessary register macros.
Use FIELD_PREP() instead of maintaining a pair of shift and mask.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
Series, applied to u-boot-uniphier.
drivers/serial/serial_uniphier.c | 75 ++++++++++++++------------------ 1 file changed, 32 insertions(+), 43 deletions(-)
diff --git a/drivers/serial/serial_uniphier.c b/drivers/serial/serial_uniphier.c index c7f46e5598..2ffab004bd 100644 --- a/drivers/serial/serial_uniphier.c +++ b/drivers/serial/serial_uniphier.c @@ -7,6 +7,8 @@
#include <common.h> #include <dm.h> +#include <linux/bitfield.h> +#include <linux/bitops.h> #include <linux/bug.h> #include <linux/io.h> #include <linux/serial_reg.h> @@ -15,77 +17,67 @@ #include <serial.h> #include <fdtdec.h>
-/*
- Note: Register map is slightly different from that of 16550.
- */
-struct uniphier_serial {
u32 rx; /* In: Receive buffer */
-#define tx rx /* Out: Transmit buffer */
u32 ier; /* Interrupt Enable Register */
u32 iir; /* In: Interrupt ID Register */
u32 char_fcr; /* Charactor / FIFO Control Register */
u32 lcr_mcr; /* Line/Modem Control Register */
-#define LCR_SHIFT 8 -#define LCR_MASK (0xff << (LCR_SHIFT))
u32 lsr; /* In: Line Status Register */
u32 msr; /* In: Modem Status Register */
u32 __rsv0;
u32 __rsv1;
u32 dlr; /* Divisor Latch Register */
-}; +#define UNIPHIER_UART_REGSHIFT 2
+#define UNIPHIER_UART_RX (0 << (UNIPHIER_UART_REGSHIFT)) +#define UNIPHIER_UART_TX UNIPHIER_UART_RX +/* bit[15:8] = CHAR, bit[7:0] = FCR */ +#define UNIPHIER_UART_CHAR_FCR (3 << (UNIPHIER_UART_REGSHIFT)) +/* bit[15:8] = LCR, bit[7:0] = MCR */ +#define UNIPHIER_UART_LCR_MCR (4 << (UNIPHIER_UART_REGSHIFT)) +#define UNIPHIER_UART_LCR_MASK GENMASK(15, 8) +#define UNIPHIER_UART_LSR (5 << (UNIPHIER_UART_REGSHIFT)) +/* Divisor Latch Register */ +#define UNIPHIER_UART_DLR (9 << (UNIPHIER_UART_REGSHIFT))
struct uniphier_serial_priv {
struct uniphier_serial __iomem *membase;
void __iomem *membase; unsigned int uartclk;
};
-#define uniphier_serial_port(dev) \
((struct uniphier_serial_priv *)dev_get_priv(dev))->membase
static int uniphier_serial_setbrg(struct udevice *dev, int baudrate) { struct uniphier_serial_priv *priv = dev_get_priv(dev);
struct uniphier_serial __iomem *port = uniphier_serial_port(dev);
const unsigned int mode_x_div = 16;
static const unsigned int mode_x_div = 16; unsigned int divisor; divisor = DIV_ROUND_CLOSEST(priv->uartclk, mode_x_div * baudrate);
writel(divisor, &port->dlr);
writel(divisor, priv->membase + UNIPHIER_UART_DLR); return 0;
}
static int uniphier_serial_getc(struct udevice *dev) {
struct uniphier_serial __iomem *port = uniphier_serial_port(dev);
struct uniphier_serial_priv *priv = dev_get_priv(dev);
if (!(readl(&port->lsr) & UART_LSR_DR))
if (!(readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_DR)) return -EAGAIN;
return readl(&port->rx);
return readl(priv->membase + UNIPHIER_UART_RX);
}
static int uniphier_serial_putc(struct udevice *dev, const char c) {
struct uniphier_serial __iomem *port = uniphier_serial_port(dev);
struct uniphier_serial_priv *priv = dev_get_priv(dev);
if (!(readl(&port->lsr) & UART_LSR_THRE))
if (!(readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_THRE)) return -EAGAIN;
writel(c, &port->tx);
writel(c, priv->membase + UNIPHIER_UART_TX); return 0;
}
static int uniphier_serial_pending(struct udevice *dev, bool input) {
struct uniphier_serial __iomem *port = uniphier_serial_port(dev);
struct uniphier_serial_priv *priv = dev_get_priv(dev); if (input)
return readl(&port->lsr) & UART_LSR_DR;
return readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_DR; else
return !(readl(&port->lsr) & UART_LSR_THRE);
return !(readl(priv->membase + UNIPHIER_UART_LSR) & UART_LSR_THRE);
}
/* @@ -113,7 +105,6 @@ static const struct uniphier_serial_clk_data uniphier_serial_clk_data[] = { static int uniphier_serial_probe(struct udevice *dev) { struct uniphier_serial_priv *priv = dev_get_priv(dev);
struct uniphier_serial __iomem *port; const struct uniphier_serial_clk_data *clk_data; ofnode root_node; fdt_addr_t base;
@@ -123,12 +114,10 @@ static int uniphier_serial_probe(struct udevice *dev) if (base == FDT_ADDR_T_NONE) return -EINVAL;
port = devm_ioremap(dev, base, SZ_64);
if (!port)
priv->membase = devm_ioremap(dev, base, SZ_64);
if (!priv->membase) return -ENOMEM;
priv->membase = port;
root_node = ofnode_path("/"); clk_data = uniphier_serial_clk_data; while (clk_data->compatible) {
@@ -143,10 +132,10 @@ static int uniphier_serial_probe(struct udevice *dev)
priv->uartclk = clk_data->clk_rate;
tmp = readl(&port->lcr_mcr);
tmp &= ~LCR_MASK;
tmp |= UART_LCR_WLEN8 << LCR_SHIFT;
writel(tmp, &port->lcr_mcr);
tmp = readl(priv->membase + UNIPHIER_UART_LCR_MCR);
tmp &= ~UNIPHIER_UART_LCR_MASK;
tmp |= FIELD_PREP(UNIPHIER_UART_LCR_MASK, UART_LCR_WLEN8);
writel(tmp, priv->membase + UNIPHIER_UART_LCR_MCR); return 0;
}
2.25.1
participants (2)
-
Masahiro Yamada
-
Masahiro Yamada