* [PATCH] serial: exar: Fix stuck MSIs
@ 2017-04-22 9:36 Jan Kiszka
2017-04-24 8:59 ` Andy Shevchenko
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2017-04-22 9:36 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee,
Andy Shevchenko
[-- Attachment #1.1: Type: text/plain, Size: 2837 bytes --]
From: Jan Kiszka <jan.kiszka@siemens.com>
After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
without further interrupts because of the special wake-up event these
chips send. They are only cleared by reading INT0. As we fail to do so
during startup and shutdown, we can leave the interrupt line asserted,
which is fatal with edge-triggered MSIs.
Add the required reading of INT0 to startup and shutdown. Also account
for the fact that a pending wake-up interrupt means we have to return 1
from exar_handle_irq.
An alternative approach would have been disabling the wake-up interrupt.
Unfortunately, this feature (REGB[17] = 1) is not available on the
XR17D15X.
Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Regression of upcoming 4.11.
drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6119516ef5fc..3a3667880fcf 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -47,6 +47,7 @@
/*
* These are definitions for the Exar XR17V35X and XR17(C|D)15X
*/
+#define UART_EXAR_INT0 0x80
#define UART_EXAR_SLEEP 0x8b /* Sleep mode */
#define UART_EXAR_DVID 0x8d /* Device identification */
@@ -1869,17 +1870,13 @@ static int serial8250_default_handle_irq(struct uart_port *port)
static int exar_handle_irq(struct uart_port *port)
{
unsigned int iir = serial_port_in(port, UART_IIR);
- int ret;
+ int ret = 0;
- ret = serial8250_handle_irq(port, iir);
+ if (((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X)) &&
+ serial_port_in(port, UART_EXAR_INT0) != 0)
+ ret = 1;
- if ((port->type == PORT_XR17V35X) ||
- (port->type == PORT_XR17D15X)) {
- serial_port_in(port, 0x80);
- serial_port_in(port, 0x81);
- serial_port_in(port, 0x82);
- serial_port_in(port, 0x83);
- }
+ ret |= serial8250_handle_irq(port, iir);
return ret;
}
@@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port *port)
serial_port_in(port, UART_RX);
serial_port_in(port, UART_IIR);
serial_port_in(port, UART_MSR);
+ if ((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X))
+ serial_port_in(port, UART_EXAR_INT0);
/*
* At this point, there's no way the LSR could still be 0xff;
@@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port *port)
serial_port_in(port, UART_RX);
serial_port_in(port, UART_IIR);
serial_port_in(port, UART_MSR);
+ if ((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X))
+ serial_port_in(port, UART_EXAR_INT0);
up->lsr_saved_flags = 0;
up->msr_saved_flags = 0;
--
2.12.0
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] serial: exar: Fix stuck MSIs
2017-04-22 9:36 [PATCH] serial: exar: Fix stuck MSIs Jan Kiszka
@ 2017-04-24 8:59 ` Andy Shevchenko
2017-04-24 9:06 ` Jan Kiszka
0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2017-04-24 8:59 UTC (permalink / raw)
To: Jan Kiszka, Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee
On Sat, 2017-04-22 at 11:36 +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
> without further interrupts because of the special wake-up event these
> chips send. They are only cleared by reading INT0. As we fail to do so
> during startup and shutdown, we can leave the interrupt line asserted,
> which is fatal with edge-triggered MSIs.
>
> Add the required reading of INT0 to startup and shutdown. Also account
> for the fact that a pending wake-up interrupt means we have to return
> 1
> from exar_handle_irq.
>
> An alternative approach would have been disabling the wake-up
> interrupt.
> Unfortunately, this feature (REGB[17] = 1) is not available on the
> XR17D15X.
>
> Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> Regression of upcoming 4.11.
>
> drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index 6119516ef5fc..3a3667880fcf 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -47,6 +47,7 @@
> /*
> * These are definitions for the Exar XR17V35X and XR17(C|D)15X
> */
> +#define UART_EXAR_INT0 0x80
> #define UART_EXAR_SLEEP 0x8b /* Sleep mode */
> #define UART_EXAR_DVID 0x8d /* Device
> identification */
>
> @@ -1869,17 +1870,13 @@ static int
> serial8250_default_handle_irq(struct uart_port *port)
> static int exar_handle_irq(struct uart_port *port)
> {
> unsigned int iir = serial_port_in(port, UART_IIR);
> - int ret;
> + int ret = 0;
>
> - ret = serial8250_handle_irq(port, iir);
> + if (((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X)) &&
> + serial_port_in(port, UART_EXAR_INT0) != 0)
> + ret = 1;
>
> - if ((port->type == PORT_XR17V35X) ||
> - (port->type == PORT_XR17D15X)) {
> - serial_port_in(port, 0x80);
> - serial_port_in(port, 0x81);
> - serial_port_in(port, 0x82);
> - serial_port_in(port, 0x83);
You replaced 4 reads by one. I'm suspecting that on multi-port cards you
still need to read all of them (I assume they are called INT0, INT1,
...). Perhaps you need a helper where you do that and call it from all
necessary places.
> - }
> + ret |= serial8250_handle_irq(port, iir);
>
> return ret;
> }
> @@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port
> *port)
> serial_port_in(port, UART_RX);
> serial_port_in(port, UART_IIR);
> serial_port_in(port, UART_MSR);
> + if ((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X))
> + serial_port_in(port, UART_EXAR_INT0);
>
> /*
> * At this point, there's no way the LSR could still be 0xff;
> @@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port
> *port)
> serial_port_in(port, UART_RX);
> serial_port_in(port, UART_IIR);
> serial_port_in(port, UART_MSR);
> + if ((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X))
> + serial_port_in(port, UART_EXAR_INT0);
> up->lsr_saved_flags = 0;
> up->msr_saved_flags = 0;
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] serial: exar: Fix stuck MSIs
2017-04-24 8:59 ` Andy Shevchenko
@ 2017-04-24 9:06 ` Jan Kiszka
2017-04-24 9:45 ` Andy Shevchenko
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2017-04-24 9:06 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee
[-- Attachment #1.1: Type: text/plain, Size: 3632 bytes --]
On 2017-04-24 10:59, Andy Shevchenko wrote:
> On Sat, 2017-04-22 at 11:36 +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
>> without further interrupts because of the special wake-up event these
>> chips send. They are only cleared by reading INT0. As we fail to do so
>> during startup and shutdown, we can leave the interrupt line asserted,
>> which is fatal with edge-triggered MSIs.
>>
>> Add the required reading of INT0 to startup and shutdown. Also account
>> for the fact that a pending wake-up interrupt means we have to return
>> 1
>> from exar_handle_irq.
>>
>> An alternative approach would have been disabling the wake-up
>> interrupt.
>> Unfortunately, this feature (REGB[17] = 1) is not available on the
>> XR17D15X.
>>
>> Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Regression of upcoming 4.11.
>>
>> drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
>> 1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c
>> b/drivers/tty/serial/8250/8250_port.c
>> index 6119516ef5fc..3a3667880fcf 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -47,6 +47,7 @@
>> /*
>> * These are definitions for the Exar XR17V35X and XR17(C|D)15X
>> */
>> +#define UART_EXAR_INT0 0x80
>> #define UART_EXAR_SLEEP 0x8b /* Sleep mode */
>> #define UART_EXAR_DVID 0x8d /* Device
>> identification */
>>
>> @@ -1869,17 +1870,13 @@ static int
>> serial8250_default_handle_irq(struct uart_port *port)
>> static int exar_handle_irq(struct uart_port *port)
>> {
>> unsigned int iir = serial_port_in(port, UART_IIR);
>> - int ret;
>> + int ret = 0;
>>
>> - ret = serial8250_handle_irq(port, iir);
>> + if (((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X)) &&
>> + serial_port_in(port, UART_EXAR_INT0) != 0)
>> + ret = 1;
>>
>> - if ((port->type == PORT_XR17V35X) ||
>> - (port->type == PORT_XR17D15X)) {
>
>> - serial_port_in(port, 0x80);
>> - serial_port_in(port, 0x81);
>> - serial_port_in(port, 0x82);
>> - serial_port_in(port, 0x83);
>
> You replaced 4 reads by one. I'm suspecting that on multi-port cards you
> still need to read all of them (I assume they are called INT0, INT1,
> ...). Perhaps you need a helper where you do that and call it from all
> necessary places.
Nope, we never had to read them all: "Wake-up Indicator is cleared by
reading the INT0 register." (Exar manual) INT0 contains the interrupt
status for all channels.
Jan
>
>> - }
>> + ret |= serial8250_handle_irq(port, iir);
>>
>> return ret;
>> }
>> @@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port
>> *port)
>> serial_port_in(port, UART_RX);
>> serial_port_in(port, UART_IIR);
>> serial_port_in(port, UART_MSR);
>> + if ((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X))
>> + serial_port_in(port, UART_EXAR_INT0);
>>
>> /*
>> * At this point, there's no way the LSR could still be 0xff;
>> @@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port
>> *port)
>> serial_port_in(port, UART_RX);
>> serial_port_in(port, UART_IIR);
>> serial_port_in(port, UART_MSR);
>> + if ((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X))
>> + serial_port_in(port, UART_EXAR_INT0);
>> up->lsr_saved_flags = 0;
>> up->msr_saved_flags = 0;
>>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] serial: exar: Fix stuck MSIs
2017-04-24 9:06 ` Jan Kiszka
@ 2017-04-24 9:45 ` Andy Shevchenko
2017-04-24 10:30 ` [PATCH v2] " Jan Kiszka
0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2017-04-24 9:45 UTC (permalink / raw)
To: Jan Kiszka, Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee
On Mon, 2017-04-24 at 11:06 +0200, Jan Kiszka wrote:
> On 2017-04-24 10:59, Andy Shevchenko wrote:
> > On Sat, 2017-04-22 at 11:36 +0200, Jan Kiszka wrote:
> > > From: Jan Kiszka <jan.kiszka@siemens.com>
> > >
> > > After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
> > > without further interrupts because of the special wake-up event
> > > these
> > > chips send. They are only cleared by reading INT0. As we fail to
> > > do so
> > > during startup and shutdown, we can leave the interrupt line
> > > asserted,
> > > which is fatal with edge-triggered MSIs.
> > >
> > > Add the required reading of INT0 to startup and shutdown. Also
> > > account
> > > for the fact that a pending wake-up interrupt means we have to
> > > return
> > > 1
> > > from exar_handle_irq.
> > >
> > > An alternative approach would have been disabling the wake-up
> > > interrupt.
> > > Unfortunately, this feature (REGB[17] = 1) is not available on the
> > > XR17D15X.
> > >
> > > Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > ---
> > >
> > > Regression of upcoming 4.11.
> > >
> > > drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
> > > 1 file changed, 10 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/8250/8250_port.c
> > > b/drivers/tty/serial/8250/8250_port.c
> > > index 6119516ef5fc..3a3667880fcf 100644
> > > --- a/drivers/tty/serial/8250/8250_port.c
> > > +++ b/drivers/tty/serial/8250/8250_port.c
> > > @@ -47,6 +47,7 @@
> > > /*
> > > * These are definitions for the Exar XR17V35X and XR17(C|D)15X
> > > */
> > > +#define UART_EXAR_INT0 0x80
> > > #define UART_EXAR_SLEEP 0x8b /* Sleep mode
> > > */
> > > #define UART_EXAR_DVID 0x8d /* Device
> > > identification */
> > >
> > > @@ -1869,17 +1870,13 @@ static int
> > > serial8250_default_handle_irq(struct uart_port *port)
> > > static int exar_handle_irq(struct uart_port *port)
> > > {
> > > unsigned int iir = serial_port_in(port, UART_IIR);
> > > - int ret;
> > > + int ret = 0;
> > >
> > > - ret = serial8250_handle_irq(port, iir);
> > > + if (((port->type == PORT_XR17V35X) || (port->type ==
> > > PORT_XR17D15X)) &&
> > > + serial_port_in(port, UART_EXAR_INT0) != 0)
> > > + ret = 1;
> > >
> > > - if ((port->type == PORT_XR17V35X) ||
> > > - (port->type == PORT_XR17D15X)) {
> > > - serial_port_in(port, 0x80);
> > > - serial_port_in(port, 0x81);
> > > - serial_port_in(port, 0x82);
> > > - serial_port_in(port, 0x83);
> >
> > You replaced 4 reads by one. I'm suspecting that on multi-port cards
> > you
> > still need to read all of them (I assume they are called INT0, INT1,
> > ...). Perhaps you need a helper where you do that and call it from
> > all
> > necessary places.
>
> Nope, we never had to read them all: "Wake-up Indicator is cleared by
> reading the INT0 register." (Exar manual) INT0 contains the interrupt
> status for all channels.
Perhaps it needs to be mentioned in commit message as well.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2] serial: exar: Fix stuck MSIs
2017-04-24 9:45 ` Andy Shevchenko
@ 2017-04-24 10:30 ` Jan Kiszka
2017-04-24 12:07 ` Andy Shevchenko
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2017-04-24 10:30 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee
After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
without further interrupts because of the special wake-up event these
chips send. They are only cleared by reading INT0. As we fail to do so
during startup and shutdown, we can leave the interrupt line asserted,
which is fatal with edge-triggered MSIs.
Add the required reading of INT0 to startup and shutdown. Also account
for the fact that a pending wake-up interrupt means we have to return 1
from exar_handle_irq. Drop the unneeded reading of INT1..3 along with
this - those never reset anything.
An alternative approach would have been disabling the wake-up interrupt.
Unfortunately, this feature (REGB[17] = 1) is not available on the
XR17D15X.
Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Changes in v2:
- mention removal of INT1..3 reading in commit log [Andy]
(no functional changes)
drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6119516ef5fc..3a3667880fcf 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -47,6 +47,7 @@
/*
* These are definitions for the Exar XR17V35X and XR17(C|D)15X
*/
+#define UART_EXAR_INT0 0x80
#define UART_EXAR_SLEEP 0x8b /* Sleep mode */
#define UART_EXAR_DVID 0x8d /* Device identification */
@@ -1869,17 +1870,13 @@ static int serial8250_default_handle_irq(struct uart_port *port)
static int exar_handle_irq(struct uart_port *port)
{
unsigned int iir = serial_port_in(port, UART_IIR);
- int ret;
+ int ret = 0;
- ret = serial8250_handle_irq(port, iir);
+ if (((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X)) &&
+ serial_port_in(port, UART_EXAR_INT0) != 0)
+ ret = 1;
- if ((port->type == PORT_XR17V35X) ||
- (port->type == PORT_XR17D15X)) {
- serial_port_in(port, 0x80);
- serial_port_in(port, 0x81);
- serial_port_in(port, 0x82);
- serial_port_in(port, 0x83);
- }
+ ret |= serial8250_handle_irq(port, iir);
return ret;
}
@@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port *port)
serial_port_in(port, UART_RX);
serial_port_in(port, UART_IIR);
serial_port_in(port, UART_MSR);
+ if ((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X))
+ serial_port_in(port, UART_EXAR_INT0);
/*
* At this point, there's no way the LSR could still be 0xff;
@@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port *port)
serial_port_in(port, UART_RX);
serial_port_in(port, UART_IIR);
serial_port_in(port, UART_MSR);
+ if ((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X))
+ serial_port_in(port, UART_EXAR_INT0);
up->lsr_saved_flags = 0;
up->msr_saved_flags = 0;
--
2.12.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] serial: exar: Fix stuck MSIs
2017-04-24 10:30 ` [PATCH v2] " Jan Kiszka
@ 2017-04-24 12:07 ` Andy Shevchenko
2017-05-13 7:09 ` Jan Kiszka
0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2017-04-24 12:07 UTC (permalink / raw)
To: Jan Kiszka, Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee
On Mon, 2017-04-24 at 12:30 +0200, Jan Kiszka wrote:
> After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
> without further interrupts because of the special wake-up event these
> chips send. They are only cleared by reading INT0. As we fail to do so
> during startup and shutdown, we can leave the interrupt line asserted,
> which is fatal with edge-triggered MSIs.
>
> Add the required reading of INT0 to startup and shutdown. Also account
> for the fact that a pending wake-up interrupt means we have to return
> 1
> from exar_handle_irq. Drop the unneeded reading of INT1..3 along with
> this - those never reset anything.
>
> An alternative approach would have been disabling the wake-up
> interrupt.
> Unfortunately, this feature (REGB[17] = 1) is not available on the
> XR17D15X.
FWIW:
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> Changes in v2:
> - mention removal of INT1..3 reading in commit log [Andy]
> (no functional changes)
>
> drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index 6119516ef5fc..3a3667880fcf 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -47,6 +47,7 @@
> /*
> * These are definitions for the Exar XR17V35X and XR17(C|D)15X
> */
> +#define UART_EXAR_INT0 0x80
> #define UART_EXAR_SLEEP 0x8b /* Sleep mode */
> #define UART_EXAR_DVID 0x8d /* Device
> identification */
>
> @@ -1869,17 +1870,13 @@ static int
> serial8250_default_handle_irq(struct uart_port *port)
> static int exar_handle_irq(struct uart_port *port)
> {
> unsigned int iir = serial_port_in(port, UART_IIR);
> - int ret;
> + int ret = 0;
>
> - ret = serial8250_handle_irq(port, iir);
> + if (((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X)) &&
> + serial_port_in(port, UART_EXAR_INT0) != 0)
> + ret = 1;
>
> - if ((port->type == PORT_XR17V35X) ||
> - (port->type == PORT_XR17D15X)) {
> - serial_port_in(port, 0x80);
> - serial_port_in(port, 0x81);
> - serial_port_in(port, 0x82);
> - serial_port_in(port, 0x83);
> - }
> + ret |= serial8250_handle_irq(port, iir);
>
> return ret;
> }
> @@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port
> *port)
> serial_port_in(port, UART_RX);
> serial_port_in(port, UART_IIR);
> serial_port_in(port, UART_MSR);
> + if ((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X))
> + serial_port_in(port, UART_EXAR_INT0);
>
> /*
> * At this point, there's no way the LSR could still be 0xff;
> @@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port
> *port)
> serial_port_in(port, UART_RX);
> serial_port_in(port, UART_IIR);
> serial_port_in(port, UART_MSR);
> + if ((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X))
> + serial_port_in(port, UART_EXAR_INT0);
> up->lsr_saved_flags = 0;
> up->msr_saved_flags = 0;
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] serial: exar: Fix stuck MSIs
2017-04-24 12:07 ` Andy Shevchenko
@ 2017-05-13 7:09 ` Jan Kiszka
2017-05-14 11:32 ` Greg Kroah-Hartman
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2017-05-13 7:09 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andy Shevchenko, Linux Kernel Mailing List, linux-serial,
Sudip Mukherjee
On 2017-04-24 14:07, Andy Shevchenko wrote:
> On Mon, 2017-04-24 at 12:30 +0200, Jan Kiszka wrote:
>> After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
>> without further interrupts because of the special wake-up event these
>> chips send. They are only cleared by reading INT0. As we fail to do so
>> during startup and shutdown, we can leave the interrupt line asserted,
>> which is fatal with edge-triggered MSIs.
>>
>> Add the required reading of INT0 to startup and shutdown. Also account
>> for the fact that a pending wake-up interrupt means we have to return
>> 1
>> from exar_handle_irq. Drop the unneeded reading of INT1..3 along with
>> this - those never reset anything.
>>
>> An alternative approach would have been disabling the wake-up
>> interrupt.
>> Unfortunately, this feature (REGB[17] = 1) is not available on the
>> XR17D15X.
>
> FWIW:
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
Ping. Needs to go into stable 4.11 now as well.
Jan
>>
>> Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Changes in v2:
>> - mention removal of INT1..3 reading in commit log [Andy]
>> (no functional changes)
>>
>> drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
>> 1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c
>> b/drivers/tty/serial/8250/8250_port.c
>> index 6119516ef5fc..3a3667880fcf 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -47,6 +47,7 @@
>> /*
>> * These are definitions for the Exar XR17V35X and XR17(C|D)15X
>> */
>> +#define UART_EXAR_INT0 0x80
>> #define UART_EXAR_SLEEP 0x8b /* Sleep mode */
>> #define UART_EXAR_DVID 0x8d /* Device
>> identification */
>>
>> @@ -1869,17 +1870,13 @@ static int
>> serial8250_default_handle_irq(struct uart_port *port)
>> static int exar_handle_irq(struct uart_port *port)
>> {
>> unsigned int iir = serial_port_in(port, UART_IIR);
>> - int ret;
>> + int ret = 0;
>>
>> - ret = serial8250_handle_irq(port, iir);
>> + if (((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X)) &&
>> + serial_port_in(port, UART_EXAR_INT0) != 0)
>> + ret = 1;
>>
>> - if ((port->type == PORT_XR17V35X) ||
>> - (port->type == PORT_XR17D15X)) {
>> - serial_port_in(port, 0x80);
>> - serial_port_in(port, 0x81);
>> - serial_port_in(port, 0x82);
>> - serial_port_in(port, 0x83);
>> - }
>> + ret |= serial8250_handle_irq(port, iir);
>>
>> return ret;
>> }
>> @@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port
>> *port)
>> serial_port_in(port, UART_RX);
>> serial_port_in(port, UART_IIR);
>> serial_port_in(port, UART_MSR);
>> + if ((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X))
>> + serial_port_in(port, UART_EXAR_INT0);
>>
>> /*
>> * At this point, there's no way the LSR could still be 0xff;
>> @@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port
>> *port)
>> serial_port_in(port, UART_RX);
>> serial_port_in(port, UART_IIR);
>> serial_port_in(port, UART_MSR);
>> + if ((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X))
>> + serial_port_in(port, UART_EXAR_INT0);
>> up->lsr_saved_flags = 0;
>> up->msr_saved_flags = 0;
>>
>
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] serial: exar: Fix stuck MSIs
2017-05-13 7:09 ` Jan Kiszka
@ 2017-05-14 11:32 ` Greg Kroah-Hartman
0 siblings, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2017-05-14 11:32 UTC (permalink / raw)
To: Jan Kiszka
Cc: Andy Shevchenko, Linux Kernel Mailing List, linux-serial,
Sudip Mukherjee
On Sat, May 13, 2017 at 09:09:17AM +0200, Jan Kiszka wrote:
> On 2017-04-24 14:07, Andy Shevchenko wrote:
> > On Mon, 2017-04-24 at 12:30 +0200, Jan Kiszka wrote:
> >> After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
> >> without further interrupts because of the special wake-up event these
> >> chips send. They are only cleared by reading INT0. As we fail to do so
> >> during startup and shutdown, we can leave the interrupt line asserted,
> >> which is fatal with edge-triggered MSIs.
> >>
> >> Add the required reading of INT0 to startup and shutdown. Also account
> >> for the fact that a pending wake-up interrupt means we have to return
> >> 1
> >> from exar_handle_irq. Drop the unneeded reading of INT1..3 along with
> >> this - those never reset anything.
> >>
> >> An alternative approach would have been disabling the wake-up
> >> interrupt.
> >> Unfortunately, this feature (REGB[17] = 1) is not available on the
> >> XR17D15X.
> >
> > FWIW:
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
>
> Ping. Needs to go into stable 4.11 now as well.
Relax, I'll get to it after 4.12-rc1 is out...
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-05-14 11:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-22 9:36 [PATCH] serial: exar: Fix stuck MSIs Jan Kiszka
2017-04-24 8:59 ` Andy Shevchenko
2017-04-24 9:06 ` Jan Kiszka
2017-04-24 9:45 ` Andy Shevchenko
2017-04-24 10:30 ` [PATCH v2] " Jan Kiszka
2017-04-24 12:07 ` Andy Shevchenko
2017-05-13 7:09 ` Jan Kiszka
2017-05-14 11:32 ` Greg Kroah-Hartman
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).