diff mbox

[U-Boot] mtd: nand: mxs: fix cache alignment for cache lines >32

Message ID 20160802065518.24140-1-stefan@agner.ch
State Accepted
Commit 2a83c95fdb9c2f735d1c30c71bc52f6fd8aa0f97
Delegated to: Stefano Babic
Headers show

Commit Message

Stefan Agner Aug. 2, 2016, 6:55 a.m. UTC 
From: Stefan Agner <stefan.agner@toradex.com>

Currently the command buffer gets allocated with a size of 32 bytes.
This causes warning messages on systems with cache lines bigger than
32 bytes:
CACHE: Misaligned operation at range [9df17a00, 9df17a20]

Define command buffer to be at least 32 bytes, but more if cache
line is bigger.

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
---
This appeared after Simon enable the message in check_cache_range
by default...

 drivers/mtd/nand/mxs_nand.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Stefano Babic Aug. 2, 2016, 8:17 a.m. UTC | #1 
Hi Stefan,

On 02/08/2016 08:55, Stefan Agner wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
> 
> Currently the command buffer gets allocated with a size of 32 bytes.
> This causes warning messages on systems with cache lines bigger than
> 32 bytes:
> CACHE: Misaligned operation at range [9df17a00, 9df17a20]
> 

I've never seen this on NAND....

> Define command buffer to be at least 32 bytes, but more if cache
> line is bigger.
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> ---
> This appeared after Simon enable the message in check_cache_range
> by default...

ok, this explains why !

> 
>  drivers/mtd/nand/mxs_nand.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index 94fc5c1..4bf564e 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -37,7 +37,12 @@
>  #endif
>  #define	MXS_NAND_METADATA_SIZE			10
>  #define	MXS_NAND_BITS_PER_ECC_LEVEL		13
> +
> +#if !defined(CONFIG_SYS_CACHELINE_SIZE) || CONFIG_SYS_CACHELINE_SIZE < 32
>  #define	MXS_NAND_COMMAND_BUFFER_SIZE		32
> +#else
> +#define	MXS_NAND_COMMAND_BUFFER_SIZE		CONFIG_SYS_CACHELINE_SIZE
> +#endif
>  
>  #define	MXS_NAND_BCH_TIMEOUT			10000
>  
> 

Reviewed-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic
Stefano Babic Aug. 3, 2016, 7:57 a.m. UTC | #2 
On 02/08/2016 08:55, Stefan Agner wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
> 
> Currently the command buffer gets allocated with a size of 32 bytes.
> This causes warning messages on systems with cache lines bigger than
> 32 bytes:
> CACHE: Misaligned operation at range [9df17a00, 9df17a20]
> 
> Define command buffer to be at least 32 bytes, but more if cache
> line is bigger.
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> ---


Applied to -u-boot-imx, thanks !

Best regards,
Stefano Babic
Fabio Estevam Aug. 3, 2016, 1:51 p.m. UTC | #3 
On Tue, Aug 2, 2016 at 3:55 AM, Stefan Agner <stefan@agner.ch> wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
>
> Currently the command buffer gets allocated with a size of 32 bytes.
> This causes warning messages on systems with cache lines bigger than
> 32 bytes:
> CACHE: Misaligned operation at range [9df17a00, 9df17a20]
>
> Define command buffer to be at least 32 bytes, but more if cache
> line is bigger.
>
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> ---
> This appeared after Simon enable the message in check_cache_range
> by default...

On mx6ul pico I also get similar warnings even though NAND is not used
on this board:

U-Boot 2016.09-rc1-00245-gad6a303 (Aug 03 2016 - 10:31:52 -0300)

CPU:   Freescale i.MX6UL rev1.0 at 396 MHz
Reset cause: WDOG
Board: PICO-IMX6UL-EMMC
I2C:   ready
DRAM:  256 MiB
CACHE: Misaligned operation at range [8fff0000, 8fff0004]
CACHE: Misaligned operation at range [8fff0024, 8fff0028]
PMIC: PFUZE3000 DEV_ID=0x30 REV_ID=0x11
MMC:   FSL_SDHC: 0
*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   FEC
Hit any key to stop autoboot:  0

Looks like we need a more generic fix?

Thanks
Stefan Agner Aug. 3, 2016, 4:13 p.m. UTC | #4 
On 2016-08-03 06:51, Fabio Estevam wrote:
> On Tue, Aug 2, 2016 at 3:55 AM, Stefan Agner <stefan@agner.ch> wrote:
>> From: Stefan Agner <stefan.agner@toradex.com>
>>
>> Currently the command buffer gets allocated with a size of 32 bytes.
>> This causes warning messages on systems with cache lines bigger than
>> 32 bytes:
>> CACHE: Misaligned operation at range [9df17a00, 9df17a20]
>>
>> Define command buffer to be at least 32 bytes, but more if cache
>> line is bigger.
>>
>> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>> ---
>> This appeared after Simon enable the message in check_cache_range
>> by default...
> 
> On mx6ul pico I also get similar warnings even though NAND is not used
> on this board:
> 
> U-Boot 2016.09-rc1-00245-gad6a303 (Aug 03 2016 - 10:31:52 -0300)
> 
> CPU:   Freescale i.MX6UL rev1.0 at 396 MHz
> Reset cause: WDOG
> Board: PICO-IMX6UL-EMMC
> I2C:   ready
> DRAM:  256 MiB
> CACHE: Misaligned operation at range [8fff0000, 8fff0004]
> CACHE: Misaligned operation at range [8fff0024, 8fff0028]
> PMIC: PFUZE3000 DEV_ID=0x30 REV_ID=0x11
> MMC:   FSL_SDHC: 0
> *** Warning - bad CRC, using default environment
> 
> In:    serial
> Out:   serial
> Err:   serial
> Net:   FEC
> Hit any key to stop autoboot:  0
> 
> Looks like we need a more generic fix?

As you noted, this particular case is due to cache flush of the page
table and should be fixed with:
arm: cache: always flush cache line size for page table

Afaik, there is no such thing as a generic fix for cache line alignment
issues... Every call site need to make sure to keep data cache line
aligned, especially in case a external device (DMA) are modifying the
same data...

--
Stefan
Fabio Estevam Aug. 3, 2016, 5:39 p.m. UTC | #5 
On Wed, Aug 3, 2016 at 1:13 PM, Stefan Agner <stefan@agner.ch> wrote:

> As you noted, this particular case is due to cache flush of the page
> table and should be fixed with:
> arm: cache: always flush cache line size for page table

Yes, just noticed that on a imx7d-sdb I still get these cache warnings
even with "arm: cache: always flush cache line size for page table"
applied:

....
Filename 'zImage'.
Load address: 0x80800000
Loading: #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #############################################
         9.1 MiB/s
done
Bytes transferred = 7334512 (6fea70 hex)
CACHE: Misaligned operation at range [80800000, 80efea70]
BOOTP broadcast 1
*** Unhandled DHCP Option in OFFER/ACK: 44
*** Unhandled DHCP Option in OFFER/ACK: 46
*** Unhandled DHCP Option in OFFER/ACK: 44
*** Unhandled DHCP Option in OFFER/ACK: 46
DHCP client bound to address 10.29.244.54 (356 ms)
Using FEC0 device
TFTP from server 10.29.244.110; our IP address is 10.29.244.54
Filename 'imx7d-sdb.dtb'.
Load address: 0x83000000
Loading: ##
         376 KiB/s
done
Bytes transferred = 27742 (6c5e hex)
CACHE: Misaligned operation at range [83000000, 83006c5e]
Kernel image @ 0x80800000 [ 0x000000 - 0x6fea70 ]
## Flattened Device Tree blob at 83000000
   Booting using the fdt blob at 0x83000000
   Using Device Tree in place at 83000000, end 83009c5d

Starting kernel ...

CACHE: Misaligned operation at range [00900000, 00900529]
[    0.000000] Booting Linux on physical CPU 0x0
Tom Rini Aug. 3, 2016, 6:09 p.m. UTC | #6 
On Wed, Aug 03, 2016 at 02:39:47PM -0300, Fabio Estevam wrote:
> On Wed, Aug 3, 2016 at 1:13 PM, Stefan Agner <stefan@agner.ch> wrote:
> 
> > As you noted, this particular case is due to cache flush of the page
> > table and should be fixed with:
> > arm: cache: always flush cache line size for page table
> 
> Yes, just noticed that on a imx7d-sdb I still get these cache warnings
> even with "arm: cache: always flush cache line size for page table"
> applied:
> 
> ....
> Filename 'zImage'.
> Load address: 0x80800000
> Loading: #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #################################################################
>          #############################################
>          9.1 MiB/s
> done
> Bytes transferred = 7334512 (6fea70 hex)
> CACHE: Misaligned operation at range [80800000, 80efea70]

I feel like we must have done something wrong of late, can you bisect
when these came in?  Thanks!
Hannes Schmelzer Aug. 3, 2016, 8:05 p.m. UTC | #7 
On 08/03/2016 08:09 PM, Tom Rini wrote:
> On Wed, Aug 03, 2016 at 02:39:47PM -0300, Fabio Estevam wrote:
>> On Wed, Aug 3, 2016 at 1:13 PM, Stefan Agner <stefan@agner.ch> wrote:
>>
>>> As you noted, this particular case is due to cache flush of the page
>>> table and should be fixed with:
>>> arm: cache: always flush cache line size for page table
>> Yes, just noticed that on a imx7d-sdb I still get these cache warnings
>> even with "arm: cache: always flush cache line size for page table"
>> applied:
>>
>> ....
>> Filename 'zImage'.
>> Load address: 0x80800000
>> Loading: #################################################################
>>           #################################################################
>>           #################################################################
>>           #################################################################
>>           #################################################################
>>           #################################################################
>>           #################################################################
>>           #############################################
>>           9.1 MiB/s
>> done
>> Bytes transferred = 7334512 (6fea70 hex)
>> CACHE: Misaligned operation at range [80800000, 80efea70]
> I feel like we must have done something wrong of late, can you bisect
> when these came in?  Thanks!
The mistake(s) must have been long time ago, but since commit 
bcc53bf095893fbdae531a9a7b5d4ef4a125a7fc it comes out visible.

I think we have to check every those warning for correct alignment.

cheers,
Hannes
diff mbox

Patch

diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index 94fc5c1..4bf564e 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -37,7 +37,12 @@ 
 #endif
 #define	MXS_NAND_METADATA_SIZE			10
 #define	MXS_NAND_BITS_PER_ECC_LEVEL		13
+
+#if !defined(CONFIG_SYS_CACHELINE_SIZE) || CONFIG_SYS_CACHELINE_SIZE < 32
 #define	MXS_NAND_COMMAND_BUFFER_SIZE		32
+#else
+#define	MXS_NAND_COMMAND_BUFFER_SIZE		CONFIG_SYS_CACHELINE_SIZE
+#endif
 
 #define	MXS_NAND_BCH_TIMEOUT			10000