2017-06-12 15:37:29

by Aisheng Dong

[permalink] [raw]
Subject: [PATCH V3 0/7] tty: serial: lpuart: add imx7ulp support

This patch series mainly intends to add imx7ulp support which is also
using FSL lpuart.

The lpuart in imx7ulp is basically the same as ls1021a. It's also
32 bit width register, but unlike ls1021a, it's little endian.
Besides that, imx7ulp lpuart has a minor different register layout
from ls1021a that it has four extra registers (verid, param, global,
pincfg) located at the beginning of register map, which are currently
not used by the driver and less to be used later.

Furthermore, this patch serial also add a new more accurate baud rate
calculation method as MX7ULP can't divide a suitable baud rate
with the default setting.

Currently the new baud rate calculation is only enabled on MX7ULP.
However, i guess the Layerscape may also be able to use it as there
seems to be no difference in baud rate setting register after checking
the Layerscape Reference Manual.

As i don't have Layerscape boards, i can't test it, so i only enable it
for MX7ULP by default to avoid a potential break.

I copied LayerScape guys in this series and hope they can help test later.
If it works on Layerscape as well, then they can switch to the new setting
too and totally remove the old stuff.

ChangeLog:
v2->v3:
* Remove global lpuart_is_be.
Instead use struct uart_port's iotype member.
lpuart32_read/write API prototype is also updated to use the iotype to
distingush the endians. And most importantly, this way also works with
earlycon.

v1->v2:
* Patch 2/4/5 chagned, other no changes.
See individuals for details.

Dong Aisheng (7):
tty: serial: lpuart: introduce lpuart_soc_data to represent SoC
property
tty: serial: lpuart: refactor lpuart32_{read|write} prototype
tty: serial: lpuart: add little endian 32 bit register support
dt-bindings: serial: fsl-lpuart: add i.MX7ULP support
tty: serial: lpuart: add imx7ulp support
tty: serial: lpuart: add earlycon support for imx7ulp
tty: serial: lpuart: add a more accurate baud rate calculation method

.../devicetree/bindings/serial/fsl-lpuart.txt | 2 +
drivers/tty/serial/fsl_lpuart.c | 286 ++++++++++++++-------
2 files changed, 201 insertions(+), 87 deletions(-)

--
2.7.4


2017-06-12 16:18:17

by Aisheng Dong

[permalink] [raw]
Subject: [PATCH V3 7/7] tty: serial: lpuart: add a more accurate baud rate calculation method

On new LPUART versions, the oversampling ratio for the receiver can be
changed from 4x (00011) to 32x (11111) which could help us get a more
accurate baud rate divider.

The idea is to use the best OSR (over-sampling rate) possible.
Note, OSR is typically hard-set to 16 in other LPUART instantiations.
Loop to find the best OSR value possible, one that generates minimum
baud diff iterate through the rest of the supported values of OSR.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Stefan Agner <[email protected]>
Cc: Mingkai Hu <[email protected]>
Cc: Yangbo Lu <[email protected]>
Acked-by: Fugang Duan <[email protected]>
Signed-off-by: Dong Aisheng <[email protected]>

---
ChangeLog:
v2->v3:
* enable new baud rate calculation method by default to get Layerscape
platforms run and test.
* break out in case we already find the minum baud_diff
v1->v2:
* No changes
---
drivers/tty/serial/fsl_lpuart.c | 79 +++++++++++++++++++++++++++++++++++++----
1 file changed, 72 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 6a725cb..a146289 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -140,6 +140,8 @@
#define UARTBAUD_SBNS 0x00002000
#define UARTBAUD_SBR 0x00000000
#define UARTBAUD_SBR_MASK 0x1fff
+#define UARTBAUD_OSR_MASK 0x1f
+#define UARTBAUD_OSR_SHIFT 24

#define UARTSTAT_LBKDIF 0x80000000
#define UARTSTAT_RXEDGIF 0x40000000
@@ -1511,6 +1513,75 @@ lpuart_set_termios(struct uart_port *port, struct ktermios *termios,
}

static void
+lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
+{
+ u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
+ u32 clk = sport->port.uartclk;
+
+ /*
+ * The idea is to use the best OSR (over-sampling rate) possible.
+ * Note, OSR is typically hard-set to 16 in other LPUART instantiations.
+ * Loop to find the best OSR value possible, one that generates minimum
+ * baud_diff iterate through the rest of the supported values of OSR.
+ *
+ * Calculation Formula:
+ * Baud Rate = baud clock / ((OSR+1) × SBR)
+ */
+ baud_diff = baudrate;
+ osr = 0;
+ sbr = 0;
+
+ for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
+ /* calculate the temporary sbr value */
+ tmp_sbr = (clk / (baudrate * tmp_osr));
+ if (tmp_sbr == 0)
+ tmp_sbr = 1;
+
+ /*
+ * calculate the baud rate difference based on the temporary
+ * osr and sbr values
+ */
+ tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;
+
+ /* select best values between sbr and sbr+1 */
+ tmp = clk / (tmp_osr * (tmp_sbr + 1));
+ if (tmp_diff > (baudrate - tmp)) {
+ tmp_diff = baudrate - tmp;
+ tmp_sbr++;
+ }
+
+ if (tmp_diff <= baud_diff) {
+ baud_diff = tmp_diff;
+ osr = tmp_osr;
+ sbr = tmp_sbr;
+
+ if (!baud_diff)
+ break;
+ }
+ }
+
+ /* handle buadrate outside acceptable rate */
+ if (baud_diff > ((baudrate / 100) * 3))
+ dev_warn(sport->port.dev,
+ "unacceptable baud rate difference of more than 3%%\n");
+
+ tmp = lpuart32_read(&sport->port, UARTBAUD);
+
+ if ((osr > 3) && (osr < 8))
+ tmp |= UARTBAUD_BOTHEDGE;
+
+ tmp &= ~(UARTBAUD_OSR_MASK << UARTBAUD_OSR_SHIFT);
+ tmp |= (((osr-1) & UARTBAUD_OSR_MASK) << UARTBAUD_OSR_SHIFT);
+
+ tmp &= ~UARTBAUD_SBR_MASK;
+ tmp |= sbr & UARTBAUD_SBR_MASK;
+
+ tmp &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
+
+ lpuart32_write(&sport->port, tmp, UARTBAUD);
+}
+
+static void
lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
struct ktermios *old)
{
@@ -1519,7 +1590,6 @@ lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
unsigned long ctrl, old_ctrl, bd, modem;
unsigned int baud;
unsigned int old_csize = old ? old->c_cflag & CSIZE : CS8;
- unsigned int sbr;

ctrl = old_ctrl = lpuart32_read(&sport->port, UARTCTRL);
bd = lpuart32_read(&sport->port, UARTBAUD);
@@ -1616,12 +1686,7 @@ lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
lpuart32_write(&sport->port, old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
UARTCTRL);

- sbr = sport->port.uartclk / (16 * baud);
- bd &= ~UARTBAUD_SBR_MASK;
- bd |= sbr & UARTBAUD_SBR_MASK;
- bd |= UARTBAUD_BOTHEDGE;
- bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
- lpuart32_write(&sport->port, bd, UARTBAUD);
+ lpuart32_serial_setbrg(sport, baud);
lpuart32_write(&sport->port, modem, UARTMODIR);
lpuart32_write(&sport->port, ctrl, UARTCTRL);
/* restore control register */
--
2.7.4

2017-06-12 15:37:54

by Aisheng Dong

[permalink] [raw]
Subject: [PATCH V3 3/7] tty: serial: lpuart: add little endian 32 bit register support

Use standard port->iotype to distinguish endian difference. Note as we
read/write register by checking iotype dynamically, we need to initialize
the iotype correctly for earlycon as well to avoid a break.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]> (supporter:TTY LAYER)
Cc: Stefan Agner <[email protected]>
Cc: Mingkai Hu <[email protected]>
Cc: Yangbo Lu <[email protected]>
Cc: Fugang Duan <[email protected]>
Signed-off-by: Dong Aisheng <[email protected]>

ChangeLog:
v2->v3:
* Instead of using global var, use standard port->iotype to distinguish
endian difference.
v1->v2:
* No changes
---
drivers/tty/serial/fsl_lpuart.c | 43 +++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index ed9db2f..bbf47a0 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -280,15 +280,29 @@ MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
/* Forward declare this for the dma callbacks*/
static void lpuart_dma_tx_complete(void *arg);

-static inline u32 lpuart32_read(struct uart_port *port, u32 reg_off)
-{
- return ioread32be(port->membase + reg_off);
+static inline u32 lpuart32_read(struct uart_port *port, u32 off)
+{
+ switch (port->iotype) {
+ case UPIO_MEM32:
+ return readl(port->membase + off);
+ case UPIO_MEM32BE:
+ return ioread32be(port->membase + off);
+ default:
+ return 0;
+ };
}

static inline void lpuart32_write(struct uart_port *port, u32 val,
- u32 reg_off)
+ u32 off)
{
- iowrite32be(val, port->membase + reg_off);
+ switch (port->iotype) {
+ case UPIO_MEM32:
+ writel(val, port->membase + off);
+ break;
+ case UPIO_MEM32BE:
+ iowrite32be(val, port->membase + off);
+ break;
+ };
}

static void lpuart_stop_tx(struct uart_port *port)
@@ -602,7 +616,7 @@ static irqreturn_t lpuart_txint(int irq, void *dev_id)

spin_lock_irqsave(&sport->port.lock, flags);
if (sport->port.x_char) {
- if (sport->port.iotype & UPIO_MEM32BE)
+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
lpuart32_write(&sport->port, sport->port.x_char, UARTDATA);
else
writeb(sport->port.x_char, sport->port.membase + UARTDR);
@@ -610,14 +624,14 @@ static irqreturn_t lpuart_txint(int irq, void *dev_id)
}

if (uart_circ_empty(xmit) || uart_tx_stopped(&sport->port)) {
- if (sport->port.iotype & UPIO_MEM32BE)
+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
lpuart32_stop_tx(&sport->port);
else
lpuart_stop_tx(&sport->port);
goto out;
}

- if (sport->port.iotype & UPIO_MEM32BE)
+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
lpuart32_transmit_buffer(sport);
else
lpuart_transmit_buffer(sport);
@@ -1890,12 +1904,12 @@ static int __init lpuart_console_setup(struct console *co, char *options)
if (options)
uart_parse_options(options, &baud, &parity, &bits, &flow);
else
- if (sport->port.iotype & UPIO_MEM32BE)
+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
lpuart32_console_get_options(sport, &baud, &parity, &bits);
else
lpuart_console_get_options(sport, &baud, &parity, &bits);

- if (sport->port.iotype & UPIO_MEM32BE)
+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
lpuart32_setup_watermark(sport);
else
lpuart_setup_watermark(sport);
@@ -1954,6 +1968,7 @@ static int __init lpuart32_early_console_setup(struct earlycon_device *device,
if (!device->port.membase)
return -ENODEV;

+ device->port.iotype = UPIO_MEM32BE;
device->con->write = lpuart32_early_write;
return 0;
}
@@ -2015,7 +2030,7 @@ static int lpuart_probe(struct platform_device *pdev)
}
sport->port.irq = ret;
sport->port.iotype = sdata->iotype;
- if (sport->port.iotype & UPIO_MEM32BE)
+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
sport->port.ops = &lpuart32_pops;
else
sport->port.ops = &lpuart_pops;
@@ -2042,7 +2057,7 @@ static int lpuart_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, &sport->port);

- if (sport->port.iotype & UPIO_MEM32BE)
+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
lpuart_reg.cons = LPUART32_CONSOLE;
else
lpuart_reg.cons = LPUART_CONSOLE;
@@ -2095,7 +2110,7 @@ static int lpuart_suspend(struct device *dev)
struct lpuart_port *sport = dev_get_drvdata(dev);
unsigned long temp;

- if (sport->port.iotype & UPIO_MEM32BE) {
+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE)) {
/* disable Rx/Tx and interrupts */
temp = lpuart32_read(&sport->port, UARTCTRL);
temp &= ~(UARTCTRL_TE | UARTCTRL_TIE | UARTCTRL_TCIE);
@@ -2146,7 +2161,7 @@ static int lpuart_resume(struct device *dev)
if (sport->port.suspended && !sport->port.irq_wake)
clk_prepare_enable(sport->clk);

- if (sport->port.iotype & UPIO_MEM32BE) {
+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE)) {
lpuart32_setup_watermark(sport);
temp = lpuart32_read(&sport->port, UARTCTRL);
temp |= (UARTCTRL_RIE | UARTCTRL_TIE | UARTCTRL_RE |
--
2.7.4

2017-06-12 16:20:07

by Aisheng Dong

[permalink] [raw]
Subject: [PATCH V3 2/7] tty: serial: lpuart: refactor lpuart32_{read|write} prototype

Due to the original lpuart32_read/write takes no port specific
information arguments, it's hard to distinguish port difference
within the API. Although it works before, but not suitable anymore
when adding more new chips support.

So let's convert it to accept a new struct uart_port argument
to make it be able to retrieve more port specific information.
This is a preparation for the later adding new chips support
more easily. No functions changes.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Stefan Agner <[email protected]>
Cc: Mingkai Hu <[email protected]>
Cc: Yangbo Lu <[email protected]>
Cc: Fugang Duan <[email protected]>
Signed-off-by: Dong Aisheng <[email protected]>

---
ChangeLog:
v2->v3:
* newly introduced
---
drivers/tty/serial/fsl_lpuart.c | 123 ++++++++++++++++++++--------------------
1 file changed, 62 insertions(+), 61 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 4daabc6..ed9db2f 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -280,14 +280,15 @@ MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
/* Forward declare this for the dma callbacks*/
static void lpuart_dma_tx_complete(void *arg);

-static u32 lpuart32_read(void __iomem *addr)
+static inline u32 lpuart32_read(struct uart_port *port, u32 reg_off)
{
- return ioread32be(addr);
+ return ioread32be(port->membase + reg_off);
}

-static void lpuart32_write(u32 val, void __iomem *addr)
+static inline void lpuart32_write(struct uart_port *port, u32 val,
+ u32 reg_off)
{
- iowrite32be(val, addr);
+ iowrite32be(val, port->membase + reg_off);
}

static void lpuart_stop_tx(struct uart_port *port)
@@ -303,9 +304,9 @@ static void lpuart32_stop_tx(struct uart_port *port)
{
unsigned long temp;

- temp = lpuart32_read(port->membase + UARTCTRL);
+ temp = lpuart32_read(port, UARTCTRL);
temp &= ~(UARTCTRL_TIE | UARTCTRL_TCIE);
- lpuart32_write(temp, port->membase + UARTCTRL);
+ lpuart32_write(port, temp, UARTCTRL);
}

static void lpuart_stop_rx(struct uart_port *port)
@@ -320,8 +321,8 @@ static void lpuart32_stop_rx(struct uart_port *port)
{
unsigned long temp;

- temp = lpuart32_read(port->membase + UARTCTRL);
- lpuart32_write(temp & ~UARTCTRL_RE, port->membase + UARTCTRL);
+ temp = lpuart32_read(port, UARTCTRL);
+ lpuart32_write(port, temp & ~UARTCTRL_RE, UARTCTRL);
}

static void lpuart_dma_tx(struct lpuart_port *sport)
@@ -520,14 +521,14 @@ static inline void lpuart32_transmit_buffer(struct lpuart_port *sport)
struct circ_buf *xmit = &sport->port.state->xmit;
unsigned long txcnt;

- txcnt = lpuart32_read(sport->port.membase + UARTWATER);
+ txcnt = lpuart32_read(&sport->port, UARTWATER);
txcnt = txcnt >> UARTWATER_TXCNT_OFF;
txcnt &= UARTWATER_COUNT_MASK;
while (!uart_circ_empty(xmit) && (txcnt < sport->txfifo_size)) {
- lpuart32_write(xmit->buf[xmit->tail], sport->port.membase + UARTDATA);
+ lpuart32_write(&sport->port, xmit->buf[xmit->tail], UARTDATA);
xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
sport->port.icount.tx++;
- txcnt = lpuart32_read(sport->port.membase + UARTWATER);
+ txcnt = lpuart32_read(&sport->port, UARTWATER);
txcnt = txcnt >> UARTWATER_TXCNT_OFF;
txcnt &= UARTWATER_COUNT_MASK;
}
@@ -563,10 +564,10 @@ static void lpuart32_start_tx(struct uart_port *port)
struct lpuart_port *sport = container_of(port, struct lpuart_port, port);
unsigned long temp;

- temp = lpuart32_read(port->membase + UARTCTRL);
- lpuart32_write(temp | UARTCTRL_TIE, port->membase + UARTCTRL);
+ temp = lpuart32_read(port, UARTCTRL);
+ lpuart32_write(port, temp | UARTCTRL_TIE, UARTCTRL);

- if (lpuart32_read(port->membase + UARTSTAT) & UARTSTAT_TDRE)
+ if (lpuart32_read(port, UARTSTAT) & UARTSTAT_TDRE)
lpuart32_transmit_buffer(sport);
}

@@ -589,7 +590,7 @@ static unsigned int lpuart_tx_empty(struct uart_port *port)

static unsigned int lpuart32_tx_empty(struct uart_port *port)
{
- return (lpuart32_read(port->membase + UARTSTAT) & UARTSTAT_TC) ?
+ return (lpuart32_read(port, UARTSTAT) & UARTSTAT_TC) ?
TIOCSER_TEMT : 0;
}

@@ -602,7 +603,7 @@ static irqreturn_t lpuart_txint(int irq, void *dev_id)
spin_lock_irqsave(&sport->port.lock, flags);
if (sport->port.x_char) {
if (sport->port.iotype & UPIO_MEM32BE)
- lpuart32_write(sport->port.x_char, sport->port.membase + UARTDATA);
+ lpuart32_write(&sport->port, sport->port.x_char, UARTDATA);
else
writeb(sport->port.x_char, sport->port.membase + UARTDR);
goto out;
@@ -702,15 +703,15 @@ static irqreturn_t lpuart32_rxint(int irq, void *dev_id)

spin_lock_irqsave(&sport->port.lock, flags);

- while (!(lpuart32_read(sport->port.membase + UARTFIFO) & UARTFIFO_RXEMPT)) {
+ while (!(lpuart32_read(&sport->port, UARTFIFO) & UARTFIFO_RXEMPT)) {
flg = TTY_NORMAL;
sport->port.icount.rx++;
/*
* to clear the FE, OR, NF, FE, PE flags,
* read STAT then read DATA reg
*/
- sr = lpuart32_read(sport->port.membase + UARTSTAT);
- rx = lpuart32_read(sport->port.membase + UARTDATA);
+ sr = lpuart32_read(&sport->port, UARTSTAT);
+ rx = lpuart32_read(&sport->port, UARTDATA);
rx &= 0x3ff;

if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx))
@@ -777,18 +778,18 @@ static irqreturn_t lpuart32_int(int irq, void *dev_id)
struct lpuart_port *sport = dev_id;
unsigned long sts, rxcount;

- sts = lpuart32_read(sport->port.membase + UARTSTAT);
- rxcount = lpuart32_read(sport->port.membase + UARTWATER);
+ sts = lpuart32_read(&sport->port, UARTSTAT);
+ rxcount = lpuart32_read(&sport->port, UARTWATER);
rxcount = rxcount >> UARTWATER_RXCNT_OFF;

if (sts & UARTSTAT_RDRF || rxcount > 0)
lpuart32_rxint(irq, dev_id);

if ((sts & UARTSTAT_TDRE) &&
- !(lpuart32_read(sport->port.membase + UARTBAUD) & UARTBAUD_TDMAE))
+ !(lpuart32_read(&sport->port, UARTBAUD) & UARTBAUD_TDMAE))
lpuart_txint(irq, dev_id);

- lpuart32_write(sts, sport->port.membase + UARTSTAT);
+ lpuart32_write(&sport->port, sts, UARTSTAT);
return IRQ_HANDLED;
}

@@ -1049,7 +1050,7 @@ static unsigned int lpuart32_get_mctrl(struct uart_port *port)
unsigned int temp = 0;
unsigned long reg;

- reg = lpuart32_read(port->membase + UARTMODIR);
+ reg = lpuart32_read(port, UARTMODIR);
if (reg & UARTMODIR_TXCTSE)
temp |= TIOCM_CTS;

@@ -1084,7 +1085,7 @@ static void lpuart32_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
unsigned long temp;

- temp = lpuart32_read(port->membase + UARTMODIR) &
+ temp = lpuart32_read(port, UARTMODIR) &
~(UARTMODIR_RXRTSE | UARTMODIR_TXCTSE);

if (mctrl & TIOCM_RTS)
@@ -1093,7 +1094,7 @@ static void lpuart32_set_mctrl(struct uart_port *port, unsigned int mctrl)
if (mctrl & TIOCM_CTS)
temp |= UARTMODIR_TXCTSE;

- lpuart32_write(temp, port->membase + UARTMODIR);
+ lpuart32_write(port, temp, UARTMODIR);
}

static void lpuart_break_ctl(struct uart_port *port, int break_state)
@@ -1112,12 +1113,12 @@ static void lpuart32_break_ctl(struct uart_port *port, int break_state)
{
unsigned long temp;

- temp = lpuart32_read(port->membase + UARTCTRL) & ~UARTCTRL_SBK;
+ temp = lpuart32_read(port, UARTCTRL) & ~UARTCTRL_SBK;

if (break_state != 0)
temp |= UARTCTRL_SBK;

- lpuart32_write(temp, port->membase + UARTCTRL);
+ lpuart32_write(port, temp, UARTCTRL);
}

static void lpuart_setup_watermark(struct lpuart_port *sport)
@@ -1157,24 +1158,24 @@ static void lpuart32_setup_watermark(struct lpuart_port *sport)
unsigned long val, ctrl;
unsigned long ctrl_saved;

- ctrl = lpuart32_read(sport->port.membase + UARTCTRL);
+ ctrl = lpuart32_read(&sport->port, UARTCTRL);
ctrl_saved = ctrl;
ctrl &= ~(UARTCTRL_TIE | UARTCTRL_TCIE | UARTCTRL_TE |
UARTCTRL_RIE | UARTCTRL_RE);
- lpuart32_write(ctrl, sport->port.membase + UARTCTRL);
+ lpuart32_write(&sport->port, ctrl, UARTCTRL);

/* enable FIFO mode */
- val = lpuart32_read(sport->port.membase + UARTFIFO);
+ val = lpuart32_read(&sport->port, UARTFIFO);
val |= UARTFIFO_TXFE | UARTFIFO_RXFE;
val |= UARTFIFO_TXFLUSH | UARTFIFO_RXFLUSH;
- lpuart32_write(val, sport->port.membase + UARTFIFO);
+ lpuart32_write(&sport->port, val, UARTFIFO);

/* set the watermark */
val = (0x1 << UARTWATER_RXWATER_OFF) | (0x0 << UARTWATER_TXWATER_OFF);
- lpuart32_write(val, sport->port.membase + UARTWATER);
+ lpuart32_write(&sport->port, val, UARTWATER);

/* Restore cr2 */
- lpuart32_write(ctrl_saved, sport->port.membase + UARTCTRL);
+ lpuart32_write(&sport->port, ctrl_saved, UARTCTRL);
}

static void rx_dma_timer_init(struct lpuart_port *sport)
@@ -1250,7 +1251,7 @@ static int lpuart32_startup(struct uart_port *port)
unsigned long temp;

/* determine FIFO size */
- temp = lpuart32_read(sport->port.membase + UARTFIFO);
+ temp = lpuart32_read(&sport->port, UARTFIFO);

sport->txfifo_size = 0x1 << (((temp >> UARTFIFO_TXSIZE_OFF) &
UARTFIFO_FIFOSIZE_MASK) - 1);
@@ -1267,10 +1268,10 @@ static int lpuart32_startup(struct uart_port *port)

lpuart32_setup_watermark(sport);

- temp = lpuart32_read(sport->port.membase + UARTCTRL);
+ temp = lpuart32_read(&sport->port, UARTCTRL);
temp |= (UARTCTRL_RIE | UARTCTRL_TIE | UARTCTRL_RE | UARTCTRL_TE);
temp |= UARTCTRL_ILIE;
- lpuart32_write(temp, sport->port.membase + UARTCTRL);
+ lpuart32_write(&sport->port, temp, UARTCTRL);

spin_unlock_irqrestore(&sport->port.lock, flags);
return 0;
@@ -1319,10 +1320,10 @@ static void lpuart32_shutdown(struct uart_port *port)
spin_lock_irqsave(&port->lock, flags);

/* disable Rx/Tx and interrupts */
- temp = lpuart32_read(port->membase + UARTCTRL);
+ temp = lpuart32_read(port, UARTCTRL);
temp &= ~(UARTCTRL_TE | UARTCTRL_RE |
UARTCTRL_TIE | UARTCTRL_TCIE | UARTCTRL_RIE);
- lpuart32_write(temp, port->membase + UARTCTRL);
+ lpuart32_write(port, temp, UARTCTRL);

spin_unlock_irqrestore(&port->lock, flags);

@@ -1497,9 +1498,9 @@ lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
unsigned int old_csize = old ? old->c_cflag & CSIZE : CS8;
unsigned int sbr;

- ctrl = old_ctrl = lpuart32_read(sport->port.membase + UARTCTRL);
- bd = lpuart32_read(sport->port.membase + UARTBAUD);
- modem = lpuart32_read(sport->port.membase + UARTMODIR);
+ ctrl = old_ctrl = lpuart32_read(&sport->port, UARTCTRL);
+ bd = lpuart32_read(&sport->port, UARTBAUD);
+ modem = lpuart32_read(&sport->port, UARTMODIR);
/*
* only support CS8 and CS7, and for CS7 must enable PE.
* supported mode:
@@ -1585,21 +1586,21 @@ lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
uart_update_timeout(port, termios->c_cflag, baud);

/* wait transmit engin complete */
- while (!(lpuart32_read(sport->port.membase + UARTSTAT) & UARTSTAT_TC))
+ while (!(lpuart32_read(&sport->port, UARTSTAT) & UARTSTAT_TC))
barrier();

/* disable transmit and receive */
- lpuart32_write(old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
- sport->port.membase + UARTCTRL);
+ lpuart32_write(&sport->port, old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
+ UARTCTRL);

sbr = sport->port.uartclk / (16 * baud);
bd &= ~UARTBAUD_SBR_MASK;
bd |= sbr & UARTBAUD_SBR_MASK;
bd |= UARTBAUD_BOTHEDGE;
bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
- lpuart32_write(bd, sport->port.membase + UARTBAUD);
- lpuart32_write(modem, sport->port.membase + UARTMODIR);
- lpuart32_write(ctrl, sport->port.membase + UARTCTRL);
+ lpuart32_write(&sport->port, bd, UARTBAUD);
+ lpuart32_write(&sport->port, modem, UARTMODIR);
+ lpuart32_write(&sport->port, ctrl, UARTCTRL);
/* restore control register */

spin_unlock_irqrestore(&sport->port.lock, flags);
@@ -1702,10 +1703,10 @@ static void lpuart_console_putchar(struct uart_port *port, int ch)

static void lpuart32_console_putchar(struct uart_port *port, int ch)
{
- while (!(lpuart32_read(port->membase + UARTSTAT) & UARTSTAT_TDRE))
+ while (!(lpuart32_read(port, UARTSTAT) & UARTSTAT_TDRE))
barrier();

- lpuart32_write(ch, port->membase + UARTDATA);
+ lpuart32_write(port, ch, UARTDATA);
}

static void
@@ -1753,18 +1754,18 @@ lpuart32_console_write(struct console *co, const char *s, unsigned int count)
spin_lock_irqsave(&sport->port.lock, flags);

/* first save CR2 and then disable interrupts */
- cr = old_cr = lpuart32_read(sport->port.membase + UARTCTRL);
+ cr = old_cr = lpuart32_read(&sport->port, UARTCTRL);
cr |= (UARTCTRL_TE | UARTCTRL_RE);
cr &= ~(UARTCTRL_TIE | UARTCTRL_TCIE | UARTCTRL_RIE);
- lpuart32_write(cr, sport->port.membase + UARTCTRL);
+ lpuart32_write(&sport->port, cr, UARTCTRL);

uart_console_write(&sport->port, s, count, lpuart32_console_putchar);

/* wait for transmitter finish complete and restore CR2 */
- while (!(lpuart32_read(sport->port.membase + UARTSTAT) & UARTSTAT_TC))
+ while (!(lpuart32_read(&sport->port, UARTSTAT) & UARTSTAT_TC))
barrier();

- lpuart32_write(old_cr, sport->port.membase + UARTCTRL);
+ lpuart32_write(&sport->port, old_cr, UARTCTRL);

if (locked)
spin_unlock_irqrestore(&sport->port.lock, flags);
@@ -1830,14 +1831,14 @@ lpuart32_console_get_options(struct lpuart_port *sport, int *baud,
unsigned long cr, bd;
unsigned int sbr, uartclk, baud_raw;

- cr = lpuart32_read(sport->port.membase + UARTCTRL);
+ cr = lpuart32_read(&sport->port, UARTCTRL);
cr &= UARTCTRL_TE | UARTCTRL_RE;
if (!cr)
return;

/* ok, the port was enabled */

- cr = lpuart32_read(sport->port.membase + UARTCTRL);
+ cr = lpuart32_read(&sport->port, UARTCTRL);

*parity = 'n';
if (cr & UARTCTRL_PE) {
@@ -1852,7 +1853,7 @@ lpuart32_console_get_options(struct lpuart_port *sport, int *baud,
else
*bits = 8;

- bd = lpuart32_read(sport->port.membase + UARTBAUD);
+ bd = lpuart32_read(&sport->port, UARTBAUD);
bd &= UARTBAUD_SBR_MASK;
sbr = bd;
uartclk = clk_get_rate(sport->clk);
@@ -2096,9 +2097,9 @@ static int lpuart_suspend(struct device *dev)

if (sport->port.iotype & UPIO_MEM32BE) {
/* disable Rx/Tx and interrupts */
- temp = lpuart32_read(sport->port.membase + UARTCTRL);
+ temp = lpuart32_read(&sport->port, UARTCTRL);
temp &= ~(UARTCTRL_TE | UARTCTRL_TIE | UARTCTRL_TCIE);
- lpuart32_write(temp, sport->port.membase + UARTCTRL);
+ lpuart32_write(&sport->port, temp, UARTCTRL);
} else {
/* disable Rx/Tx and interrupts */
temp = readb(sport->port.membase + UARTCR2);
@@ -2147,10 +2148,10 @@ static int lpuart_resume(struct device *dev)

if (sport->port.iotype & UPIO_MEM32BE) {
lpuart32_setup_watermark(sport);
- temp = lpuart32_read(sport->port.membase + UARTCTRL);
+ temp = lpuart32_read(&sport->port, UARTCTRL);
temp |= (UARTCTRL_RIE | UARTCTRL_TIE | UARTCTRL_RE |
UARTCTRL_TE | UARTCTRL_ILIE);
- lpuart32_write(temp, sport->port.membase + UARTCTRL);
+ lpuart32_write(&sport->port, temp, UARTCTRL);
} else {
lpuart_setup_watermark(sport);
temp = readb(sport->port.membase + UARTCR2);
--
2.7.4

2017-06-12 16:20:49

by Aisheng Dong

[permalink] [raw]
Subject: [PATCH V3 1/7] tty: serial: lpuart: introduce lpuart_soc_data to represent SoC property

This is used to dynamically check the SoC specific lpuart properies.
Currently only the iotype is added, it functions the same as before.
With this, new chips with different iotype will be more easily added.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Stefan Agner <[email protected]>
Cc: Mingkai Hu <[email protected]>
Cc: Yangbo Lu <[email protected]>
Cc: Fugang Duan <[email protected]>
Signed-off-by: Dong Aisheng <[email protected]>

---
ChangeLog:
v2->v3:
* use standard iotype flags instead of private is_32 member
v1->v2:
* make all soc_data const
---
drivers/tty/serial/fsl_lpuart.c | 48 ++++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 15df1ba7..4daabc6 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -236,7 +236,6 @@ struct lpuart_port {
struct clk *clk;
unsigned int txfifo_size;
unsigned int rxfifo_size;
- bool lpuart32;

bool lpuart_dma_tx_use;
bool lpuart_dma_rx_use;
@@ -258,13 +257,22 @@ struct lpuart_port {
wait_queue_head_t dma_wait;
};

+struct lpuart_soc_data {
+ char iotype;
+};
+
+static const struct lpuart_soc_data vf_data = {
+ .iotype = UPIO_MEM,
+};
+
+static const struct lpuart_soc_data ls_data = {
+ .iotype = UPIO_MEM32BE,
+
+};
+
static const struct of_device_id lpuart_dt_ids[] = {
- {
- .compatible = "fsl,vf610-lpuart",
- },
- {
- .compatible = "fsl,ls1021a-lpuart",
- },
+ { .compatible = "fsl,vf610-lpuart", .data = &vf_data, },
+ { .compatible = "fsl,ls1021a-lpuart", .data = &ls_data, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
@@ -593,7 +601,7 @@ static irqreturn_t lpuart_txint(int irq, void *dev_id)

spin_lock_irqsave(&sport->port.lock, flags);
if (sport->port.x_char) {
- if (sport->lpuart32)
+ if (sport->port.iotype & UPIO_MEM32BE)
lpuart32_write(sport->port.x_char, sport->port.membase + UARTDATA);
else
writeb(sport->port.x_char, sport->port.membase + UARTDR);
@@ -601,14 +609,14 @@ static irqreturn_t lpuart_txint(int irq, void *dev_id)
}

if (uart_circ_empty(xmit) || uart_tx_stopped(&sport->port)) {
- if (sport->lpuart32)
+ if (sport->port.iotype & UPIO_MEM32BE)
lpuart32_stop_tx(&sport->port);
else
lpuart_stop_tx(&sport->port);
goto out;
}

- if (sport->lpuart32)
+ if (sport->port.iotype & UPIO_MEM32BE)
lpuart32_transmit_buffer(sport);
else
lpuart_transmit_buffer(sport);
@@ -1881,12 +1889,12 @@ static int __init lpuart_console_setup(struct console *co, char *options)
if (options)
uart_parse_options(options, &baud, &parity, &bits, &flow);
else
- if (sport->lpuart32)
+ if (sport->port.iotype & UPIO_MEM32BE)
lpuart32_console_get_options(sport, &baud, &parity, &bits);
else
lpuart_console_get_options(sport, &baud, &parity, &bits);

- if (sport->lpuart32)
+ if (sport->port.iotype & UPIO_MEM32BE)
lpuart32_setup_watermark(sport);
else
lpuart_setup_watermark(sport);
@@ -1971,6 +1979,9 @@ static struct uart_driver lpuart_reg = {

static int lpuart_probe(struct platform_device *pdev)
{
+ const struct of_device_id *of_id = of_match_device(lpuart_dt_ids,
+ &pdev->dev);
+ const struct lpuart_soc_data *sdata = of_id->data;
struct device_node *np = pdev->dev.of_node;
struct lpuart_port *sport;
struct resource *res;
@@ -1988,8 +1999,6 @@ static int lpuart_probe(struct platform_device *pdev)
return ret;
}
sport->port.line = ret;
- sport->lpuart32 = of_device_is_compatible(np, "fsl,ls1021a-lpuart");
-
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
sport->port.membase = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(sport->port.membase))
@@ -1998,15 +2007,14 @@ static int lpuart_probe(struct platform_device *pdev)
sport->port.mapbase = res->start;
sport->port.dev = &pdev->dev;
sport->port.type = PORT_LPUART;
- sport->port.iotype = UPIO_MEM;
ret = platform_get_irq(pdev, 0);
if (ret < 0) {
dev_err(&pdev->dev, "cannot obtain irq\n");
return ret;
}
sport->port.irq = ret;
-
- if (sport->lpuart32)
+ sport->port.iotype = sdata->iotype;
+ if (sport->port.iotype & UPIO_MEM32BE)
sport->port.ops = &lpuart32_pops;
else
sport->port.ops = &lpuart_pops;
@@ -2033,7 +2041,7 @@ static int lpuart_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, &sport->port);

- if (sport->lpuart32)
+ if (sport->port.iotype & UPIO_MEM32BE)
lpuart_reg.cons = LPUART32_CONSOLE;
else
lpuart_reg.cons = LPUART_CONSOLE;
@@ -2086,7 +2094,7 @@ static int lpuart_suspend(struct device *dev)
struct lpuart_port *sport = dev_get_drvdata(dev);
unsigned long temp;

- if (sport->lpuart32) {
+ if (sport->port.iotype & UPIO_MEM32BE) {
/* disable Rx/Tx and interrupts */
temp = lpuart32_read(sport->port.membase + UARTCTRL);
temp &= ~(UARTCTRL_TE | UARTCTRL_TIE | UARTCTRL_TCIE);
@@ -2137,7 +2145,7 @@ static int lpuart_resume(struct device *dev)
if (sport->port.suspended && !sport->port.irq_wake)
clk_prepare_enable(sport->clk);

- if (sport->lpuart32) {
+ if (sport->port.iotype & UPIO_MEM32BE) {
lpuart32_setup_watermark(sport);
temp = lpuart32_read(sport->port.membase + UARTCTRL);
temp |= (UARTCTRL_RIE | UARTCTRL_TIE | UARTCTRL_RE |
--
2.7.4

2017-06-12 15:37:48

by Aisheng Dong

[permalink] [raw]
Subject: [PATCH V3 4/7] dt-bindings: serial: fsl-lpuart: add i.MX7ULP support

The lpuart of imx7ulp is basically the same as ls1021a. It's also
32 bit width register, but unlike ls1021a, it's little endian.
Besides that, imx7ulp lpuart has a minor different register layout
from ls1021a.

Cc: [email protected]
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Stefan Agner <[email protected]>
Cc: Mingkai Hu <[email protected]>
Cc: Yangbo Lu <[email protected]>
Acked-by: Rob Herring <[email protected]>
Acked-by: Fugang Duan <[email protected]>
Signed-off-by: Dong Aisheng <[email protected]>
---
Documentation/devicetree/bindings/serial/fsl-lpuart.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/fsl-lpuart.txt b/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
index c95005e..a1252a0 100644
--- a/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
+++ b/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
@@ -6,6 +6,8 @@ Required properties:
on Vybrid vf610 SoC with 8-bit register organization
- "fsl,ls1021a-lpuart" for lpuart compatible with the one integrated
on LS1021A SoC with 32-bit big-endian register organization
+ - "fsl,imx7ulp-lpuart" for lpuart compatible with the one integrated
+ on i.MX7ULP SoC with 32-bit little-endian register organization
- reg : Address and length of the register set for the device
- interrupts : Should contain uart interrupt
- clocks : phandle + clock specifier pairs, one for each entry in clock-names
--
2.7.4

2017-06-12 16:21:42

by Aisheng Dong

[permalink] [raw]
Subject: [PATCH V3 6/7] tty: serial: lpuart: add earlycon support for imx7ulp

earlycon is executed quite early before the device tree probe,
so we need correctly initialize the port membase and iotype for
imx7ulp during early console setup before using.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Stefan Agner <[email protected]>
Cc: Mingkai Hu <[email protected]>
Cc: Yangbo Lu <[email protected]>
Acked-by: Fugang Duan <[email protected]>
Signed-off-by: Dong Aisheng <[email protected]>

---
Change Log:
v2->v3:
* use standard port->iotype to represent endians
v1->v2:
* updated due to lpuart_reg_off removed
---
drivers/tty/serial/fsl_lpuart.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 9d05e53..6a725cb 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1982,8 +1982,21 @@ static int __init lpuart32_early_console_setup(struct earlycon_device *device,
return 0;
}

+static int __init lpuart32_imx_early_console_setup(struct earlycon_device *device,
+ const char *opt)
+{
+ if (!device->port.membase)
+ return -ENODEV;
+
+ device->port.iotype = UPIO_MEM32;
+ device->port.membase += IMX_REG_OFF;
+ device->con->write = lpuart32_early_write;
+
+ return 0;
+}
OF_EARLYCON_DECLARE(lpuart, "fsl,vf610-lpuart", lpuart_early_console_setup);
OF_EARLYCON_DECLARE(lpuart32, "fsl,ls1021a-lpuart", lpuart32_early_console_setup);
+OF_EARLYCON_DECLARE(lpuart32, "fsl,imx7ulp-lpuart", lpuart32_imx_early_console_setup);
EARLYCON_DECLARE(lpuart, lpuart_early_console_setup);
EARLYCON_DECLARE(lpuart32, lpuart32_early_console_setup);

--
2.7.4

2017-06-12 16:22:03

by Aisheng Dong

[permalink] [raw]
Subject: [PATCH V3 5/7] tty: serial: lpuart: add imx7ulp support

The lpuart of imx7ulp is basically the same as ls1021a. It's also
32 bit width register, but unlike ls1021a, it's little endian.
Besides that, imx7ulp lpuart has a minor different register layout
from ls1021a that it has four extra registers (verid, param, global,
pincfg) located at the beginning of register map, which are currently
not used by the driver and less to be used later.

To ease the register difference handling, we add a reg_off member
in lpuart_soc_data structure to represent if the normal
lpuart32_{read|write} requires plus a offset to hide the issue.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Stefan Agner <[email protected]>
Cc: Mingkai Hu <[email protected]>
Cc: Yangbo Lu <[email protected]>
Cc: Fugang Duan <[email protected]>
Signed-off-by: Dong Aisheng <[email protected]>

---
ChangeLog:
v2->v3:
* use standard port->iotype to represent the endians.
v1->v2:
* remove lpuart_reg_off according to Stefan's suggestion
---
drivers/tty/serial/fsl_lpuart.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index bbf47a0..9d05e53 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -231,6 +231,9 @@
#define DEV_NAME "ttyLP"
#define UART_NR 6

+/* IMX lpuart has four extra unused regs located at the beginning */
+#define IMX_REG_OFF 0x10
+
struct lpuart_port {
struct uart_port port;
struct clk *clk;
@@ -259,6 +262,7 @@ struct lpuart_port {

struct lpuart_soc_data {
char iotype;
+ u8 reg_off;
};

static const struct lpuart_soc_data vf_data = {
@@ -267,12 +271,17 @@ static const struct lpuart_soc_data vf_data = {

static const struct lpuart_soc_data ls_data = {
.iotype = UPIO_MEM32BE,
+};

+static struct lpuart_soc_data imx_data = {
+ .iotype = UPIO_MEM32,
+ .reg_off = IMX_REG_OFF,
};

static const struct of_device_id lpuart_dt_ids[] = {
{ .compatible = "fsl,vf610-lpuart", .data = &vf_data, },
{ .compatible = "fsl,ls1021a-lpuart", .data = &ls_data, },
+ { .compatible = "fsl,imx7ulp-lpuart", .data = &imx_data, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
@@ -2020,6 +2029,7 @@ static int lpuart_probe(struct platform_device *pdev)
if (IS_ERR(sport->port.membase))
return PTR_ERR(sport->port.membase);

+ sport->port.membase += sdata->reg_off;
sport->port.mapbase = res->start;
sport->port.dev = &pdev->dev;
sport->port.type = PORT_LPUART;
--
2.7.4

2017-06-12 17:49:43

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH V3 1/7] tty: serial: lpuart: introduce lpuart_soc_data to represent SoC property

On Mon, Jun 12, 2017 at 6:37 PM, Dong Aisheng <[email protected]> wrote:
> This is used to dynamically check the SoC specific lpuart properies.
> Currently only the iotype is added, it functions the same as before.
> With this, new chips with different iotype will be more easily added.


> +struct lpuart_soc_data {
> + char iotype;
> +};
> +
> +static const struct lpuart_soc_data vf_data = {
> + .iotype = UPIO_MEM,
> +};
> +
> +static const struct lpuart_soc_data ls_data = {
> + .iotype = UPIO_MEM32BE,

> +

Redundant.

> +};

And now most interesting part...

> - if (sport->lpuart32)
> + if (sport->port.iotype & UPIO_MEM32BE)
> lpuart32_write(sport->port.x_char, sport->port.membase + UARTDATA);
> else
> writeb(sport->port.x_char, sport->port.membase + UARTDR);

> - if (sport->lpuart32)
> + if (sport->port.iotype & UPIO_MEM32BE)
> lpuart32_stop_tx(&sport->port);
> else
> lpuart_stop_tx(&sport->port);

> - if (sport->lpuart32)
> + if (sport->port.iotype & UPIO_MEM32BE)
> lpuart32_transmit_buffer(sport);
> else
> lpuart_transmit_buffer(sport);

> - if (sport->lpuart32)
> + if (sport->port.iotype & UPIO_MEM32BE)
> lpuart32_console_get_options(sport, &baud, &parity, &bits);
> else
> lpuart_console_get_options(sport, &baud, &parity, &bits);

> - if (sport->lpuart32)
> + if (sport->port.iotype & UPIO_MEM32BE)
> lpuart32_setup_watermark(sport);
> else
> lpuart_setup_watermark(sport);

> - if (sport->lpuart32)
> + sport->port.iotype = sdata->iotype;
> + if (sport->port.iotype & UPIO_MEM32BE)
> sport->port.ops = &lpuart32_pops;
> else
> sport->port.ops = &lpuart_pops;

> - if (sport->lpuart32)
> + if (sport->port.iotype & UPIO_MEM32BE)
> lpuart_reg.cons = LPUART32_CONSOLE;
> else
> lpuart_reg.cons = LPUART_CONSOLE;

...all above since you introduced nice struct, can get rid of conditionals.
Instead it might be a members of the struct above.

(I dunno if it's good to have in this patch, but at list a follow up
could be nice to have)

> - if (sport->lpuart32) {
> + if (sport->port.iotype & UPIO_MEM32BE) {
> /* disable Rx/Tx and interrupts */
> temp = lpuart32_read(sport->port.membase + UARTCTRL);
> temp &= ~(UARTCTRL_TE | UARTCTRL_TIE | UARTCTRL_TCIE);

> - if (sport->lpuart32) {
> + if (sport->port.iotype & UPIO_MEM32BE) {
> lpuart32_setup_watermark(sport);
> temp = lpuart32_read(sport->port.membase + UARTCTRL);
> temp |= (UARTCTRL_RIE | UARTCTRL_TIE | UARTCTRL_RE |

Above are questionable, might be not need to convert them.

So, in any case above is a sighting which you could address (separately).

--
With Best Regards,
Andy Shevchenko

2017-06-13 00:29:13

by kernel test robot

[permalink] [raw]
Subject: [PATCH] tty: serial: lpuart: fix semicolon.cocci warnings

drivers/tty/serial/fsl_lpuart.c:305:2-3: Unneeded semicolon


Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Dong Aisheng <[email protected]>
Signed-off-by: Fengguang Wu <[email protected]>
---

fsl_lpuart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -302,7 +302,7 @@ static inline void lpuart32_write(struct
case UPIO_MEM32BE:
iowrite32be(val, port->membase + off);
break;
- };
+ }
}

static void lpuart_stop_tx(struct uart_port *port)

2017-06-13 00:29:27

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH V3 3/7] tty: serial: lpuart: add little endian 32 bit register support

Hi Dong,

[auto build test WARNING on tty/tty-testing]
[also build test WARNING on v4.12-rc5 next-20170609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Dong-Aisheng/tty-serial-lpuart-add-imx7ulp-support/20170613-045714
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing


coccinelle warnings: (new ones prefixed by >>)

>> drivers/tty/serial/fsl_lpuart.c:305:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

2017-06-13 02:28:41

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH V3 1/7] tty: serial: lpuart: introduce lpuart_soc_data to represent SoC property

On Mon, Jun 12, 2017 at 08:49:36PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 12, 2017 at 6:37 PM, Dong Aisheng <[email protected]> wrote:
> > This is used to dynamically check the SoC specific lpuart properies.
> > Currently only the iotype is added, it functions the same as before.
> > With this, new chips with different iotype will be more easily added.
>
>
> > +struct lpuart_soc_data {
> > + char iotype;
> > +};
> > +
> > +static const struct lpuart_soc_data vf_data = {
> > + .iotype = UPIO_MEM,
> > +};
> > +
> > +static const struct lpuart_soc_data ls_data = {
> > + .iotype = UPIO_MEM32BE,
>
> > +
>
> Redundant.

My mistake to introduce one more extra blank line...

>
> > +};
>
> And now most interesting part...
>
> > - if (sport->lpuart32)
> > + if (sport->port.iotype & UPIO_MEM32BE)
> > lpuart32_write(sport->port.x_char, sport->port.membase + UARTDATA);
> > else
> > writeb(sport->port.x_char, sport->port.membase + UARTDR);
>
> > - if (sport->lpuart32)
> > + if (sport->port.iotype & UPIO_MEM32BE)
> > lpuart32_stop_tx(&sport->port);
> > else
> > lpuart_stop_tx(&sport->port);
>
> > - if (sport->lpuart32)
> > + if (sport->port.iotype & UPIO_MEM32BE)
> > lpuart32_transmit_buffer(sport);
> > else
> > lpuart_transmit_buffer(sport);
>
> > - if (sport->lpuart32)
> > + if (sport->port.iotype & UPIO_MEM32BE)
> > lpuart32_console_get_options(sport, &baud, &parity, &bits);
> > else
> > lpuart_console_get_options(sport, &baud, &parity, &bits);
>
> > - if (sport->lpuart32)
> > + if (sport->port.iotype & UPIO_MEM32BE)
> > lpuart32_setup_watermark(sport);
> > else
> > lpuart_setup_watermark(sport);
>
> > - if (sport->lpuart32)
> > + sport->port.iotype = sdata->iotype;
> > + if (sport->port.iotype & UPIO_MEM32BE)
> > sport->port.ops = &lpuart32_pops;
> > else
> > sport->port.ops = &lpuart_pops;
>
> > - if (sport->lpuart32)
> > + if (sport->port.iotype & UPIO_MEM32BE)
> > lpuart_reg.cons = LPUART32_CONSOLE;
> > else
> > lpuart_reg.cons = LPUART_CONSOLE;
>
> ...all above since you introduced nice struct, can get rid of conditionals.
> Instead it might be a members of the struct above.
>
> (I dunno if it's good to have in this patch, but at list a follow up
> could be nice to have)
>

Yes, to clean up all conditionals, much more things need to be done,
so a separate follow up patch may be better.

This patch only address iotype which is just the same as before.

> > - if (sport->lpuart32) {
> > + if (sport->port.iotype & UPIO_MEM32BE) {
> > /* disable Rx/Tx and interrupts */
> > temp = lpuart32_read(sport->port.membase + UARTCTRL);
> > temp &= ~(UARTCTRL_TE | UARTCTRL_TIE | UARTCTRL_TCIE);
>
> > - if (sport->lpuart32) {
> > + if (sport->port.iotype & UPIO_MEM32BE) {
> > lpuart32_setup_watermark(sport);
> > temp = lpuart32_read(sport->port.membase + UARTCTRL);
> > temp |= (UARTCTRL_RIE | UARTCTRL_TIE | UARTCTRL_RE |
>
> Above are questionable, might be not need to convert them.
>
> So, in any case above is a sighting which you could address (separately).

Yes, seems not a easy convert which can be investigated later.

Thanks for the review.

Regards
Dong Aisheng

>
> --
> With Best Regards,
> Andy Shevchenko

2017-06-13 02:38:10

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH] tty: serial: lpuart: fix semicolon.cocci warnings

Hi Fengguang,

On Tue, Jun 13, 2017 at 08:28:41AM +0800, kbuild test robot wrote:
> drivers/tty/serial/fsl_lpuart.c:305:2-3: Unneeded semicolon
>
>
> Remove unneeded semicolon.
>
> Generated by: scripts/coccinelle/misc/semicolon.cocci
>
> CC: Dong Aisheng <[email protected]>
> Signed-off-by: Fengguang Wu <[email protected]>

Seems not catched by checkpatch.

> ---
>
> fsl_lpuart.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -302,7 +302,7 @@ static inline void lpuart32_write(struct
> case UPIO_MEM32BE:
> iowrite32be(val, port->membase + off);
> break;
> - };
> + }

lpuart32_read needs the same fixing.

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index a66ed23..ed0bf18 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -288,7 +288,7 @@ static inline u32 lpuart32_read(struct uart_port *port, u32 off)
return ioread32be(port->membase + off);
default:
return 0;
- };
+ }
}

static inline void lpuart32_write(struct uart_port *port, u32 val,
@@ -301,7 +301,7 @@ static inline void lpuart32_write(struct uart_port *port, u32 val,
case UPIO_MEM32BE:
iowrite32be(val, port->membase + off);
break;
- };
+ }
}

Thanks for the catching.

Regards
Dong Aisheng

> }
>
> static void lpuart_stop_tx(struct uart_port *port)

2017-06-13 03:02:39

by Andy Duan

[permalink] [raw]
Subject: RE: [PATCH V3 5/7] tty: serial: lpuart: add imx7ulp support

From: Dong Aisheng <[email protected]> Sent: Monday, June 12, 2017 11:37 PM
>The lpuart of imx7ulp is basically the same as ls1021a. It's also
>32 bit width register, but unlike ls1021a, it's little endian.
>Besides that, imx7ulp lpuart has a minor different register layout from ls1021a
>that it has four extra registers (verid, param, global,
>pincfg) located at the beginning of register map, which are currently not used
>by the driver and less to be used later.
>
>To ease the register difference handling, we add a reg_off member in
>lpuart_soc_data structure to represent if the normal lpuart32_{read|write}
>requires plus a offset to hide the issue.
>
>Cc: Greg Kroah-Hartman <[email protected]>
>Cc: Jiri Slaby <[email protected]>
>Cc: Stefan Agner <[email protected]>
>Cc: Mingkai Hu <[email protected]>
>Cc: Yangbo Lu <[email protected]>
>Cc: Fugang Duan <[email protected]>
>Signed-off-by: Dong Aisheng <[email protected]>
>
>---
>ChangeLog:
>v2->v3:
> * use standard port->iotype to represent the endians.
>v1->v2:
> * remove lpuart_reg_off according to Stefan's suggestion
>---
> drivers/tty/serial/fsl_lpuart.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index
>bbf47a0..9d05e53 100644
>--- a/drivers/tty/serial/fsl_lpuart.c
>+++ b/drivers/tty/serial/fsl_lpuart.c
>@@ -231,6 +231,9 @@
> #define DEV_NAME "ttyLP"
> #define UART_NR 6
>
>+/* IMX lpuart has four extra unused regs located at the beginning */
>+#define IMX_REG_OFF 0x10
>+
> struct lpuart_port {
> struct uart_port port;
> struct clk *clk;
>@@ -259,6 +262,7 @@ struct lpuart_port {
>
> struct lpuart_soc_data {
> char iotype;
>+ u8 reg_off;
> };
>
> static const struct lpuart_soc_data vf_data = { @@ -267,12 +271,17 @@ static
>const struct lpuart_soc_data vf_data = {
>
> static const struct lpuart_soc_data ls_data = {
> .iotype = UPIO_MEM32BE,
>+};
>
>+static struct lpuart_soc_data imx_data = {
>+ .iotype = UPIO_MEM32,
>+ .reg_off = IMX_REG_OFF,
> };
>
> static const struct of_device_id lpuart_dt_ids[] = {
> { .compatible = "fsl,vf610-lpuart", .data = &vf_data, },
> { .compatible = "fsl,ls1021a-lpuart", .data = &ls_data, },
>+ { .compatible = "fsl,imx7ulp-lpuart", .data = &imx_data, },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, lpuart_dt_ids); @@ -2020,6 +2029,7 @@ static
>int lpuart_probe(struct platform_device *pdev)
> if (IS_ERR(sport->port.membase))
> return PTR_ERR(sport->port.membase);
>
>+ sport->port.membase += sdata->reg_off;
> sport->port.mapbase = res->start;

Also update the mapbase.

> sport->port.dev = &pdev->dev;
> sport->port.type = PORT_LPUART;
>--
>2.7.4

2017-06-13 03:09:23

by Andy Duan

[permalink] [raw]
Subject: RE: [PATCH V3 3/7] tty: serial: lpuart: add little endian 32 bit register support

From: Dong Aisheng <[email protected]> Sent: Monday, June 12, 2017 11:37 PM
>To: [email protected]
>Cc: [email protected]; [email protected];
>[email protected]; [email protected]; Andy Duan
><[email protected]>; [email protected]; Mingkai Hu
><[email protected]>; Y.b. Lu <[email protected]>;
>[email protected]; [email protected];
>[email protected]; A.S. Dong <[email protected]>
>Subject: [PATCH V3 3/7] tty: serial: lpuart: add little endian 32 bit register
>support
>
>Use standard port->iotype to distinguish endian difference. Note as we
>read/write register by checking iotype dynamically, we need to initialize the
>iotype correctly for earlycon as well to avoid a break.
>
>Cc: Greg Kroah-Hartman <[email protected]>
>Cc: Jiri Slaby <[email protected]> (supporter:TTY LAYER)
>Cc: Stefan Agner <[email protected]>
>Cc: Mingkai Hu <[email protected]>
>Cc: Yangbo Lu <[email protected]>
>Cc: Fugang Duan <[email protected]>
>Signed-off-by: Dong Aisheng <[email protected]>
>
>ChangeLog:
>v2->v3:
> * Instead of using global var, use standard port->iotype to distinguish
> endian difference.
>v1->v2:
> * No changes
>---
> drivers/tty/serial/fsl_lpuart.c | 43 +++++++++++++++++++++++++++----------
>----
> 1 file changed, 29 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index
>ed9db2f..bbf47a0 100644
>--- a/drivers/tty/serial/fsl_lpuart.c
>+++ b/drivers/tty/serial/fsl_lpuart.c
>@@ -280,15 +280,29 @@ MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
> /* Forward declare this for the dma callbacks*/ static void
>lpuart_dma_tx_complete(void *arg);
>
>-static inline u32 lpuart32_read(struct uart_port *port, u32 reg_off) -{
>- return ioread32be(port->membase + reg_off);
>+static inline u32 lpuart32_read(struct uart_port *port, u32 off) {
>+ switch (port->iotype) {
>+ case UPIO_MEM32:
>+ return readl(port->membase + off);
>+ case UPIO_MEM32BE:
>+ return ioread32be(port->membase + off);
>+ default:
>+ return 0;
>+ };
> }
>
> static inline void lpuart32_write(struct uart_port *port, u32 val,
>- u32 reg_off)
>+ u32 off)
> {
>- iowrite32be(val, port->membase + reg_off);
>+ switch (port->iotype) {
>+ case UPIO_MEM32:
>+ writel(val, port->membase + off);
>+ break;
>+ case UPIO_MEM32BE:
>+ iowrite32be(val, port->membase + off);
>+ break;
>+ };
> }
>
> static void lpuart_stop_tx(struct uart_port *port) @@ -602,7 +616,7 @@
>static irqreturn_t lpuart_txint(int irq, void *dev_id)
>
> spin_lock_irqsave(&sport->port.lock, flags);
> if (sport->port.x_char) {
>- if (sport->port.iotype & UPIO_MEM32BE)
>+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> lpuart32_write(&sport->port, sport->port.x_char,
>UARTDATA);
> else
> writeb(sport->port.x_char, sport->port.membase +
>UARTDR); @@ -610,14 +624,14 @@ static irqreturn_t lpuart_txint(int irq, void
>*dev_id)
> }
>
> if (uart_circ_empty(xmit) || uart_tx_stopped(&sport->port)) {
>- if (sport->port.iotype & UPIO_MEM32BE)
>+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))

Can use one macro instead of sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE ?

> lpuart32_stop_tx(&sport->port);
> else
> lpuart_stop_tx(&sport->port);
> goto out;
> }
>
>- if (sport->port.iotype & UPIO_MEM32BE)
>+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> lpuart32_transmit_buffer(sport);
> else
> lpuart_transmit_buffer(sport);
>@@ -1890,12 +1904,12 @@ static int __init lpuart_console_setup(struct
>console *co, char *options)
> if (options)
> uart_parse_options(options, &baud, &parity, &bits, &flow);
> else
>- if (sport->port.iotype & UPIO_MEM32BE)
>+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> lpuart32_console_get_options(sport, &baud, &parity,
>&bits);
> else
> lpuart_console_get_options(sport, &baud, &parity,
>&bits);
>
>- if (sport->port.iotype & UPIO_MEM32BE)
>+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> lpuart32_setup_watermark(sport);
> else
> lpuart_setup_watermark(sport);
>@@ -1954,6 +1968,7 @@ static int __init lpuart32_early_console_setup(struct
>earlycon_device *device,
> if (!device->port.membase)
> return -ENODEV;
>
>+ device->port.iotype = UPIO_MEM32BE;
> device->con->write = lpuart32_early_write;
> return 0;
> }
>@@ -2015,7 +2030,7 @@ static int lpuart_probe(struct platform_device *pdev)
> }
> sport->port.irq = ret;
> sport->port.iotype = sdata->iotype;
>- if (sport->port.iotype & UPIO_MEM32BE)
>+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> sport->port.ops = &lpuart32_pops;
> else
> sport->port.ops = &lpuart_pops;
>@@ -2042,7 +2057,7 @@ static int lpuart_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, &sport->port);
>
>- if (sport->port.iotype & UPIO_MEM32BE)
>+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> lpuart_reg.cons = LPUART32_CONSOLE;
> else
> lpuart_reg.cons = LPUART_CONSOLE;
>@@ -2095,7 +2110,7 @@ static int lpuart_suspend(struct device *dev)
> struct lpuart_port *sport = dev_get_drvdata(dev);
> unsigned long temp;
>
>- if (sport->port.iotype & UPIO_MEM32BE) {
>+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE)) {
> /* disable Rx/Tx and interrupts */
> temp = lpuart32_read(&sport->port, UARTCTRL);
> temp &= ~(UARTCTRL_TE | UARTCTRL_TIE | UARTCTRL_TCIE);
>@@ -2146,7 +2161,7 @@ static int lpuart_resume(struct device *dev)
> if (sport->port.suspended && !sport->port.irq_wake)
> clk_prepare_enable(sport->clk);
>
>- if (sport->port.iotype & UPIO_MEM32BE) {
>+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE)) {
> lpuart32_setup_watermark(sport);
> temp = lpuart32_read(&sport->port, UARTCTRL);
> temp |= (UARTCTRL_RIE | UARTCTRL_TIE | UARTCTRL_RE |
>--
>2.7.4

2017-06-13 03:27:23

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH V3 3/7] tty: serial: lpuart: add little endian 32 bit register support

> -----Original Message-----
> From: Andy Duan
> Sent: Tuesday, June 13, 2017 11:09 AM
> To: A.s. Dong; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Mingkai Hu;
> Y.b. Lu; [email protected]; [email protected];
> [email protected]; A.s. Dong
> Subject: RE: [PATCH V3 3/7] tty: serial: lpuart: add little endian 32 bit
> register support
>
> From: Dong Aisheng <[email protected]> Sent: Monday, June 12, 2017
> 11:37 PM
> >To: [email protected]
> >Cc: [email protected]; [email protected];
> >[email protected]; [email protected]; Andy Duan
> ><[email protected]>; [email protected]; Mingkai Hu
> ><[email protected]>; Y.b. Lu <[email protected]>;
> >[email protected]; [email protected];
> >[email protected]; A.S. Dong <[email protected]>
> >Subject: [PATCH V3 3/7] tty: serial: lpuart: add little endian 32 bit
> >register support
> >
> >Use standard port->iotype to distinguish endian difference. Note as we
> >read/write register by checking iotype dynamically, we need to
> >initialize the iotype correctly for earlycon as well to avoid a break.
> >
> >Cc: Greg Kroah-Hartman <[email protected]>
> >Cc: Jiri Slaby <[email protected]> (supporter:TTY LAYER)
> >Cc: Stefan Agner <[email protected]>
> >Cc: Mingkai Hu <[email protected]>
> >Cc: Yangbo Lu <[email protected]>
> >Cc: Fugang Duan <[email protected]>
> >Signed-off-by: Dong Aisheng <[email protected]>
> >
> >ChangeLog:
> >v2->v3:
> > * Instead of using global var, use standard port->iotype to distinguish
> > endian difference.
> >v1->v2:
> > * No changes
> >---
> > drivers/tty/serial/fsl_lpuart.c | 43
> >+++++++++++++++++++++++++++----------
> >----
> > 1 file changed, 29 insertions(+), 14 deletions(-)
> >
> >diff --git a/drivers/tty/serial/fsl_lpuart.c
> >b/drivers/tty/serial/fsl_lpuart.c index
> >ed9db2f..bbf47a0 100644
> >--- a/drivers/tty/serial/fsl_lpuart.c
> >+++ b/drivers/tty/serial/fsl_lpuart.c
> >@@ -280,15 +280,29 @@ MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
> > /* Forward declare this for the dma callbacks*/ static void
> >lpuart_dma_tx_complete(void *arg);
> >
> >-static inline u32 lpuart32_read(struct uart_port *port, u32 reg_off) -{
> >- return ioread32be(port->membase + reg_off);
> >+static inline u32 lpuart32_read(struct uart_port *port, u32 off) {
> >+ switch (port->iotype) {
> >+ case UPIO_MEM32:
> >+ return readl(port->membase + off);
> >+ case UPIO_MEM32BE:
> >+ return ioread32be(port->membase + off);
> >+ default:
> >+ return 0;
> >+ };
> > }
> >
> > static inline void lpuart32_write(struct uart_port *port, u32 val,
> >- u32 reg_off)
> >+ u32 off)
> > {
> >- iowrite32be(val, port->membase + reg_off);
> >+ switch (port->iotype) {
> >+ case UPIO_MEM32:
> >+ writel(val, port->membase + off);
> >+ break;
> >+ case UPIO_MEM32BE:
> >+ iowrite32be(val, port->membase + off);
> >+ break;
> >+ };
> > }
> >
> > static void lpuart_stop_tx(struct uart_port *port) @@ -602,7 +616,7 @@
> >static irqreturn_t lpuart_txint(int irq, void *dev_id)
> >
> > spin_lock_irqsave(&sport->port.lock, flags);
> > if (sport->port.x_char) {
> >- if (sport->port.iotype & UPIO_MEM32BE)
> >+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> > lpuart32_write(&sport->port, sport->port.x_char,
> UARTDATA);
> > else
> > writeb(sport->port.x_char, sport->port.membase + UARTDR);
> @@
> >-610,14 +624,14 @@ static irqreturn_t lpuart_txint(int irq, void
> >*dev_id)
> > }
> >
> > if (uart_circ_empty(xmit) || uart_tx_stopped(&sport->port)) {
> >- if (sport->port.iotype & UPIO_MEM32BE)
> >+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
>
> Can use one macro instead of sport->port.iotype & (UPIO_MEM32 |
> UPIO_MEM32BE ?
>

UPIO_MEM32 and UPIO_MEM32BE are serial core definitions.
If we use one macro for it, then the macro seems be better in serial core.
But I don't think it's quite necessary.

Explicit using also make the code look clearer.

Regards
Dong Aisheng

> > lpuart32_stop_tx(&sport->port);
> > else
> > lpuart_stop_tx(&sport->port);
> > goto out;
> > }
> >
> >- if (sport->port.iotype & UPIO_MEM32BE)
> >+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> > lpuart32_transmit_buffer(sport);
> > else
> > lpuart_transmit_buffer(sport);
> >@@ -1890,12 +1904,12 @@ static int __init lpuart_console_setup(struct
> >console *co, char *options)
> > if (options)
> > uart_parse_options(options, &baud, &parity, &bits, &flow);
> > else
> >- if (sport->port.iotype & UPIO_MEM32BE)
> >+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> > lpuart32_console_get_options(sport, &baud, &parity,
> &bits);
> > else
> > lpuart_console_get_options(sport, &baud, &parity, &bits);
> >
> >- if (sport->port.iotype & UPIO_MEM32BE)
> >+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> > lpuart32_setup_watermark(sport);
> > else
> > lpuart_setup_watermark(sport);
> >@@ -1954,6 +1968,7 @@ static int __init
> >lpuart32_early_console_setup(struct
> >earlycon_device *device,
> > if (!device->port.membase)
> > return -ENODEV;
> >
> >+ device->port.iotype = UPIO_MEM32BE;
> > device->con->write = lpuart32_early_write;
> > return 0;
> > }
> >@@ -2015,7 +2030,7 @@ static int lpuart_probe(struct platform_device
> *pdev)
> > }
> > sport->port.irq = ret;
> > sport->port.iotype = sdata->iotype;
> >- if (sport->port.iotype & UPIO_MEM32BE)
> >+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> > sport->port.ops = &lpuart32_pops;
> > else
> > sport->port.ops = &lpuart_pops;
> >@@ -2042,7 +2057,7 @@ static int lpuart_probe(struct platform_device
> >*pdev)
> >
> > platform_set_drvdata(pdev, &sport->port);
> >
> >- if (sport->port.iotype & UPIO_MEM32BE)
> >+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE))
> > lpuart_reg.cons = LPUART32_CONSOLE;
> > else
> > lpuart_reg.cons = LPUART_CONSOLE;
> >@@ -2095,7 +2110,7 @@ static int lpuart_suspend(struct device *dev)
> > struct lpuart_port *sport = dev_get_drvdata(dev);
> > unsigned long temp;
> >
> >- if (sport->port.iotype & UPIO_MEM32BE) {
> >+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE)) {
> > /* disable Rx/Tx and interrupts */
> > temp = lpuart32_read(&sport->port, UARTCTRL);
> > temp &= ~(UARTCTRL_TE | UARTCTRL_TIE | UARTCTRL_TCIE); @@ -
> 2146,7
> >+2161,7 @@ static int lpuart_resume(struct device *dev)
> > if (sport->port.suspended && !sport->port.irq_wake)
> > clk_prepare_enable(sport->clk);
> >
> >- if (sport->port.iotype & UPIO_MEM32BE) {
> >+ if (sport->port.iotype & (UPIO_MEM32 | UPIO_MEM32BE)) {
> > lpuart32_setup_watermark(sport);
> > temp = lpuart32_read(&sport->port, UARTCTRL);
> > temp |= (UARTCTRL_RIE | UARTCTRL_TIE | UARTCTRL_RE |
> >--
> >2.7.4

2017-06-13 03:32:44

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH V3 5/7] tty: serial: lpuart: add imx7ulp support

> -----Original Message-----
> From: Andy Duan
> Sent: Tuesday, June 13, 2017 11:02 AM
> To: A.s. Dong; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Mingkai Hu;
> Y.b. Lu; [email protected]; [email protected];
> [email protected]; A.s. Dong
> Subject: RE: [PATCH V3 5/7] tty: serial: lpuart: add imx7ulp support
>
> From: Dong Aisheng <[email protected]> Sent: Monday, June 12, 2017
> 11:37 PM
> >The lpuart of imx7ulp is basically the same as ls1021a. It's also
> >32 bit width register, but unlike ls1021a, it's little endian.
> >Besides that, imx7ulp lpuart has a minor different register layout from
> >ls1021a that it has four extra registers (verid, param, global,
> >pincfg) located at the beginning of register map, which are currently
> >not used by the driver and less to be used later.
> >
> >To ease the register difference handling, we add a reg_off member in
> >lpuart_soc_data structure to represent if the normal
> >lpuart32_{read|write} requires plus a offset to hide the issue.
> >
> >Cc: Greg Kroah-Hartman <[email protected]>
> >Cc: Jiri Slaby <[email protected]>
> >Cc: Stefan Agner <[email protected]>
> >Cc: Mingkai Hu <[email protected]>
> >Cc: Yangbo Lu <[email protected]>
> >Cc: Fugang Duan <[email protected]>
> >Signed-off-by: Dong Aisheng <[email protected]>
> >
> >---
> >ChangeLog:
> >v2->v3:
> > * use standard port->iotype to represent the endians.
> >v1->v2:
> > * remove lpuart_reg_off according to Stefan's suggestion
> >---
> > drivers/tty/serial/fsl_lpuart.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> >diff --git a/drivers/tty/serial/fsl_lpuart.c
> >b/drivers/tty/serial/fsl_lpuart.c index
> >bbf47a0..9d05e53 100644
> >--- a/drivers/tty/serial/fsl_lpuart.c
> >+++ b/drivers/tty/serial/fsl_lpuart.c
> >@@ -231,6 +231,9 @@
> > #define DEV_NAME "ttyLP"
> > #define UART_NR 6
> >
> >+/* IMX lpuart has four extra unused regs located at the beginning */
> >+#define IMX_REG_OFF 0x10
> >+
> > struct lpuart_port {
> > struct uart_port port;
> > struct clk *clk;
> >@@ -259,6 +262,7 @@ struct lpuart_port {
> >
> > struct lpuart_soc_data {
> > char iotype;
> >+ u8 reg_off;
> > };
> >
> > static const struct lpuart_soc_data vf_data = { @@ -267,12 +271,17 @@
> >static const struct lpuart_soc_data vf_data = {
> >
> > static const struct lpuart_soc_data ls_data = {
> > .iotype = UPIO_MEM32BE,
> >+};
> >
> >+static struct lpuart_soc_data imx_data = {
> >+ .iotype = UPIO_MEM32,
> >+ .reg_off = IMX_REG_OFF,
> > };
> >
> > static const struct of_device_id lpuart_dt_ids[] = {
> > { .compatible = "fsl,vf610-lpuart", .data = &vf_data, },
> > { .compatible = "fsl,ls1021a-lpuart", .data = &ls_data, },
> >+ { .compatible = "fsl,imx7ulp-lpuart", .data = &imx_data, },
> > { /* sentinel */ }
> > };
> > MODULE_DEVICE_TABLE(of, lpuart_dt_ids); @@ -2020,6 +2029,7 @@ static
> >int lpuart_probe(struct platform_device *pdev)
> > if (IS_ERR(sport->port.membase))
> > return PTR_ERR(sport->port.membase);
> >
> >+ sport->port.membase += sdata->reg_off;
> > sport->port.mapbase = res->start;
>
> Also update the mapbase.
>

The idea behind is only do the quirk for io remapped address.
Mapbase(physical address) is not needed for lpuart32 currently.

Probably it could be changed when lpuart32 DMA function is added.

Regards
Dong Aisheng

> > sport->port.dev = &pdev->dev;
> > sport->port.type = PORT_LPUART;
> >--
> >2.7.4