LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] 8250: make probing for TXEN bug a config option
@ 2006-12-26 16:43 Vitaly Wool
  2007-02-14  6:46 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Vitaly Wool @ 2006-12-26 16:43 UTC (permalink / raw)
  To: akpm; +Cc: linux-kernel

Hello Andrew,

probing for UART_BUG_TXEN in 8250 driver leads to weird effects on some ARM boards (pnx4008 for instance). That is, the driver detects  UART_BUG_TXEN (though it apparently shouldn't) and it leads to symbol loss in console on input (i. e. you input 'a' and you get nothing, then you input 'b' and you get 'a', then you input 'c' and get 'b' and so on).

The patch below makes this very probing a configuration option turned on by default.

 drivers/serial/8250.c  |    5 ++++-
 drivers/serial/Kconfig |   10 ++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

Signed-off-by: Vitaly Wool <vitalywool@gmail.com>

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 51f3c73..cf3eb31 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1645,6 +1645,7 @@ static int serial8250_startup(struct uar
 
 	serial8250_set_mctrl(&up->port, up->port.mctrl);
 
+#ifndef CONFIG_SERIAL_8250_DONT_TEST_BUG_TXEN
 	/*
 	 * Do a quick test to see if we receive an
 	 * interrupt when we enable the TX irq.
@@ -1660,7 +1661,9 @@ static int serial8250_startup(struct uar
 			pr_debug("ttyS%d - enabling bad tx status workarounds\n",
 				 port->line);
 		}
-	} else {
+	} else
+#endif
+	{
 		up->bugs &= ~UART_BUG_TXEN;
 	}
 
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 2978c09..7efcaf3 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -223,6 +223,16 @@ config SERIAL_8250_DETECT_IRQ
 
 	  If unsure, say N.
 
+config SERIAL_8250_DONT_TEST_BUG_TXEN
+	bool "Don't probe for TXEN bug"
+	depends on SERIAL_8250_EXTENDED
+	help
+	  Say Y here if you don't want the kernel to probe for TXEN bug
+	  on your serial port and try to workaround it. It might lead to
+	  character loss on some boards, though this is quite a rare case.
+
+	  If unsure, say N.
+
 config SERIAL_8250_RSA
 	bool "Support RSA serial ports"
 	depends on SERIAL_8250_EXTENDED

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] 8250: make probing for TXEN bug a config option
  2006-12-26 16:43 [PATCH] 8250: make probing for TXEN bug a config option Vitaly Wool
@ 2007-02-14  6:46 ` Andrew Morton
       [not found]   ` <acd2a5930702140037yff0b77dl2714ea7869bdde54@mail.gmail.com>
  2007-03-14 21:38   ` Vitaly Wool
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2007-02-14  6:46 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-kernel

On Tue, 26 Dec 2006 19:43:17 +0300 Vitaly Wool <vitalywool@gmail.com> wrote:

        ^^^^^^^^^^^  Sorry.

> Hello Andrew,
> 
> probing for UART_BUG_TXEN in 8250 driver leads to weird effects on some ARM boards (pnx4008 for instance). That is, the driver detects  UART_BUG_TXEN (though it apparently shouldn't) and it leads to symbol loss in console on input (i. e. you input 'a' and you get nothing, then you input 'b' and you get 'a', then you input 'c' and get 'b' and so on).
> 
> The patch below makes this very probing a configuration option turned on by default.
> 
>  drivers/serial/8250.c  |    5 ++++-
>  drivers/serial/Kconfig |   10 ++++++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> Signed-off-by: Vitaly Wool <vitalywool@gmail.com>
> 
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index 51f3c73..cf3eb31 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -1645,6 +1645,7 @@ static int serial8250_startup(struct uar
>  
>  	serial8250_set_mctrl(&up->port, up->port.mctrl);
>  
> +#ifndef CONFIG_SERIAL_8250_DONT_TEST_BUG_TXEN
>  	/*
>  	 * Do a quick test to see if we receive an
>  	 * interrupt when we enable the TX irq.
> @@ -1660,7 +1661,9 @@ static int serial8250_startup(struct uar
>  			pr_debug("ttyS%d - enabling bad tx status workarounds\n",
>  				 port->line);
>  		}
> -	} else {
> +	} else
> +#endif
> +	{
>  		up->bugs &= ~UART_BUG_TXEN;
>  	}
>  
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 2978c09..7efcaf3 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -223,6 +223,16 @@ config SERIAL_8250_DETECT_IRQ
>  
>  	  If unsure, say N.
>  
> +config SERIAL_8250_DONT_TEST_BUG_TXEN
> +	bool "Don't probe for TXEN bug"
> +	depends on SERIAL_8250_EXTENDED
> +	help
> +	  Say Y here if you don't want the kernel to probe for TXEN bug
> +	  on your serial port and try to workaround it. It might lead to
> +	  character loss on some boards, though this is quite a rare case.
> +
> +	  If unsure, say N.
> +
>  config SERIAL_8250_RSA
>  	bool "Support RSA serial ports"
>  	depends on SERIAL_8250_EXTENDED

I think this should be a module option/boot parameter, not a config-time
option.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] 8250: make probing for TXEN bug a config option
       [not found]   ` <acd2a5930702140037yff0b77dl2714ea7869bdde54@mail.gmail.com>
@ 2007-02-14  8:47     ` Andrew Morton
  2007-02-14 13:41       ` Vitaly Wool
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2007-02-14  8:47 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-kernel

On Wed, 14 Feb 2007 11:37:52 +0300 "Vitaly Wool" <vitalywool@gmail.com> wrote:

> On 2/14/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > I think this should be a module option/boot parameter, not a config-time
> > option.
> >
> >
> Hmm, why? I can't think of a platform where one 8250-compatible UART is
> problematic and another isn't :)
> 

Is it not possible that the same kernel package can be installed on systems
which do and don't need this feature?  If so, we don't want to force the
provider of that package to create two packages.

That, plus the chances of the package creator actually knowing about this
option aren't great.

Generally, if it can be done at runtime it is better to do so, no?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] 8250: make probing for TXEN bug a config option
  2007-02-14  8:47     ` Andrew Morton
@ 2007-02-14 13:41       ` Vitaly Wool
  2007-02-14 18:20         ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Vitaly Wool @ 2007-02-14 13:41 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

On 2/14/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 14 Feb 2007 11:37:52 +0300 "Vitaly Wool" <vitalywool@gmail.com> wrote:
> > Hmm, why? I can't think of a platform where one 8250-compatible UART is
> > problematic and another isn't :)
> >
>
> Is it not possible that the same kernel package can be installed on systems
> which do and don't need this feature?  If so, we don't want to force the
> provider of that package to create two packages.
>
> That, plus the chances of the package creator actually knowing about this
> option aren't great.
>
> Generally, if it can be done at runtime it is better to do so, no?

Okay, yes, I see your point. The same kernel might actually be
supporting several machines.

But having that as a config option doesn't look too attractive to me.
What about adding a new flag to plat_serial 8250 stuff instead?

Vitaly

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] 8250: make probing for TXEN bug a config option
  2007-02-14 13:41       ` Vitaly Wool
@ 2007-02-14 18:20         ` Andrew Morton
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2007-02-14 18:20 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-kernel

On Wed, 14 Feb 2007 05:41:53 -0800
"Vitaly Wool" <vitalywool@gmail.com> wrote:

> On 2/14/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Wed, 14 Feb 2007 11:37:52 +0300 "Vitaly Wool" <vitalywool@gmail.com> wrote:
> > > Hmm, why? I can't think of a platform where one 8250-compatible UART is
> > > problematic and another isn't :)
> > >
> >
> > Is it not possible that the same kernel package can be installed on systems
> > which do and don't need this feature?  If so, we don't want to force the
> > provider of that package to create two packages.
> >
> > That, plus the chances of the package creator actually knowing about this
> > option aren't great.
> >
> > Generally, if it can be done at runtime it is better to do so, no?
> 
> Okay, yes, I see your point. The same kernel might actually be
> supporting several machines.
> 
> But having that as a config option doesn't look too attractive to me.
> What about adding a new flag to plat_serial 8250 stuff instead?

plat_serial8250_port.flags?  Dunno, I'm unfamiliar with it.  That seems to be how
the share_irqs option is handled.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] 8250: make probing for TXEN bug a config option
  2007-02-14  6:46 ` Andrew Morton
       [not found]   ` <acd2a5930702140037yff0b77dl2714ea7869bdde54@mail.gmail.com>
@ 2007-03-14 21:38   ` Vitaly Wool
  1 sibling, 0 replies; 6+ messages in thread
From: Vitaly Wool @ 2007-03-14 21:38 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

On 2/14/07, Andrew Morton <akpm@linux-foundation.org> wrote:

> I think this should be a module option/boot parameter, not a config-time
> option.

NM, the recent changes in 8250 driver eliminated the need for this patch :)

Vitaly

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-03-14 21:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-12-26 16:43 [PATCH] 8250: make probing for TXEN bug a config option Vitaly Wool
2007-02-14  6:46 ` Andrew Morton
     [not found]   ` <acd2a5930702140037yff0b77dl2714ea7869bdde54@mail.gmail.com>
2007-02-14  8:47     ` Andrew Morton
2007-02-14 13:41       ` Vitaly Wool
2007-02-14 18:20         ` Andrew Morton
2007-03-14 21:38   ` Vitaly Wool

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).