* [PATCH] serial: 8250_pnp: pass IRQ shared flag to UART ports
@ 2020-02-09 4:42 Li RongQing
2020-02-10 10:09 ` Andy Shevchenko
0 siblings, 1 reply; 4+ messages in thread
From: Li RongQing @ 2020-02-09 4:42 UTC (permalink / raw)
To: gregkh, jslaby, haolee.swjtu, andriy.shevchenko, linux-serial
On some systems IRQ lines might be shared between multiple devices.
If so, the irqflags have to be configured accordingly. The reason is:
The 8250 port startup code performs IRQ tests *before* the IRQ handler
for that particular port is registered.
This commit fixed the below issue:
[ 973.782131] 8250 request irq 00000000f5a0e2ae 00000000f5a0e2ae 0
[ 973.785414] genirq: Flags mismatch irq 16. 00000004 (ttyS0) vs. 00000084 (ipmi_si)
[ 973.788741] CPU: 0 PID: 1 Comm: systemd Tainted: G E 4.19.0-1.0.0.1.rc2 #5
[ 973.792112] Hardware name: Huawei TaiShan 2280 V2/BC82AMDDA, BIOS 0.18 06/10/2019
[ 973.795577] Call trace:
[ 973.799018] dump_backtrace+0x0/0x198
[ 973.802493] show_stack+0x24/0x30
[ 973.805965] dump_stack+0x9c/0xbc
[ 973.809357] __setup_irq+0x150/0x6c0
[ 973.812663] request_threaded_irq+0xe8/0x180
[ 973.815891] univ8250_setup_irq+0x278/0x2a0
[ 973.819007] serial8250_do_startup+0x468/0x818
[ 973.822060] serial8250_startup+0x38/0x48
[ 973.825099] uart_startup.part.9+0x11c/0x270
[ 973.828156] uart_port_activate+0x64/0x98
[ 973.831247] tty_port_open+0xac/0x110
[ 973.834349] uart_open+0x2c/0x40
[ 973.837415] tty_open+0x110/0x3f8
[ 973.840464] chrdev_open+0xc8/0x248
[ 973.843584] do_dentry_open+0x118/0x358
[ 973.846615] vfs_open+0x38/0x48
[ 973.849621] do_last+0x23c/0x808
[ 973.852610] path_openat+0x88/0x260
[ 973.855596] do_filp_open+0x88/0x100
[ 973.858582] do_sys_open+0x188/0x218
[ 973.861564] __arm64_sys_openat+0x2c/0x38
[ 973.864575] el0_svc_common+0xac/0x178
[ 973.867592] el0_svc_handler+0x38/0x78
[ 973.870570] el0_svc+0x8/0xc
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
drivers/tty/serial/8250/8250_pnp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/8250/8250_pnp.c b/drivers/tty/serial/8250/8250_pnp.c
index de90d681b64c..10c260928952 100644
--- a/drivers/tty/serial/8250/8250_pnp.c
+++ b/drivers/tty/serial/8250/8250_pnp.c
@@ -476,6 +476,7 @@ serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
uart.port.flags |= UPF_SHARE_IRQ;
uart.port.uartclk = 1843200;
uart.port.dev = &dev->dev;
+ uart.port.irqflags |= IRQF_SHARED;
line = serial8250_register_8250_port(&uart);
if (line < 0 || (flags & CIR_PORT))
--
2.16.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] serial: 8250_pnp: pass IRQ shared flag to UART ports
2020-02-09 4:42 [PATCH] serial: 8250_pnp: pass IRQ shared flag to UART ports Li RongQing
@ 2020-02-10 10:09 ` Andy Shevchenko
2020-02-10 12:54 ` 答复: " Li,Rongqing
0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2020-02-10 10:09 UTC (permalink / raw)
To: Li RongQing; +Cc: gregkh, jslaby, haolee.swjtu, linux-serial
On Sun, Feb 09, 2020 at 12:42:27PM +0800, Li RongQing wrote:
> On some systems IRQ lines might be shared between multiple devices.
> If so, the irqflags have to be configured accordingly. The reason is:
> The 8250 port startup code performs IRQ tests *before* the IRQ handler
> for that particular port is registered.
Thanks for the report.
Before we proceed with it, can we have more information about the device in question?
How is it enumerated? What is in resources (ACPI / or ...?) for this device?
Also how IPMI is being involved to all this and why?
> This commit fixed the below issue:
> [ 973.782131] 8250 request irq 00000000f5a0e2ae 00000000f5a0e2ae 0
> [ 973.785414] genirq: Flags mismatch irq 16. 00000004 (ttyS0) vs. 00000084 (ipmi_si)
> [ 973.788741] CPU: 0 PID: 1 Comm: systemd Tainted: G E 4.19.0-1.0.0.1.rc2 #5
> [ 973.792112] Hardware name: Huawei TaiShan 2280 V2/BC82AMDDA, BIOS 0.18 06/10/2019
> [ 973.795577] Call trace:
> [ 973.799018] dump_backtrace+0x0/0x198
> [ 973.802493] show_stack+0x24/0x30
> [ 973.805965] dump_stack+0x9c/0xbc
> [ 973.809357] __setup_irq+0x150/0x6c0
> [ 973.812663] request_threaded_irq+0xe8/0x180
> [ 973.815891] univ8250_setup_irq+0x278/0x2a0
> [ 973.819007] serial8250_do_startup+0x468/0x818
> [ 973.822060] serial8250_startup+0x38/0x48
Nit: no need to put entire stack for this.
> --- a/drivers/tty/serial/8250/8250_pnp.c
> +++ b/drivers/tty/serial/8250/8250_pnp.c
> @@ -476,6 +476,7 @@ serial_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
> uart.port.flags |= UPF_SHARE_IRQ;
> uart.port.uartclk = 1843200;
> uart.port.dev = &dev->dev;
> + uart.port.irqflags |= IRQF_SHARED;
Why not to use UPF_SHARE_IRQ flags instead?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 4+ messages in thread
* 答复: [PATCH] serial: 8250_pnp: pass IRQ shared flag to UART ports
2020-02-10 10:09 ` Andy Shevchenko
@ 2020-02-10 12:54 ` Li,Rongqing
2020-02-11 13:47 ` Andy Shevchenko
0 siblings, 1 reply; 4+ messages in thread
From: Li,Rongqing @ 2020-02-10 12:54 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: gregkh, jslaby, haolee.swjtu, linux-serial
> -----邮件原件-----
> 发件人: Andy Shevchenko [mailto:andriy.shevchenko@linux.intel.com]
> 发送时间: 2020年2月10日 18:10
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: gregkh@linuxfoundation.org; jslaby@suse.com;
> haolee.swjtu@gmail.com; linux-serial@vger.kernel.org
> 主题: Re: [PATCH] serial: 8250_pnp: pass IRQ shared flag to UART ports
>
> On Sun, Feb 09, 2020 at 12:42:27PM +0800, Li RongQing wrote:
> > On some systems IRQ lines might be shared between multiple devices.
> > If so, the irqflags have to be configured accordingly. The reason is:
> > The 8250 port startup code performs IRQ tests *before* the IRQ handler
> > for that particular port is registered.
>
> Thanks for the report.
>
> Before we proceed with it, can we have more information about the device in
> question?
> How is it enumerated? What is in resources (ACPI / or ...?) for this device?
> Also how IPMI is being involved to all this and why?
>
This is arm server, resource is from dsdt, and detail is
8 {
7 Device (UART)
6 {
5 Name (_HID, "PNP0501" /* 16550A-compatible COM Serial Port */) // _HID: Hardware ID
4 Name (_UID, Zero) // _UID: Unique ID
3 Name (_CCA, One) // _CCA: Cache Coherency Attribute
2 Name (_DSD, Package (0x02) // _DSD: Device-Specific Data
1 {
7748 ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
1 Package (0x01)
2 {
3 Package (0x02)
4 {
5 "clock-frequency",
6 0x001C2000
7 }
8 }
9 })
10 Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
11 {
12 QWordMemory (ResourceConsumer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
13 0x0000000000000000, // Granularity
14 0x00000003F00002F8, // Range Minimum
15 0x00000003F00002FF, // Range Maximum
16 0x0000000000000000, // Translation Offset
17 0x0000000000000008, // Length
18 ,, , AddressRangeMemory, TypeStatic)
19 Interrupt (ResourceConsumer, Level, ActiveHigh, Shared, ,, )
20 {
21 0x000001E4,
22 }
23 })
24 }
25 }
26
9 Scope (_SB)
8 {
7 Device (IPI0)
6 {
5 Name (_HID, "IPI0001") // _HID: Hardware ID
4 Method (_IFT, 0, NotSerialized) // _IFT: IPMI Interface Type
3 {
2 Return (0x03)
1 }
10659
1 Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
2 {
3 QWordMemory (ResourceConsumer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
4 0x0000000000000000, // Granularity
5 0x00000003F00000E4, // Range Minimum
6 0x00000003F00000E7, // Range Maximum
7 0x0000000000000000, // Translation Offset
8 0x0000000000000004, // Length
9 ,, , AddressRangeMemory, TypeStatic)
10 Interrupt (ResourceConsumer, Level, ActiveHigh, Shared, ,, )
11 {
12 0x000001E4,
13 }
14 })
15 }
16 }
> > This commit fixed the below issue:
> > [ 973.782131] 8250 request irq 00000000f5a0e2ae 00000000f5a0e2ae 0 [
> > 973.785414] genirq: Flags mismatch irq 16. 00000004 (ttyS0) vs. 00000084
> (ipmi_si)
> > [ 973.788741] CPU: 0 PID: 1 Comm: systemd Tainted: G E
> 4.19.0-1.0.0.1.rc2 #5
> > [ 973.792112] Hardware name: Huawei TaiShan 2280 V2/BC82AMDDA,
> BIOS
> > 0.18 06/10/2019 [ 973.795577] Call trace:
> > [ 973.799018] dump_backtrace+0x0/0x198 [ 973.802493]
> > show_stack+0x24/0x30 [ 973.805965] dump_stack+0x9c/0xbc [
> > 973.809357] __setup_irq+0x150/0x6c0 [ 973.812663]
> > request_threaded_irq+0xe8/0x180 [ 973.815891]
> > univ8250_setup_irq+0x278/0x2a0 [ 973.819007]
> > serial8250_do_startup+0x468/0x818 [ 973.822060]
> > serial8250_startup+0x38/0x48
>
> Nit: no need to put entire stack for this.
>
> > --- a/drivers/tty/serial/8250/8250_pnp.c
> > +++ b/drivers/tty/serial/8250/8250_pnp.c
> > @@ -476,6 +476,7 @@ serial_pnp_probe(struct pnp_dev *dev, const struct
> pnp_device_id *dev_id)
> > uart.port.flags |= UPF_SHARE_IRQ;
> > uart.port.uartclk = 1843200;
> > uart.port.dev = &dev->dev;
> > + uart.port.irqflags |= IRQF_SHARED;
>
> Why not to use UPF_SHARE_IRQ flags instead?
> commit 54e53b2e8081e9eaba865e745ca61de9a8eccb18
And I think we has the same issue.
Author: Kurt Kanzenbach <kurt@linutronix.de>
Date: Fri Mar 16 12:31:58 2018 +0100
tty: serial: 8250: pass IRQ shared flag to UART ports
On some systems IRQ lines between multiple UARTs might be shared. If so, the
irqflags have to be configured accordingly. The reason is: The 8250 port startup
code performs IRQ tests *before* the IRQ handler for that particular port is
registered. This is performed in serial8250_do_startup(). This function checks
whether IRQF_SHARED is configured and only then disables the IRQ line while
testing.
This test is performed upon each open() of the UART device. Imagine two UARTs
share the same IRQ line: On is already opened and the IRQ is active. When the
second UART is opened, the IRQ line has to be disabled while performing IRQ
tests. Otherwise an IRQ might handler might be invoked, but the the IRQ itself
cannot be handled, because the corresponding handler isn't registered,
yet. That's because the 8250 code uses a chain-handler and invokes the
corresponding port's IRQ handling rountines himself.
Unfortunately this IRQF_SHARED flag isn't configured for UARTs probed via device
tree even if the IRQs are shared. This way, the actual and shared IRQ line isn't
disabled while performing tests and the kernel correctly detects a spurious
IRQ. So, adding this flag to the DT probe solves the issue.
Note: The UPF_SHARE_IRQ flag is configured unconditionally. Therefore, the
IRQF_SHARED flag can be set unconditionally as well.
Example stacktrace by performing echo 1 > /dev/ttyS2 on a non-patched system:
|irq 85: nobody cared (try booting with the "irqpoll" option)
| [...]
|handlers:
|[<ffff0000080fc628>] irq_default_primary_handler threaded [<ffff00000855fbb8>] serial8250_interrupt
|Disabling IRQ #85
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 9835b1c1cbe1..3de8d6a41246 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -149,6 +149,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
port->uartclk = clk;
port->flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP
| UPF_FIXED_PORT | UPF_FIXED_TYPE;
+ port->irqflags |= IRQF_SHARED;
if (of_property_read_bool(np, "no-loopback-test"))
port->flags |= UPF_SKIP_TEST;
-Li
> --
> With Best Regards,
> Andy Shevchenko
>
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: 答复: [PATCH] serial: 8250_pnp: pass IRQ shared flag to UART ports
2020-02-10 12:54 ` 答复: " Li,Rongqing
@ 2020-02-11 13:47 ` Andy Shevchenko
0 siblings, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2020-02-11 13:47 UTC (permalink / raw)
To: Li,Rongqing; +Cc: gregkh, jslaby, haolee.swjtu, linux-serial
On Mon, Feb 10, 2020 at 12:54:47PM +0000, Li,Rongqing wrote:
> > -----邮件原件-----
> > 发件人: Andy Shevchenko [mailto:andriy.shevchenko@linux.intel.com]
> > 发送时间: 2020年2月10日 18:10
> > 收件人: Li,Rongqing <lirongqing@baidu.com>
> > 抄送: gregkh@linuxfoundation.org; jslaby@suse.com;
> > haolee.swjtu@gmail.com; linux-serial@vger.kernel.org
> > 主题: Re: [PATCH] serial: 8250_pnp: pass IRQ shared flag to UART ports
> > On Sun, Feb 09, 2020 at 12:42:27PM +0800, Li RongQing wrote:
> > > On some systems IRQ lines might be shared between multiple devices.
> > > If so, the irqflags have to be configured accordingly. The reason is:
> > > The 8250 port startup code performs IRQ tests *before* the IRQ handler
> > > for that particular port is registered.
> >
> > Thanks for the report.
> >
> > Before we proceed with it, can we have more information about the device in
> > question?
> > How is it enumerated? What is in resources (ACPI / or ...?) for this device?
> > Also how IPMI is being involved to all this and why?
> >
>
> This is arm server, resource is from dsdt, and detail is
> And I think we has the same issue.
>
> Author: Kurt Kanzenbach <kurt@linutronix.de>
> Date: Fri Mar 16 12:31:58 2018 +0100
>
> tty: serial: 8250: pass IRQ shared flag to UART ports
Thanks for sharing additional information.
Funny that the mentioned commit describes everything and had done a symptomatic
healing instead of fixing it properly.
So, I will send a patch later Cc'ing you. I would like to be informed if it
helps (in form of Tested-by tag).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-11 13:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-09 4:42 [PATCH] serial: 8250_pnp: pass IRQ shared flag to UART ports Li RongQing
2020-02-10 10:09 ` Andy Shevchenko
2020-02-10 12:54 ` 答复: " Li,Rongqing
2020-02-11 13:47 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.