Picon
Picon

[PATCH 1/2] usb: renesas_usbhs: (cosmetic) simplify list operations

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski <at> gmx.de>
---
 drivers/usb/renesas_usbhs/fifo.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index 72339bd..5d543e3 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -75,8 +75,7 @@ void usbhs_pkt_push(struct usbhs_pipe *pipe, struct usbhs_pkt *pkt,
 		pipe->handler = &usbhsf_null_handler;
 	}

-	list_del_init(&pkt->node);
-	list_add_tail(&pkt->node, &pipe->list);
+	list_move_tail(&pkt->node, &pipe->list);

 	/*
 	 * each pkt must hold own handler.
@@ -106,7 +105,7 @@ static struct usbhs_pkt *__usbhsf_pkt_get(struct usbhs_pipe *pipe)
 	if (list_empty(&pipe->list))
 		return NULL;

-	return list_entry(pipe->list.next, struct usbhs_pkt, node);
+	return list_first_entry(&pipe->list, struct usbhs_pkt, node);
 }

 struct usbhs_pkt *usbhs_pkt_pop(struct usbhs_pipe *pipe, struct usbhs_pkt *pkt)
--

-- 
1.7.2.5
(Continue reading)

Picon
Picon

[PATCH 2/2] usb: fix renesas_usbhs to not schedule in atomic context

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 <at> 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);
(Continue reading)

Felipe Balbi | 13 Feb 13:08
Picon
Favicon

Re: [PATCH 2/2] usb: fix renesas_usbhs to not schedule in atomic context

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
(Continue reading)

Picon
Picon

Re: [PATCH 2/2] usb: fix renesas_usbhs to not schedule in atomic context

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
(Continue reading)

Felipe Balbi | 13 Feb 13:09
Picon
Favicon

Re: [PATCH 1/2] usb: renesas_usbhs: (cosmetic) simplify list operations

On Thu, Feb 09, 2012 at 11:17:47PM +0100, Guennadi Liakhovetski wrote:
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@...>

if (patch_has_no_commit_log(path)) {
	kfree(patch);
	continue;
}

--

-- 
balbi
Picon
Picon

Re: [PATCH 1/2] usb: renesas_usbhs: (cosmetic) simplify list operations

On Mon, 13 Feb 2012, Felipe Balbi wrote:

> On Thu, Feb 09, 2012 at 11:17:47PM +0100, Guennadi Liakhovetski wrote:
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski <at> gmx.de>
> 
> if (patch_has_no_commit_log(path)) {
> 	kfree(patch);
> 	continue;
> }

It is a common practice to omit a commit log in trivial patches, where the 
subject says it all, just look through git log. IMHO this is one of those 
cases - what else can you add to the statement in subj?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Felipe Balbi | 13 Feb 13:22
Picon
Favicon

Re: [PATCH 1/2] usb: renesas_usbhs: (cosmetic) simplify list operations

On Mon, Feb 13, 2012 at 01:19:36PM +0100, Guennadi Liakhovetski wrote:
> On Mon, 13 Feb 2012, Felipe Balbi wrote:
> 
> > On Thu, Feb 09, 2012 at 11:17:47PM +0100, Guennadi Liakhovetski wrote:
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski <at> gmx.de>
> > 
> > if (patch_has_no_commit_log(path)) {
> > 	kfree(patch);
> > 	continue;
> > }
> 
> It is a common practice to omit a commit log in trivial patches, where the 
> subject says it all, just look through git log. IMHO this is one of those 
> cases - what else can you add to the statement in subj?

something like:

"list.h already provide helpers to find the first entry and to move the
tail of one list to another list. This patch simply uses those helpers,
no functional changes".

--

-- 
balbi

Gmane