Skip to content

Commit

Permalink
sc16is7xx: Fix for "Unexpected interrupt: 8"
Browse files Browse the repository at this point in the history
The SC16IS752 has an Enhanced Feature Register which is aliased at the
same address as the Interrupt Identification Register; accessing it
requires that a magic value is written to the Line Configuration
Register. If an interrupt is raised while the EFR is mapped in then
the ISR won't be able to access the IIR, leading to the "Unexpected
interrupt" error messages.

Avoid the problem by claiming a mutex around accesses to the EFR
register, also claiming the mutex in the interrupt handler work
item (this is equivalent to disabling interrupts to interlock against
a non-threaded interrupt handler).

See: #2529

Signed-off-by: Phil Elwell <phil@raspberrypi.org>
  • Loading branch information
Phil Elwell committed Jan 15, 2019
1 parent 05b4ba8 commit aa02ab7
Showing 1 changed file with 28 additions and 0 deletions.
28 changes: 28 additions & 0 deletions drivers/tty/serial/sc16is7xx.c
Expand Up @@ -328,6 +328,7 @@ struct sc16is7xx_port {
struct kthread_worker kworker;
struct task_struct *kworker_task;
struct kthread_work irq_work;
struct mutex efr_lock;
struct sc16is7xx_one p[0];
};

Expand Down Expand Up @@ -499,6 +500,21 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud)
div /= 4;
}

/* In an amazing feat of design, the Enhanced Features Register shares
* the address of the Interrupt Identification Register, and is
* switched in by writing a magic value (0xbf) to the Line Control
* Register. Any interrupt firing during this time will see the EFR
* where it expects the IIR to be, leading to "Unexpected interrupt"
* messages.
*
* Prevent this possibility by claiming a mutex while accessing the
* EFR, and claiming the same mutex from within the interrupt handler.
* This is similar to disabling the interrupt, but that doesn't work
* because the bulk of the interrupt processing is run as a workqueue
* job in thread context.
*/
mutex_lock(&s->efr_lock);

lcr = sc16is7xx_port_read(port, SC16IS7XX_LCR_REG);

/* Open the LCR divisors for configuration */
Expand All @@ -514,6 +530,8 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud)
/* Put LCR back to the normal mode */
sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);

mutex_unlock(&s->efr_lock);

sc16is7xx_port_update(port, SC16IS7XX_MCR_REG,
SC16IS7XX_MCR_CLKSEL_BIT,
prescaler);
Expand Down Expand Up @@ -698,6 +716,8 @@ static void sc16is7xx_ist(struct kthread_work *ws)
{
struct sc16is7xx_port *s = to_sc16is7xx_port(ws, irq_work);

mutex_lock(&s->efr_lock);

while (1) {
bool keep_polling = false;
int i;
Expand All @@ -707,6 +727,8 @@ static void sc16is7xx_ist(struct kthread_work *ws)
if (!keep_polling)
break;
}

mutex_unlock(&s->efr_lock);
}

static irqreturn_t sc16is7xx_irq(int irq, void *dev_id)
Expand Down Expand Up @@ -901,6 +923,9 @@ static void sc16is7xx_set_termios(struct uart_port *port,
if (!(termios->c_cflag & CREAD))
port->ignore_status_mask |= SC16IS7XX_LSR_BRK_ERROR_MASK;

/* As above, claim the mutex while accessing the EFR. */
mutex_lock(&s->efr_lock);

sc16is7xx_port_write(port, SC16IS7XX_LCR_REG,
SC16IS7XX_LCR_CONF_MODE_B);

Expand All @@ -922,6 +947,8 @@ static void sc16is7xx_set_termios(struct uart_port *port,
/* Update LCR register */
sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);

mutex_unlock(&s->efr_lock);

/* Get baud rate generator configuration */
baud = uart_get_baud_rate(port, termios, old,
port->uartclk / 16 / 4 / 0xffff,
Expand Down Expand Up @@ -1187,6 +1214,7 @@ static int sc16is7xx_probe(struct device *dev,
s->regmap = regmap;
s->devtype = devtype;
dev_set_drvdata(dev, s);
mutex_init(&s->efr_lock);

kthread_init_worker(&s->kworker);
kthread_init_work(&s->irq_work, sc16is7xx_ist);
Expand Down

0 comments on commit aa02ab7

Please sign in to comment.