linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/2] usb: fix renesas_usbhs to not schedule in atomic context
       [not found] <Pine.LNX.4.64.1202092313560.18719@axis700.grange>
@ 2012-02-09 22:17 ` Guennadi Liakhovetski
  2012-02-13 12:08   ` Felipe Balbi
  0 siblings, 1 reply; 3+ messages in thread
From: Guennadi Liakhovetski @ 2012-02-09 22:17 UTC (permalink / raw)
  To: linux-usb
  Cc: Felipe Balbi, Shimoda, Yoshihiro, linux-sh, Magnus Damm,
	linux-kernel, Vinod Koul, Paul Mundt

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 <g.liakhovetski@gmx.de>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/2] usb: fix renesas_usbhs to not schedule in atomic context
  2012-02-09 22:17 ` [PATCH 2/2] usb: fix renesas_usbhs to not schedule in atomic context Guennadi Liakhovetski
@ 2012-02-13 12:08   ` Felipe Balbi
  2012-02-13 12:17     ` Guennadi Liakhovetski
  0 siblings, 1 reply; 3+ messages in thread
From: Felipe Balbi @ 2012-02-13 12:08 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-usb, Felipe Balbi, Shimoda, Yoshihiro, linux-sh,
	Magnus Damm, linux-kernel, Vinod Koul, Paul Mundt

[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/2] usb: fix renesas_usbhs to not schedule in atomic context
  2012-02-13 12:08   ` Felipe Balbi
@ 2012-02-13 12:17     ` Guennadi Liakhovetski
  0 siblings, 0 replies; 3+ messages in thread
From: Guennadi Liakhovetski @ 2012-02-13 12:17 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: linux-usb, Shimoda, Yoshihiro, linux-sh, Magnus Damm,
	linux-kernel, Vinod Koul, Paul Mundt

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/

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-02-13 12:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <Pine.LNX.4.64.1202092313560.18719@axis700.grange>
2012-02-09 22:17 ` [PATCH 2/2] usb: fix renesas_usbhs to not schedule in atomic context Guennadi Liakhovetski
2012-02-13 12:08   ` Felipe Balbi
2012-02-13 12:17     ` Guennadi Liakhovetski

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).