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

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

[sudhakar]

From: Sudhakar Mamillapalli <sudhakar@fb.com>


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 <sudhakar@fb.com>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Nhan H Mai <nhan.h.mai@intel.com>
---
 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(-)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 5b149b4..5fb0157 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -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));
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index da2b0b0..858dca8 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -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
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index f51bf2e..2db407a 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -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))
-- 
1.7.8.4

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

[Williams, Dan J]

On Tue, Apr 3, 2012 at 12:43 PM, sudhakar <sudhakar@fb.com> wrote:
>
> From: Sudhakar Mamillapalli <sudhakar@fb.com>
>
>
> 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 <sudhakar@fb.com>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Acked-by: Nhan H Mai <nhan.h.mai@intel.com>

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'

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

[Alan Cox]

On Tue, 3 Apr 2012 12:43:20 -0700
sudhakar <sudhakar@fb.com> wrote:

> 
> From: Sudhakar Mamillapalli <sudhakar@fb.com>
> 
> 
> 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

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

[Greg KH]

On Tue, Apr 03, 2012 at 09:58:32PM +0100, Alan Cox wrote:
> On Tue, 3 Apr 2012 12:43:20 -0700
> sudhakar <sudhakar@fb.com> wrote:
> 
> > 
> > From: Sudhakar Mamillapalli <sudhakar@fb.com>
> > 
> > 
> > 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.

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

greg k-h

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

[Williams, Dan J]

On Mon, Apr 9, 2012 at 9:47 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Apr 03, 2012 at 09:58:32PM +0100, Alan Cox wrote:
>> On Tue, 3 Apr 2012 12:43:20 -0700
>> sudhakar <sudhakar@fb.com> wrote:
>>
>> >
>> > From: Sudhakar Mamillapalli <sudhakar@fb.com>
>> >
>> >
>> > 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.
>
> 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

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

[Greg KH]

On Mon, Apr 09, 2012 at 09:53:47AM -0700, Williams, Dan J wrote:
> On Mon, Apr 9, 2012 at 9:47 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Apr 03, 2012 at 09:58:32PM +0100, Alan Cox wrote:
> >> On Tue, 3 Apr 2012 12:43:20 -0700
> >> sudhakar <sudhakar@fb.com> wrote:
> >>
> >> >
> >> > From: Sudhakar Mamillapalli <sudhakar@fb.com>
> >> >
> >> >
> >> > 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.
> >
> > 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.

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

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

[Williams, Dan J]

On Mon, Apr 9, 2012 at 10:48 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Apr 09, 2012 at 09:53:47AM -0700, Williams, Dan J wrote:
>> On Mon, Apr 9, 2012 at 9:47 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Tue, Apr 03, 2012 at 09:58:32PM +0100, Alan Cox wrote:
>> >> On Tue, 3 Apr 2012 12:43:20 -0700
>> >> sudhakar <sudhakar@fb.com> wrote:
>> >>
>> >> >
>> >> > From: Sudhakar Mamillapalli <sudhakar@fb.com>
>> >> >
>> >> >
>> >> > 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.
>> >
>> > 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.
>
> 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