| Message ID | 20200406230855.13772-1-hyunki00.koo@samsung.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
On 07. 04. 20, 1:08, Hyunki Koo wrote: > Support 32-bit access for the TX/RX hold registers UTXH and URXH. > > This is required for some newer SoCs. > > Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com> ... > --- > drivers/tty/serial/samsung_tty.c | 76 +++++++++++++++++++++++++++++++++------- > 1 file changed, 64 insertions(+), 12 deletions(-) > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index 73f951d65b93..bdf1d4d12cb1 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -154,12 +154,47 @@ struct s3c24xx_uart_port { ... > -#define wr_regb(port, reg, val) writeb_relaxed(val, portaddr(port, reg)) > +static void wr_reg(struct uart_port *port, u32 reg, u32 val) > +{ > + switch (port->iotype) { > + case UPIO_MEM: > + writeb_relaxed(val, portaddr(port, reg)); > + break; > + case UPIO_MEM32: > + writel_relaxed(val, portaddr(port, reg)); > + break; > + } > +} > + > #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg)) > > +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val) You need to explain, why you need this _barrier variant now. This change should be done in a separate patch too. > +{ > + switch (port->iotype) { > + case UPIO_MEM: > + writeb(val, portaddr(port, reg)); > + break; > + case UPIO_MEM32: > + writel(val, portaddr(port, reg)); > + break; > + } > +} > + > /* Byte-order aware bit setting/clearing functions. */ > > static inline void s3c24xx_set_bit(struct uart_port *port, int idx, thanks,
On 07. 04. 20, 1:49, Jiri Slaby wrote: > On 07. 04. 20, 1:08, Hyunki Koo wrote: > > Support 32-bit access for the TX/RX hold registers UTXH and URXH. > > > > This is required for some newer SoCs. > > > > Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com> > ... > > --- > > drivers/tty/serial/samsung_tty.c | 76 > > +++++++++++++++++++++++++++++++++------- > > 1 file changed, 64 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/tty/serial/samsung_tty.c > > b/drivers/tty/serial/samsung_tty.c > > index 73f951d65b93..bdf1d4d12cb1 100644 > > --- a/drivers/tty/serial/samsung_tty.c > > +++ b/drivers/tty/serial/samsung_tty.c > > @@ -154,12 +154,47 @@ struct s3c24xx_uart_port { > ... > > -#define wr_regb(port, reg, val) writeb_relaxed(val, portaddr(port, > > reg)) > > +static void wr_reg(struct uart_port *port, u32 reg, u32 val) { > > + switch (port->iotype) { > > + case UPIO_MEM: > > + writeb_relaxed(val, portaddr(port, reg)); > > + break; > > + case UPIO_MEM32: > > + writel_relaxed(val, portaddr(port, reg)); > > + break; > > + } > > +} > > + > > #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, > > reg)) > > > > +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val) > > You need to explain, why you need this _barrier variant now. This change > should be done in a separate patch too. > > > +{ > > + switch (port->iotype) { > > + case UPIO_MEM: > > + writeb(val, portaddr(port, reg)); > > + break; > > + case UPIO_MEM32: > > + writel(val, portaddr(port, reg)); > > + break; > > + } > > +} > > + > > /* Byte-order aware bit setting/clearing functions. */ > > > > static inline void s3c24xx_set_bit(struct uart_port *port, int idx, > > thanks, > -- > js > suse labs The purpose of this patch is to support 32bit access for registers, and it is also working exsisting device. There are 3 operations what I have to to change which are rd_regb, wr_regb, and writeb. rd_regb, wr_regb are changed to rd_reg, wr_reg. and writeb is changed to wr_reg_barrier. So I make as a one patch. wr_reg_barrier is not a different patch, itis just replaced from writeb.
On Tue, Apr 07, 2020 at 06:49:29AM +0200, Jiri Slaby wrote: > On 07. 04. 20, 1:08, Hyunki Koo wrote: > > Support 32-bit access for the TX/RX hold registers UTXH and URXH. > > > > This is required for some newer SoCs. > > > > Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com> > ... > > --- > > drivers/tty/serial/samsung_tty.c | 76 +++++++++++++++++++++++++++++++++------- > > 1 file changed, 64 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > > index 73f951d65b93..bdf1d4d12cb1 100644 > > --- a/drivers/tty/serial/samsung_tty.c > > +++ b/drivers/tty/serial/samsung_tty.c > > @@ -154,12 +154,47 @@ struct s3c24xx_uart_port { > ... > > -#define wr_regb(port, reg, val) writeb_relaxed(val, portaddr(port, reg)) > > +static void wr_reg(struct uart_port *port, u32 reg, u32 val) > > +{ > > + switch (port->iotype) { > > + case UPIO_MEM: > > + writeb_relaxed(val, portaddr(port, reg)); > > + break; > > + case UPIO_MEM32: > > + writel_relaxed(val, portaddr(port, reg)); > > + break; > > + } > > +} > > + > > #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg)) > > > > +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val) > > You need to explain, why you need this _barrier variant now. This change > should be done in a separate patch too. There is no functional change in regard of barrier. The ordered IO was used there before. Best regards, Krzysztof
On Tue, Apr 07, 2020 at 08:08:49AM +0900, Hyunki Koo wrote: > Support 32-bit access for the TX/RX hold registers UTXH and URXH. > > This is required for some newer SoCs. > > Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com> > --- Why I am adding these for the third time? Tested-by: Krzysztof Kozlowski <krzk@kernel.org> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
On 07. 04. 20, 8:26, Krzysztof Kozlowski wrote: > On Tue, Apr 07, 2020 at 08:08:49AM +0900, Hyunki Koo wrote: >> Support 32-bit access for the TX/RX hold registers UTXH and URXH. >> >> This is required for some newer SoCs. >> >> Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com> >> --- > > Why I am adding these for the third time? I don't know as I don't care about your tags anyway. > Tested-by: Krzysztof Kozlowski <krzk@kernel.org> > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
On 07. 04. 20, 8:24, Krzysztof Kozlowski wrote: > On Tue, Apr 07, 2020 at 06:49:29AM +0200, Jiri Slaby wrote: >> On 07. 04. 20, 1:08, Hyunki Koo wrote: >>> Support 32-bit access for the TX/RX hold registers UTXH and URXH. >>> >>> This is required for some newer SoCs. >>> >>> Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com> >> ... >>> --- >>> drivers/tty/serial/samsung_tty.c | 76 +++++++++++++++++++++++++++++++++------- >>> 1 file changed, 64 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c >>> index 73f951d65b93..bdf1d4d12cb1 100644 >>> --- a/drivers/tty/serial/samsung_tty.c >>> +++ b/drivers/tty/serial/samsung_tty.c >>> @@ -154,12 +154,47 @@ struct s3c24xx_uart_port { >> ... >>> -#define wr_regb(port, reg, val) writeb_relaxed(val, portaddr(port, reg)) >>> +static void wr_reg(struct uart_port *port, u32 reg, u32 val) >>> +{ >>> + switch (port->iotype) { >>> + case UPIO_MEM: >>> + writeb_relaxed(val, portaddr(port, reg)); >>> + break; >>> + case UPIO_MEM32: >>> + writel_relaxed(val, portaddr(port, reg)); >>> + break; >>> + } >>> +} >>> + >>> #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg)) >>> >>> +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val) >> >> You need to explain, why you need this _barrier variant now. This change >> should be done in a separate patch too. > > There is no functional change in regard of barrier. The ordered IO was > used there before. The patch changes one wr_reg to wr_reg_barrier without any explanation. This will hardly be accepted. thanks,
On 07. 04. 20, 8:28, Jiri Slaby wrote: > On 07. 04. 20, 8:26, Krzysztof Kozlowski wrote: >> On Tue, Apr 07, 2020 at 08:08:49AM +0900, Hyunki Koo wrote: >>> Support 32-bit access for the TX/RX hold registers UTXH and URXH. >>> >>> This is required for some newer SoCs. >>> >>> Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com> >>> --- >> >> Why I am adding these for the third time? > > I don't know as I don't care about your tags anyway. Sorry, my bad, I was somehow mislead by thunderbird, thinking I am replying to a different thread. sorry,
On Tue, Apr 07, 2020 at 3:27 :00PM +0900, Krzysztof Kozlowski wrote: > On Tue, Apr 07, 2020 at 08:08:49AM +0900, Hyunki Koo wrote: > > Support 32-bit access for the TX/RX hold registers UTXH and URXH. > > > > This is required for some newer SoCs. > > > > Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com> > > --- > > Why I am adding these for the third time? Sorry, I didn't knew that, I will keep this next time > > Tested-by: Krzysztof Kozlowski <krzk@kernel.org> > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> > > Best regards, > Krzysztof
On Tue, Apr 07, 2020 at 08:32:56AM +0200, Jiri Slaby wrote: > On 07. 04. 20, 8:24, Krzysztof Kozlowski wrote: > > On Tue, Apr 07, 2020 at 06:49:29AM +0200, Jiri Slaby wrote: > >> On 07. 04. 20, 1:08, Hyunki Koo wrote: > >>> Support 32-bit access for the TX/RX hold registers UTXH and URXH. > >>> > >>> This is required for some newer SoCs. > >>> > >>> Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com> > >> ... > >>> --- > >>> drivers/tty/serial/samsung_tty.c | 76 +++++++++++++++++++++++++++++++++------- > >>> 1 file changed, 64 insertions(+), 12 deletions(-) > >>> > >>> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > >>> index 73f951d65b93..bdf1d4d12cb1 100644 > >>> --- a/drivers/tty/serial/samsung_tty.c > >>> +++ b/drivers/tty/serial/samsung_tty.c > >>> @@ -154,12 +154,47 @@ struct s3c24xx_uart_port { > >> ... > >>> -#define wr_regb(port, reg, val) writeb_relaxed(val, portaddr(port, reg)) > >>> +static void wr_reg(struct uart_port *port, u32 reg, u32 val) > >>> +{ > >>> + switch (port->iotype) { > >>> + case UPIO_MEM: > >>> + writeb_relaxed(val, portaddr(port, reg)); > >>> + break; > >>> + case UPIO_MEM32: > >>> + writel_relaxed(val, portaddr(port, reg)); > >>> + break; > >>> + } > >>> +} > >>> + > >>> #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg)) > >>> > >>> +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val) > >> > >> You need to explain, why you need this _barrier variant now. This change > >> should be done in a separate patch too. > > > > There is no functional change in regard of barrier. The ordered IO was > > used there before. > > The patch changes one wr_reg to wr_reg_barrier without any explanation. > This will hardly be accepted. I cannot find such change... I see only: @@ -2612,7 +2664,7 @@ static void samsung_early_putc(struct uart_port *port, int c) - writeb(c, port->membase + S3C2410_UTXH); + wr_reg_barrier(port, S3C2410_UTXH, c); which is the same except 'b' -> 'b/l'. Best regards, Krzysztof
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index 73f951d65b93..bdf1d4d12cb1 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -154,12 +154,47 @@ struct s3c24xx_uart_port { #define portaddrl(port, reg) \ ((unsigned long *)(unsigned long)((port)->membase + (reg))) -#define rd_regb(port, reg) (readb_relaxed(portaddr(port, reg))) +static u32 rd_reg(struct uart_port *port, u32 reg) +{ + switch (port->iotype) { + case UPIO_MEM: + return readb_relaxed(portaddr(port, reg)); + case UPIO_MEM32: + return readl_relaxed(portaddr(port, reg)); + default: + return 0; + } + return 0; +} + #define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg))) -#define wr_regb(port, reg, val) writeb_relaxed(val, portaddr(port, reg)) +static void wr_reg(struct uart_port *port, u32 reg, u32 val) +{ + switch (port->iotype) { + case UPIO_MEM: + writeb_relaxed(val, portaddr(port, reg)); + break; + case UPIO_MEM32: + writel_relaxed(val, portaddr(port, reg)); + break; + } +} + #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg)) +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val) +{ + switch (port->iotype) { + case UPIO_MEM: + writeb(val, portaddr(port, reg)); + break; + case UPIO_MEM32: + writel(val, portaddr(port, reg)); + break; + } +} + /* Byte-order aware bit setting/clearing functions. */ static inline void s3c24xx_set_bit(struct uart_port *port, int idx, @@ -714,7 +749,7 @@ static void s3c24xx_serial_rx_drain_fifo(struct s3c24xx_uart_port *ourport) fifocnt--; uerstat = rd_regl(port, S3C2410_UERSTAT); - ch = rd_regb(port, S3C2410_URXH); + ch = rd_reg(port, S3C2410_URXH); if (port->flags & UPF_CONS_FLOW) { int txe = s3c24xx_serial_txempty_nofifo(port); @@ -826,7 +861,7 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id) } if (port->x_char) { - wr_regb(port, S3C2410_UTXH, port->x_char); + wr_reg(port, S3C2410_UTXH, port->x_char); port->icount.tx++; port->x_char = 0; goto out; @@ -852,7 +887,7 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id) if (rd_regl(port, S3C2410_UFSTAT) & ourport->info->tx_fifofull) break; - wr_regb(port, S3C2410_UTXH, xmit->buf[xmit->tail]); + wr_reg(port, S3C2410_UTXH, xmit->buf[xmit->tail]); xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1); port->icount.tx++; count--; @@ -916,7 +951,7 @@ static unsigned int s3c24xx_serial_tx_empty(struct uart_port *port) /* no modem control lines */ static unsigned int s3c24xx_serial_get_mctrl(struct uart_port *port) { - unsigned int umstat = rd_regb(port, S3C2410_UMSTAT); + unsigned int umstat = rd_reg(port, S3C2410_UMSTAT); if (umstat & S3C2410_UMSTAT_CTS) return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS; @@ -1974,7 +2009,7 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node; struct s3c24xx_uart_port *ourport; int index = probe_index; - int ret; + int ret, prop = 0; if (np) { ret = of_alias_get_id(np, "serial"); @@ -2000,10 +2035,27 @@ static int s3c24xx_serial_probe(struct platform_device *pdev) dev_get_platdata(&pdev->dev) : ourport->drv_data->def_cfg; - if (np) + if (np) { of_property_read_u32(np, "samsung,uart-fifosize", &ourport->port.fifosize); + if (of_property_read_u32(np, "reg-io-width", &prop) == 0) { + switch (prop) { + case 1: + ourport->port.iotype = UPIO_MEM; + break; + case 4: + ourport->port.iotype = UPIO_MEM32; + break; + default: + dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", + prop); + ret = -EINVAL; + break; + } + } + } + if (ourport->drv_data->fifosize[index]) ourport->port.fifosize = ourport->drv_data->fifosize[index]; else if (ourport->info->fifosize) @@ -2185,7 +2237,7 @@ static int s3c24xx_serial_get_poll_char(struct uart_port *port) if (s3c24xx_serial_rx_fifocnt(ourport, ufstat) == 0) return NO_POLL_CHAR; - return rd_regb(port, S3C2410_URXH); + return rd_reg(port, S3C2410_URXH); } static void s3c24xx_serial_put_poll_char(struct uart_port *port, @@ -2200,7 +2252,7 @@ static void s3c24xx_serial_put_poll_char(struct uart_port *port, while (!s3c24xx_serial_console_txrdy(port, ufcon)) cpu_relax(); - wr_regb(port, S3C2410_UTXH, c); + wr_reg(port, S3C2410_UTXH, c); } #endif /* CONFIG_CONSOLE_POLL */ @@ -2212,7 +2264,7 @@ s3c24xx_serial_console_putchar(struct uart_port *port, int ch) while (!s3c24xx_serial_console_txrdy(port, ufcon)) cpu_relax(); - wr_regb(port, S3C2410_UTXH, ch); + wr_reg(port, S3C2410_UTXH, ch); } static void @@ -2612,7 +2664,7 @@ static void samsung_early_putc(struct uart_port *port, int c) else samsung_early_busyuart(port); - writeb(c, port->membase + S3C2410_UTXH); + wr_reg_barrier(port, S3C2410_UTXH, c); } static void samsung_early_write(struct console *con, const char *s,
Support 32-bit access for the TX/RX hold registers UTXH and URXH. This is required for some newer SoCs. Signed-off-by: Hyunki Koo <hyunki00.koo@samsung.com> --- v2: line 954 : change rd_regl to rd_reg in for backward compatibility. line 2031: Add init value for ourport->port.iotype to UPIO_MEM v3: line 2031: remove redundant init value for ourport->port.iotype v4: correct variable types and change misleading function name v5: add dt-binding and go as first patch in this series. v6: no change in this patch, only chaged in [PATCH v6 1/2] --- drivers/tty/serial/samsung_tty.c | 76 +++++++++++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 12 deletions(-)