Re: [PATCH] dmaengine: dw-edma: Fix linked list physical address calculation on non-64 bits architectures

[Vinod Koul <vkoul@kernel.org>]

On 13-08-20, 16:13, Gustavo Pimentel wrote:
> Fix linked list physical address calculation on non-64 bits architectures.
> 
> The paddr variable is phys_addr_t type, which can assume a different
> type (u64 or u32) depending on the conditional compilation flag
> CONFIG_PHYS_ADDR_T_64BIT.
> 
> Since this variable is used in with upper_32 bits() macro to get the
> value from 32 to 63 bits, on a non-64 bits architecture this variable
> will assume a u32 type, it can cause a compilation warning.
> 
> This issue was reported by a Coverity analysis.
> 
> Fixes: 7e4b8a4fbe2c ("dmaengine: Add Synopsys eDMA IP version 0 support")
> 
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  drivers/dma/dw-edma/dw-edma-v0-core.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 692de47..cfabbf5 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -229,8 +229,13 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
>  	/* Channel control */
>  	SET_LL(&llp->control, control);
>  	/* Linked list  - low, high */
> -	SET_LL(&llp->llp_low, lower_32_bits(chunk->ll_region.paddr));
> -	SET_LL(&llp->llp_high, upper_32_bits(chunk->ll_region.paddr));
> +	#ifdef CONFIG_PHYS_ADDR_T_64BIT
> +		SET_LL(&llp->llp_low, lower_32_bits(chunk->ll_region.paddr));
> +		SET_LL(&llp->llp_high, upper_32_bits(chunk->ll_region.paddr));
> +	#else /* CONFIG_PHYS_ADDR_T_64BIT */
> +		SET_LL(&llp->llp_low, chunk->ll_region.paddr);
> +		SET_LL(&llp->llp_high, 0x0);

Shouldn't upper_32_bits(chunk->ll_region.paddr) return zero for non
64bit archs?

> +	#endif /* CONFIG_PHYS_ADDR_T_64BIT */
>  }
>  
>  void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> @@ -257,10 +262,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  		SET_CH(dw, chan->dir, chan->id, ch_control1,
>  		       (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
>  		/* Linked list - low, high */
> -		SET_CH(dw, chan->dir, chan->id, llp_low,
> -		       lower_32_bits(chunk->ll_region.paddr));
> -		SET_CH(dw, chan->dir, chan->id, llp_high,
> -		       upper_32_bits(chunk->ll_region.paddr));
> +		#ifdef CONFIG_PHYS_ADDR_T_64BIT
> +			SET_CH(dw, chan->dir, chan->id, llp_low,
> +			       lower_32_bits(chunk->ll_region.paddr));
> +			SET_CH(dw, chan->dir, chan->id, llp_high,
> +			       upper_32_bits(chunk->ll_region.paddr));
> +		#else /* CONFIG_PHYS_ADDR_T_64BIT */
> +			SET_CH(dw, chan->dir, chan->id, llp_low,
> +			       chunk->ll_region.paddr);
> +			SET_CH(dw, chan->dir, chan->id, llp_high, 0x0);
> +		#endif /* CONFIG_PHYS_ADDR_T_64BIT*/
>  	}
>  	/* Doorbell */
>  	SET_RW(dw, chan->dir, doorbell,
> -- 
> 2.7.4

-- 
~Vinod