On Mon, Apr 11, 2022 at 11:33:21AM +0300, Ilpo Järvinen wrote:
> This change adds 9th bit multipoint addressing mode for DW UART using
> the new ioctls introduced in the previous change. 9th bit addressing
> can be used only when HW RS485 is available.
>
> Updating RAR (receive address register) is bit tricky because
> busy indication is not be available when DW UART is strictly
> 16550 compatible, which is the case with the hardware I was
> testing with. RAR should not be updated while receive is in
> progress which is now achieved by deasserting RE and waiting
> for one frame (in case rx would be in progress, the driver
> seems to have no way of knowing it w/o busy indication).
...
> +static void dw8250_update_rar(struct uart_port *p, u32 addr)
> +{
> + u32 re_en = dw8250_readl_ext(p, DW_UART_RE_EN);
> +
> + /*
> + * RAR shouldn't be changed while receiving. Thus, de-assert RE_EN
> + * if asserted and wait.
> + */
> + if (re_en)
> + dw8250_writel_ext(p, DW_UART_RE_EN, 0);
> + dw8250_wait_re_deassert(p);
> + dw8250_writel_ext(p, DW_UART_RAR, addr);
> + if (re_en)
> + dw8250_writel_ext(p, DW_UART_RE_EN, re_en);
I would write it
u32 re_en;
/*
* RAR shouldn't be changed while receiving. Thus, de-assert RE_EN
* if asserted and wait.
*/
re_en = dw8250_readl_ext(p, DW_UART_RE_EN);
if (re_en) {
dw8250_writel_ext(p, DW_UART_RE_EN, 0);
dw8250_wait_re_deassert(p);
dw8250_writel_ext(p, DW_UART_RAR, addr);
dw8250_writel_ext(p, DW_UART_RE_EN, re_en);
} else {
dw8250_wait_re_deassert(p);
dw8250_writel_ext(p, DW_UART_RAR, addr);
}
or something similar with extracting these two lines into another function.
But it's up to you, it's just pure style thingy.
> +}
...
> + dw8250_writel_ext(p, DW_UART_TAR, addr->addr & 0xff);
GENMASK() ?
Perhaps even a separate definition?
...
> + addr->addr = dw8250_readl_ext(p, DW_UART_TAR) & 0xff;
Ditto.
...
> + addr->addr = dw8250_readl_ext(p, DW_UART_RAR) & 0xff;
Ditto.
--
With Best Regards,
Andy Shevchenko
