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

On Mon, Sep 02, 2013 at 07:32:53AM +0000, Lu Jingchang-B35083 wrote:

quoted

quoted

quoted

quoted

quoted

quoted

+	chan->private = fn_param;

why do you need to use chan->private?

[Lu Jingchang]
The private used here is to store the slot_id information, which

must

quoted

quoted

be used

quoted

by the DMAMUX in alloc_chan_resources function. Thanks.

Why dont you pass this in struct dma_slave_config memeber slave_id

for

quoted

quoted

this.

[Lu Jingchang-b35083]
I will drop this private and setup the slave_id directly in the filter

function.
why in filter? before calling prepare function you can set the slave
config

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

~Vinod