linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Mario Kleiner <mario.kleiner.de@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de>,
	"open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>,
	Jiri Slaby <jirislaby@kernel.org>
Subject: Re: [EXT] [PATCH] serial: 8250_pci: Avoid irq sharing for MSI(-X) interrupts.
Date: Fri, 13 Aug 2021 20:04:39 +0300	[thread overview]
Message-ID: <CAHp75VdLR8K_eYwfyfVZUyaOhtq0OyyHA0n0bcjWwNpFOEt1xg@mail.gmail.com> (raw)
In-Reply-To: <CAEsyxyhqxUMw6nV2ULn3VS8Jax+r95+pZD-sZh5-NDOje3C6NQ@mail.gmail.com>

On Fri, Aug 13, 2021 at 7:45 PM Mario Kleiner
<mario.kleiner.de@gmail.com> wrote:
>
> Btw., one question to the experts, after much more grepping through
> the irq handling code and being no wiser than before:
>
> My understanding of MSI(-X) is that one of the big advantages over
> legacy IRQ is that there are so many available MSI IRQ's, and so IRQ
> sharing is normally not needed. So why did the driver choose an MSI
> irq shared with the USB host controller? Or iow., why did
>
> pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES); and uart.port.irq
> = pci_irq_vector(dev, 0); return a IRQ number that was already in use
> at that time? Is this something to be expected, or some quirk specific
> to serial drivers or this hw?

My theory is that they are sharing the same pin on the PCI bus and
drivers have a SHARED flag and some code inside IRQ core decides that
it's fine to have them shared. But I think the best person to answer
is Thomas.

> > > > > On 29/07/2021 06:33, Mario Kleiner wrote:
> > > > > > This attempts to fix a bug found with a serial port card which uses
> > > > > > an MCS9922 chip, one of the 4 models for which MSI-X interrupts are
> > > > > > currently supported. I don't possess such a card, and i'm not
> > > > > > experienced with the serial subsystem, so this patch is based on what
> > > > > > i think i found as a likely reason for failure, based on walking the
> > > > > > user who actually owns the card through some diagnostic.
> > > > >
> > > > > As there's currently some (stuck) discussion on how to generally handle
> > > > > MSI capable serial cards, and this is issue related to some degree, let
> > > > > me Cc Jiri and Andy.
> > > >
> > > > Thanks, Ralf!
> > > >
> > > > > > The user who reported the problem finds the following in his dmesg
> > > > > > output for the relevant ttyS4 and ttyS5:
> > > > > >
> > > > > > [    0.580425] serial 0000:02:00.0: enabling device (0000 -> 0003)
> > > > > > [    0.601448] 0000:02:00.0: ttyS4 at I/O 0x3010 (irq = 125, base_baud = 115200) is a ST16650V2
> > > > > > [    0.603089] serial 0000:02:00.1: enabling device (0000 -> 0003)
> > > > > > [    0.624119] 0000:02:00.1: ttyS5 at I/O 0x3000 (irq = 126, base_baud = 115200) is a ST16650V2
> > > > > > ...
> > > > > > [    6.323784] genirq: Flags mismatch irq 128. 00000080 (ttyS5) vs. 00000000 (xhci_hcd)
> > > > > > [    6.324128] genirq: Flags mismatch irq 128. 00000080 (ttyS5) vs. 00000000 (xhci_hcd)
> > > > > > ...
> > > > > >
> > > > > > Output of setserial -a:
> > > > > >
> > > > > > /dev/ttyS4, Line 4, UART: 16650V2, Port: 0x3010, IRQ: 127
> > > > > >       Baud_base: 115200, close_delay: 50, divisor: 0
> > > > > >       closing_wait: 3000
> > > > > >       Flags: spd_normal skip_test
> > > > > >
> > > > > > This suggests to me that the serial driver wants to register and share a
> > > > > > MSI/MSI-X irq 128 with the xhci_hcd driver, whereas the xhci driver does
> > > > > > not want to share the irq, as flags 0x00000080 (== IRQF_SHARED) from the
> > > > > > serial port driver means to share the irq, and this mismatch ends in some
> > > > > > failed irq init?
> > > > > >
> > > > > > With this setup, data reception works very unreliable, with dropped data,
> > > > > > already at a transmission rate of only a 16 Bytes chunk every 1/120th of
> > > > > > a second, ie. 1920 Bytes/sec, presumably due to rx fifo overflow due to
> > > > > > mishandled or not used at all rx irq's?
> > > > > >
> > > > > > See full discussion thread with attempted diagnosis at:
> > > > > >
> > > > > > https://psychtoolbox.discourse.group/t/issues-with-iscan-serial-port-recording/3886
> > > > > >
> > > > > > Disabling the use of MSI interrupts for the serial port pci card did
> > > > > > fix the reliability problems. The user executed the following sequence
> > > > > > of commands to achieve this:
> > > > > >
> > > > > > # Disable PCI serial port driver, shut down card:
> > > > > > echo 0000:02:00.0 | sudo tee /sys/bus/pci/drivers/serial/unbind
> > > > > > echo 0000:02:00.1 | sudo tee /sys/bus/pci/drivers/serial/unbind
> > > > > >
> > > > > > # Disallow use of MSI/MSI-X interrupts on pci serial port card:
> > > > > > echo 0 | sudo tee /sys/bus/pci/devices/0000:02:00.0/msi_bus
> > > > > > echo 0 | sudo tee /sys/bus/pci/devices/0000:02:00.1/msi_bus
> > > > > >
> > > > > > # Restart driver, reinitialize card, hopefully without MSI irqs now:
> > > > > > echo 0000:02:00.0 | sudo tee /sys/bus/pci/drivers/serial/bind
> > > > > > echo 0000:02:00.1 | sudo tee /sys/bus/pci/drivers/serial/bind
> > > > > >
> > > > > > This resulted in the following log output:
> > > > > >
> > > > > > [   82.179021] pci 0000:02:00.0: MSI/MSI-X disallowed for future drivers
> > > > > > [   87.003031] pci 0000:02:00.1: MSI/MSI-X disallowed for future drivers
> > > > > > [   98.537010] 0000:02:00.0: ttyS4 at I/O 0x3010 (irq = 17, base_baud = 115200) is a ST16650V2
> > > > > > [  103.648124] 0000:02:00.1: ttyS5 at I/O 0x3000 (irq = 18, base_baud = 115200) is a ST16650V2
> > > > > >
> > > > > > This patch attempts to fix the problem by disabling irq sharing when
> > > > > > using MSI irq's. Note that all i know for sure is that disabling MSI
> > > >
> > > > In general the shared MSI interrupts are weird things that can be done
> > > > with IRQs.
> > > >
> > > > > > irq's fixed the problem for the user, so this patch could be wrong and
> > > > > > is untested. Please review with caution, keeping this in mind.
> > > >
> > > > I think it's a good idea in general. I have no objections.

-- 
With Best Regards,
Andy Shevchenko

      reply	other threads:[~2021-08-13 17:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29  4:33 [PATCH] serial: 8250_pci: Avoid irq sharing for MSI(-X) interrupts Mario Kleiner
2021-07-29  8:37 ` [EXT] " Ralf Ramsauer
2021-07-29 15:25   ` Andy Shevchenko
2021-07-30  8:20     ` Greg Kroah-Hartman
2021-07-30  8:36       ` Andy Shevchenko
2021-08-13 16:45         ` Mario Kleiner
2021-08-13 17:04           ` Andy Shevchenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHp75VdLR8K_eYwfyfVZUyaOhtq0OyyHA0n0bcjWwNpFOEt1xg@mail.gmail.com \
    --to=andy.shevchenko@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=mario.kleiner.de@gmail.com \
    --cc=ralf.ramsauer@oth-regensburg.de \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).