diff mbox series

[v2,07/11] mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers

Message ID 20211112191523.1943825-8-sean.anderson@seco.com
State Superseded
Delegated to: Peng Fan
Headers show
Series fsl_esdhc_imx: port several patches from fsl_esdhc | expand

Commit Message

Sean Anderson Nov. 12, 2021, 7:15 p.m. UTC 
[ fsl_esdhc commit da86e8cfcb03ed5c1d8e0718bc8bc8583e60ced8 ]

SDMA can only do DMA with 32 bit addresses. This is true for all
architectures (just doesn't apply to 32 bit ones). Simplify the code and
remove unnecessary CONFIG_FSL_LAYERSCAPE.

Also make the error message more concise.

Signed-off-by: Michael Walle <michael@walle.cc>
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/mmc/fsl_esdhc_imx.c | 33 ++++++---------------------------
 1 file changed, 6 insertions(+), 27 deletions(-)

Comments

Jaehoon Chung Nov. 15, 2021, 8:39 a.m. UTC | #1 
On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit da86e8cfcb03ed5c1d8e0718bc8bc8583e60ced8 ]
> 
> SDMA can only do DMA with 32 bit addresses. This is true for all
> architectures (just doesn't apply to 32 bit ones). Simplify the code and
> remove unnecessary CONFIG_FSL_LAYERSCAPE.
> 
> Also make the error message more concise.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Just add minor comments.

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 33 ++++++---------------------------
>  1 file changed, 6 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index 95cde9e410..3588056759 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -282,10 +282,7 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  {
>  	int timeout;
>  	struct fsl_esdhc *regs = priv->esdhc_regs;
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
> -	defined(CONFIG_IMX8ULP)
>  	dma_addr_t addr;
> -#endif
>  	uint wml_value;
>  
>  	wml_value = data->blocksize/4;
> @@ -296,16 +293,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  
>  		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
> -	defined(CONFIG_IMX8ULP)
>  		addr = virt_to_phys((void *)(data->dest));
>  		if (upper_32_bits(addr))
> -			printf("Error found for upper 32 bits\n");
> -		else
> -			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
> -#else
> -		esdhc_write32(&regs->dsaddr, (u32)data->dest);
> -#endif
> +			printf("Cannot use 64 bit addresses with SDMA\n");
> +		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>  #endif
>  	} else {
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> @@ -334,16 +325,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>  		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
>  					wml_value << 16);
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
> -		defined(CONFIG_IMX8ULP)
>  		addr = virt_to_phys((void *)(data->src));
>  		if (upper_32_bits(addr))
> -			printf("Error found for upper 32 bits\n");
> -		else
> -			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
> -#else
> -		esdhc_write32(&regs->dsaddr, (u32)data->src);
> -#endif
> +			printf("Cannot use 64 bit addresses with SDMA\n");

Fix indentation?

> +		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>  #endif
>  	}
>  
> @@ -400,18 +385,12 @@ static void check_and_invalidate_dcache_range
>  	unsigned end = 0;
>  	unsigned size = roundup(ARCH_DMA_MINALIGN,
>  				data->blocks*data->blocksize);
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
> -	defined(CONFIG_IMX8ULP)
>  	dma_addr_t addr;
>  
>  	addr = virt_to_phys((void *)(data->dest));
>  	if (upper_32_bits(addr))
> -		printf("Error found for upper 32 bits\n");
> -	else
> -		start = lower_32_bits(addr);
> -#else
> -	start = (unsigned)data->dest;
> -#endif
> +		printf("Cannot use 64 bit addresses with SDMA\n");

Ditto.

> +	start = lower_32_bits(addr);
>  	end = start + size;
>  	invalidate_dcache_range(start, end);
>  }
>
Sean Anderson Nov. 15, 2021, 3:12 p.m. UTC | #2 
On 11/15/21 3:39 AM, Jaehoon Chung wrote:
> On 11/13/21 4:15 AM, Sean Anderson wrote:
>> [ fsl_esdhc commit da86e8cfcb03ed5c1d8e0718bc8bc8583e60ced8 ]
>> 
>> SDMA can only do DMA with 32 bit addresses. This is true for all
>> architectures (just doesn't apply to 32 bit ones). Simplify the code and
>> remove unnecessary CONFIG_FSL_LAYERSCAPE.
>> 
>> Also make the error message more concise.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> 
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> 
> Just add minor comments.
> 
>> ---
>> 
>> (no changes since v1)
>> 
>>  drivers/mmc/fsl_esdhc_imx.c | 33 ++++++---------------------------
>>  1 file changed, 6 insertions(+), 27 deletions(-)
>> 
>> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
>> index 95cde9e410..3588056759 100644
>> --- a/drivers/mmc/fsl_esdhc_imx.c
>> +++ b/drivers/mmc/fsl_esdhc_imx.c
>> @@ -282,10 +282,7 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>  {
>>  	int timeout;
>>  	struct fsl_esdhc *regs = priv->esdhc_regs;
>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>> -	defined(CONFIG_IMX8ULP)
>>  	dma_addr_t addr;
>> -#endif
>>  	uint wml_value;
>>  
>>  	wml_value = data->blocksize/4;
>> @@ -296,16 +293,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>  
>>  		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>> -	defined(CONFIG_IMX8ULP)
>>  		addr = virt_to_phys((void *)(data->dest));
>>  		if (upper_32_bits(addr))
>> -			printf("Error found for upper 32 bits\n");
>> -		else
>> -			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>> -#else
>> -		esdhc_write32(&regs->dsaddr, (u32)data->dest);
>> -#endif
>> +			printf("Cannot use 64 bit addresses with SDMA\n");
>> +		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>  #endif
>>  	} else {
>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>> @@ -334,16 +325,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>  		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
>>  					wml_value << 16);
>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>> -		defined(CONFIG_IMX8ULP)
>>  		addr = virt_to_phys((void *)(data->src));
>>  		if (upper_32_bits(addr))
>> -			printf("Error found for upper 32 bits\n");
>> -		else
>> -			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>> -#else
>> -		esdhc_write32(&regs->dsaddr, (u32)data->src);
>> -#endif
>> +			printf("Cannot use 64 bit addresses with SDMA\n");
> 
> Fix indentation?

Looks OK to me. The final code is

		if (upper_32_bits(addr))
			printf("Cannot use 64 bit addresses with SDMA\n");


> 
>> +		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>  #endif
>>  	}
>>  
>> @@ -400,18 +385,12 @@ static void check_and_invalidate_dcache_range
>>  	unsigned end = 0;
>>  	unsigned size = roundup(ARCH_DMA_MINALIGN,
>>  				data->blocks*data->blocksize);
>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>> -	defined(CONFIG_IMX8ULP)
>>  	dma_addr_t addr;
>>  
>>  	addr = virt_to_phys((void *)(data->dest));
>>  	if (upper_32_bits(addr))
>> -		printf("Error found for upper 32 bits\n");
>> -	else
>> -		start = lower_32_bits(addr);
>> -#else
>> -	start = (unsigned)data->dest;
>> -#endif
>> +		printf("Cannot use 64 bit addresses with SDMA\n");
> 
> Ditto.

	if (upper_32_bits(addr))
		printf("Cannot use 64 bit addresses with SDMA\n");

--Sean

>> +	start = lower_32_bits(addr);
>>  	end = start + size;
>>  	invalidate_dcache_range(start, end);
>>  }
>> 
>
Jaehoon Chung Nov. 15, 2021, 10:18 p.m. UTC | #3 
On 11/16/21 12:12 AM, Sean Anderson wrote:
> 
> 
> On 11/15/21 3:39 AM, Jaehoon Chung wrote:
>> On 11/13/21 4:15 AM, Sean Anderson wrote:
>>> [ fsl_esdhc commit da86e8cfcb03ed5c1d8e0718bc8bc8583e60ced8 ]
>>>
>>> SDMA can only do DMA with 32 bit addresses. This is true for all
>>> architectures (just doesn't apply to 32 bit ones). Simplify the code and
>>> remove unnecessary CONFIG_FSL_LAYERSCAPE.
>>>
>>> Also make the error message more concise.
>>>
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>>
>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>>
>> Just add minor comments.
>>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>  drivers/mmc/fsl_esdhc_imx.c | 33 ++++++---------------------------
>>>  1 file changed, 6 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
>>> index 95cde9e410..3588056759 100644
>>> --- a/drivers/mmc/fsl_esdhc_imx.c
>>> +++ b/drivers/mmc/fsl_esdhc_imx.c
>>> @@ -282,10 +282,7 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>>  {
>>>      int timeout;
>>>      struct fsl_esdhc *regs = priv->esdhc_regs;
>>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>>> -    defined(CONFIG_IMX8ULP)
>>>      dma_addr_t addr;
>>> -#endif
>>>      uint wml_value;
>>>  
>>>      wml_value = data->blocksize/4;
>>> @@ -296,16 +293,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>>  
>>>          esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>>> -    defined(CONFIG_IMX8ULP)
>>>          addr = virt_to_phys((void *)(data->dest));
>>>          if (upper_32_bits(addr))
>>> -            printf("Error found for upper 32 bits\n");
>>> -        else
>>> -            esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>> -#else
>>> -        esdhc_write32(&regs->dsaddr, (u32)data->dest);
>>> -#endif
>>> +            printf("Cannot use 64 bit addresses with SDMA\n");
>>> +        esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>>  #endif
>>>      } else {
>>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>>> @@ -334,16 +325,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
>>>          esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
>>>                      wml_value << 16);
>>>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
>>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>>> -        defined(CONFIG_IMX8ULP)
>>>          addr = virt_to_phys((void *)(data->src));
>>>          if (upper_32_bits(addr))
>>> -            printf("Error found for upper 32 bits\n");
>>> -        else
>>> -            esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>> -#else
>>> -        esdhc_write32(&regs->dsaddr, (u32)data->src);
>>> -#endif
>>> +            printf("Cannot use 64 bit addresses with SDMA\n");
>>
>> Fix indentation?
> 
> Looks OK to me. The final code is

It's my mis-reading. Thanks

Best Regards,
Jaehoon Chung

> 
>         if (upper_32_bits(addr))
>             printf("Cannot use 64 bit addresses with SDMA\n");
> 
> 
>>
>>> +        esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>>>  #endif
>>>      }
>>>  
>>> @@ -400,18 +385,12 @@ static void check_and_invalidate_dcache_range
>>>      unsigned end = 0;
>>>      unsigned size = roundup(ARCH_DMA_MINALIGN,
>>>                  data->blocks*data->blocksize);
>>> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
>>> -    defined(CONFIG_IMX8ULP)
>>>      dma_addr_t addr;
>>>  
>>>      addr = virt_to_phys((void *)(data->dest));
>>>      if (upper_32_bits(addr))
>>> -        printf("Error found for upper 32 bits\n");
>>> -    else
>>> -        start = lower_32_bits(addr);
>>> -#else
>>> -    start = (unsigned)data->dest;
>>> -#endif
>>> +        printf("Cannot use 64 bit addresses with SDMA\n");
>>
>> Ditto.
> 
>     if (upper_32_bits(addr))
>         printf("Cannot use 64 bit addresses with SDMA\n");
> 
> --Sean
> 
>>> +    start = lower_32_bits(addr);
>>>      end = start + size;
>>>      invalidate_dcache_range(start, end);
>>>  }
>>>
>>
>
diff mbox series

Patch

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 95cde9e410..3588056759 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -282,10 +282,7 @@  static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 {
 	int timeout;
 	struct fsl_esdhc *regs = priv->esdhc_regs;
-#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
-	defined(CONFIG_IMX8ULP)
 	dma_addr_t addr;
-#endif
 	uint wml_value;
 
 	wml_value = data->blocksize/4;
@@ -296,16 +293,10 @@  static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 
 		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
 #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
-	defined(CONFIG_IMX8ULP)
 		addr = virt_to_phys((void *)(data->dest));
 		if (upper_32_bits(addr))
-			printf("Error found for upper 32 bits\n");
-		else
-			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
-#else
-		esdhc_write32(&regs->dsaddr, (u32)data->dest);
-#endif
+			printf("Cannot use 64 bit addresses with SDMA\n");
+		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
 #endif
 	} else {
 #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
@@ -334,16 +325,10 @@  static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
 					wml_value << 16);
 #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
-		defined(CONFIG_IMX8ULP)
 		addr = virt_to_phys((void *)(data->src));
 		if (upper_32_bits(addr))
-			printf("Error found for upper 32 bits\n");
-		else
-			esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
-#else
-		esdhc_write32(&regs->dsaddr, (u32)data->src);
-#endif
+			printf("Cannot use 64 bit addresses with SDMA\n");
+		esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
 #endif
 	}
 
@@ -400,18 +385,12 @@  static void check_and_invalidate_dcache_range
 	unsigned end = 0;
 	unsigned size = roundup(ARCH_DMA_MINALIGN,
 				data->blocks*data->blocksize);
-#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) || \
-	defined(CONFIG_IMX8ULP)
 	dma_addr_t addr;
 
 	addr = virt_to_phys((void *)(data->dest));
 	if (upper_32_bits(addr))
-		printf("Error found for upper 32 bits\n");
-	else
-		start = lower_32_bits(addr);
-#else
-	start = (unsigned)data->dest;
-#endif
+		printf("Cannot use 64 bit addresses with SDMA\n");
+	start = lower_32_bits(addr);
 	end = start + size;
 	invalidate_dcache_range(start, end);
 }