Re: [PATCH] dmaengine/coh901318: fix slave submission semantics

[Koul, Vinod]

On Thu, 2011-07-07 at 07:39 +0530, Vinod Koul wrote:
> On Fri, 2011-07-01 at 16:47 +0200, Linus Walleij wrote:
> > From: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > 
> > While testing Per Forlins MMC speed improvements I noticed a
> > semantic bug in the COH901318 driver: it will write to channel
> > registers in the prep_slave_sg() function, instead of deferring
> > it to later, breaking the assumption from the drivers to be able
> > to queue up new jobs while another job is running. Fix this by
> > storing up the initial register writes in the job descriptors
> > and write them to hardware when we process the descriptor
> > instead. Now the stress tests work.
> > 
> > Acked-by: Per Forlin <per.forlin@xxxxxxxxxx>
> > Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > ---
> Looks good, it is in my queue...
Applied thanks

> >  drivers/dma/coh901318.c |   19 ++++++++++++-------
> >  1 files changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
> > index af8c0b5..a7fca16 100644
> > --- a/drivers/dma/coh901318.c
> > +++ b/drivers/dma/coh901318.c
> > @@ -40,6 +40,8 @@ struct coh901318_desc {
> >  	struct coh901318_lli *lli;
> >  	enum dma_data_direction dir;
> >  	unsigned long flags;
> > +	u32 head_config;
> > +	u32 head_ctrl;
> >  };
> >  
> >  struct coh901318_base {
> > @@ -660,6 +662,9 @@ static struct coh901318_desc *coh901318_queue_start(struct coh901318_chan *cohc)
> >  
> >  		coh901318_desc_submit(cohc, cohd);
> >  
> > +		/* Program the transaction head */
> > +		coh901318_set_conf(cohc, cohd->head_config);
> > +		coh901318_set_ctrl(cohc, cohd->head_ctrl);
> >  		coh901318_prep_linked_list(cohc, cohd->lli);
> >  
> >  		/* start dma job on this channel */
> > @@ -1090,8 +1095,6 @@ coh901318_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
> >  	} else
> >  		goto err_direction;
> >  
> > -	coh901318_set_conf(cohc, config);
> > -
> >  	/* The dma only supports transmitting packages up to
> >  	 * MAX_DMA_PACKET_SIZE. Calculate to total number of
> >  	 * dma elemts required to send the entire sg list
> > @@ -1128,16 +1131,18 @@ coh901318_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
> >  	if (ret)
> >  		goto err_lli_fill;
> >  
> > -	/*
> > -	 * Set the default ctrl for the channel to the one from the lli,
> > -	 * things may have changed due to odd buffer alignment etc.
> > -	 */
> > -	coh901318_set_ctrl(cohc, lli->control);
> >  
> >  	COH_DBG(coh901318_list_print(cohc, lli));
> >  
> >  	/* Pick a descriptor to handle this transfer */
> >  	cohd = coh901318_desc_get(cohc);
> > +	cohd->head_config = config;
> > +	/*
> > +	 * Set the default head ctrl for the channel to the one from the
> > +	 * lli, things may have changed due to odd buffer alignment
> > +	 * etc.
> > +	 */
> > +	cohd->head_ctrl = lli->control;
> >  	cohd->dir = direction;
> >  	cohd->flags = flags;
> >  	cohd->desc.tx_submit = coh901318_tx_submit;
> 


-- 
~Vinod

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/