There is one more place where lqasc_tx_ready() can be used now:
lqasc_console_putchar(). So replace the open-coded variant by the
helper.
Suggested-by: Ilpo Järvinen <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/lantiq.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index 6da1b7496c6c..ba9739af30ed 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -606,15 +606,12 @@ static const struct uart_ops lqasc_pops = {
static void
lqasc_console_putchar(struct uart_port *port, unsigned char ch)
{
- int fifofree;
-
if (!port->membase)
return;
- do {
- fifofree = (__raw_readl(port->membase + LTQ_ASC_FSTAT)
- & ASCFSTAT_TXFREEMASK) >> ASCFSTAT_TXFREEOFF;
- } while (fifofree == 0);
+ while (!lqasc_tx_ready(port))
+ ;
+
writeb(ch, port->membase + LTQ_ASC_TBUF);
}
--
2.37.3
TX space reads from the control register are performed in various forms
on 4 places in altera_jtaguart. Unify all those and do the read and
masking on a single place.
The new helper altera_jtaguart_tx_space() uses FIELD_GET(), so we can
drop ALTERA_JTAGUART_CONTROL_WSPACE_OFF now.
Cc: Tobias Klauser <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/altera_jtaguart.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
index 23f339757894..ac8ce418de36 100644
--- a/drivers/tty/serial/altera_jtaguart.c
+++ b/drivers/tty/serial/altera_jtaguart.c
@@ -9,6 +9,7 @@
* (C) Copyright 2010, Tobias Klauser <[email protected]>
*/
+#include <linux/bitfield.h>
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/interrupt.h>
@@ -48,7 +49,6 @@
#define ALTERA_JTAGUART_CONTROL_WI_MSK 0x00000200
#define ALTERA_JTAGUART_CONTROL_AC_MSK 0x00000400
#define ALTERA_JTAGUART_CONTROL_WSPACE_MSK 0xFFFF0000
-#define ALTERA_JTAGUART_CONTROL_WSPACE_OFF 16
/*
* Local per-uart structure.
@@ -59,10 +59,19 @@ struct altera_jtaguart {
unsigned long imr; /* Local IMR mirror */
};
+static unsigned int altera_jtaguart_tx_space(struct uart_port *port, u32 *ctlp)
+{
+ u32 ctl = readl(port->membase + ALTERA_JTAGUART_CONTROL_REG);
+
+ if (ctlp)
+ *ctlp = ctl;
+
+ return FIELD_GET(ALTERA_JTAGUART_CONTROL_WSPACE_MSK, ctl);
+}
+
static unsigned int altera_jtaguart_tx_empty(struct uart_port *port)
{
- return (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
- ALTERA_JTAGUART_CONTROL_WSPACE_MSK) ? TIOCSER_TEMT : 0;
+ return altera_jtaguart_tx_space(port, NULL) ? TIOCSER_TEMT : 0;
}
static unsigned int altera_jtaguart_get_mctrl(struct uart_port *port)
@@ -150,9 +159,7 @@ static void altera_jtaguart_tx_chars(struct altera_jtaguart *pp)
pending = uart_circ_chars_pending(xmit);
if (pending > 0) {
- count = (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
- ALTERA_JTAGUART_CONTROL_WSPACE_MSK) >>
- ALTERA_JTAGUART_CONTROL_WSPACE_OFF;
+ count = altera_jtaguart_tx_space(port, NULL);
if (count > pending)
count = pending;
if (count > 0) {
@@ -298,12 +305,11 @@ static struct altera_jtaguart altera_jtaguart_ports[ALTERA_JTAGUART_MAXPORTS];
#if defined(CONFIG_SERIAL_ALTERA_JTAGUART_CONSOLE_BYPASS)
static void altera_jtaguart_console_putc(struct uart_port *port, unsigned char c)
{
- unsigned long status;
unsigned long flags;
+ u32 status;
spin_lock_irqsave(&port->lock, flags);
- while (((status = readl(port->membase + ALTERA_JTAGUART_CONTROL_REG)) &
- ALTERA_JTAGUART_CONTROL_WSPACE_MSK) == 0) {
+ while (!altera_jtaguart_tx_space(port, &status)) {
if ((status & ALTERA_JTAGUART_CONTROL_AC_MSK) == 0) {
spin_unlock_irqrestore(&port->lock, flags);
return; /* no connection activity */
@@ -321,8 +327,7 @@ static void altera_jtaguart_console_putc(struct uart_port *port, unsigned char c
unsigned long flags;
spin_lock_irqsave(&port->lock, flags);
- while ((readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
- ALTERA_JTAGUART_CONTROL_WSPACE_MSK) == 0) {
+ while (!altera_jtaguart_tx_space(port, NULL)) {
spin_unlock_irqrestore(&port->lock, flags);
cpu_relax();
spin_lock_irqsave(&port->lock, flags);
--
2.37.3
On Tue, 27 Sep 2022, Jiri Slaby wrote:
> There is one more place where lqasc_tx_ready() can be used now:
> lqasc_console_putchar(). So replace the open-coded variant by the
> helper.
>
> Suggested-by: Ilpo Järvinen <[email protected]>
> Signed-off-by: Jiri Slaby <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
> ---
> drivers/tty/serial/lantiq.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
> index 6da1b7496c6c..ba9739af30ed 100644
> --- a/drivers/tty/serial/lantiq.c
> +++ b/drivers/tty/serial/lantiq.c
> @@ -606,15 +606,12 @@ static const struct uart_ops lqasc_pops = {
> static void
> lqasc_console_putchar(struct uart_port *port, unsigned char ch)
> {
> - int fifofree;
> -
> if (!port->membase)
> return;
>
> - do {
> - fifofree = (__raw_readl(port->membase + LTQ_ASC_FSTAT)
> - & ASCFSTAT_TXFREEMASK) >> ASCFSTAT_TXFREEOFF;
> - } while (fifofree == 0);
> + while (!lqasc_tx_ready(port))
> + ;
> +
> writeb(ch, port->membase + LTQ_ASC_TBUF);
> }
>
>
On 2022-09-27 at 13:18:18 +0200, Jiri Slaby <[email protected]> wrote:
> TX space reads from the control register are performed in various forms
> on 4 places in altera_jtaguart. Unify all those and do the read and
> masking on a single place.
>
> The new helper altera_jtaguart_tx_space() uses FIELD_GET(), so we can
> drop ALTERA_JTAGUART_CONTROL_WSPACE_OFF now.
>
> Cc: Tobias Klauser <[email protected]>
> Signed-off-by: Jiri Slaby <[email protected]>
Acked-by: Tobias Klauser <[email protected]>
Thanks
On Tue, 27 Sep 2022, Jiri Slaby wrote:
> TX space reads from the control register are performed in various forms
> on 4 places in altera_jtaguart. Unify all those and do the read and
> masking on a single place.
>
> The new helper altera_jtaguart_tx_space() uses FIELD_GET(), so we can
> drop ALTERA_JTAGUART_CONTROL_WSPACE_OFF now.
>
> Cc: Tobias Klauser <[email protected]>
> Signed-off-by: Jiri Slaby <[email protected]>
> ---
> drivers/tty/serial/altera_jtaguart.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
> index 23f339757894..ac8ce418de36 100644
> --- a/drivers/tty/serial/altera_jtaguart.c
> +++ b/drivers/tty/serial/altera_jtaguart.c
> @@ -9,6 +9,7 @@
> * (C) Copyright 2010, Tobias Klauser <[email protected]>
> */
>
> +#include <linux/bitfield.h>
> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> @@ -48,7 +49,6 @@
> #define ALTERA_JTAGUART_CONTROL_WI_MSK 0x00000200
> #define ALTERA_JTAGUART_CONTROL_AC_MSK 0x00000400
> #define ALTERA_JTAGUART_CONTROL_WSPACE_MSK 0xFFFF0000
> -#define ALTERA_JTAGUART_CONTROL_WSPACE_OFF 16
>
> /*
> * Local per-uart structure.
> @@ -59,10 +59,19 @@ struct altera_jtaguart {
> unsigned long imr; /* Local IMR mirror */
> };
>
> +static unsigned int altera_jtaguart_tx_space(struct uart_port *port, u32 *ctlp)
> +{
> + u32 ctl = readl(port->membase + ALTERA_JTAGUART_CONTROL_REG);
> +
> + if (ctlp)
> + *ctlp = ctl;
> +
> + return FIELD_GET(ALTERA_JTAGUART_CONTROL_WSPACE_MSK, ctl);
> +}
> +
> static unsigned int altera_jtaguart_tx_empty(struct uart_port *port)
> {
> - return (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
> - ALTERA_JTAGUART_CONTROL_WSPACE_MSK) ? TIOCSER_TEMT : 0;
> + return altera_jtaguart_tx_space(port, NULL) ? TIOCSER_TEMT : 0;
> }
>
> static unsigned int altera_jtaguart_get_mctrl(struct uart_port *port)
> @@ -150,9 +159,7 @@ static void altera_jtaguart_tx_chars(struct altera_jtaguart *pp)
>
> pending = uart_circ_chars_pending(xmit);
> if (pending > 0) {
> - count = (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
> - ALTERA_JTAGUART_CONTROL_WSPACE_MSK) >>
> - ALTERA_JTAGUART_CONTROL_WSPACE_OFF;
> + count = altera_jtaguart_tx_space(port, NULL);
> if (count > pending)
> count = pending;
> if (count > 0) {
> @@ -298,12 +305,11 @@ static struct altera_jtaguart altera_jtaguart_ports[ALTERA_JTAGUART_MAXPORTS];
> #if defined(CONFIG_SERIAL_ALTERA_JTAGUART_CONSOLE_BYPASS)
> static void altera_jtaguart_console_putc(struct uart_port *port, unsigned char c)
> {
> - unsigned long status;
> unsigned long flags;
> + u32 status;
>
> spin_lock_irqsave(&port->lock, flags);
> - while (((status = readl(port->membase + ALTERA_JTAGUART_CONTROL_REG)) &
> - ALTERA_JTAGUART_CONTROL_WSPACE_MSK) == 0) {
> + while (!altera_jtaguart_tx_space(port, &status)) {
> if ((status & ALTERA_JTAGUART_CONTROL_AC_MSK) == 0) {
> spin_unlock_irqrestore(&port->lock, flags);
> return; /* no connection activity */
> @@ -321,8 +327,7 @@ static void altera_jtaguart_console_putc(struct uart_port *port, unsigned char c
> unsigned long flags;
>
> spin_lock_irqsave(&port->lock, flags);
> - while ((readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
> - ALTERA_JTAGUART_CONTROL_WSPACE_MSK) == 0) {
> + while (!altera_jtaguart_tx_space(port, NULL)) {
> spin_unlock_irqrestore(&port->lock, flags);
> cpu_relax();
> spin_lock_irqsave(&port->lock, flags);
Reviewed-by: Ilpo J?rvinen <[email protected]>
With caveats. This is not incorrect per se but I don't particularly like
that pointer thing done in altera_jtaguart_tx_space(). IMHO,
altera_jtaguart_console_putc() that needs the control register also for
other uses than tx space check should read it locally and
__altera_jtaguart_tx_space() could be added to handle just the
tx space check without read. But this is of course just my opinion,
there's no technical issue things done either way.
--
i.