Messages in this thread | | | From | "A.S. Dong" <> | Subject | RE: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support | Date | Wed, 17 May 2017 06:01:37 +0000 |
| |
> -----Original Message----- > From: Nikita Yushchenko [mailto:nikita.yoush@cogentembedded.com] > Sent: Wednesday, May 17, 2017 1:44 PM > To: Dong Aisheng > Cc: A.S. Dong; linux-serial@vger.kernel.org; Andy Duan; > gregkh@linuxfoundation.org; Y.B. Lu; linux-kernel@vger.kernel.org; > stefan@agner.ch; Mingkai Hu; jslaby@suse.com; linux-arm- > kernel@lists.infradead.org > 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
| |