Skip to contentSkip to conversation

8250_pci: Replace commits 448ac154 and e86ff4a

PATCH In review sudhakar 4 participants 7 messages Apr 3, 2012 – Apr 9, 2012 TTY layer and serial drivers

lore
Share & tools
lore

sudhakar

author
#0 lore raw

PATCH 1/2 8250_pci: Replace commits 448ac154 and e86ff4a

From: Sudhakar Mamillapalli <[email protected]>

The following commits don't completely fix the KT serial missing interrupt problems. So they have been replaced instead by enabling serial backup timer. The iir-once + msi tried approach tried to enforce one read of the iir per THRE event, but its defeated by any other event occurring at the wrong time, particularly MSR events.

448ac15 serial/8250_pci: setup-quirk workaround for the kt serial controller e86ff4a serial/8250_pci: init-quirk msi support for kt serial controller

Signed-off-by Sudhakar Mamillapalli <[email protected]>
Acked-by Dan Williams <[email protected]>
Acked-by Nhan H Mai <[email protected]>
---
 drivers/tty/serial/8250/8250.c     |   12 ++++++------
 drivers/tty/serial/8250/8250_pci.c |   16 +---------------
 include/linux/serial_core.h        |    2 +-
 3 files changed, 8 insertions(+), 22 deletions(-)
drivers/tty/serial/8250/8250.c +6 -6
index 5b149b4..5fb0157 100644
@@ -1572,13 +1572,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)

@@ -2037,10 +2035,12 @@ static int serial8250_startup(struct uart_port *port)
 		spin_unlock_irqrestore(&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));
drivers/tty/serial/8250/8250_pci.c +1 -15
index da2b0b0..858dca8 100644
@@ -1096,7 +1096,7 @@ 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;
+	port->flags |= UPF_BUG_THRE;
 	return skip_tx_en_setup(priv, board, port, idx);
 }
 

@@ -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
include/linux/serial_core.h +1 -1
index f51bf2e..2db407a 100644
@@ -357,7 +357,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_IIR_ONCE		((__force upf_t) (1 << 26))
+#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))
Signature · 1 line
1.7.8.4
#1 lore raw
Quoted 10 lines · sudhakar

From: Sudhakar Mamillapalli <[email protected]>

The following commits don't completely fix the KT serial missing interrupt problems.  So they have been replaced instead by enabling serial backup timer.  The iir-once + msi tried approach tried to enforce one read of the iir per THRE event, but its defeated by any other event occurring at the wrong time, particularly MSR events.

Quoted 6 lines

448ac15 serial/8250_pci: setup-quirk workaround for the kt serial controller e86ff4a serial/8250_pci: init-quirk msi support for kt serial controller

Signed-off-by Sudhakar Mamillapalli <[email protected]>
Acked-by Dan Williams <[email protected]>
Acked-by Nhan H Mai <[email protected]>

Just a note that this should also be tagged for 3.3-stable since the msi setup hack in e86ff4a breaks suspend/resume like this:

[  365.250523] sysfs: cannot create duplicate filename

'/devices/pci0000:00/0000:00:16.3/msi_irqs'
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
In reply to sudhakar
Quoted 4 lines · On Tue, 3 Apr 2012 12:43:20 -0700 sudhakar <[email protected]> wrote:

From: Sudhakar Mamillapalli <[email protected]>

Quoted 8 lines

The following commits don't completely fix the KT serial missing interrupt problems. So they have been replaced instead by enabling serial backup timer. The iir-once + msi tried approach tried to enforce one read of the iir per THRE event, but its defeated by any other event occurring at the wrong time, particularly MSR events.

448ac15 serial/8250_pci: setup-quirk workaround for the kt serial controller e86ff4a serial/8250_pci: init-quirk msi support for kt serial controller

These are already in tree. If you are reversing them and adding a different fix then please post the series as

- two revert patches exactly reverting the two commits
- a follow on patch or two adding the replacement

That makes it much easier to review and much more obvious. It also helps make testing simpler.

Alan

maintainer
#3 lore raw
Quoted 16 lines · Alan Cox
Quoted 4 lines · On Tue, 3 Apr 2012 12:43:20 -0700 sudhakar <[email protected]> wrote:

From: Sudhakar Mamillapalli <[email protected]>

Quoted 8 lines

The following commits don't completely fix the KT serial missing interrupt problems. So they have been replaced instead by enabling serial backup timer. The iir-once + msi tried approach tried to enforce one read of the iir per THRE event, but its defeated by any other event occurring at the wrong time, particularly MSR events.

448ac15 serial/8250_pci: setup-quirk workaround for the kt serial controller e86ff4a serial/8250_pci: init-quirk msi support for kt serial controller

Quoted 8 lines

These are already in tree. If you are reversing them and adding a different fix then please post the series as

- two revert patches exactly reverting the two commits
- a follow on patch or two adding the replacement

That makes it much easier to review and much more obvious. It also helps make testing simpler.

I agree, Sudhakar, can you please redo these 2 patches?

greg k-h

#4 lore raw
Quoted 20 lines · Greg KH
Quoted 19 lines · Alan Cox
Quoted 4 lines · On Tue, 3 Apr 2012 12:43:20 -0700 sudhakar <[email protected]> wrote:

From: Sudhakar Mamillapalli <[email protected]>

Quoted 8 lines

The following commits don't completely fix the KT serial missing interrupt problems.  So they have been replaced instead by enabling serial backup timer.  The iir-once + msi tried approach tried to enforce one read of the iir per THRE event, but its defeated by any other event occurring at the wrong time, particularly MSR events.

448ac15 serial/8250_pci: setup-quirk workaround for the kt serial controller e86ff4a serial/8250_pci: init-quirk msi support for kt serial controller

These are already in tree. If you are reversing them and adding a different fix then please post the series as

Quoted 8 lines
Quoted 5 lines

- two revert patches exactly reverting the two commits
- a follow on patch or two adding the replacement

That makes it much easier to review and much more obvious. It also helps make testing simpler.

I agree, Sudhakar, can you please redo these 2 patches?

Hi Greg,

This was in the series:

[PATCH 0/6] rework quirks for the "kt" serial port [1]

...but I have just noticed it collides with current git, and I also need to address Stephen's comments for Tegra. Expect a v2 shortly.

--
Dan

[1]: http://marc.info/?l=linux-serial&m=133373702606784&w=2
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#5 lore raw
Quoted 30 lines · Dan J
Quoted 20 lines · Greg KH
Quoted 19 lines · Alan Cox
Quoted 4 lines · On Tue, 3 Apr 2012 12:43:20 -0700 sudhakar <[email protected]> wrote:

From: Sudhakar Mamillapalli <[email protected]>

Quoted 8 lines

The following commits don't completely fix the KT serial missing interrupt problems.  So they have been replaced instead by enabling serial backup timer.  The iir-once + msi tried approach tried to enforce one read of the iir per THRE event, but its defeated by any other event occurring at the wrong time, particularly MSR events.

448ac15 serial/8250_pci: setup-quirk workaround for the kt serial controller e86ff4a serial/8250_pci: init-quirk msi support for kt serial controller

These are already in tree. If you are reversing them and adding a different fix then please post the series as

Quoted 8 lines
Quoted 5 lines

- two revert patches exactly reverting the two commits
- a follow on patch or two adding the replacement

That makes it much easier to review and much more obvious. It also helps make testing simpler.

I agree, Sudhakar, can you please redo these 2 patches?

Quoted 8 lines

Hi Greg,

This was in the series:

[PATCH 0/6] rework quirks for the "kt" serial port [1]

...but I have just noticed it collides with current git, and I also need to address Stephen's comments for Tegra. Expect a v2 shortly.

Ok, thanks. Note, I just applied the first 3 to go to Linus soon, those didn't need to be reworked, right?

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw
Quoted 35 lines · Greg KH
Quoted 34 lines · Dan J
Quoted 20 lines · Greg KH
Quoted 19 lines · Alan Cox
Quoted 4 lines · On Tue, 3 Apr 2012 12:43:20 -0700 sudhakar <[email protected]> wrote:

From: Sudhakar Mamillapalli <[email protected]>

Quoted 8 lines

The following commits don't completely fix the KT serial missing interrupt problems.  So they have been replaced instead by enabling serial backup timer.  The iir-once + msi tried approach tried to enforce one read of the iir per THRE event, but its defeated by any other event occurring at the wrong time, particularly MSR events.

448ac15 serial/8250_pci: setup-quirk workaround for the kt serial controller e86ff4a serial/8250_pci: init-quirk msi support for kt serial controller

These are already in tree. If you are reversing them and adding a different fix then please post the series as

Quoted 8 lines
Quoted 5 lines

- two revert patches exactly reverting the two commits
- a follow on patch or two adding the replacement

That makes it much easier to review and much more obvious. It also helps make testing simpler.

I agree, Sudhakar, can you please redo these 2 patches?

Hi Greg,

This was in the series:

Quoted 8 lines
Quoted 4 lines

[PATCH 0/6] rework quirks for the "kt" serial port [1]

...but I have just noticed it collides with current git, and I also need to address Stephen's comments for Tegra.  Expect a v2 shortly.

Ok, thanks.  Note, I just applied the first 3 to go to Linus soon, those didn't need to be reworked, right?

Correct, those were ready to go. Thanks!

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html