RE: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support

[A.S. Dong]

> -----Original Message-----
> From: Nikita Yushchenko [mailto:nikita.yoush@xxxxxxxxxxxxxxxxxx]
> Sent: Wednesday, May 17, 2017 1:44 PM
> To: Dong Aisheng
> Cc: A.S. Dong; linux-serial@xxxxxxxxxxxxxxx; Andy Duan;
> gregkh@xxxxxxxxxxxxxxxxxxx; Y.B. Lu; linux-kernel@xxxxxxxxxxxxxxx;
> stefan@xxxxxxxx; Mingkai Hu; jslaby@xxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit
> register support
> 
> >>> @@ -2000,6 +2007,7 @@ static int lpuart_probe(struct platform_device
> *pdev)
> >>>  	}
> >>>  	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