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

-----Original Message-----
From: Vinod Koul [mailto:vinod.koul at intel.com]
Sent: Monday, September 02, 2013 2:37 PM
To: Lu Jingchang-B35083
Cc: shawn.guo at linaro.org; linux-kernel at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; devicetree at vger.kernel.org
Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

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

quoted

quoted

-----Original Message-----
From: Vinod Koul [mailto:vinod.koul at intel.com]
Sent: Monday, September 02, 2013 12:51 PM
To: Lu Jingchang-B35083
Cc: shawn.guo at linaro.org; linux-kernel at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; devicetree at vger.kernel.org
Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver

support

quoted

quoted

On Thu, Aug 29, 2013 at 03:32:04AM +0000, Lu Jingchang-B35083 wrote:

Please use a right MUA and wrap your lines at 80chars...

quoted

quoted

quoted

[Lu Jingchang]

No need to put your name :)

[Lu Jingchang-b35083]
Aha, the Microsoft Outlook adds this automatically with default option.

You can change that!!

Thanks.

quoted

quoted

quoted

quoted

quoted

+static bool fsl_edma_filter_fn(struct dma_chan *chan, void

*fn_param)

quoted

quoted

quoted

+{
+	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;
+
+	fsl_chan->slot_id = fparam->slot_id;
+	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?



Best Regareds,
Jingchang