[RFC,1/6] dma: Add Synopsys eDMA IP core driver

[Vinod Koul <vkoul@kernel.org>]

On 17-12-18, 15:56, Gustavo Pimentel wrote:

> >> +
> >> +#define SET(reg, name, val)			\
> >> +	reg.name = val
> >> +
> >> +#define SET_BOTH_CH(name, value)		\
> >> +	do {					\
> >> +		SET(dw->wr_edma, name, value);	\
> >> +		SET(dw->rd_edma, name, value);	\
> >> +	} while (0)
> > 
> > I am not sure how this helps, makes things not explicit..
> 
> Since this driver has 2 channels (write and read) I'd like to simplify all the
> configurations that I've to make on both channels (avoiding any omission), that
> why I created this macro.

So in order to configure a channel you need to write to both?

> Should I add some comment on top of this macro or do you think that is better to
> replicate the code for each channel?

That will help to explain..

> >> +
> >> +	err = ops->device_config(dchan);
> > 
> > okay what does this callback do. You are already under and dmaengine fwk
> > so what is the need to add one more abstraction layer, can you explain
> > that in details please
> 
> This callback just configures the eDMA HW block interrupt address (abort and
> done) and data for each channel. This callback could easily moved to the
> dw_edma_probe() where each channel is created at first.
> Should I do it in your opinion?

My question is about callbacks in this driver in general. You are
already under a dmaengine fwk, so this is adding one more layer on top
with callbacks implementing low level stiff. Why do we need another layer
is the question..