2022-06-28 13:57:20

by Ilpo Järvinen

Subject: [PATCH 3/4] serial: 8250_dw: Move 16550 compatible & LCR checks to dw8250_verify_write()

Rename dw8250_check_lcr() -> dw8250_verify_write() and add comment.
Move LCR and 16550_compatible checks there. As offset is now passed and
dw8250_verify_write() ensures it's UART_LCR, offset can use used
instead of explicit UART_LCR.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/tty/serial/8250/8250_dw.c | 35 +++++++++++++++----------------
1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index f18975b4d2c7..fc367d44f86d 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -87,14 +87,24 @@ static void dw8250_force_idle(struct uart_port *p)
(void)p->serial_in(p, UART_RX);
}

-static void dw8250_check_lcr(struct uart_port *p, int value)
+/*
+ * DW UART can be configured to indicate BUSY in USR (with
+ * UART_16550_COMPATIBLE=NO or version prior to introducing that option).
+ * If BUSY is set while writing to LCR register, the write is ignored and
+ * needs to be retries.
+ */
+static void dw8250_verify_write(struct uart_port *p, int offset, int value)
{
- void __iomem *reg_offset = p->membase + (UART_LCR << p->regshift);
+ void __iomem *reg_offset = p->membase + (offset << p->regshift);
+ struct dw8250_data *d = to_dw8250_data(p->private_data);
int tries = 1000;

+ if ((offset != UART_LCR) || !d->uart_16550_compatible)
+ return;
+
/* Make sure LCR write wasn't ignored */
while (tries--) {
- unsigned int lcr = p->serial_in(p, UART_LCR);
+ unsigned int lcr = p->serial_in(p, offset);

if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
return;
@@ -145,12 +155,9 @@ static void dw8250_tx_wait_empty(struct uart_port *p)

static void dw8250_serial_out(struct uart_port *p, int offset, int value)
{
- struct dw8250_data *d = to_dw8250_data(p->private_data);
-
writeb(value, p->membase + (offset << p->regshift));

- if (offset == UART_LCR && !d->uart_16550_compatible)
- dw8250_check_lcr(p, value);
+ dw8250_verify_write(p, offset, value);
}

static void dw8250_serial_out38x(struct uart_port *p, int offset, int value)
@@ -181,26 +188,20 @@ static unsigned int dw8250_serial_inq(struct uart_port *p, int offset)

static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
{
- struct dw8250_data *d = to_dw8250_data(p->private_data);
-
value &= 0xff;
__raw_writeq(value, p->membase + (offset << p->regshift));
/* Read back to ensure register write ordering. */
__raw_readq(p->membase + (UART_LCR << p->regshift));

- if (offset == UART_LCR && !d->uart_16550_compatible)
- dw8250_check_lcr(p, value);
+ dw8250_verify_write(p, offset, value);
}
#endif /* CONFIG_64BIT */

static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
{
- struct dw8250_data *d = to_dw8250_data(p->private_data);
-
writel(value, p->membase + (offset << p->regshift));

- if (offset == UART_LCR && !d->uart_16550_compatible)
- dw8250_check_lcr(p, value);
+ dw8250_verify_write(p, offset, value);
}

static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
@@ -212,12 +213,10 @@ static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)

static void dw8250_serial_out32be(struct uart_port *p, int offset, int value)
{
- struct dw8250_data *d = to_dw8250_data(p->private_data);

iowrite32be(value, p->membase + (offset << p->regshift));

- if (offset == UART_LCR && !d->uart_16550_compatible)
- dw8250_check_lcr(p, value);
+ dw8250_verify_write(p, offset, value);
}

static unsigned int dw8250_serial_in32be(struct uart_port *p, int offset)
--
2.30.2

2022-06-28 20:48:07

by Andy Shevchenko

Subject: Re: [PATCH 3/4] serial: 8250_dw: Move 16550 compatible & LCR checks to dw8250_verify_write()

On Tue, Jun 28, 2022 at 3:43 PM Ilpo Järvinen
<[email protected]> wrote:
>
> Rename dw8250_check_lcr() -> dw8250_verify_write() and add comment.
> Move LCR and 16550_compatible checks there. As offset is now passed and
> dw8250_verify_write() ensures it's UART_LCR, offset can use used
> instead of explicit UART_LCR.

...

> +/*
> + * DW UART can be configured to indicate BUSY in USR (with
> + * UART_16550_COMPATIBLE=NO or version prior to introducing that option).
> + * If BUSY is set while writing to LCR register, the write is ignored and
> + * needs to be retries.

retried

> + */


--
With Best Regards,
Andy Shevchenko