Re: [PATCH char-misc-next 11/19] dma: Add support to program MIC x100 status descriptiors

[Sudeep Dutt <sudeep.dutt@intel.com>]

On Thu, 2015-08-20 at 10:50 +0530, Vinod Koul wrote:
> On Mon, Jul 27, 2015 at 04:58:17PM -0700, Ashutosh Dixit wrote:
> > +/* Program a status descriptor with phys as address and value to be written */
> > +static int mic_dma_do_status_update(struct mic_dma_chan *ch, dma_addr_t phys,
> > +				    u64 value)
> > +{
> > +	int ret = mic_dma_avail_desc_ring_space(ch, 4);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +	ret = 0;
> ?
> > +	mic_dma_prep_status_desc(&ch->desc_ring[ch->head],
> > +				 value, phys, false);
> > +	mic_dma_hw_ring_inc_head(ch);
> > +	return ret;
> 
> return 0 then?
> 

Sounds good. We will change this in the next revision of the patch
series which will be posted after the 4.3 merge window closes.

> > +}
> > +
> >  static inline void mic_dma_issue_pending(struct dma_chan *ch)
> >  {
> >  	struct mic_dma_chan *mic_ch = to_mic_dma_chan(ch);
> > @@ -287,9 +310,28 @@ mic_dma_prep_memcpy_lock(struct dma_chan *ch, dma_addr_t dma_dest,
> >  		return NULL;
> >  
> >  	spin_lock(&mic_ch->prep_lock);
> > +	if (len == 8) {
> magic number?
> 

The MIC X100 DMA engine requires cache line (64 bytes) aligned
src/dst/len. We use the length of 8 bytes as a special case for
programming the status descriptor since it writes an 8 byte value. We
will use a macro in the next revision of the patch series.

> > +		/*
> > +		 * This is a hack to program status descriptor since
> > +		 * DMA engine API doesn't have support for this.
> > +		 */
> what do you mean by programming status descriptor, what do you need to
> program?
> 

The MIC X100 DMA engine has a special status descriptor which writes an
8 byte immediate data value to a destination address. It is used to
signal completion of all DMA descriptors prior to the status descriptor.
The DMA engine API does not allow drivers to pass a 8 byte value. We are
allowed to pass a source physical address but we cannot determine the
source value within the DMA driver using that information specifically
with the IOMMU enabled. We have added this workaround so that we can
program this special status update descriptor without making any changes
to the DMA engine API.

Thanks for the review!

Sudeep Dutt