All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
@ 2011-01-05 12:26 ` Michael Williamson
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Williamson @ 2011-01-05 12:26 UTC (permalink / raw)
  To: davinci-linux-open-source
  Cc: linux-arm-kernel, khilman, linux-serial, gregkh, Michael Williamson

On the da850/omap-l138/am18x family of SoCs, up to three on chip UARTS may be
configured.  These peripherals support the standard Tx/Rx signals as well as
CTS/RTS hardware flow control signals.  The pins on these SOC's associated with
these signals are multiplexed; e.g., the pin providing UART0_TXD capability
also provides SPI0 chip select line 5 output capability.  The configuration of
the pin multiplexing occurs during platform initialization (or by earlier
bootloader operations).

There is a problem with the multiplexing implementation on these SOCs.  Only
the output and output enable portions of the I/O section of the pin are
multiplexed.  All peripheral input functions remain connected to a given pin
regardless of configuration.

In many configurations of these parts, providing a UART with Tx/Rx capability
is needed, but the HW flow control capability is not.  Furthermore, the pins
associated with the CTS inputs on these UARTS are often configured to support
a different peripheral, and they may be active/toggling during runtime.  This
can result in false modem status (CTS) interrupts being asserted to the 8250
driver controlling the associated Tx/Rx pins, and will impact system
performance.

The 8250 serial driver platform data does not provide a direct mechanism to
tell the driver to disable modem status (i.e., CTS) interrupts for a given
port.  As a work-around, intercept register writes to the IER and disable
modem status interrupts.

This patch was tested using a MityDSP-L138 SOM having UART1 enabled with the
associated CTS pin connected to a clock (configured for the AHCLKX function).

Background / problem reports related to this issue are captured in the links
below:
http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/36701.aspx
http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg19524.html

Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
Tested-by: Michael Williamson <michael.williamson@criticallink.com>
---
This is against the linux-davinci tree.

Changes from original proposed patch:
- instead of overriding set_termios, now override serial_out driver hook
  and intercept writes to the MSR.

An alternate patch was proposed that modified the serial core driver and added a UPF_NO_MSR
flag.  There was resistance to this patch.  The reasoning is that the core 8250 driver code
should not continue to get muddied by platform hacks.

I'm wondering, given this and the original proposed patch, if the set_termios
override might be better?  This patch incurs a test penalty every time the UART
is accessed; whereas the original patch only incurs a penalty on IOCTL calls.  The
set_termios would at least report the proper IOCTL information for CRTSCTS 
when probed, I think, instead of just quietly lying about it...

 arch/arm/mach-davinci/include/mach/serial.h |    2 ++
 arch/arm/mach-davinci/serial.c              |   19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/serial.h b/arch/arm/mach-davinci/include/mach/serial.h
index 8051110..8f7f5e5 100644
--- a/arch/arm/mach-davinci/include/mach/serial.h
+++ b/arch/arm/mach-davinci/include/mach/serial.h
@@ -49,6 +49,8 @@
 struct davinci_uart_config {
 	/* Bit field of UARTs present; bit 0 --> UART1 */
 	unsigned int enabled_uarts;
+	/* Bit field of modem status interrupt disables; bit 0 -> UART1 */
+	unsigned int disable_msi;
 };
 
 extern int davinci_serial_init(struct davinci_uart_config *);
diff --git a/arch/arm/mach-davinci/serial.c b/arch/arm/mach-davinci/serial.c
index 1875740..83d44c0 100644
--- a/arch/arm/mach-davinci/serial.c
+++ b/arch/arm/mach-davinci/serial.c
@@ -31,6 +31,22 @@
 #include <mach/serial.h>
 #include <mach/cputype.h>
 
+static void davinci_serial_out_nomsi(struct uart_port *p, int offset, int value)
+{
+	int lcr, lcr_off;
+
+	if (offset == UART_IER) {
+		lcr_off = UART_LCR << p->regshift;
+		lcr = readb(p->membase + lcr_off);
+		/* don't override DLM setting, or probing operations */
+		if (!(lcr & UART_LCR_DLAB) && p->type != PORT_UNKNOWN)
+			value &= ~UART_IER_MSI;
+	}
+
+	offset <<= p->regshift;
+	writeb(value, p->membase + offset);
+}
+
 static inline unsigned int serial_read_reg(struct plat_serial8250_port *up,
 					   int offset)
 {
@@ -109,6 +125,9 @@ int __init davinci_serial_init(struct davinci_uart_config *info)
 
 		if (p->membase && p->type != PORT_AR7)
 			davinci_serial_reset(p);
+
+		if (info->disable_msi & (1 << i))
+			p->serial_out = davinci_serial_out_nomsi;
 	}
 
 	return platform_device_register(soc_info->serial_dev);
-- 
1.7.0.4


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

* [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
@ 2011-01-05 12:26 ` Michael Williamson
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Williamson @ 2011-01-05 12:26 UTC (permalink / raw)
  To: linux-arm-kernel

On the da850/omap-l138/am18x family of SoCs, up to three on chip UARTS may be
configured.  These peripherals support the standard Tx/Rx signals as well as
CTS/RTS hardware flow control signals.  The pins on these SOC's associated with
these signals are multiplexed; e.g., the pin providing UART0_TXD capability
also provides SPI0 chip select line 5 output capability.  The configuration of
the pin multiplexing occurs during platform initialization (or by earlier
bootloader operations).

There is a problem with the multiplexing implementation on these SOCs.  Only
the output and output enable portions of the I/O section of the pin are
multiplexed.  All peripheral input functions remain connected to a given pin
regardless of configuration.

In many configurations of these parts, providing a UART with Tx/Rx capability
is needed, but the HW flow control capability is not.  Furthermore, the pins
associated with the CTS inputs on these UARTS are often configured to support
a different peripheral, and they may be active/toggling during runtime.  This
can result in false modem status (CTS) interrupts being asserted to the 8250
driver controlling the associated Tx/Rx pins, and will impact system
performance.

The 8250 serial driver platform data does not provide a direct mechanism to
tell the driver to disable modem status (i.e., CTS) interrupts for a given
port.  As a work-around, intercept register writes to the IER and disable
modem status interrupts.

This patch was tested using a MityDSP-L138 SOM having UART1 enabled with the
associated CTS pin connected to a clock (configured for the AHCLKX function).

Background / problem reports related to this issue are captured in the links
below:
http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/36701.aspx
http://www.mail-archive.com/davinci-linux-open-source at linux.davincidsp.com/msg19524.html

Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
Tested-by: Michael Williamson <michael.williamson@criticallink.com>
---
This is against the linux-davinci tree.

Changes from original proposed patch:
- instead of overriding set_termios, now override serial_out driver hook
  and intercept writes to the MSR.

An alternate patch was proposed that modified the serial core driver and added a UPF_NO_MSR
flag.  There was resistance to this patch.  The reasoning is that the core 8250 driver code
should not continue to get muddied by platform hacks.

I'm wondering, given this and the original proposed patch, if the set_termios
override might be better?  This patch incurs a test penalty every time the UART
is accessed; whereas the original patch only incurs a penalty on IOCTL calls.  The
set_termios would at least report the proper IOCTL information for CRTSCTS 
when probed, I think, instead of just quietly lying about it...

 arch/arm/mach-davinci/include/mach/serial.h |    2 ++
 arch/arm/mach-davinci/serial.c              |   19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/serial.h b/arch/arm/mach-davinci/include/mach/serial.h
index 8051110..8f7f5e5 100644
--- a/arch/arm/mach-davinci/include/mach/serial.h
+++ b/arch/arm/mach-davinci/include/mach/serial.h
@@ -49,6 +49,8 @@
 struct davinci_uart_config {
 	/* Bit field of UARTs present; bit 0 --> UART1 */
 	unsigned int enabled_uarts;
+	/* Bit field of modem status interrupt disables; bit 0 -> UART1 */
+	unsigned int disable_msi;
 };
 
 extern int davinci_serial_init(struct davinci_uart_config *);
diff --git a/arch/arm/mach-davinci/serial.c b/arch/arm/mach-davinci/serial.c
index 1875740..83d44c0 100644
--- a/arch/arm/mach-davinci/serial.c
+++ b/arch/arm/mach-davinci/serial.c
@@ -31,6 +31,22 @@
 #include <mach/serial.h>
 #include <mach/cputype.h>
 
+static void davinci_serial_out_nomsi(struct uart_port *p, int offset, int value)
+{
+	int lcr, lcr_off;
+
+	if (offset == UART_IER) {
+		lcr_off = UART_LCR << p->regshift;
+		lcr = readb(p->membase + lcr_off);
+		/* don't override DLM setting, or probing operations */
+		if (!(lcr & UART_LCR_DLAB) && p->type != PORT_UNKNOWN)
+			value &= ~UART_IER_MSI;
+	}
+
+	offset <<= p->regshift;
+	writeb(value, p->membase + offset);
+}
+
 static inline unsigned int serial_read_reg(struct plat_serial8250_port *up,
 					   int offset)
 {
@@ -109,6 +125,9 @@ int __init davinci_serial_init(struct davinci_uart_config *info)
 
 		if (p->membase && p->type != PORT_AR7)
 			davinci_serial_reset(p);
+
+		if (info->disable_msi & (1 << i))
+			p->serial_out = davinci_serial_out_nomsi;
 	}
 
 	return platform_device_register(soc_info->serial_dev);
-- 
1.7.0.4

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

* [PATCH 2/2 v1] davinci: mityomapl138 platform: disable MS interrupts on UART1
  2011-01-05 12:26 ` Michael Williamson
@ 2011-01-05 12:26   ` Michael Williamson
  -1 siblings, 0 replies; 14+ messages in thread
From: Michael Williamson @ 2011-01-05 12:26 UTC (permalink / raw)
  To: davinci-linux-open-source
  Cc: linux-arm-kernel, khilman, linux-serial, gregkh, Michael Williamson

All supported configurations of the MityDSP-l138 and MityARM-1808
SoMs use the pin associated with CTS on UART1 as either an AHCLKX
input or a USB_REFCLKIN input.  Disable modem status interrupts
associated with this pin.

Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
---
This is against the linux-davinci tree.

 arch/arm/mach-davinci/board-mityomapl138.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
index 0bb5f0c..10ea4cb 100644
--- a/arch/arm/mach-davinci/board-mityomapl138.c
+++ b/arch/arm/mach-davinci/board-mityomapl138.c
@@ -283,7 +283,8 @@ static void __init mityomapl138_setup_nand(void)
 }
 
 static struct davinci_uart_config mityomapl138_uart_config __initdata = {
-	.enabled_uarts = 0x7,
+	.enabled_uarts	= 0x7,
+	.disable_msi	= 0x2,
 };
 
 static const short mityomap_mii_pins[] = {
-- 
1.7.0.4


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

* [PATCH 2/2 v1] davinci: mityomapl138 platform: disable MS interrupts on UART1
@ 2011-01-05 12:26   ` Michael Williamson
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Williamson @ 2011-01-05 12:26 UTC (permalink / raw)
  To: linux-arm-kernel

All supported configurations of the MityDSP-l138 and MityARM-1808
SoMs use the pin associated with CTS on UART1 as either an AHCLKX
input or a USB_REFCLKIN input.  Disable modem status interrupts
associated with this pin.

Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
---
This is against the linux-davinci tree.

 arch/arm/mach-davinci/board-mityomapl138.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
index 0bb5f0c..10ea4cb 100644
--- a/arch/arm/mach-davinci/board-mityomapl138.c
+++ b/arch/arm/mach-davinci/board-mityomapl138.c
@@ -283,7 +283,8 @@ static void __init mityomapl138_setup_nand(void)
 }
 
 static struct davinci_uart_config mityomapl138_uart_config __initdata = {
-	.enabled_uarts = 0x7,
+	.enabled_uarts	= 0x7,
+	.disable_msi	= 0x2,
 };
 
 static const short mityomap_mii_pins[] = {
-- 
1.7.0.4

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

* RE: [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
  2011-01-05 12:26 ` Michael Williamson
@ 2011-01-11  8:17   ` Manjunathappa, Prakash
  -1 siblings, 0 replies; 14+ messages in thread
From: Manjunathappa, Prakash @ 2011-01-11  8:17 UTC (permalink / raw)
  To: Michael Williamson, davinci-linux-open-source
  Cc: khilman, gregkh, linux-serial, linux-arm-kernel

Hi Michael,

On Wed, Jan 05, 2011 at 17:56:31, Michael Williamson wrote:
> On the da850/omap-l138/am18x family of SoCs, up to three on chip UARTS may be
> configured.  These peripherals support the standard Tx/Rx signals as well as
> CTS/RTS hardware flow control signals.  The pins on these SOC's associated with
> these signals are multiplexed; e.g., the pin providing UART0_TXD capability
> also provides SPI0 chip select line 5 output capability.  The configuration of
> the pin multiplexing occurs during platform initialization (or by earlier
> bootloader operations).
> 
> There is a problem with the multiplexing implementation on these SOCs.  Only
> the output and output enable portions of the I/O section of the pin are
> multiplexed.  All peripheral input functions remain connected to a given pin
> regardless of configuration.
> 
> In many configurations of these parts, providing a UART with Tx/Rx capability
> is needed, but the HW flow control capability is not.  Furthermore, the pins
> associated with the CTS inputs on these UARTS are often configured to support
> a different peripheral, and they may be active/toggling during runtime.  This
> can result in false modem status (CTS) interrupts being asserted to the 8250
> driver controlling the associated Tx/Rx pins, and will impact system
> performance.
> 
> The 8250 serial driver platform data does not provide a direct mechanism to
> tell the driver to disable modem status (i.e., CTS) interrupts for a given
> port.  As a work-around, intercept register writes to the IER and disable
> modem status interrupts.
> 
> This patch was tested using a MityDSP-L138 SOM having UART1 enabled with the
> associated CTS pin connected to a clock (configured for the AHCLKX function).
> 
> Background / problem reports related to this issue are captured in the links
> below:
> http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/36701.aspx
> http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg19524.html
> 
> Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
> Tested-by: Michael Williamson <michael.williamson@criticallink.com>
> ---
> This is against the linux-davinci tree.
> 
> Changes from original proposed patch:
> - instead of overriding set_termios, now override serial_out driver hook
>   and intercept writes to the MSR.
> 
> An alternate patch was proposed that modified the serial core driver and added a UPF_NO_MSR
> flag.  There was resistance to this patch.  The reasoning is that the core 8250 driver code
> should not continue to get muddied by platform hacks.
> 
> I'm wondering, given this and the original proposed patch, if the set_termios
> override might be better?  This patch incurs a test penalty every time the UART
> is accessed; whereas the original patch only incurs a penalty on IOCTL calls.  The
> set_termios would at least report the proper IOCTL information for CRTSCTS 
> when probed, I think, instead of just quietly lying about it...
> 
>  arch/arm/mach-davinci/include/mach/serial.h |    2 ++
>  arch/arm/mach-davinci/serial.c              |   19 +++++++++++++++++++
>  2 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/include/mach/serial.h b/arch/arm/mach-davinci/include/mach/serial.h
> index 8051110..8f7f5e5 100644
> --- a/arch/arm/mach-davinci/include/mach/serial.h
> +++ b/arch/arm/mach-davinci/include/mach/serial.h
> @@ -49,6 +49,8 @@
>  struct davinci_uart_config {
>  	/* Bit field of UARTs present; bit 0 --> UART1 */
>  	unsigned int enabled_uarts;
> +	/* Bit field of modem status interrupt disables; bit 0 -> UART1 */
> +	unsigned int disable_msi;
>  };
>  
>  extern int davinci_serial_init(struct davinci_uart_config *);
> diff --git a/arch/arm/mach-davinci/serial.c b/arch/arm/mach-davinci/serial.c
> index 1875740..83d44c0 100644
> --- a/arch/arm/mach-davinci/serial.c
> +++ b/arch/arm/mach-davinci/serial.c
> @@ -31,6 +31,22 @@
>  #include <mach/serial.h>
>  #include <mach/cputype.h>
>  
> +static void davinci_serial_out_nomsi(struct uart_port *p, int offset, int value)
> +{
> +	int lcr, lcr_off;
> +
> +	if (offset == UART_IER) {
> +		lcr_off = UART_LCR << p->regshift;
> +		lcr = readb(p->membase + lcr_off);
> +		/* don't override DLM setting, or probing operations */
> +		if (!(lcr & UART_LCR_DLAB) && p->type != PORT_UNKNOWN)
> +			value &= ~UART_IER_MSI;
> +	}
> +
> +	offset <<= p->regshift;
> +	writeb(value, p->membase + offset);
> +}
> +
>  static inline unsigned int serial_read_reg(struct plat_serial8250_port *up,
>  					   int offset)
>  {
> @@ -109,6 +125,9 @@ int __init davinci_serial_init(struct davinci_uart_config *info)
>  
>  		if (p->membase && p->type != PORT_AR7)
>  			davinci_serial_reset(p);
> +
> +		if (info->disable_msi & (1 << i))
> +			p->serial_out = davinci_serial_out_nomsi;
>  	}
>  
>  	return platform_device_register(soc_info->serial_dev);
> -- 
> 1.7.0.4
> 
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
> 
When status of CTS input is wrong, why can't we say flow control not supported on particular port and return an error to application trying to enable hardware flow control(i.e. set CRTSCTS). I am assuming you are seeing the spurious interrupts only on enabling hardware flow control(as modem status interrupt is enabled on setting the CRTSCTS).

Thanks,
Prakash


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

* [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
@ 2011-01-11  8:17   ` Manjunathappa, Prakash
  0 siblings, 0 replies; 14+ messages in thread
From: Manjunathappa, Prakash @ 2011-01-11  8:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Michael,

On Wed, Jan 05, 2011 at 17:56:31, Michael Williamson wrote:
> On the da850/omap-l138/am18x family of SoCs, up to three on chip UARTS may be
> configured.  These peripherals support the standard Tx/Rx signals as well as
> CTS/RTS hardware flow control signals.  The pins on these SOC's associated with
> these signals are multiplexed; e.g., the pin providing UART0_TXD capability
> also provides SPI0 chip select line 5 output capability.  The configuration of
> the pin multiplexing occurs during platform initialization (or by earlier
> bootloader operations).
> 
> There is a problem with the multiplexing implementation on these SOCs.  Only
> the output and output enable portions of the I/O section of the pin are
> multiplexed.  All peripheral input functions remain connected to a given pin
> regardless of configuration.
> 
> In many configurations of these parts, providing a UART with Tx/Rx capability
> is needed, but the HW flow control capability is not.  Furthermore, the pins
> associated with the CTS inputs on these UARTS are often configured to support
> a different peripheral, and they may be active/toggling during runtime.  This
> can result in false modem status (CTS) interrupts being asserted to the 8250
> driver controlling the associated Tx/Rx pins, and will impact system
> performance.
> 
> The 8250 serial driver platform data does not provide a direct mechanism to
> tell the driver to disable modem status (i.e., CTS) interrupts for a given
> port.  As a work-around, intercept register writes to the IER and disable
> modem status interrupts.
> 
> This patch was tested using a MityDSP-L138 SOM having UART1 enabled with the
> associated CTS pin connected to a clock (configured for the AHCLKX function).
> 
> Background / problem reports related to this issue are captured in the links
> below:
> http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/36701.aspx
> http://www.mail-archive.com/davinci-linux-open-source at linux.davincidsp.com/msg19524.html
> 
> Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
> Tested-by: Michael Williamson <michael.williamson@criticallink.com>
> ---
> This is against the linux-davinci tree.
> 
> Changes from original proposed patch:
> - instead of overriding set_termios, now override serial_out driver hook
>   and intercept writes to the MSR.
> 
> An alternate patch was proposed that modified the serial core driver and added a UPF_NO_MSR
> flag.  There was resistance to this patch.  The reasoning is that the core 8250 driver code
> should not continue to get muddied by platform hacks.
> 
> I'm wondering, given this and the original proposed patch, if the set_termios
> override might be better?  This patch incurs a test penalty every time the UART
> is accessed; whereas the original patch only incurs a penalty on IOCTL calls.  The
> set_termios would at least report the proper IOCTL information for CRTSCTS 
> when probed, I think, instead of just quietly lying about it...
> 
>  arch/arm/mach-davinci/include/mach/serial.h |    2 ++
>  arch/arm/mach-davinci/serial.c              |   19 +++++++++++++++++++
>  2 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/include/mach/serial.h b/arch/arm/mach-davinci/include/mach/serial.h
> index 8051110..8f7f5e5 100644
> --- a/arch/arm/mach-davinci/include/mach/serial.h
> +++ b/arch/arm/mach-davinci/include/mach/serial.h
> @@ -49,6 +49,8 @@
>  struct davinci_uart_config {
>  	/* Bit field of UARTs present; bit 0 --> UART1 */
>  	unsigned int enabled_uarts;
> +	/* Bit field of modem status interrupt disables; bit 0 -> UART1 */
> +	unsigned int disable_msi;
>  };
>  
>  extern int davinci_serial_init(struct davinci_uart_config *);
> diff --git a/arch/arm/mach-davinci/serial.c b/arch/arm/mach-davinci/serial.c
> index 1875740..83d44c0 100644
> --- a/arch/arm/mach-davinci/serial.c
> +++ b/arch/arm/mach-davinci/serial.c
> @@ -31,6 +31,22 @@
>  #include <mach/serial.h>
>  #include <mach/cputype.h>
>  
> +static void davinci_serial_out_nomsi(struct uart_port *p, int offset, int value)
> +{
> +	int lcr, lcr_off;
> +
> +	if (offset == UART_IER) {
> +		lcr_off = UART_LCR << p->regshift;
> +		lcr = readb(p->membase + lcr_off);
> +		/* don't override DLM setting, or probing operations */
> +		if (!(lcr & UART_LCR_DLAB) && p->type != PORT_UNKNOWN)
> +			value &= ~UART_IER_MSI;
> +	}
> +
> +	offset <<= p->regshift;
> +	writeb(value, p->membase + offset);
> +}
> +
>  static inline unsigned int serial_read_reg(struct plat_serial8250_port *up,
>  					   int offset)
>  {
> @@ -109,6 +125,9 @@ int __init davinci_serial_init(struct davinci_uart_config *info)
>  
>  		if (p->membase && p->type != PORT_AR7)
>  			davinci_serial_reset(p);
> +
> +		if (info->disable_msi & (1 << i))
> +			p->serial_out = davinci_serial_out_nomsi;
>  	}
>  
>  	return platform_device_register(soc_info->serial_dev);
> -- 
> 1.7.0.4
> 
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source at linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
> 
When status of CTS input is wrong, why can't we say flow control not supported on particular port and return an error to application trying to enable hardware flow control(i.e. set CRTSCTS). I am assuming you are seeing the spurious interrupts only on enabling hardware flow control(as modem status interrupt is enabled on setting the CRTSCTS).

Thanks,
Prakash

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

* Re: [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
  2011-01-11  8:17   ` Manjunathappa, Prakash
@ 2011-01-11 12:42     ` Michael Williamson
  -1 siblings, 0 replies; 14+ messages in thread
From: Michael Williamson @ 2011-01-11 12:42 UTC (permalink / raw)
  To: Manjunathappa, Prakash
  Cc: davinci-linux-open-source, khilman, gregkh, linux-serial,
	linux-arm-kernel

On 1/11/2011 3:17 AM, Manjunathappa, Prakash wrote:

> Hi Michael,
> 
> On Wed, Jan 05, 2011 at 17:56:31, Michael Williamson wrote:
>> On the da850/omap-l138/am18x family of SoCs, up to three on chip UARTS may be
>> configured.  These peripherals support the standard Tx/Rx signals as well as
>> CTS/RTS hardware flow control signals.  The pins on these SOC's associated with
>> these signals are multiplexed; e.g., the pin providing UART0_TXD capability
>> also provides SPI0 chip select line 5 output capability.  The configuration of
>> the pin multiplexing occurs during platform initialization (or by earlier
>> bootloader operations).
>>
>> There is a problem with the multiplexing implementation on these SOCs.  Only
>> the output and output enable portions of the I/O section of the pin are
>> multiplexed.  All peripheral input functions remain connected to a given pin
>> regardless of configuration.
>>
>> In many configurations of these parts, providing a UART with Tx/Rx capability
>> is needed, but the HW flow control capability is not.  Furthermore, the pins
>> associated with the CTS inputs on these UARTS are often configured to support
>> a different peripheral, and they may be active/toggling during runtime.  This
>> can result in false modem status (CTS) interrupts being asserted to the 8250
>> driver controlling the associated Tx/Rx pins, and will impact system
>> performance.
>>
>> The 8250 serial driver platform data does not provide a direct mechanism to
>> tell the driver to disable modem status (i.e., CTS) interrupts for a given
>> port.  As a work-around, intercept register writes to the IER and disable
>> modem status interrupts.
>>
>> This patch was tested using a MityDSP-L138 SOM having UART1 enabled with the
>> associated CTS pin connected to a clock (configured for the AHCLKX function).
>>
>> Background / problem reports related to this issue are captured in the links
>> below:
>> http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/36701.aspx
>> http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg19524.html
>>
>> Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
>> Tested-by: Michael Williamson <michael.williamson@criticallink.com>
>> ---
>> This is against the linux-davinci tree.
>>
>> Changes from original proposed patch:
>> - instead of overriding set_termios, now override serial_out driver hook
>>   and intercept writes to the MSR.
>>
>> An alternate patch was proposed that modified the serial core driver and added a UPF_NO_MSR
>> flag.  There was resistance to this patch.  The reasoning is that the core 8250 driver code
>> should not continue to get muddied by platform hacks.
>>
>> I'm wondering, given this and the original proposed patch, if the set_termios
>> override might be better?  This patch incurs a test penalty every time the UART
>> is accessed; whereas the original patch only incurs a penalty on IOCTL calls.  The
>> set_termios would at least report the proper IOCTL information for CRTSCTS 
>> when probed, I think, instead of just quietly lying about it...
>>
>>  arch/arm/mach-davinci/include/mach/serial.h |    2 ++
>>  arch/arm/mach-davinci/serial.c              |   19 +++++++++++++++++++
>>  2 files changed, 21 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/include/mach/serial.h b/arch/arm/mach-davinci/include/mach/serial.h
>> index 8051110..8f7f5e5 100644
>> --- a/arch/arm/mach-davinci/include/mach/serial.h
>> +++ b/arch/arm/mach-davinci/include/mach/serial.h
>> @@ -49,6 +49,8 @@
>>  struct davinci_uart_config {
>>  	/* Bit field of UARTs present; bit 0 --> UART1 */
>>  	unsigned int enabled_uarts;
>> +	/* Bit field of modem status interrupt disables; bit 0 -> UART1 */
>> +	unsigned int disable_msi;
>>  };
>>  
>>  extern int davinci_serial_init(struct davinci_uart_config *);
>> diff --git a/arch/arm/mach-davinci/serial.c b/arch/arm/mach-davinci/serial.c
>> index 1875740..83d44c0 100644
>> --- a/arch/arm/mach-davinci/serial.c
>> +++ b/arch/arm/mach-davinci/serial.c
>> @@ -31,6 +31,22 @@
>>  #include <mach/serial.h>
>>  #include <mach/cputype.h>
>>  
>> +static void davinci_serial_out_nomsi(struct uart_port *p, int offset, int value)
>> +{
>> +	int lcr, lcr_off;
>> +
>> +	if (offset == UART_IER) {
>> +		lcr_off = UART_LCR << p->regshift;
>> +		lcr = readb(p->membase + lcr_off);
>> +		/* don't override DLM setting, or probing operations */
>> +		if (!(lcr & UART_LCR_DLAB) && p->type != PORT_UNKNOWN)
>> +			value &= ~UART_IER_MSI;
>> +	}
>> +
>> +	offset <<= p->regshift;
>> +	writeb(value, p->membase + offset);
>> +}
>> +
>>  static inline unsigned int serial_read_reg(struct plat_serial8250_port *up,
>>  					   int offset)
>>  {
>> @@ -109,6 +125,9 @@ int __init davinci_serial_init(struct davinci_uart_config *info)
>>  
>>  		if (p->membase && p->type != PORT_AR7)
>>  			davinci_serial_reset(p);
>> +
>> +		if (info->disable_msi & (1 << i))
>> +			p->serial_out = davinci_serial_out_nomsi;
>>  	}
>>  
>>  	return platform_device_register(soc_info->serial_dev);
>> -- 
>> 1.7.0.4
>>
>> _______________________________________________
>> Davinci-linux-open-source mailing list
>> Davinci-linux-open-source@linux.davincidsp.com
>> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>>
> When status of CTS input is wrong, why can't we say flow control not
> supported on particular port and return an error to application trying to
> enable hardware flow control(i.e. set CRTSCTS). I am assuming you are seeing
> the spurious interrupts only on enabling hardware flow control(as modem
> status interrupt is enabled on setting the CRTSCTS).
> 



This was pretty much what the original patch I had proposed did.  There are
actually two termios flags that cause MS interrupts to be enabled by the
driver, CLOCAL (disabled) or CRTSCTS (enabled).  I observed that some
process after the kernel had completed booting (getty, I think)
was calling set_termios with CLOCAL disabled (not CRTSCTS enabled).  I could
probably alter the getty args to work around it, but it's a bit of a hole to
not have something in the kernel to preclude sending the OS to "interrupt 
hell".

Is there another way to accomplish what you are suggesting that is different
from the original patch?

Thanks for the comments.

-Mike

Background: The original patch (not accepted) is here
https://patchwork.kernel.org/patch/442671/

This kicked a second email thread:
http://marc.info/?l=linux-serial&m=129409970003827&w=4

>From that thread, it was suggested to add a port flag to the driver
flags structure for disabling MS interrupts.

Driver Tweak patch (not accepted):
https://patchwork.kernel.org/patch/450771/

>From that thread, it was suggested to override the I/O methods and hide
the problem back in the platform code (this patch).

All three proposed patches work for me.  I'd gladly submit a fourth if
suggested -- this problem pretty much kills a couple of platform
configurations we are working on...

> Thanks,
> Prakash
> 





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

* [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
@ 2011-01-11 12:42     ` Michael Williamson
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Williamson @ 2011-01-11 12:42 UTC (permalink / raw)
  To: linux-arm-kernel

On 1/11/2011 3:17 AM, Manjunathappa, Prakash wrote:

> Hi Michael,
> 
> On Wed, Jan 05, 2011 at 17:56:31, Michael Williamson wrote:
>> On the da850/omap-l138/am18x family of SoCs, up to three on chip UARTS may be
>> configured.  These peripherals support the standard Tx/Rx signals as well as
>> CTS/RTS hardware flow control signals.  The pins on these SOC's associated with
>> these signals are multiplexed; e.g., the pin providing UART0_TXD capability
>> also provides SPI0 chip select line 5 output capability.  The configuration of
>> the pin multiplexing occurs during platform initialization (or by earlier
>> bootloader operations).
>>
>> There is a problem with the multiplexing implementation on these SOCs.  Only
>> the output and output enable portions of the I/O section of the pin are
>> multiplexed.  All peripheral input functions remain connected to a given pin
>> regardless of configuration.
>>
>> In many configurations of these parts, providing a UART with Tx/Rx capability
>> is needed, but the HW flow control capability is not.  Furthermore, the pins
>> associated with the CTS inputs on these UARTS are often configured to support
>> a different peripheral, and they may be active/toggling during runtime.  This
>> can result in false modem status (CTS) interrupts being asserted to the 8250
>> driver controlling the associated Tx/Rx pins, and will impact system
>> performance.
>>
>> The 8250 serial driver platform data does not provide a direct mechanism to
>> tell the driver to disable modem status (i.e., CTS) interrupts for a given
>> port.  As a work-around, intercept register writes to the IER and disable
>> modem status interrupts.
>>
>> This patch was tested using a MityDSP-L138 SOM having UART1 enabled with the
>> associated CTS pin connected to a clock (configured for the AHCLKX function).
>>
>> Background / problem reports related to this issue are captured in the links
>> below:
>> http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/36701.aspx
>> http://www.mail-archive.com/davinci-linux-open-source at linux.davincidsp.com/msg19524.html
>>
>> Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
>> Tested-by: Michael Williamson <michael.williamson@criticallink.com>
>> ---
>> This is against the linux-davinci tree.
>>
>> Changes from original proposed patch:
>> - instead of overriding set_termios, now override serial_out driver hook
>>   and intercept writes to the MSR.
>>
>> An alternate patch was proposed that modified the serial core driver and added a UPF_NO_MSR
>> flag.  There was resistance to this patch.  The reasoning is that the core 8250 driver code
>> should not continue to get muddied by platform hacks.
>>
>> I'm wondering, given this and the original proposed patch, if the set_termios
>> override might be better?  This patch incurs a test penalty every time the UART
>> is accessed; whereas the original patch only incurs a penalty on IOCTL calls.  The
>> set_termios would at least report the proper IOCTL information for CRTSCTS 
>> when probed, I think, instead of just quietly lying about it...
>>
>>  arch/arm/mach-davinci/include/mach/serial.h |    2 ++
>>  arch/arm/mach-davinci/serial.c              |   19 +++++++++++++++++++
>>  2 files changed, 21 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/include/mach/serial.h b/arch/arm/mach-davinci/include/mach/serial.h
>> index 8051110..8f7f5e5 100644
>> --- a/arch/arm/mach-davinci/include/mach/serial.h
>> +++ b/arch/arm/mach-davinci/include/mach/serial.h
>> @@ -49,6 +49,8 @@
>>  struct davinci_uart_config {
>>  	/* Bit field of UARTs present; bit 0 --> UART1 */
>>  	unsigned int enabled_uarts;
>> +	/* Bit field of modem status interrupt disables; bit 0 -> UART1 */
>> +	unsigned int disable_msi;
>>  };
>>  
>>  extern int davinci_serial_init(struct davinci_uart_config *);
>> diff --git a/arch/arm/mach-davinci/serial.c b/arch/arm/mach-davinci/serial.c
>> index 1875740..83d44c0 100644
>> --- a/arch/arm/mach-davinci/serial.c
>> +++ b/arch/arm/mach-davinci/serial.c
>> @@ -31,6 +31,22 @@
>>  #include <mach/serial.h>
>>  #include <mach/cputype.h>
>>  
>> +static void davinci_serial_out_nomsi(struct uart_port *p, int offset, int value)
>> +{
>> +	int lcr, lcr_off;
>> +
>> +	if (offset == UART_IER) {
>> +		lcr_off = UART_LCR << p->regshift;
>> +		lcr = readb(p->membase + lcr_off);
>> +		/* don't override DLM setting, or probing operations */
>> +		if (!(lcr & UART_LCR_DLAB) && p->type != PORT_UNKNOWN)
>> +			value &= ~UART_IER_MSI;
>> +	}
>> +
>> +	offset <<= p->regshift;
>> +	writeb(value, p->membase + offset);
>> +}
>> +
>>  static inline unsigned int serial_read_reg(struct plat_serial8250_port *up,
>>  					   int offset)
>>  {
>> @@ -109,6 +125,9 @@ int __init davinci_serial_init(struct davinci_uart_config *info)
>>  
>>  		if (p->membase && p->type != PORT_AR7)
>>  			davinci_serial_reset(p);
>> +
>> +		if (info->disable_msi & (1 << i))
>> +			p->serial_out = davinci_serial_out_nomsi;
>>  	}
>>  
>>  	return platform_device_register(soc_info->serial_dev);
>> -- 
>> 1.7.0.4
>>
>> _______________________________________________
>> Davinci-linux-open-source mailing list
>> Davinci-linux-open-source at linux.davincidsp.com
>> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>>
> When status of CTS input is wrong, why can't we say flow control not
> supported on particular port and return an error to application trying to
> enable hardware flow control(i.e. set CRTSCTS). I am assuming you are seeing
> the spurious interrupts only on enabling hardware flow control(as modem
> status interrupt is enabled on setting the CRTSCTS).
> 



This was pretty much what the original patch I had proposed did.  There are
actually two termios flags that cause MS interrupts to be enabled by the
driver, CLOCAL (disabled) or CRTSCTS (enabled).  I observed that some
process after the kernel had completed booting (getty, I think)
was calling set_termios with CLOCAL disabled (not CRTSCTS enabled).  I could
probably alter the getty args to work around it, but it's a bit of a hole to
not have something in the kernel to preclude sending the OS to "interrupt 
hell".

Is there another way to accomplish what you are suggesting that is different
from the original patch?

Thanks for the comments.

-Mike

Background: The original patch (not accepted) is here
https://patchwork.kernel.org/patch/442671/

This kicked a second email thread:
http://marc.info/?l=linux-serial&m=129409970003827&w=4

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

* RE: [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
  2011-01-11 12:42     ` Michael Williamson
@ 2011-01-11 15:43         ` Manjunathappa, Prakash
  -1 siblings, 0 replies; 14+ messages in thread
From: Manjunathappa, Prakash @ 2011-01-11 15:43 UTC (permalink / raw)
  To: Michael Williamson
  Cc: khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR,
	davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
	gregkh-l3A5Bk7waGM,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-serial-u79uwXL29TY76Z2rM5mHXA

On Tue, Jan 11, 2011 at 18:12:25, Michael Williamson wrote:
> [...]
> This was pretty much what the original patch I had proposed did.  There are
> actually two termios flags that cause MS interrupts to be enabled by the
> driver, CLOCAL (disabled) or CRTSCTS (enabled).  I observed that some
> process after the kernel had completed booting (getty, I think)
> was calling set_termios with CLOCAL disabled (not CRTSCTS enabled).  I could
> probably alter the getty args to work around it, but it's a bit of a hole to
> not have something in the kernel to preclude sending the OS to "interrupt 
> hell".
> 
> Is there another way to accomplish what you are suggesting that is different
> from the original patch?
> 
> Thanks for the comments.
> 
> -Mike
> 
> Background: The original patch (not accepted) is here
> https://patchwork.kernel.org/patch/442671/
> 
> This kicked a second email thread:
> http://marc.info/?l=linux-serial&m=129409970003827&w=4
> 
> From that thread, it was suggested to add a port flag to the driver
> flags structure for disabling MS interrupts.
> 
> Driver Tweak patch (not accepted):
> https://patchwork.kernel.org/patch/450771/
> 
> From that thread, it was suggested to override the I/O methods and hide
> the problem back in the platform code (this patch).
> 
> All three proposed patches work for me.  I'd gladly submit a fourth if
> suggested -- this problem pretty much kills a couple of platform
> configurations we are working on...
> 
> > Thanks,
> > Prakash
> > 
Thanks for clarifying. Looks like this is what we can do as of now.

Thanks,
Prakash

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

* [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
@ 2011-01-11 15:43         ` Manjunathappa, Prakash
  0 siblings, 0 replies; 14+ messages in thread
From: Manjunathappa, Prakash @ 2011-01-11 15:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 11, 2011 at 18:12:25, Michael Williamson wrote:
> [...]
> This was pretty much what the original patch I had proposed did.  There are
> actually two termios flags that cause MS interrupts to be enabled by the
> driver, CLOCAL (disabled) or CRTSCTS (enabled).  I observed that some
> process after the kernel had completed booting (getty, I think)
> was calling set_termios with CLOCAL disabled (not CRTSCTS enabled).  I could
> probably alter the getty args to work around it, but it's a bit of a hole to
> not have something in the kernel to preclude sending the OS to "interrupt 
> hell".
> 
> Is there another way to accomplish what you are suggesting that is different
> from the original patch?
> 
> Thanks for the comments.
> 
> -Mike
> 
> Background: The original patch (not accepted) is here
> https://patchwork.kernel.org/patch/442671/
> 
> This kicked a second email thread:
> http://marc.info/?l=linux-serial&m=129409970003827&w=4
> 
> From that thread, it was suggested to add a port flag to the driver
> flags structure for disabling MS interrupts.
> 
> Driver Tweak patch (not accepted):
> https://patchwork.kernel.org/patch/450771/
> 
> From that thread, it was suggested to override the I/O methods and hide
> the problem back in the platform code (this patch).
> 
> All three proposed patches work for me.  I'd gladly submit a fourth if
> suggested -- this problem pretty much kills a couple of platform
> configurations we are working on...
> 
> > Thanks,
> > Prakash
> > 
Thanks for clarifying. Looks like this is what we can do as of now.

Thanks,
Prakash

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

* Re: [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
  2011-01-05 12:26 ` Michael Williamson
@ 2011-01-29 21:01   ` Michael Williamson
  -1 siblings, 0 replies; 14+ messages in thread
From: Michael Williamson @ 2011-01-29 21:01 UTC (permalink / raw)
  To: Michael Williamson
  Cc: davinci-linux-open-source, linux-arm-kernel, khilman,
	linux-serial, gregkh

Hi Kevin...

On 01/05/2011 07:26 AM, Michael Williamson wrote:
> On the da850/omap-l138/am18x family of SoCs, up to three on chip UARTS may be
> configured.  These peripherals support the standard Tx/Rx signals as well as
> CTS/RTS hardware flow control signals.  The pins on these SOC's associated with
> these signals are multiplexed; e.g., the pin providing UART0_TXD capability
> also provides SPI0 chip select line 5 output capability.  The configuration of
> the pin multiplexing occurs during platform initialization (or by earlier
> bootloader operations).
> 
> There is a problem with the multiplexing implementation on these SOCs.  Only
> the output and output enable portions of the I/O section of the pin are
> multiplexed.  All peripheral input functions remain connected to a given pin
> regardless of configuration.
> 
> In many configurations of these parts, providing a UART with Tx/Rx capability
> is needed, but the HW flow control capability is not.  Furthermore, the pins
> associated with the CTS inputs on these UARTS are often configured to support
> a different peripheral, and they may be active/toggling during runtime.  This
> can result in false modem status (CTS) interrupts being asserted to the 8250
> driver controlling the associated Tx/Rx pins, and will impact system
> performance.
> 
> The 8250 serial driver platform data does not provide a direct mechanism to
> tell the driver to disable modem status (i.e., CTS) interrupts for a given
> port.  As a work-around, intercept register writes to the IER and disable
> modem status interrupts.
> 
> This patch was tested using a MityDSP-L138 SOM having UART1 enabled with the
> associated CTS pin connected to a clock (configured for the AHCLKX function).
> 
> Background / problem reports related to this issue are captured in the links
> below:
> http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/36701.aspx
> http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg19524.html
> 
> Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
> Tested-by: Michael Williamson <michael.williamson@criticallink.com>
> ---
> This is against the linux-davinci tree.
> 
> Changes from original proposed patch:
> - instead of overriding set_termios, now override serial_out driver hook
>   and intercept writes to the MSR.
> 
> An alternate patch was proposed that modified the serial core driver and added a UPF_NO_MSR
> flag.  There was resistance to this patch.  The reasoning is that the core 8250 driver code
> should not continue to get muddied by platform hacks.
> 

I'd like to withdraw this patch.  It works, but it's inefficient and ugly, and 
I also get the feeling it isn't going to make it in it's current form anyway.

I have another patch that is limited to just the mityomapl138 platform that 
I would like to submit in place of this.  No other board appears to have this
problem, so it makes sense to constrain the fix to platform file that does.

-Mike

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

* [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
@ 2011-01-29 21:01   ` Michael Williamson
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Williamson @ 2011-01-29 21:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Kevin...

On 01/05/2011 07:26 AM, Michael Williamson wrote:
> On the da850/omap-l138/am18x family of SoCs, up to three on chip UARTS may be
> configured.  These peripherals support the standard Tx/Rx signals as well as
> CTS/RTS hardware flow control signals.  The pins on these SOC's associated with
> these signals are multiplexed; e.g., the pin providing UART0_TXD capability
> also provides SPI0 chip select line 5 output capability.  The configuration of
> the pin multiplexing occurs during platform initialization (or by earlier
> bootloader operations).
> 
> There is a problem with the multiplexing implementation on these SOCs.  Only
> the output and output enable portions of the I/O section of the pin are
> multiplexed.  All peripheral input functions remain connected to a given pin
> regardless of configuration.
> 
> In many configurations of these parts, providing a UART with Tx/Rx capability
> is needed, but the HW flow control capability is not.  Furthermore, the pins
> associated with the CTS inputs on these UARTS are often configured to support
> a different peripheral, and they may be active/toggling during runtime.  This
> can result in false modem status (CTS) interrupts being asserted to the 8250
> driver controlling the associated Tx/Rx pins, and will impact system
> performance.
> 
> The 8250 serial driver platform data does not provide a direct mechanism to
> tell the driver to disable modem status (i.e., CTS) interrupts for a given
> port.  As a work-around, intercept register writes to the IER and disable
> modem status interrupts.
> 
> This patch was tested using a MityDSP-L138 SOM having UART1 enabled with the
> associated CTS pin connected to a clock (configured for the AHCLKX function).
> 
> Background / problem reports related to this issue are captured in the links
> below:
> http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/36701.aspx
> http://www.mail-archive.com/davinci-linux-open-source at linux.davincidsp.com/msg19524.html
> 
> Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
> Tested-by: Michael Williamson <michael.williamson@criticallink.com>
> ---
> This is against the linux-davinci tree.
> 
> Changes from original proposed patch:
> - instead of overriding set_termios, now override serial_out driver hook
>   and intercept writes to the MSR.
> 
> An alternate patch was proposed that modified the serial core driver and added a UPF_NO_MSR
> flag.  There was resistance to this patch.  The reasoning is that the core 8250 driver code
> should not continue to get muddied by platform hacks.
> 

I'd like to withdraw this patch.  It works, but it's inefficient and ugly, and 
I also get the feeling it isn't going to make it in it's current form anyway.

I have another patch that is limited to just the mityomapl138 platform that 
I would like to submit in place of this.  No other board appears to have this
problem, so it makes sense to constrain the fix to platform file that does.

-Mike

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

* Re: [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
  2011-01-29 21:01   ` Michael Williamson
@ 2011-02-01 20:53     ` Kevin Hilman
  -1 siblings, 0 replies; 14+ messages in thread
From: Kevin Hilman @ 2011-02-01 20:53 UTC (permalink / raw)
  To: Michael Williamson
  Cc: davinci-linux-open-source, gregkh, linux-serial, linux-arm-kernel

Michael Williamson <michael.williamson@criticallink.com> writes:

> Hi Kevin...
>
> On 01/05/2011 07:26 AM, Michael Williamson wrote:
>> On the da850/omap-l138/am18x family of SoCs, up to three on chip UARTS may be
>> configured.  These peripherals support the standard Tx/Rx signals as well as
>> CTS/RTS hardware flow control signals.  The pins on these SOC's associated with
>> these signals are multiplexed; e.g., the pin providing UART0_TXD capability
>> also provides SPI0 chip select line 5 output capability.  The configuration of
>> the pin multiplexing occurs during platform initialization (or by earlier
>> bootloader operations).
>> 
>> There is a problem with the multiplexing implementation on these SOCs.  Only
>> the output and output enable portions of the I/O section of the pin are
>> multiplexed.  All peripheral input functions remain connected to a given pin
>> regardless of configuration.
>> 
>> In many configurations of these parts, providing a UART with Tx/Rx capability
>> is needed, but the HW flow control capability is not.  Furthermore, the pins
>> associated with the CTS inputs on these UARTS are often configured to support
>> a different peripheral, and they may be active/toggling during runtime.  This
>> can result in false modem status (CTS) interrupts being asserted to the 8250
>> driver controlling the associated Tx/Rx pins, and will impact system
>> performance.
>> 
>> The 8250 serial driver platform data does not provide a direct mechanism to
>> tell the driver to disable modem status (i.e., CTS) interrupts for a given
>> port.  As a work-around, intercept register writes to the IER and disable
>> modem status interrupts.
>> 
>> This patch was tested using a MityDSP-L138 SOM having UART1 enabled with the
>> associated CTS pin connected to a clock (configured for the AHCLKX function).
>> 
>> Background / problem reports related to this issue are captured in the links
>> below:
>> http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/36701.aspx
>> http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg19524.html
>> 
>> Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
>> Tested-by: Michael Williamson <michael.williamson@criticallink.com>
>> ---
>> This is against the linux-davinci tree.
>> 
>> Changes from original proposed patch:
>> - instead of overriding set_termios, now override serial_out driver hook
>>   and intercept writes to the MSR.
>> 
>> An alternate patch was proposed that modified the serial core driver and added a UPF_NO_MSR
>> flag.  There was resistance to this patch.  The reasoning is that the core 8250 driver code
>> should not continue to get muddied by platform hacks.
>> 
>
> I'd like to withdraw this patch.  It works, but it's inefficient and ugly, and 
> I also get the feeling it isn't going to make it in it's current form anyway.
>
> I have another patch that is limited to just the mityomapl138 platform that 
> I would like to submit in place of this.  No other board appears to have this
> problem, so it makes sense to constrain the fix to platform file that does.
>

OK

Kevin

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

* [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS
@ 2011-02-01 20:53     ` Kevin Hilman
  0 siblings, 0 replies; 14+ messages in thread
From: Kevin Hilman @ 2011-02-01 20:53 UTC (permalink / raw)
  To: linux-arm-kernel

Michael Williamson <michael.williamson@criticallink.com> writes:

> Hi Kevin...
>
> On 01/05/2011 07:26 AM, Michael Williamson wrote:
>> On the da850/omap-l138/am18x family of SoCs, up to three on chip UARTS may be
>> configured.  These peripherals support the standard Tx/Rx signals as well as
>> CTS/RTS hardware flow control signals.  The pins on these SOC's associated with
>> these signals are multiplexed; e.g., the pin providing UART0_TXD capability
>> also provides SPI0 chip select line 5 output capability.  The configuration of
>> the pin multiplexing occurs during platform initialization (or by earlier
>> bootloader operations).
>> 
>> There is a problem with the multiplexing implementation on these SOCs.  Only
>> the output and output enable portions of the I/O section of the pin are
>> multiplexed.  All peripheral input functions remain connected to a given pin
>> regardless of configuration.
>> 
>> In many configurations of these parts, providing a UART with Tx/Rx capability
>> is needed, but the HW flow control capability is not.  Furthermore, the pins
>> associated with the CTS inputs on these UARTS are often configured to support
>> a different peripheral, and they may be active/toggling during runtime.  This
>> can result in false modem status (CTS) interrupts being asserted to the 8250
>> driver controlling the associated Tx/Rx pins, and will impact system
>> performance.
>> 
>> The 8250 serial driver platform data does not provide a direct mechanism to
>> tell the driver to disable modem status (i.e., CTS) interrupts for a given
>> port.  As a work-around, intercept register writes to the IER and disable
>> modem status interrupts.
>> 
>> This patch was tested using a MityDSP-L138 SOM having UART1 enabled with the
>> associated CTS pin connected to a clock (configured for the AHCLKX function).
>> 
>> Background / problem reports related to this issue are captured in the links
>> below:
>> http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/t/36701.aspx
>> http://www.mail-archive.com/davinci-linux-open-source at linux.davincidsp.com/msg19524.html
>> 
>> Signed-off-by: Michael Williamson <michael.williamson@criticallink.com>
>> Tested-by: Michael Williamson <michael.williamson@criticallink.com>
>> ---
>> This is against the linux-davinci tree.
>> 
>> Changes from original proposed patch:
>> - instead of overriding set_termios, now override serial_out driver hook
>>   and intercept writes to the MSR.
>> 
>> An alternate patch was proposed that modified the serial core driver and added a UPF_NO_MSR
>> flag.  There was resistance to this patch.  The reasoning is that the core 8250 driver code
>> should not continue to get muddied by platform hacks.
>> 
>
> I'd like to withdraw this patch.  It works, but it's inefficient and ugly, and 
> I also get the feeling it isn't going to make it in it's current form anyway.
>
> I have another patch that is limited to just the mityomapl138 platform that 
> I would like to submit in place of this.  No other board appears to have this
> problem, so it makes sense to constrain the fix to platform file that does.
>

OK

Kevin

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

end of thread, other threads:[~2011-02-01 20:53 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-05 12:26 [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS Michael Williamson
2011-01-05 12:26 ` Michael Williamson
2011-01-05 12:26 ` [PATCH 2/2 v1] davinci: mityomapl138 platform: disable MS interrupts on UART1 Michael Williamson
2011-01-05 12:26   ` Michael Williamson
2011-01-11  8:17 ` [PATCH 1/2 v1] davinci: support disabling modem status interrupts on SOC UARTS Manjunathappa, Prakash
2011-01-11  8:17   ` Manjunathappa, Prakash
2011-01-11 12:42   ` Michael Williamson
2011-01-11 12:42     ` Michael Williamson
     [not found]     ` <4D2C5031.30507-wZX4cNJlHJ2sVWG7oymsAA@public.gmane.org>
2011-01-11 15:43       ` Manjunathappa, Prakash
2011-01-11 15:43         ` Manjunathappa, Prakash
2011-01-29 21:01 ` Michael Williamson
2011-01-29 21:01   ` Michael Williamson
2011-02-01 20:53   ` Kevin Hilman
2011-02-01 20:53     ` Kevin Hilman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.