[PATCH 1/6] Revert "serial/8250_pci: init-quirk msi support for kt serial controller"

[PATCH 1/6] Revert "serial/8250_pci: init-quirk msi support for kt serial controller"

[Dan Williams]

This reverts commit e86ff4a63c9fdd875ba8492577cd1ad2252f525c.

This tried to enforce the semantics of one interrupt per iir read of the
THRE (transmit-hold empty) status, but events from other sources
(particularly modem status) defeat this guarantee.

This change also broke 8250_pci suspend/resume support as
pciserial_resume_ports() re-runs .init() quirks, but does not run
.exit() quirks in pciserial_suspend_ports() leading to reports like:

  sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:16.3/msi_irqs'

...and a subsequent crash.  The mismatch of init/exit at suspend/resume
seems like a bug in its own right.

[stable: 3.3.x]
Cc: <stable@vger.kernel.org>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Sudhakar Mamillapalli <sudhakar@fb.com>
Reported-by: Nhan H Mai <nhan.h.mai@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/tty/serial/8250/8250_pci.c |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index da2b0b0..1d4ccf8 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1118,18 +1118,6 @@ pci_xr17c154_setup(struct serial_private *priv,
 	return pci_default_setup(priv, board, port, idx);
 }
 
-static int try_enable_msi(struct pci_dev *dev)
-{
-	/* use msi if available, but fallback to legacy otherwise */
-	pci_enable_msi(dev);
-	return 0;
-}
-
-static void disable_msi(struct pci_dev *dev)
-{
-	pci_disable_msi(dev);
-}
-
 #define PCI_VENDOR_ID_SBSMODULARIO	0x124B
 #define PCI_SUBVENDOR_ID_SBSMODULARIO	0x124B
 #define PCI_DEVICE_ID_OCTPRO		0x0001
@@ -1249,9 +1237,7 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = {
 		.device		= PCI_DEVICE_ID_INTEL_PATSBURG_KT,
 		.subvendor	= PCI_ANY_ID,
 		.subdevice	= PCI_ANY_ID,
-		.init		= try_enable_msi,
 		.setup		= kt_serial_setup,
-		.exit		= disable_msi,
 	},
 	/*
 	 * ITE

[PATCH 2/6] Revert "serial/8250_pci: setup-quirk workaround for the kt serial controller"

[Dan Williams]

This reverts commit 448ac154c957c4580531fa0c8f2045816fe2f0e7.

The semantic of UPF_IIR_ONCE is only guaranteed to workaround the race
condition in the kt serial's iir register if the only source of
interrupts is THRE (fifo-empty) events.  An modem status event at the
wrong time can again cause an iir read to drop the 'empty' status
leading to a hang.  So, revert this in preparation for using the
existing "I don't trust my iir register" workaround in the 8250 core
(UART_BUG_THRE).

[stable: 3.3.x]
Cc: <stable@vger.kernel.org>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Sudhakar Mamillapalli <sudhakar@fb.com>
Reported-by: Nhan H Mai <nhan.h.mai@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/tty/serial/8250/8250.c     |    4 +---
 drivers/tty/serial/8250/8250_pci.c |   17 +----------------
 include/linux/serial_core.h        |    1 -
 3 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 9b7336f..ec7d99b 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -1592,13 +1592,11 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
 	do {
 		struct uart_8250_port *up;
 		struct uart_port *port;
-		bool skip;
 
 		up = list_entry(l, struct uart_8250_port, list);
 		port = &up->port;
-		skip = pass_counter && up->port.flags & UPF_IIR_ONCE;
 
-		if (!skip && port->handle_irq(port)) {
+		if (port->handle_irq(port)) {
 			handled = 1;
 			end = NULL;
 		} else if (end == NULL)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 1d4ccf8..105dcfb 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1092,14 +1092,6 @@ static int skip_tx_en_setup(struct serial_private *priv,
 	return pci_default_setup(priv, board, port, idx);
 }
 
-static int kt_serial_setup(struct serial_private *priv,
-			   const struct pciserial_board *board,
-			   struct uart_port *port, int idx)
-{
-	port->flags |= UPF_IIR_ONCE;
-	return skip_tx_en_setup(priv, board, port, idx);
-}
-
 static int pci_eg20t_init(struct pci_dev *dev)
 {
 #if defined(CONFIG_SERIAL_PCH_UART) || defined(CONFIG_SERIAL_PCH_UART_MODULE)
@@ -1118,6 +1110,7 @@ pci_xr17c154_setup(struct serial_private *priv,
 	return pci_default_setup(priv, board, port, idx);
 }
 
+/* This should be in linux/pci_ids.h */
 #define PCI_VENDOR_ID_SBSMODULARIO	0x124B
 #define PCI_SUBVENDOR_ID_SBSMODULARIO	0x124B
 #define PCI_DEVICE_ID_OCTPRO		0x0001
@@ -1147,7 +1140,6 @@ pci_xr17c154_setup(struct serial_private *priv,
 #define PCI_DEVICE_ID_OXSEMI_16PCI958	0x9538
 #define PCIE_DEVICE_ID_NEO_2_OX_IBM	0x00F6
 #define PCI_DEVICE_ID_PLX_CRONYX_OMEGA	0xc001
-#define PCI_DEVICE_ID_INTEL_PATSBURG_KT 0x1d3d
 
 /* Unknown vendors/cards - this should not be in linux/pci_ids.h */
 #define PCI_SUBDEVICE_ID_UNKNOWN_0x1584	0x1584
@@ -1232,13 +1224,6 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = {
 		.subdevice	= PCI_ANY_ID,
 		.setup		= ce4100_serial_setup,
 	},
-	{
-		.vendor		= PCI_VENDOR_ID_INTEL,
-		.device		= PCI_DEVICE_ID_INTEL_PATSBURG_KT,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= kt_serial_setup,
-	},
 	/*
 	 * ITE
 	 */
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index c91ace7..c56d114 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -355,7 +355,6 @@ struct uart_port {
 #define UPF_CONS_FLOW		((__force upf_t) (1 << 23))
 #define UPF_SHARE_IRQ		((__force upf_t) (1 << 24))
 #define UPF_EXAR_EFR		((__force upf_t) (1 << 25))
-#define UPF_IIR_ONCE		((__force upf_t) (1 << 26))
 /* The exact UART type is known and should not be probed.  */
 #define UPF_FIXED_TYPE		((__force upf_t) (1 << 27))
 #define UPF_BOOT_AUTOCONF	((__force upf_t) (1 << 28))

[PATCH 3/6] serial/8250_pci: add a "force background timer" flag and use it for the "kt" serial port

[Dan Williams]

Workaround dropped notifications in the iir register.  Register reads
coincident with new interrupt notifications sometimes result in this
device clearing the interrupt event without reporting it in the read
data.

The serial core already has a heuristic for determining when a device
has an untrustworthy iir register.  In this case when we apriori know
that the iir is faulty use a flag (UPF_BUG_THRE) to bypass the test and
force usage of the background timer.

Cc: Alan Cox <alan@linux.intel.com>
[stable: 3.3.x]
Cc: <stable@vger.kernel.org>
Reported-by: Nhan H Mai <nhan.h.mai@intel.com>
Reported-by: Sudhakar Mamillapalli <sudhakar@fb.com>
Tested-by: Nhan H Mai <nhan.h.mai@intel.com>
Tested-by: Sudhakar Mamillapalli <sudhakar@fb.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/tty/serial/8250/8250.c     |    8 +++++---
 drivers/tty/serial/8250/8250_pci.c |   17 ++++++++++++++++-
 include/linux/serial_core.h        |    1 +
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index ec7d99b..0e2c703 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -2055,10 +2055,12 @@ static int serial8250_startup(struct uart_port *port)
 		spin_unlock_irqrestore(&up->port.lock, flags);
 
 		/*
-		 * If the interrupt is not reasserted, setup a timer to
-		 * kick the UART on a regular basis.
+		 * If the interrupt is not reasserted, or we otherwise
+		 * don't trust the iir, setup a timer to kick the UART
+		 * on a regular basis.
 		 */
-		if (!(iir1 & UART_IIR_NO_INT) && (iir & UART_IIR_NO_INT)) {
+		if ((!(iir1 & UART_IIR_NO_INT) && (iir & UART_IIR_NO_INT)) ||
+		    up->port.flags & UPF_BUG_THRE) {
 			up->bugs |= UART_BUG_THRE;
 			pr_debug("ttyS%d - using backup timer\n",
 				 serial_index(port));
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 105dcfb..858dca8 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1092,6 +1092,14 @@ static int skip_tx_en_setup(struct serial_private *priv,
 	return pci_default_setup(priv, board, port, idx);
 }
 
+static int kt_serial_setup(struct serial_private *priv,
+			   const struct pciserial_board *board,
+			   struct uart_port *port, int idx)
+{
+	port->flags |= UPF_BUG_THRE;
+	return skip_tx_en_setup(priv, board, port, idx);
+}
+
 static int pci_eg20t_init(struct pci_dev *dev)
 {
 #if defined(CONFIG_SERIAL_PCH_UART) || defined(CONFIG_SERIAL_PCH_UART_MODULE)
@@ -1110,7 +1118,6 @@ pci_xr17c154_setup(struct serial_private *priv,
 	return pci_default_setup(priv, board, port, idx);
 }
 
-/* This should be in linux/pci_ids.h */
 #define PCI_VENDOR_ID_SBSMODULARIO	0x124B
 #define PCI_SUBVENDOR_ID_SBSMODULARIO	0x124B
 #define PCI_DEVICE_ID_OCTPRO		0x0001
@@ -1140,6 +1147,7 @@ pci_xr17c154_setup(struct serial_private *priv,
 #define PCI_DEVICE_ID_OXSEMI_16PCI958	0x9538
 #define PCIE_DEVICE_ID_NEO_2_OX_IBM	0x00F6
 #define PCI_DEVICE_ID_PLX_CRONYX_OMEGA	0xc001
+#define PCI_DEVICE_ID_INTEL_PATSBURG_KT 0x1d3d
 
 /* Unknown vendors/cards - this should not be in linux/pci_ids.h */
 #define PCI_SUBDEVICE_ID_UNKNOWN_0x1584	0x1584
@@ -1224,6 +1232,13 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = {
 		.subdevice	= PCI_ANY_ID,
 		.setup		= ce4100_serial_setup,
 	},
+	{
+		.vendor		= PCI_VENDOR_ID_INTEL,
+		.device		= PCI_DEVICE_ID_INTEL_PATSBURG_KT,
+		.subvendor	= PCI_ANY_ID,
+		.subdevice	= PCI_ANY_ID,
+		.setup		= kt_serial_setup,
+	},
 	/*
 	 * ITE
 	 */
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index c56d114..cbb3d12 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -355,6 +355,7 @@ struct uart_port {
 #define UPF_CONS_FLOW		((__force upf_t) (1 << 23))
 #define UPF_SHARE_IRQ		((__force upf_t) (1 << 24))
 #define UPF_EXAR_EFR		((__force upf_t) (1 << 25))
+#define UPF_BUG_THRE		((__force upf_t) (1 << 26))
 /* The exact UART type is known and should not be probed.  */
 #define UPF_FIXED_TYPE		((__force upf_t) (1 << 27))
 #define UPF_BOOT_AUTOCONF	((__force upf_t) (1 << 28))

Re: [PATCH 1/6] Revert "serial/8250_pci: init-quirk msi support for kt serial controller"

[Alan Cox]

On Fri, 06 Apr 2012 11:49:37 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> This reverts commit e86ff4a63c9fdd875ba8492577cd1ad2252f525c.

Series

Acked-by: Alan Cox <alan@linux.intel.com>