Re: [PATCH v2 2/3] dma: edma: add device_channel_caps() support

[Vinod Koul <vinod.koul@intel.com>]

On Thu, Jan 10, 2013 at 02:07:05PM -0500, Matt Porter wrote:
> Implement device_channel_caps().
> 
> EDMA has a finite set of PaRAM slots available for linking
> a multi-segment SG transfer. In order to prevent any one
> channel from consuming all PaRAM slots to fulfill a large SG
> transfer, the driver reports a static per-channel max number
> of SG segments it will handle.
> 
> The maximum size of SG segment is limited by the slave config
> maxburst and addr_width for the channel in question. These values
> are used from the current channel config to calculate and return
> the max segment length cap.
> 
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
>  drivers/dma/edma.c |   27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 82c8672..fc4b9db 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -70,6 +70,7 @@ struct edma_chan {
>  	bool				alloced;
>  	int				slot[EDMA_MAX_SLOTS];
>  	struct dma_slave_config		cfg;
> +	struct dmaengine_chan_caps	caps;
>  };
>  
>  struct edma_cc {
> @@ -462,6 +463,28 @@ static void edma_issue_pending(struct dma_chan *chan)
>  	spin_unlock_irqrestore(&echan->vchan.lock, flags);
>  }
>  
> +static struct dmaengine_chan_caps
> +*edma_get_channel_caps(struct dma_chan *chan, enum dma_transfer_direction dir)
> +{
> +	struct edma_chan *echan;
> +	enum dma_slave_buswidth width = 0;
> +	u32 burst = 0;
> +
> +	if (chan) {
I think this check is redundant.
> +		echan = to_edma_chan(chan);
> +		if (dir == DMA_MEM_TO_DEV) {
> +			width = echan->cfg.dst_addr_width;
> +			burst = echan->cfg.dst_maxburst;
Per you API example burst and width is not set yet, so this doesn't make sense
> +		} else if (dir == DMA_DEV_TO_MEM) {
> +			width = echan->cfg.src_addr_width;
> +			burst = echan->cfg.src_maxburst;
> +		}
> +		echan->caps.seg_len = (SZ_64K - 1) * width * burst;
Also the defination of API is max, above computation doesn't make sense to me!

--
~Vinod