* [RFC] ns16550: support for UART with broken FIFO @ 2012-01-17 10:39 Antony Pavlov 2012-01-17 13:09 ` Sascha Hauer 0 siblings, 1 reply; 6+ messages in thread From: Antony Pavlov @ 2012-01-17 10:39 UTC (permalink / raw) To: barebox Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> --- drivers/serial/serial_ns16550.c | 19 +++++++++++++++++-- include/ns16550.h | 2 ++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c index 1217a5f..01de964 100644 --- a/drivers/serial/serial_ns16550.c +++ b/drivers/serial/serial_ns16550.c @@ -139,6 +139,8 @@ static unsigned int ns16550_calc_divisor(struct console_device *cdev, static void ns16550_serial_init_port(struct console_device *cdev) { unsigned int baud_divisor; + struct NS16550_plat *plat = (struct NS16550_plat *) + cdev->dev->platform_data; /* Setup the serial port with the defaults first */ baud_divisor = ns16550_calc_divisor(cdev, CONFIG_BAUDRATE); @@ -153,7 +155,13 @@ static void ns16550_serial_init_port(struct console_device *cdev) ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); ns16550_write(cdev, LCRVAL, lcr); ns16550_write(cdev, MCRVAL, mcr); - ns16550_write(cdev, FCRVAL, fcr); + + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); + } else { + ns16550_write(cdev, FCRVAL, fcr); + } + #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS ns16550_write(cdev, 0x00, mdr1); #endif @@ -211,6 +219,8 @@ static int ns16550_tstc(struct console_device *cdev) static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) { unsigned int baud_divisor = ns16550_calc_divisor(cdev, baud_rate); + struct NS16550_plat *plat = (struct NS16550_plat *) + cdev->dev->platform_data; ns16550_write(cdev, 0x00, ier); ns16550_write(cdev, LCR_BKSE, lcr); @@ -218,7 +228,12 @@ static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); ns16550_write(cdev, LCRVAL, lcr); ns16550_write(cdev, MCRVAL, mcr); - ns16550_write(cdev, FCRVAL, fcr); + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); + } else { + ns16550_write(cdev, FCRVAL, fcr); + } + return 0; } diff --git a/include/ns16550.h b/include/ns16550.h index 5fd52fa..4a41b93 100644 --- a/include/ns16550.h +++ b/include/ns16550.h @@ -52,6 +52,8 @@ struct NS16550_plat { unsigned char reg_offset); int shift; + unsigned int flags; +#define NS16650_FLAG_DISABLE_FIFO 1 }; #endif /* __NS16650_PLATFORM_H_ */ -- 1.7.8.3 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] ns16550: support for UART with broken FIFO 2012-01-17 10:39 [RFC] ns16550: support for UART with broken FIFO Antony Pavlov @ 2012-01-17 13:09 ` Sascha Hauer 2012-01-17 13:27 ` Antony Pavlov 0 siblings, 1 reply; 6+ messages in thread From: Sascha Hauer @ 2012-01-17 13:09 UTC (permalink / raw) To: Antony Pavlov; +Cc: barebox On Tue, Jan 17, 2012 at 02:39:06PM +0400, Antony Pavlov wrote: > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > --- > drivers/serial/serial_ns16550.c | 19 +++++++++++++++++-- > include/ns16550.h | 2 ++ > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c > index 1217a5f..01de964 100644 > --- a/drivers/serial/serial_ns16550.c > +++ b/drivers/serial/serial_ns16550.c > @@ -139,6 +139,8 @@ static unsigned int ns16550_calc_divisor(struct console_device *cdev, > static void ns16550_serial_init_port(struct console_device *cdev) > { > unsigned int baud_divisor; > + struct NS16550_plat *plat = (struct NS16550_plat *) > + cdev->dev->platform_data; > > /* Setup the serial port with the defaults first */ > baud_divisor = ns16550_calc_divisor(cdev, CONFIG_BAUDRATE); > @@ -153,7 +155,13 @@ static void ns16550_serial_init_port(struct console_device *cdev) > ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); > ns16550_write(cdev, LCRVAL, lcr); > ns16550_write(cdev, MCRVAL, mcr); > - ns16550_write(cdev, FCRVAL, fcr); > + > + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { > + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); > + } else { > + ns16550_write(cdev, FCRVAL, fcr); > + } unnecessary braces here.. > + > #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS > ns16550_write(cdev, 0x00, mdr1); > #endif > @@ -211,6 +219,8 @@ static int ns16550_tstc(struct console_device *cdev) > static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) > { > unsigned int baud_divisor = ns16550_calc_divisor(cdev, baud_rate); > + struct NS16550_plat *plat = (struct NS16550_plat *) > + cdev->dev->platform_data; > > ns16550_write(cdev, 0x00, ier); > ns16550_write(cdev, LCR_BKSE, lcr); > @@ -218,7 +228,12 @@ static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) > ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); > ns16550_write(cdev, LCRVAL, lcr); > ns16550_write(cdev, MCRVAL, mcr); > - ns16550_write(cdev, FCRVAL, fcr); > + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { > + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); > + } else { > + ns16550_write(cdev, FCRVAL, fcr); > + } and here. Otherwise this looks ok. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] ns16550: support for UART with broken FIFO 2012-01-17 13:09 ` Sascha Hauer @ 2012-01-17 13:27 ` Antony Pavlov 2012-01-17 14:53 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 1 reply; 6+ messages in thread From: Antony Pavlov @ 2012-01-17 13:27 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox On 17 January 2012 17:09, Sascha Hauer <s.hauer@pengutronix.de> wrote: > On Tue, Jan 17, 2012 at 02:39:06PM +0400, Antony Pavlov wrote: >> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> >> --- >> drivers/serial/serial_ns16550.c | 19 +++++++++++++++++-- >> include/ns16550.h | 2 ++ >> 2 files changed, 19 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c >> index 1217a5f..01de964 100644 >> --- a/drivers/serial/serial_ns16550.c >> +++ b/drivers/serial/serial_ns16550.c >> @@ -139,6 +139,8 @@ static unsigned int ns16550_calc_divisor(struct console_device *cdev, >> static void ns16550_serial_init_port(struct console_device *cdev) >> { >> unsigned int baud_divisor; >> + struct NS16550_plat *plat = (struct NS16550_plat *) >> + cdev->dev->platform_data; >> >> /* Setup the serial port with the defaults first */ >> baud_divisor = ns16550_calc_divisor(cdev, CONFIG_BAUDRATE); >> @@ -153,7 +155,13 @@ static void ns16550_serial_init_port(struct console_device *cdev) >> ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); >> ns16550_write(cdev, LCRVAL, lcr); >> ns16550_write(cdev, MCRVAL, mcr); >> - ns16550_write(cdev, FCRVAL, fcr); >> + >> + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { >> + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); >> + } else { >> + ns16550_write(cdev, FCRVAL, fcr); >> + } > > unnecessary braces here.. > >> + >> #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS >> ns16550_write(cdev, 0x00, mdr1); >> #endif >> @@ -211,6 +219,8 @@ static int ns16550_tstc(struct console_device *cdev) >> static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) >> { >> unsigned int baud_divisor = ns16550_calc_divisor(cdev, baud_rate); >> + struct NS16550_plat *plat = (struct NS16550_plat *) >> + cdev->dev->platform_data; >> >> ns16550_write(cdev, 0x00, ier); >> ns16550_write(cdev, LCR_BKSE, lcr); >> @@ -218,7 +228,12 @@ static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) >> ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); >> ns16550_write(cdev, LCRVAL, lcr); >> ns16550_write(cdev, MCRVAL, mcr); >> - ns16550_write(cdev, FCRVAL, fcr); >> + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { >> + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); >> + } else { >> + ns16550_write(cdev, FCRVAL, fcr); >> + } > > and here. Otherwise this looks ok. Ok, I shall remove the braces. Sascha! We have special OMAP code, e.g.: #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS ns16550_write(cdev, 0x07, mdr1); /* Disable */ #endif I think, that we can change it to something like this: if (plat->flags & NS16650_FLAG_OMAP_EXTENSIONS) ns16550_write(cdev, 0x07, mdr1); /* Disable */ -- Best regards, Antony Pavlov _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] ns16550: support for UART with broken FIFO 2012-01-17 13:27 ` Antony Pavlov @ 2012-01-17 14:53 ` Jean-Christophe PLAGNIOL-VILLARD 2012-01-17 19:40 ` Antony Pavlov 0 siblings, 1 reply; 6+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-01-17 14:53 UTC (permalink / raw) To: Antony Pavlov; +Cc: barebox On 17:27 Tue 17 Jan , Antony Pavlov wrote: > On 17 January 2012 17:09, Sascha Hauer <s.hauer@pengutronix.de> wrote: > > On Tue, Jan 17, 2012 at 02:39:06PM +0400, Antony Pavlov wrote: > >> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > >> --- > >> drivers/serial/serial_ns16550.c | 19 +++++++++++++++++-- > >> include/ns16550.h | 2 ++ > >> 2 files changed, 19 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c > >> index 1217a5f..01de964 100644 > >> --- a/drivers/serial/serial_ns16550.c > >> +++ b/drivers/serial/serial_ns16550.c > >> @@ -139,6 +139,8 @@ static unsigned int ns16550_calc_divisor(struct console_device *cdev, > >> static void ns16550_serial_init_port(struct console_device *cdev) > >> { > >> unsigned int baud_divisor; > >> + struct NS16550_plat *plat = (struct NS16550_plat *) > >> + cdev->dev->platform_data; > >> > >> /* Setup the serial port with the defaults first */ > >> baud_divisor = ns16550_calc_divisor(cdev, CONFIG_BAUDRATE); > >> @@ -153,7 +155,13 @@ static void ns16550_serial_init_port(struct console_device *cdev) > >> ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); > >> ns16550_write(cdev, LCRVAL, lcr); > >> ns16550_write(cdev, MCRVAL, mcr); > >> - ns16550_write(cdev, FCRVAL, fcr); > >> + > >> + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { > >> + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); > >> + } else { > >> + ns16550_write(cdev, FCRVAL, fcr); > >> + } > > > > unnecessary braces here.. > > > >> + > >> #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS > >> ns16550_write(cdev, 0x00, mdr1); > >> #endif > >> @@ -211,6 +219,8 @@ static int ns16550_tstc(struct console_device *cdev) > >> static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) > >> { > >> unsigned int baud_divisor = ns16550_calc_divisor(cdev, baud_rate); > >> + struct NS16550_plat *plat = (struct NS16550_plat *) > >> + cdev->dev->platform_data; > >> > >> ns16550_write(cdev, 0x00, ier); > >> ns16550_write(cdev, LCR_BKSE, lcr); > >> @@ -218,7 +228,12 @@ static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) > >> ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); > >> ns16550_write(cdev, LCRVAL, lcr); > >> ns16550_write(cdev, MCRVAL, mcr); > >> - ns16550_write(cdev, FCRVAL, fcr); > >> + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { > >> + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); > >> + } else { > >> + ns16550_write(cdev, FCRVAL, fcr); > >> + } > > > > and here. Otherwise this looks ok. > > Ok, I shall remove the braces. > > Sascha! > > We have special OMAP code, e.g.: > > #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS > ns16550_write(cdev, 0x07, mdr1); /* Disable */ > #endif > > I think, that we can change it to something like this: > > if (plat->flags & NS16650_FLAG_OMAP_EXTENSIONS) > ns16550_write(cdev, 0x07, mdr1); /* Disable */ I don't like to add dead code in the binary just use if (IS_ENABLED(CONFIG_...) Best Regards, J. _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] ns16550: support for UART with broken FIFO 2012-01-17 14:53 ` Jean-Christophe PLAGNIOL-VILLARD @ 2012-01-17 19:40 ` Antony Pavlov 2012-01-17 23:58 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 1 reply; 6+ messages in thread From: Antony Pavlov @ 2012-01-17 19:40 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox On 17 January 2012 18:53, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote: > On 17:27 Tue 17 Jan , Antony Pavlov wrote: >> On 17 January 2012 17:09, Sascha Hauer <s.hauer@pengutronix.de> wrote: >> > On Tue, Jan 17, 2012 at 02:39:06PM +0400, Antony Pavlov wrote: >> >> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> >> >> --- >> >> drivers/serial/serial_ns16550.c | 19 +++++++++++++++++-- >> >> include/ns16550.h | 2 ++ >> >> 2 files changed, 19 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c >> >> index 1217a5f..01de964 100644 >> >> --- a/drivers/serial/serial_ns16550.c >> >> +++ b/drivers/serial/serial_ns16550.c >> >> @@ -139,6 +139,8 @@ static unsigned int ns16550_calc_divisor(struct console_device *cdev, >> >> static void ns16550_serial_init_port(struct console_device *cdev) >> >> { >> >> unsigned int baud_divisor; >> >> + struct NS16550_plat *plat = (struct NS16550_plat *) >> >> + cdev->dev->platform_data; >> >> >> >> /* Setup the serial port with the defaults first */ >> >> baud_divisor = ns16550_calc_divisor(cdev, CONFIG_BAUDRATE); >> >> @@ -153,7 +155,13 @@ static void ns16550_serial_init_port(struct console_device *cdev) >> >> ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); >> >> ns16550_write(cdev, LCRVAL, lcr); >> >> ns16550_write(cdev, MCRVAL, mcr); >> >> - ns16550_write(cdev, FCRVAL, fcr); >> >> + >> >> + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { >> >> + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); >> >> + } else { >> >> + ns16550_write(cdev, FCRVAL, fcr); >> >> + } >> > >> > unnecessary braces here.. >> > >> >> + >> >> #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS >> >> ns16550_write(cdev, 0x00, mdr1); >> >> #endif >> >> @@ -211,6 +219,8 @@ static int ns16550_tstc(struct console_device *cdev) >> >> static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) >> >> { >> >> unsigned int baud_divisor = ns16550_calc_divisor(cdev, baud_rate); >> >> + struct NS16550_plat *plat = (struct NS16550_plat *) >> >> + cdev->dev->platform_data; >> >> >> >> ns16550_write(cdev, 0x00, ier); >> >> ns16550_write(cdev, LCR_BKSE, lcr); >> >> @@ -218,7 +228,12 @@ static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) >> >> ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); >> >> ns16550_write(cdev, LCRVAL, lcr); >> >> ns16550_write(cdev, MCRVAL, mcr); >> >> - ns16550_write(cdev, FCRVAL, fcr); >> >> + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { >> >> + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); >> >> + } else { >> >> + ns16550_write(cdev, FCRVAL, fcr); >> >> + } >> > >> > and here. Otherwise this looks ok. >> >> Ok, I shall remove the braces. >> >> Sascha! >> >> We have special OMAP code, e.g.: >> >> #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS >> ns16550_write(cdev, 0x07, mdr1); /* Disable */ >> #endif >> >> I think, that we can change it to something like this: >> >> if (plat->flags & NS16650_FLAG_OMAP_EXTENSIONS) >> ns16550_write(cdev, 0x07, mdr1); /* Disable */ > I don't like to add dead code in the binary > just use > if (IS_ENABLED(CONFIG_...) You is right, but there is another point of view. If you have defined CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS macro, then you will use OMAP extensions for __ALL__ ports, not only for OMAP internal port. May be OMAP have no other ports, only internal ports with extensions. But in any case it is bad design example: we have very special code (OMAP EXTENSIONS) in universal driver (ns16550), and this special code works without checking presents of the extensions. I don't like dead code too. So there are another solutions: #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS if (plat->flags & NS16650_FLAG_OMAP_EXTENSIONS) ns16550_write(cdev, 0x07, mdr1); /* Disable */ #endif or even if (IS_ENABLED(CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS) && plat->flags & NS16650_FLAG_OMAP_EXTENSIONS) ns16550_write(cdev, 0x07, mdr1); /* Disable */ -- Best regards, Antony Pavlov _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] ns16550: support for UART with broken FIFO 2012-01-17 19:40 ` Antony Pavlov @ 2012-01-17 23:58 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 0 replies; 6+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-01-17 23:58 UTC (permalink / raw) To: Antony Pavlov; +Cc: barebox On 23:40 Tue 17 Jan , Antony Pavlov wrote: > On 17 January 2012 18:53, Jean-Christophe PLAGNIOL-VILLARD > <plagnioj@jcrosoft.com> wrote: > > On 17:27 Tue 17 Jan , Antony Pavlov wrote: > >> On 17 January 2012 17:09, Sascha Hauer <s.hauer@pengutronix.de> wrote: > >> > On Tue, Jan 17, 2012 at 02:39:06PM +0400, Antony Pavlov wrote: > >> >> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > >> >> --- > >> >> drivers/serial/serial_ns16550.c | 19 +++++++++++++++++-- > >> >> include/ns16550.h | 2 ++ > >> >> 2 files changed, 19 insertions(+), 2 deletions(-) > >> >> > >> >> diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c > >> >> index 1217a5f..01de964 100644 > >> >> --- a/drivers/serial/serial_ns16550.c > >> >> +++ b/drivers/serial/serial_ns16550.c > >> >> @@ -139,6 +139,8 @@ static unsigned int ns16550_calc_divisor(struct console_device *cdev, > >> >> static void ns16550_serial_init_port(struct console_device *cdev) > >> >> { > >> >> unsigned int baud_divisor; > >> >> + struct NS16550_plat *plat = (struct NS16550_plat *) > >> >> + cdev->dev->platform_data; > >> >> > >> >> /* Setup the serial port with the defaults first */ > >> >> baud_divisor = ns16550_calc_divisor(cdev, CONFIG_BAUDRATE); > >> >> @@ -153,7 +155,13 @@ static void ns16550_serial_init_port(struct console_device *cdev) > >> >> ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); > >> >> ns16550_write(cdev, LCRVAL, lcr); > >> >> ns16550_write(cdev, MCRVAL, mcr); > >> >> - ns16550_write(cdev, FCRVAL, fcr); > >> >> + > >> >> + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { > >> >> + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); > >> >> + } else { > >> >> + ns16550_write(cdev, FCRVAL, fcr); > >> >> + } > >> > > >> > unnecessary braces here.. > >> > > >> >> + > >> >> #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS > >> >> ns16550_write(cdev, 0x00, mdr1); > >> >> #endif > >> >> @@ -211,6 +219,8 @@ static int ns16550_tstc(struct console_device *cdev) > >> >> static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) > >> >> { > >> >> unsigned int baud_divisor = ns16550_calc_divisor(cdev, baud_rate); > >> >> + struct NS16550_plat *plat = (struct NS16550_plat *) > >> >> + cdev->dev->platform_data; > >> >> > >> >> ns16550_write(cdev, 0x00, ier); > >> >> ns16550_write(cdev, LCR_BKSE, lcr); > >> >> @@ -218,7 +228,12 @@ static int ns16550_setbaudrate(struct console_device *cdev, int baud_rate) > >> >> ns16550_write(cdev, (baud_divisor >> 8) & 0xff, dlm); > >> >> ns16550_write(cdev, LCRVAL, lcr); > >> >> ns16550_write(cdev, MCRVAL, mcr); > >> >> - ns16550_write(cdev, FCRVAL, fcr); > >> >> + if (plat->flags & NS16650_FLAG_DISABLE_FIFO) { > >> >> + ns16550_write(cdev, FCRVAL & ~FCR_FIFO_EN, fcr); > >> >> + } else { > >> >> + ns16550_write(cdev, FCRVAL, fcr); > >> >> + } > >> > > >> > and here. Otherwise this looks ok. > >> > >> Ok, I shall remove the braces. > >> > >> Sascha! > >> > >> We have special OMAP code, e.g.: > >> > >> #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS > >> ns16550_write(cdev, 0x07, mdr1); /* Disable */ > >> #endif > >> > >> I think, that we can change it to something like this: > >> > >> if (plat->flags & NS16650_FLAG_OMAP_EXTENSIONS) > >> ns16550_write(cdev, 0x07, mdr1); /* Disable */ > > I don't like to add dead code in the binary > > just use > > if (IS_ENABLED(CONFIG_...) > > You is right, but there is another point of view. > > If you have defined CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS > macro, then you will use OMAP extensions for __ALL__ ports, not only > for OMAP internal port. May be OMAP have no other ports, only internal > ports with extensions. But in any case it is bad design example: we > have very special code (OMAP EXTENSIONS) in universal driver > (ns16550), and this special code works without checking presents of > the extensions. > > I don't like dead code too. So there are another solutions: > > #ifdef CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS > if (plat->flags & NS16650_FLAG_OMAP_EXTENSIONS) > ns16550_write(cdev, 0x07, mdr1); /* Disable */ > #endif > > or even > if (IS_ENABLED(CONFIG_DRIVER_SERIAL_NS16550_OMAP_EXTENSIONS) && > plat->flags & NS16650_FLAG_OMAP_EXTENSIONS) > ns16550_write(cdev, 0x07, mdr1); /* Disable */ this one Best Regards, J. > -- > Best regards, > Antony Pavlov _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-17 23:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-01-17 10:39 [RFC] ns16550: support for UART with broken FIFO Antony Pavlov 2012-01-17 13:09 ` Sascha Hauer 2012-01-17 13:27 ` Antony Pavlov 2012-01-17 14:53 ` Jean-Christophe PLAGNIOL-VILLARD 2012-01-17 19:40 ` Antony Pavlov 2012-01-17 23:58 ` Jean-Christophe PLAGNIOL-VILLARD
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox