RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

From: Lu Jingchang-B35083
Date: Tue Sep 03 2013 - 01:43:31 EST


> > How about change the filter_fn to follow:
> > static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
> > {
> > struct fsl_edma_filter_param *fparam = fn_param;
> > struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > unsigned char val;
> >
> > if (fsl_chan->edmamux->mux_id != fparam->mux_id)
> > return false;
> >
> > val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(fparam-
> >slot_id);
> > fsl_edmamux_config_chan(fsl_chan, val);
> > return true;
> > }
> > In fact the slot_id isn't need elsewhere, and if the filter return true,
> > This channel should be to this request. So no need to save the slave id,
> Right?
> something like
>
> static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
> {
> struct fsl_edma_filter_param *fparam = fn_param;
> struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
>
> if (fsl_chan->edmamux->mux_id != fparam->mux_id)
> return false;
> return true;
> }
>
> in thedriver which calls this:
>
> before prep:
>
> config->slave_id = val;
>
> dma_set_slave_config(chan, slave);
>
Do you mean the DMA_SLAVE_CONFIG device_control? Yeah, the slave driver could pass
the slave_id. But the DMA_SLAVE_CONFIG may be called more than once, and the eDMA
driver just needs to set the slave id once for any given channel, after that the
transfer is transparent to the device.
On the other hand, the DMAMUX's setting procedure requires first disable the dmamux
before setting, then if it is set in DMA_SLAVE_CONFIG, the repeated setting may be
complex and unnecessary. The channel is occupied exclusively by the peripheral.
So, according the HW feature, I think the eDMA needs only set the slave id once,
and since the of_dma helper has pass the slave id in on xlate, we can get and set
the slave id here. How do you think about this?
Thanks!









Best Regards,
Jingchang



¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶Ÿÿà -»+ƒùdš_