linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jiri Slaby <jirislaby@kernel.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	<linux-serial@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [EXT] Re: [PATCH v1 3/4] serial: 8250_pci: Always try MSI/MSI-X
Date: Fri, 16 Jul 2021 15:07:16 +0200	[thread overview]
Message-ID: <ac8ac10e-aa43-93a1-d36e-6304643375ae@oth-regensburg.de> (raw)
In-Reply-To: <CAHp75VdoaE7hCOzsRvuf=7A4mmv0NWBmwqK_mM8vO-K3YZKTUQ@mail.gmail.com>



On 14/07/2021 15:35, Andy Shevchenko wrote:
> On Wed, Jul 14, 2021 at 3:56 PM Ralf Ramsauer
> <ralf.ramsauer@oth-regensburg.de> wrote:
>> On 14/07/2021 08:54, Jiri Slaby wrote:
>>> On 13. 07. 21, 12:40, Andy Shevchenko wrote:
> 
>>> Hmm, have you checked the commit which introduced the whitelist?
>>>
>>>     Nevertheless, this needs to handled with care: while many 8250 devices
>>>     actually claim to support MSI(-X) interrupts it should not be
>>> enabled be
>>>     default. I had at least one device in my hands with broken MSI
>>>     implementation.
>>>
>>>     So better introduce a whitelist with devices that are known to support
>>>     MSI(-X) interrupts. I tested all devices mentioned in the patch.
>>>
>>>
>>> You should have at least CCed the author for an input.
>>
>> Yep, back then I was testing three different 8250 pci cards. All of them
>> claimed to support MSI, while one really worked with MSI, the one that I
>> whitelisted. So I thought it would be better to use legacy IRQs as long
>> as no one tested a specific card to work with MSI.
> 
> Can you shed a light eventually what those cards are?

So I found a no-name el-cheapo card that has some issues with MSI:

18:00.0 Serial controller: Device 1c00:3253 (rev 10) (prog-if 05 [16850])

The card comes with two serial lines. It comes perfectly up, if I enable
it to use MSI in the whitelist:

serial 0000:18:00.0: Using MSI(-X) interrupts
serial 0000:18:00.0: Setup PCI port: port 40c0, irq 104, type 0
0000:18:00.0: ttyS6 at I/O 0x40c0 (irq = 104, base_baud = 115200) is a
XR16850
serial 0000:18:00.0: Setup PCI port: port 40c8, irq 104, type 0
0000:18:00.0: ttyS7 at I/O 0x40c8 (irq = 104, base_baud = 115200) is a
XR16850

After loading 8250_pci, lspci -vvs 18:0.0 tells:

	Capabilities: [68] MSI: Enable+ Count=1/32 Maskable+ 64bit+
		Address: 00000000fee000b8  Data: 0000
		Masking: ffffffff  Pending: 00000000

Looks good so far. Now let's echo to the device.

$ echo asdf > /dev/ttyS6

-- stuck. The echoing process stucks at close():

write(1, "asdf\n", 5)                   = 5
close(1

Stuck in the sense of: the echo is still killable, no crashes. The same
happens if I try to access the device with stty. So something is odd
here. However, the Netmos cards that I whitelisted do a great job.

So I can't tell if I was just unlucky to grab a card that has issues
with MSI, and this is an exception rather than the rule…

HTH,
  Ralf


> 
>> Don't do that… And don't convert it to a blacklist. A blacklist will
>> break users until they report that something doesn't work.
> 
> White list is not okay either. MSI in general is a right thing to do.
> preventing users from MSI is asking for the performance degradation
> and IRQ resource conflicts (in case the IRQ line is shared).
> 
> Besides that, shouldn't it be rather the specific field in private (to
> 8250_pci) structure than constantly growing list?
> 

  parent reply	other threads:[~2021-07-16 13:07 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-13 10:40 [PATCH v1 1/4] serial: 8250_pci: Refactor the loop in pci_ite887x_init() Andy Shevchenko
2021-07-13 10:40 ` [PATCH v1 2/4] serial: 8250_pci: Get rid of redundant 'else' keyword Andy Shevchenko
2021-07-13 10:40 ` [PATCH v1 3/4] serial: 8250_pci: Always try MSI/MSI-X Andy Shevchenko
2021-07-14  6:54   ` Jiri Slaby
2021-07-14  7:58     ` Jiri Slaby
2021-07-14  9:31       ` Andy Shevchenko
2021-07-14  9:15     ` Andy Shevchenko
2021-07-14 16:56       ` rwright
2021-07-14 12:49     ` [EXT] " Ralf Ramsauer
2021-07-14 13:35       ` Andy Shevchenko
2021-07-14 16:49         ` Ralf Ramsauer
2021-07-16 13:07         ` Ralf Ramsauer [this message]
2021-07-16 15:01           ` Andy Shevchenko
     [not found]             ` <599a37bd-3cb4-1e4b-d5f8-936c4daae71f@oth-regensburg.de>
2021-07-16 17:27               ` Andy Shevchenko
2021-07-17 12:44                 ` Ralf Ramsauer
2021-07-13 10:40 ` [PATCH v1 4/4] serial: 8250_pci: Replace dev_*() by pci_*() macros Andy Shevchenko
2021-07-13 21:05   ` Joe Perches
2021-07-14  6:57 ` [PATCH v1 1/4] serial: 8250_pci: Refactor the loop in pci_ite887x_init() Jiri Slaby
2021-07-14 12:37   ` Andy Shevchenko
2021-07-14  8:07 ` Dan Carpenter
2021-07-14 10:44 ` Joe Perches
2021-07-14 12:36   ` Andy Shevchenko

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=ac8ac10e-aa43-93a1-d36e-6304643375ae@oth-regensburg.de \
    --to=ralf.ramsauer@oth-regensburg.de \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    /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).