Commit 66e358a8 authored by David S. Miller's avatar David S. Miller

Merge branch 'mrf24j40'

Alan Ott says:

====================
Fix race conditions in mrf24j40 interrupts

After testing with the betas of this patchset, it's been rebased and is
ready for inclusion.

David Hauweele noticed that the mrf24j40 would hang arbitrarily after some
period of heavy traffic.  Two race conditions were discovered, and the
driver was changed to use threaded interrupts, since the enable/disable of
interrupts in the driver has recently been a lighning rod whenever issues
arise related to interrupts (costing engineering time), and since threaded
interrupts are the right way to do it.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 8cc27d17 40afbb65
...@@ -82,7 +82,6 @@ struct mrf24j40 { ...@@ -82,7 +82,6 @@ struct mrf24j40 {
struct mutex buffer_mutex; /* only used to protect buf */ struct mutex buffer_mutex; /* only used to protect buf */
struct completion tx_complete; struct completion tx_complete;
struct work_struct irqwork;
u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */ u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */
}; };
...@@ -344,6 +343,8 @@ static int mrf24j40_tx(struct ieee802154_dev *dev, struct sk_buff *skb) ...@@ -344,6 +343,8 @@ static int mrf24j40_tx(struct ieee802154_dev *dev, struct sk_buff *skb)
if (ret) if (ret)
goto err; goto err;
INIT_COMPLETION(devrec->tx_complete);
/* Set TXNTRIG bit of TXNCON to send packet */ /* Set TXNTRIG bit of TXNCON to send packet */
ret = read_short_reg(devrec, REG_TXNCON, &val); ret = read_short_reg(devrec, REG_TXNCON, &val);
if (ret) if (ret)
...@@ -354,8 +355,6 @@ static int mrf24j40_tx(struct ieee802154_dev *dev, struct sk_buff *skb) ...@@ -354,8 +355,6 @@ static int mrf24j40_tx(struct ieee802154_dev *dev, struct sk_buff *skb)
val |= 0x4; val |= 0x4;
write_short_reg(devrec, REG_TXNCON, val); write_short_reg(devrec, REG_TXNCON, val);
INIT_COMPLETION(devrec->tx_complete);
/* Wait for the device to send the TX complete interrupt. */ /* Wait for the device to send the TX complete interrupt. */
ret = wait_for_completion_interruptible_timeout( ret = wait_for_completion_interruptible_timeout(
&devrec->tx_complete, &devrec->tx_complete,
...@@ -590,17 +589,6 @@ static struct ieee802154_ops mrf24j40_ops = { ...@@ -590,17 +589,6 @@ static struct ieee802154_ops mrf24j40_ops = {
static irqreturn_t mrf24j40_isr(int irq, void *data) static irqreturn_t mrf24j40_isr(int irq, void *data)
{ {
struct mrf24j40 *devrec = data; struct mrf24j40 *devrec = data;
disable_irq_nosync(irq);
schedule_work(&devrec->irqwork);
return IRQ_HANDLED;
}
static void mrf24j40_isrwork(struct work_struct *work)
{
struct mrf24j40 *devrec = container_of(work, struct mrf24j40, irqwork);
u8 intstat; u8 intstat;
int ret; int ret;
...@@ -618,7 +606,7 @@ static void mrf24j40_isrwork(struct work_struct *work) ...@@ -618,7 +606,7 @@ static void mrf24j40_isrwork(struct work_struct *work)
mrf24j40_handle_rx(devrec); mrf24j40_handle_rx(devrec);
out: out:
enable_irq(devrec->spi->irq); return IRQ_HANDLED;
} }
static int mrf24j40_probe(struct spi_device *spi) static int mrf24j40_probe(struct spi_device *spi)
...@@ -642,7 +630,6 @@ static int mrf24j40_probe(struct spi_device *spi) ...@@ -642,7 +630,6 @@ static int mrf24j40_probe(struct spi_device *spi)
mutex_init(&devrec->buffer_mutex); mutex_init(&devrec->buffer_mutex);
init_completion(&devrec->tx_complete); init_completion(&devrec->tx_complete);
INIT_WORK(&devrec->irqwork, mrf24j40_isrwork);
devrec->spi = spi; devrec->spi = spi;
spi_set_drvdata(spi, devrec); spi_set_drvdata(spi, devrec);
...@@ -688,11 +675,12 @@ static int mrf24j40_probe(struct spi_device *spi) ...@@ -688,11 +675,12 @@ static int mrf24j40_probe(struct spi_device *spi)
val &= ~0x3; /* Clear RX mode (normal) */ val &= ~0x3; /* Clear RX mode (normal) */
write_short_reg(devrec, REG_RXMCR, val); write_short_reg(devrec, REG_RXMCR, val);
ret = request_irq(spi->irq, ret = request_threaded_irq(spi->irq,
mrf24j40_isr, NULL,
IRQF_TRIGGER_FALLING, mrf24j40_isr,
dev_name(&spi->dev), IRQF_TRIGGER_LOW|IRQF_ONESHOT,
devrec); dev_name(&spi->dev),
devrec);
if (ret) { if (ret) {
dev_err(printdev(devrec), "Unable to get IRQ"); dev_err(printdev(devrec), "Unable to get IRQ");
...@@ -721,7 +709,6 @@ static int mrf24j40_remove(struct spi_device *spi) ...@@ -721,7 +709,6 @@ static int mrf24j40_remove(struct spi_device *spi)
dev_dbg(printdev(devrec), "remove\n"); dev_dbg(printdev(devrec), "remove\n");
free_irq(spi->irq, devrec); free_irq(spi->irq, devrec);
flush_work(&devrec->irqwork); /* TODO: Is this the right call? */
ieee802154_unregister_device(devrec->dev); ieee802154_unregister_device(devrec->dev);
ieee802154_free_device(devrec->dev); ieee802154_free_device(devrec->dev);
/* TODO: Will ieee802154_free_device() wait until ->xmit() is /* TODO: Will ieee802154_free_device() wait until ->xmit() is
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment