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

[Vinod Koul <vkoul@kernel.org>]

Hi Gustavo,

On 03-01-19, 09:53, Gustavo Pimentel wrote:
> Hi Vinod,
> 
> (snipped)
> 
> >>> +{
> >>> +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> >>> +	const struct dw_edma_core_ops *ops = chan2ops(chan);
> >>> +	enum dma_transfer_direction dir;
> >>> +	unsigned long flags;
> >>> +	int err = 0;
> >>> +
> >>> +	spin_lock_irqsave(&chan->vc.lock, flags);
> >>> +
> >>> +	if (!config) {
> >>> +		err = -EINVAL;
> >>> +		goto err_config;
> >>> +	}
> >>> +
> >>> +	if (chan->configured) {
> >>> +		dev_err(chan2dev(chan), ": channel already configured\n");
> >>> +		err = -EPERM;
> >>> +		goto err_config;
> >>> +	}
> >>> +
> >>> +	dir = config->direction;
> >>
> >> Direction is depreciated, I have already removed the usages, so please
> >> do not add new ones.
> >>
> >> You need to take direction for respective prep_ calls
> > 
> > Ok, I already do that. IMHO I found it strange to have the same information
> > repeated on two places. But now that you say that this is deprecated, it makes
> > sense now.
> > 
> >>
> >>> +	if (dir == DMA_DEV_TO_MEM && chan->dir == EDMA_DIR_WRITE) {
> >>> +		dev_info(chan2dev(chan),
> >>> +			": direction DMA_DEV_TO_MEM (EDMA_DIR_WRITE)\n");
> >>> +		chan->p_addr = config->src_addr;
> >>> +	} else if (dir == DMA_MEM_TO_DEV && chan->dir == EDMA_DIR_READ) {
> >>> +		dev_info(chan2dev(chan),
> >>> +			": direction DMA_MEM_TO_DEV (EDMA_DIR_READ)\n");
> >>> +		chan->p_addr = config->dst_addr;
> >>> +	} else {
> >>> +		dev_err(chan2dev(chan), ": invalid direction\n");
> >>> +		err = -EINVAL;
> >>> +		goto err_config;
> >>> +	}
> >>
> >> This should be removed
> > 
> > Yeah, it was just for validation purposes. Now that direction is deprecated on
> > the API, makes no sense to validate it.
> > 
> >>
> >>> +
> >>> +	dev_info(chan2dev(chan),
> >>> +		": src_addr(physical) = 0x%.16x\n", config->src_addr);
> >>> +	dev_info(chan2dev(chan),
> >>> +		": dst_addr(physical) = 0x%.16x\n", config->dst_addr);
> >>
> 
> I've a doubt now. As you know, for a DMA transfer you need the source and
> destination addresses, which in the limited can be swapped according to the
> direction MEM_TO_DEV/DEV_TO_MEM case.
> 
> For the sake of simplicity, I'll just consider now the MEM_TO_DEV case, since
> the other case is similar but the source and destination address are swapped.
> 
> In my code I can get some of the information that I need by using the
> sg_dma_address() in the scatter-gather list (which gives me the source address).
> 
> The remaining information I got from here, using the direction to help me to
> select which address I'll use later on the DMA transfer, in this case the
> destination address.
> 
> Since this is deprecated how should I proceed? How can I get that information?
> There is some similar function to sg_dma_address() that could give me the
> destination address?

So the direction field is deprecated but rest of the configuration comes
from from dma_slave_config. The user should set src_addr, dst_addr and
then based on direction passed in the .device_prep_dma_* call arguments
one can use one of these as peripheral address.

Also, please keep in mind that for memory to memory transfers you should
not expect the dma_slave_config to be used

Thanks