diff mbox

[U-Boot,RFC] mmc: fsl_esdhc: fix misaligned cache operation warning

Message ID 20160827223938.11778-1-clemens.gruber@pqgruber.com
State Superseded
Headers show

Commit Message

Clemens Gruber Aug. 27, 2016, 10:39 p.m. UTC
When using gzwrite to eMMC on an i.MX6Q board, the following warning
occurs repeatedly:
CACHE: Misaligned operation at range [4fd63318, 4fe63318]

I tried to cache-line align the start and end parameter for
flush_dcache_range in esdhc_setup_data.
(Same approach as in https://patchwork.ozlabs.org/patch/656470/)

After that, the "Misaligned operation" error message disappeared and
everything still works for me.

But is this safe and should I send the patch again with [PATCH] ?

Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com>
---
 drivers/mmc/fsl_esdhc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Stefan Agner Aug. 28, 2016, 11:55 p.m. UTC | #1
On 2016-08-27 15:39, Clemens Gruber wrote:
> When using gzwrite to eMMC on an i.MX6Q board, the following warning
> occurs repeatedly:
> CACHE: Misaligned operation at range [4fd63318, 4fe63318]
> 
> I tried to cache-line align the start and end parameter for
> flush_dcache_range in esdhc_setup_data.
> (Same approach as in https://patchwork.ozlabs.org/patch/656470/)
> 
> After that, the "Misaligned operation" error message disappeared and
> everything still works for me.
> 
> But is this safe and should I send the patch again with [PATCH] ?
> 
> Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com>
> ---
>  drivers/mmc/fsl_esdhc.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 103b32e..f40b424 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -222,6 +222,7 @@ static int esdhc_setup_data(struct mmc *mmc,
> struct mmc_data *data)
>  	dma_addr_t addr;
>  #endif
>  	uint wml_value;
> +	ulong start, end;
>  
>  	wml_value = data->blocksize/4;
>  
> @@ -243,9 +244,11 @@ static int esdhc_setup_data(struct mmc *mmc,
> struct mmc_data *data)
>  #endif
>  	} else {
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -		flush_dcache_range((ulong)data->src,
> -				   (ulong)data->src+data->blocks
> -					 *data->blocksize);
> +		start = (ulong)data->src;
> +		start &= ~(CONFIG_SYS_CACHELINE_SIZE - 1);
> +		end = ALIGN((ulong)data->src + data->blocks * data->blocksize,
> +			    CONFIG_SYS_CACHELINE_SIZE);
> +		flush_dcache_range(start, end);

Hm, that seems dangerous to me, especially the src rounding...

Not sure where that memory gets aligned, but I guess we should use
memalign and align the length to cache size or the like... 

--
Stefan

>  #endif
>  		if (wml_value > WML_WR_WML_MAX)
>  			wml_value = WML_WR_WML_MAX_VAL;
Clemens Gruber Aug. 29, 2016, 3:18 p.m. UTC | #2
Hi,

On Sun, Aug 28, 2016 at 04:55:36PM -0700, Stefan Agner wrote:
> Hm, that seems dangerous to me, especially the src rounding...
> 
> Not sure where that memory gets aligned, but I guess we should use
> memalign and align the length to cache size or the like... 

Yes, you are right. I looked into this some more and found the source of
the problem is actually in lib/gunzip.c and not the flush_dcache_range
call in fsl_esdhc.c.
I sent a new patch to cache-align the memory allocation in gzwrite.

Regards,
Clemens
diff mbox

Patch

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index 103b32e..f40b424 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -222,6 +222,7 @@  static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
 	dma_addr_t addr;
 #endif
 	uint wml_value;
+	ulong start, end;
 
 	wml_value = data->blocksize/4;
 
@@ -243,9 +244,11 @@  static int esdhc_setup_data(struct mmc *mmc, struct mmc_data *data)
 #endif
 	} else {
 #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
-		flush_dcache_range((ulong)data->src,
-				   (ulong)data->src+data->blocks
-					 *data->blocksize);
+		start = (ulong)data->src;
+		start &= ~(CONFIG_SYS_CACHELINE_SIZE - 1);
+		end = ALIGN((ulong)data->src + data->blocks * data->blocksize,
+			    CONFIG_SYS_CACHELINE_SIZE);
+		flush_dcache_range(start, end);
 #endif
 		if (wml_value > WML_WR_WML_MAX)
 			wml_value = WML_WR_WML_MAX_VAL;