The current renesas_usbhs driver triggers
BUG: scheduling while atomic: ksoftirqd/0/3/0x00000102
with enabled CONFIG_DEBUG_ATOMIC_SLEEP, by submitting DMA transfers from
an atomic (tasklet) context, which is not supported by the shdma dmaengine
driver. Fix it by switching to a work.
Signed-off-by: Guennadi Liakhovetski <[email protected]>
---
drivers/usb/renesas_usbhs/fifo.c | 28 +++++++++++-----------------
drivers/usb/renesas_usbhs/fifo.h | 3 ++-
2 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index 5d543e3..4d739ec 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -761,9 +761,9 @@ static int __usbhsf_dma_map_ctrl(struct usbhs_pkt *pkt, int map)
}
static void usbhsf_dma_complete(void *arg);
-static void usbhsf_dma_prepare_tasklet(unsigned long data)
+static void xfer_work(struct work_struct *work)
{
- struct usbhs_pkt *pkt = (struct usbhs_pkt *)data;
+ struct usbhs_pkt *pkt = container_of(work, struct usbhs_pkt, work);
struct usbhs_pipe *pipe = pkt->pipe;
struct usbhs_fifo *fifo = usbhs_pipe_to_fifo(pipe);
struct usbhs_priv *priv = usbhs_pipe_to_priv(pipe);
@@ -843,11 +843,8 @@ static int usbhsf_dma_prepare_push(struct usbhs_pkt *pkt, int *is_done)
pkt->trans = len;
- tasklet_init(&fifo->tasklet,
- usbhsf_dma_prepare_tasklet,
- (unsigned long)pkt);
-
- tasklet_schedule(&fifo->tasklet);
+ INIT_WORK(&pkt->work, xfer_work);
+ schedule_work(&pkt->work);
return 0;
@@ -937,11 +934,8 @@ static int usbhsf_dma_try_pop(struct usbhs_pkt *pkt, int *is_done)
pkt->trans = len;
- tasklet_init(&fifo->tasklet,
- usbhsf_dma_prepare_tasklet,
- (unsigned long)pkt);
-
- tasklet_schedule(&fifo->tasklet);
+ INIT_WORK(&pkt->work, xfer_work);
+ schedule_work(&pkt->work);
return 0;
@@ -997,7 +991,7 @@ static bool usbhsf_dma_filter(struct dma_chan *chan, void *param)
*
* usbhs doesn't recognize id = 0 as valid DMA
*/
- if (0 == slave->slave_id)
+ if (0 == slave->simple_slave.slave_id)
return false;
chan->private = slave;
@@ -1176,8 +1170,8 @@ int usbhs_fifo_probe(struct usbhs_priv *priv)
fifo->port = D0FIFO;
fifo->sel = D0FIFOSEL;
fifo->ctr = D0FIFOCTR;
- fifo->tx_slave.slave_id = usbhs_get_dparam(priv, d0_tx_id);
- fifo->rx_slave.slave_id = usbhs_get_dparam(priv, d0_rx_id);
+ fifo->tx_slave.simple_slave.slave_id = usbhs_get_dparam(priv, d0_tx_id);
+ fifo->rx_slave.simple_slave.slave_id = usbhs_get_dparam(priv, d0_rx_id);
/* D1FIFO */
fifo = usbhsf_get_d1fifo(priv);
@@ -1185,8 +1179,8 @@ int usbhs_fifo_probe(struct usbhs_priv *priv)
fifo->port = D1FIFO;
fifo->sel = D1FIFOSEL;
fifo->ctr = D1FIFOCTR;
- fifo->tx_slave.slave_id = usbhs_get_dparam(priv, d1_tx_id);
- fifo->rx_slave.slave_id = usbhs_get_dparam(priv, d1_rx_id);
+ fifo->tx_slave.simple_slave.slave_id = usbhs_get_dparam(priv, d1_tx_id);
+ fifo->rx_slave.simple_slave.slave_id = usbhs_get_dparam(priv, d1_rx_id);
return 0;
}
diff --git a/drivers/usb/renesas_usbhs/fifo.h b/drivers/usb/renesas_usbhs/fifo.h
index f68609c..c31731a 100644
--- a/drivers/usb/renesas_usbhs/fifo.h
+++ b/drivers/usb/renesas_usbhs/fifo.h
@@ -19,6 +19,7 @@
#include <linux/interrupt.h>
#include <linux/sh_dma.h>
+#include <linux/workqueue.h>
#include <asm/dma.h>
#include "pipe.h"
@@ -31,7 +32,6 @@ struct usbhs_fifo {
u32 ctr; /* xFIFOCTR */
struct usbhs_pipe *pipe;
- struct tasklet_struct tasklet;
struct dma_chan *tx_chan;
struct dma_chan *rx_chan;
@@ -53,6 +53,7 @@ struct usbhs_pkt {
struct usbhs_pkt_handle *handler;
void (*done)(struct usbhs_priv *priv,
struct usbhs_pkt *pkt);
+ struct work_struct work;
dma_addr_t dma;
void *buf;
int length;
--
1.7.2.5
Hi,
On Thu, Feb 09, 2012 at 11:17:51PM +0100, Guennadi Liakhovetski wrote:
> @@ -997,7 +991,7 @@ static bool usbhsf_dma_filter(struct dma_chan *chan, void *param)
> *
> * usbhs doesn't recognize id = 0 as valid DMA
> */
> - if (0 == slave->slave_id)
> + if (0 == slave->simple_slave.slave_id)
> return false;
>
> chan->private = slave;
> @@ -1176,8 +1170,8 @@ int usbhs_fifo_probe(struct usbhs_priv *priv)
> fifo->port = D0FIFO;
> fifo->sel = D0FIFOSEL;
> fifo->ctr = D0FIFOCTR;
> - fifo->tx_slave.slave_id = usbhs_get_dparam(priv, d0_tx_id);
> - fifo->rx_slave.slave_id = usbhs_get_dparam(priv, d0_rx_id);
> + fifo->tx_slave.simple_slave.slave_id = usbhs_get_dparam(priv, d0_tx_id);
> + fifo->rx_slave.simple_slave.slave_id = usbhs_get_dparam(priv, d0_rx_id);
what are these "simple_slave" changes ? They have nothing to do with
$SUBJECT ? In fact, breaks my gadget branch.
How many times does a maintainer have to ask for contributors to keep
changes atomic ? One patch == One single, self-contained, change.
--
balbi
On Mon, 13 Feb 2012, Felipe Balbi wrote:
> Hi,
>
> On Thu, Feb 09, 2012 at 11:17:51PM +0100, Guennadi Liakhovetski wrote:
> > @@ -997,7 +991,7 @@ static bool usbhsf_dma_filter(struct dma_chan *chan, void *param)
> > *
> > * usbhs doesn't recognize id = 0 as valid DMA
> > */
> > - if (0 == slave->slave_id)
> > + if (0 == slave->simple_slave.slave_id)
> > return false;
> >
> > chan->private = slave;
> > @@ -1176,8 +1170,8 @@ int usbhs_fifo_probe(struct usbhs_priv *priv)
> > fifo->port = D0FIFO;
> > fifo->sel = D0FIFOSEL;
> > fifo->ctr = D0FIFOCTR;
> > - fifo->tx_slave.slave_id = usbhs_get_dparam(priv, d0_tx_id);
> > - fifo->rx_slave.slave_id = usbhs_get_dparam(priv, d0_rx_id);
> > + fifo->tx_slave.simple_slave.slave_id = usbhs_get_dparam(priv, d0_tx_id);
> > + fifo->rx_slave.simple_slave.slave_id = usbhs_get_dparam(priv, d0_rx_id);
>
> what are these "simple_slave" changes ? They have nothing to do with
> $SUBJECT ? In fact, breaks my gadget branch.
Hrrrm... Right, sorry, that's clearly unrelated...
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/