On Fri, Jul 10, 2020 at 1:13 AM Masahiro Yamada
<yamada.masah...@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.masah...@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
>
--
Best Regards
Masahiro Yamada
