-----Original Message-----
From: Nikita Yushchenko [mailto:nikita.yoush at cogentembedded.com]
Sent: Wednesday, May 17, 2017 1:44 PM
To: Dong Aisheng
Cc: A.S. Dong; linux-serial at vger.kernel.org; Andy Duan;
gregkh at linuxfoundation.org; Y.B. Lu; linux-kernel at vger.kernel.org;
stefan at agner.ch; Mingkai Hu; jslaby at suse.com; linux-arm-
kernel at lists.infradead.org
Subject: Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit
register support

quoted

quoted

quoted

@@ -2000,6 +2007,7 @@ static int lpuart_probe(struct platform_device

*pdev)

quoted

quoted

quoted

 	}
 	sport->port.line = ret;
 	sport->lpuart32 = sdata->is_32;
+	lpuart_is_be = sdata->is_be;

Setting a global variable in per-device routine is quite bad design.

There is a reason for that we don't want to change the exist
lpuart32_read[write] API which is widely used in driver.
Making a global lpuart_is_be is the simplest way to do it.

Any strong blocking reason?

Code should be consistent.

Yes.

There is no good reason to have sport->lpuart32 inside sport, but
lpuart_is_be outside of it. Both these values describe properties of
particular device, and thus should be in per-device structure.

That's for special case, normally we wouldn't do that.

If that implies adding sport arg to lpuart32_(read|write), just do that.

There's another reason that we have to deal with earlycon which is
executed much early before driver probe.

And I need specificly align the endian data.
e.g.
static int __init lpuart32_early_console_setup(struct earlycon_device *device,
                                          const char *opt)
{
        if (!device->port.membase)
                return -ENODEV;

        lpuart_is_be = true;
        device->con->write = lpuart32_early_write;
        return 0;
}

static int __init lpuart32_imx_early_console_setup(struct earlycon_device *device,
                                                   const char *opt)
{
        if (!device->port.membase)
                return -ENODEV;

        lpuart_is_be = false;
        device->port.membase += IMX_REG_OFF;
        device->con->write = lpuart32_early_write;

        return 0;
}

Regards
Dong Aisheng