> -----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 -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
