diff mbox

[v2,10/14] dma: edma: Simplify direction configuration in edma_config_pset()

Message ID 1396357575-30585-11-git-send-email-peter.ujfalusi@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Ujfalusi April 1, 2014, 1:06 p.m. UTC
We only support DEV_TO_MEM or MEM_TO_DEV directions with edma driver and the
check for the direction has been already done in the function calling
edma_config_pset().
The error reporting is redundant and also the "else if ()" can be removed.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/edma.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Joel Fernandes April 10, 2014, 10:40 p.m. UTC | #1
On 04/01/2014 08:06 AM, Peter Ujfalusi wrote:
> We only support DEV_TO_MEM or MEM_TO_DEV directions with edma driver and the
> check for the direction has been already done in the function calling
> edma_config_pset().
> The error reporting is redundant and also the "else if ()" can be removed.
> 

NAK. Please don't do this. I have been working on MEM_TO_MEM support as
well so leave it as it is for future.

Thanks,
-Joel

> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/dma/edma.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 855766672aa9..d954099650ae 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -372,14 +372,12 @@ static int edma_config_pset(struct dma_chan *chan, struct edmacc_param *pset,
>  		src_cidx = cidx;
>  		dst_bidx = 0;
>  		dst_cidx = 0;
> -	} else if (direction == DMA_DEV_TO_MEM)  {
> +	} else {
> +		/* DMA_DEV_TO_MEM */
>  		src_bidx = 0;
>  		src_cidx = 0;
>  		dst_bidx = acnt;
>  		dst_cidx = cidx;
> -	} else {
> -		dev_err(dev, "%s: direction not implemented yet\n", __func__);
> -		return -EINVAL;
>  	}
>  
>  	pset->opt = EDMA_TCC(EDMA_CHAN_SLOT(echan->ch_num));
>
Peter Ujfalusi April 11, 2014, 6:39 a.m. UTC | #2
On 04/11/2014 01:40 AM, Joel Fernandes wrote:
> On 04/01/2014 08:06 AM, Peter Ujfalusi wrote:
>> We only support DEV_TO_MEM or MEM_TO_DEV directions with edma driver and the
>> check for the direction has been already done in the function calling
>> edma_config_pset().
>> The error reporting is redundant and also the "else if ()" can be removed.
>>
> 
> NAK. Please don't do this. I have been working on MEM_TO_MEM support as
> well so leave it as it is for future.

Sure. It is still valid to say that the error else {} will never going to
happen since you have the same check in the calling function and they already
filtered the non implemented direction.

I'll leave this out from v3.
Joel Fernandes April 11, 2014, 7:57 p.m. UTC | #3
On 04/11/2014 01:39 AM, Peter Ujfalusi wrote:
> On 04/11/2014 01:40 AM, Joel Fernandes wrote:
>> On 04/01/2014 08:06 AM, Peter Ujfalusi wrote:
>>> We only support DEV_TO_MEM or MEM_TO_DEV directions with edma driver and the
>>> check for the direction has been already done in the function calling
>>> edma_config_pset().
>>> The error reporting is redundant and also the "else if ()" can be removed.
>>>
>>
>> NAK. Please don't do this. I have been working on MEM_TO_MEM support as
>> well so leave it as it is for future.
> 
> Sure. It is still valid to say that the error else {} will never going to
> happen since you have the same check in the calling function and they already
> filtered the non implemented direction.
> 

That's true. Though the patch removes the else if which would mean more
work later ;)

> I'll leave this out from v3.

Thanks.

Regards,
-Joel
diff mbox

Patch

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 855766672aa9..d954099650ae 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -372,14 +372,12 @@  static int edma_config_pset(struct dma_chan *chan, struct edmacc_param *pset,
 		src_cidx = cidx;
 		dst_bidx = 0;
 		dst_cidx = 0;
-	} else if (direction == DMA_DEV_TO_MEM)  {
+	} else {
+		/* DMA_DEV_TO_MEM */
 		src_bidx = 0;
 		src_cidx = 0;
 		dst_bidx = acnt;
 		dst_cidx = cidx;
-	} else {
-		dev_err(dev, "%s: direction not implemented yet\n", __func__);
-		return -EINVAL;
 	}
 
 	pset->opt = EDMA_TCC(EDMA_CHAN_SLOT(echan->ch_num));