On 2017-05-09 23:14, Dong Aisheng wrote:

Hi Stefan,

On Wed, May 10, 2017 at 12:10 PM, Stefan Agner [off-list ref] wrote:

quoted

On 2017-05-09 00:50, Dong Aisheng wrote:

quoted

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 <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <redacted>
Cc: Fugang Duan <redacted>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index bddd041..1cdb3f9 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c

@@ -231,7 +231,11 @@
 #define DEV_NAME     "ttyLP"
 #define UART_NR              6

+/* IMX lpuart has four extra unused regs located at the beginning */
+#define IMX_REG_OFF  0x10
+
 static bool lpuart_is_be;
+static u8 lpuart_reg_off;

Global variables? That hardly works once you have two UARTs...

lpuart_reg_off is SoC specific and there's no two UART
with two different reg offset.

quoted

Instead of adding a fixed offset to any write you could just add the
offset to sport->port.membase...

That's intended as i don't want the changes to be too intrusive.
If adding offset in sport->port.xxx, then we have to change the basic
lpuart32_read/write API which is called through the whole driver.

Every lpuart32_write/read call passes port.membase, so if you offset
port.membase when it is assigned in probe you should be fine not?

--
Stefan

Regards
Dong Aisheng

quoted

--
Stefan

quoted

 struct lpuart_port {
      struct uart_port        port;

@@ -263,6 +267,7 @@ struct lpuart_port {
 struct lpuart_soc_data {
      bool    is_32;
      bool    is_be;
+     u8      reg_off;
 };

 static struct lpuart_soc_data vf_data = {

@@ -272,11 +277,19 @@ static struct lpuart_soc_data vf_data = {
 static struct lpuart_soc_data ls_data = {
      .is_32 = true,
      .is_be = true,
+     .reg_off = 0x0,
+};
+
+static struct lpuart_soc_data imx_data = {
+     .is_32 = true,
+     .is_be = false,
+     .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);

@@ -286,15 +299,16 @@ static void lpuart_dma_tx_complete(void *arg);

 static u32 lpuart32_read(void __iomem *addr)
 {
-     return lpuart_is_be ? ioread32be(addr) : readl(addr);
+     return lpuart_is_be ? ioread32be(addr + lpuart_reg_off) :
+                           readl(addr + lpuart_reg_off);
 }

 static void lpuart32_write(u32 val, void __iomem *addr)
 {
      if (lpuart_is_be)
-             iowrite32be(val, addr);
+             iowrite32be(val, addr + lpuart_reg_off);
      else
-             writel(val, addr);
+             writel(val, addr + lpuart_reg_off);
 }

 static void lpuart_stop_tx(struct uart_port *port)

@@ -2008,6 +2022,7 @@ static int lpuart_probe(struct platform_device *pdev)
      sport->port.line = ret;
      sport->lpuart32 = sdata->is_32;
      lpuart_is_be = sdata->is_be;
+     lpuart_reg_off = sdata->reg_off;

      res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
      sport->port.membase = devm_ioremap_resource(&pdev->dev, res);