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..
--
~Vinod
