diff mbox series

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

Message ID 20200709161208.223814-1-yamada.masahiro@socionext.com
State Accepted
Commit 70434ab9de795b205608067d9a5e067f30ad3629
Headers show
Series [1/3] serial: uniphier: use register macros instead of structure | expand

Commit Message

Masahiro Yamada July 9, 2020, 4:12 p.m. UTC
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 at socionext.com>
---

 drivers/serial/serial_uniphier.c | 75 ++++++++++++++------------------
 1 file changed, 32 insertions(+), 43 deletions(-)

Comments

Masahiro Yamada July 11, 2020, 2:12 p.m. UTC | #1
On Fri, Jul 10, 2020 at 1:13 AM Masahiro Yamada
<yamada.masahiro at 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 at 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
>
diff mbox series

Patch

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;
 }