mips: mt76xx: Flush d-cache in arch_misc_init() to solve d-cache issues

PATCH New Stefan Roese 2 participants 4 messages Dec 18, 2018

Subsystems (2)

MIPS Arm MediaTek

lore

Share & tools

lore

Stefan Roese

author

2018-12-18 09:27 UTC #0 lore raw

PATCH 1/2 mips: mt76xx: Flush d-cache in arch_misc_init() to solve d-cache issues

It has been noticed, that sometimes the d-cache is not in a "clean-state" when U-Boot is running on MT7688. This was detected when using the ethernet driver (which uses d-cache) and a TFTP command does not complete. Flushing the complete d-cache (again?) here seems to fix this issue.

Signed-off-by Stefan Roese <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

---
 arch/mips/Kconfig           |  2 +-
 arch/mips/mach-mt7620/cpu.c | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

arch/mips/Kconfig +1 -1

index 1b1b1d7d00..355d3b4d91 100644
@@ -79,7 +79,7 @@ config ARCH_MT7620
 	select DM_SERIAL
 	imply DM_SPI
 	imply DM_SPI_FLASH
-	select ARCH_MISC_INIT if WATCHDOG
+	select ARCH_MISC_INIT
 	select MIPS_TUNE_24KC
 	select OF_CONTROL
 	select ROM_EXCEPTION_VECTORS

arch/mips/mach-mt7620/cpu.c +13 -1

index 87cc973b75..9e0ca716f7 100644
@@ -89,9 +89,21 @@ void watchdog_reset(void)
 		wdt_reset(watchdog_dev);
 	}
 }
+#endif
 
 int arch_misc_init(void)
 {
+	/*
+	 * It has been noticed, that sometimes the d-cache is not in a
+	 * "clean-state" when U-Boot is running on MT7688. This was
+	 * detected when using the ethernet driver (which uses d-cache)
+	 * and a TFTP command does not complete. Flushing the complete
+	 * d-cache (again?) here seems to fix this issue.
+	 */
+	flush_dcache_range(gd->bd->bi_memstart,
+			   gd->bd->bi_memstart + gd->ram_size - 1);
+
+#ifdef CONFIG_WATCHDOG
 	/* Init watchdog */
 	if (uclass_get_device_by_seq(UCLASS_WDT, 0, &watchdog_dev)) {
 		debug("Watchdog: Not found by seq!\n");

@@ -103,7 +115,7 @@ int arch_misc_init(void)
 
 	wdt_start(watchdog_dev, 60000, 0);	/* 60 seconds */
 	printf("Watchdog: Started\n");
+#endif
 
 	return 0;
 }
-#endif

Signature · 1 line

2.20.1

Stefan Roese

author

2018-12-18 09:27 UTC #1 lore raw

PATCH 2/2 mips: mt76xx: linkit/gardena: Don't use CONFIG_OF_EMBED

Building with CONFIG_OF_EMBED generates build warnings, as it should only be used for debugging purposes. So let's move all MT7688 targets to CONFIG_OF_SEPARATE instead with this patch.

Signed-off-by Stefan Roese <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

---
 configs/gardena-smart-gateway-mt7688-ram_defconfig | 1 -
 configs/gardena-smart-gateway-mt7688_defconfig     | 1 -
 configs/linkit-smart-7688-ram_defconfig            | 1 -
 configs/linkit-smart-7688_defconfig                | 1 -
 4 files changed, 4 deletions(-)

configs/gardena-smart-gateway-mt7688-ram_defconfig +0 -1

index 05e7cf9f88..ae8bf2981f 100644
@@ -36,7 +36,6 @@ CONFIG_CMD_MTDPARTS=y
 CONFIG_MTDIDS_DEFAULT="spi-nand0=gd5f,nor0=spi0.0"
 CONFIG_MTDPARTS_DEFAULT="spi0.0:640k(uboot),64k(uboot_env0),64k(uboot_env1),64k(factory),-(unused);gd5f:-(nand)"
 CONFIG_CMD_UBI=y
-CONFIG_OF_EMBED=y
 CONFIG_DEFAULT_DEVICE_TREE="gardena-smart-gateway-mt7688"
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/gardena-smart-gateway-mt7688_defconfig +0 -1

index a08d1dbc9c..b7024e3dec 100644
@@ -39,7 +39,6 @@ CONFIG_CMD_MTDPARTS=y
 CONFIG_MTDIDS_DEFAULT="spi-nand0=gd5f,nor0=spi0.0"
 CONFIG_MTDPARTS_DEFAULT="spi0.0:640k(uboot),64k(uboot_env0),64k(uboot_env1),64k(factory),-(unused);gd5f:-(nand)"
 CONFIG_CMD_UBI=y
-CONFIG_OF_EMBED=y
 CONFIG_DEFAULT_DEVICE_TREE="gardena-smart-gateway-mt7688"
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/linkit-smart-7688-ram_defconfig +0 -1

index 2d20e9edd9..2d3ab7e35c 100644
@@ -26,7 +26,6 @@ CONFIG_CMD_DHCP=y
 CONFIG_CMD_MII=y
 CONFIG_CMD_PING=y
 CONFIG_CMD_TIME=y
-CONFIG_OF_EMBED=y
 CONFIG_DEFAULT_DEVICE_TREE="linkit-smart-7688"
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_NET_RANDOM_ETHADDR=y

configs/linkit-smart-7688_defconfig +0 -1

index 5add29fb33..ad34aaf640 100644
@@ -30,7 +30,6 @@ CONFIG_CMD_DHCP=y
 CONFIG_CMD_MII=y
 CONFIG_CMD_PING=y
 CONFIG_CMD_TIME=y
-CONFIG_OF_EMBED=y
 CONFIG_DEFAULT_DEVICE_TREE="linkit-smart-7688"
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_NET_RANDOM_ETHADDR=y

Signature · 1 line

2.20.1

Daniel Schwierzeck

2018-12-18 12:16 UTC #2 lore raw

PATCH 1/2 mips: mt76xx: Flush d-cache in arch_misc_init() to solve d-cache issues

In reply to Stefan Roese

Am 18.12.18 um 10:27 schrieb Stefan Roese:

Quoted 37 lines

It has been noticed, that sometimes the d-cache is not in a "clean-state" when U-Boot is running on MT7688. This was detected when using the ethernet driver (which uses d-cache) and a TFTP command does not complete. Flushing the complete d-cache (again?) here seems to fix this issue.
Signed-off-by Stefan Roese <[email protected]>
Cc Daniel Schwierzeck <[email protected]>
---
 arch/mips/Kconfig           |  2 +-
 arch/mips/mach-mt7620/cpu.c | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)
arch/mips/Kconfig +1 -1
index 1b1b1d7d00..355d3b4d91 100644
@@ -79,7 +79,7 @@ config ARCH_MT7620
 	select DM_SERIAL
 	imply DM_SPI
 	imply DM_SPI_FLASH
-	select ARCH_MISC_INIT if WATCHDOG
+	select ARCH_MISC_INIT
 	select MIPS_TUNE_24KC
 	select OF_CONTROL
 	select ROM_EXCEPTION_VECTORS
arch/mips/mach-mt7620/cpu.c +1 -0
index 87cc973b75..9e0ca716f7 100644
@@ -89,9 +89,21 @@ void watchdog_reset(void)
 		wdt_reset(watchdog_dev);
 	}
 }
+#endif
 
 int arch_misc_init(void)

arch/mips/mach-mt7620/cpu.c void watchdog_reset(void) · line 104

Quoted 10 lines

arch/mips/mach-mt7620/cpu.c +9 -0

 {
+	/*
+	 * It has been noticed, that sometimes the d-cache is not in a
+	 * "clean-state" when U-Boot is running on MT7688. This was
+	 * detected when using the ethernet driver (which uses d-cache)
+	 * and a TFTP command does not complete. Flushing the complete
+	 * d-cache (again?) here seems to fix this issue.
+	 */
+	flush_dcache_range(gd->bd->bi_memstart,
+			   gd->bd->bi_memstart + gd->ram_size - 1);

I noticed that in cache_init.S the i-cache lines is always invalidated regardless of CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD whereas the d-cache lines are not. I think we could always invalidate the d-cache lines and only fill them with zero when CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD is enabled to force good cache parity.

Could you retry with this little change?

arch/mips/lib/cache_init.S +1 -1

@@ -344,10 +344,10 @@ l1_init:
 2:     LONG_L          zero, 0(t0)
        PTR_ADDU        t0, R_DC_LINE
        bne             t0, t1, 2b
+#endif
        /* clear all tags */
        PTR_LI          t0, CKSEG0ADDR(CONFIG_MIPS_CACHE_INDEX_BASE)
        cache_loop      t0, t1, R_DC_LINE, INDEX_STORE_TAG_D
-#endif
 3:

Quoted 15 lines

arch/mips/mach-mt7620/cpu.c +3 -1

+
+#ifdef CONFIG_WATCHDOG
 	/* Init watchdog */
 	if (uclass_get_device_by_seq(UCLASS_WDT, 0, &watchdog_dev)) {
 		debug("Watchdog: Not found by seq!\n");

@@ -103,7 +115,7 @@ int arch_misc_init(void)
 
 	wdt_start(watchdog_dev, 60000, 0);	/* 60 seconds */
 	printf("Watchdog: Started\n");
+#endif
 
 	return 0;
 }
-#endif

Signature · 1 line

- Daniel

Stefan Roese

author

2018-12-18 13:04 UTC #3 lore raw

PATCH 1/2 mips: mt76xx: Flush d-cache in arch_misc_init() to solve d-cache issues

Hi Daniel,

Quoted 62 lines · Daniel Schwierzeck

Am 18.12.18 um 10:27 schrieb Stefan Roese:
Quoted 37 lines
It has been noticed, that sometimes the d-cache is not in a "clean-state" when U-Boot is running on MT7688. This was detected when using the ethernet driver (which uses d-cache) and a TFTP command does not complete. Flushing the complete d-cache (again?) here seems to fix this issue.
Signed-off-by Stefan Roese <[email protected]>
Cc Daniel Schwierzeck <[email protected]>
---
  arch/mips/Kconfig           |  2 +-
  arch/mips/mach-mt7620/cpu.c | 14 +++++++++++++-
  2 files changed, 14 insertions(+), 2 deletions(-)
arch/mips/Kconfig +1 -1
index 1b1b1d7d00..355d3b4d91 100644
@@ -79,7 +79,7 @@ config ARCH_MT7620
  	select DM_SERIAL
  	imply DM_SPI
  	imply DM_SPI_FLASH
-	select ARCH_MISC_INIT if WATCHDOG
+	select ARCH_MISC_INIT
  	select MIPS_TUNE_24KC
  	select OF_CONTROL
  	select ROM_EXCEPTION_VECTORS
arch/mips/mach-mt7620/cpu.c +1 -0
index 87cc973b75..9e0ca716f7 100644
@@ -89,9 +89,21 @@ void watchdog_reset(void)
  		wdt_reset(watchdog_dev);
  	}
  }
+#endif
 
  int arch_misc_init(void)
arch/mips/mach-mt7620/cpu.c void watchdog_reset(void) · line 104
Quoted 10 lines
arch/mips/mach-mt7620/cpu.c +9 -0
  {
+	/*
+	 * It has been noticed, that sometimes the d-cache is not in a
+	 * "clean-state" when U-Boot is running on MT7688. This was
+	 * detected when using the ethernet driver (which uses d-cache)
+	 * and a TFTP command does not complete. Flushing the complete
+	 * d-cache (again?) here seems to fix this issue.
+	 */
+	flush_dcache_range(gd->bd->bi_memstart,
+			   gd->bd->bi_memstart + gd->ram_size - 1);
I noticed that in cache_init.S the i-cache lines is always invalidated regardless of CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD whereas the d-cache lines are not. I think we could always invalidate the d-cache lines and only fill them with zero when CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD is enabled to force good cache parity.
Could you retry with this little change?

arch/mips/lib/cache_init.S l1_init: · line 352

Quoted 10 lines

arch/mips/lib/cache_init.S +1 -1

@@ -344,10 +344,10 @@ l1_init:
  2:     LONG_L          zero, 0(t0)
         PTR_ADDU        t0, R_DC_LINE
         bne             t0, t1, 2b
+#endif
         /* clear all tags */
         PTR_LI          t0, CKSEG0ADDR(CONFIG_MIPS_CACHE_INDEX_BASE)
         cache_loop      t0, t1, R_DC_LINE, INDEX_STORE_TAG_D
-#endif
  3:

Thanks for the idea, but this patch does not fix the issue, I'm afraid. Still hangs in the first TFTP command most of the time after power-up.

Please note that I also tested with CONFIG_SYS_MIPS_CACHE_INIT_RAM_LOAD enabled and this issue also occurs in this setup. The only real help to fix this first TFTP command hang is in my tests to either use the dcache_flush (as done in this current patch) or some user help command, like "cp.b 81000000 82000000 8000" to copy 32KiB of memory.

Frankly I'm also not very happy with this patch as the real reason why it is needed is not 100% clear to me. But its definitely better than using such user "cp.b xxx" hacks.

Thanks,
Stefan