[U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug

Hello,
Vagrant Cascadian asked for people to test the version of u-boot packaged for Debian Buster. I tested u-boot on my Dreamplug and found it was not working correctly. I raised a bug for Debian[1] but I have also tested with the mainline version of u-boot and found the same issues.
The first issue is that the following commit caused u-boot to no longer be able to access usb storage on the Dreamplug:
commit 93b283d49f933f95f3a6f40762936f454ac655a8 Author: Adam Ford aford173@gmail.com Date: Thu Aug 16 13:23:11 2018 -0500
ARM: CPU: arm926ejs: Consolidate cache routines to common file
Four different boards had different options for enabling cache that were virtually all the same. This consolidates these common functions into arch/arm/cpu/arm926ejs/cache.c
This also has the positive side-effect of enabling cache on the Davinci (da850) boards.
Signed-off-by: Adam Ford aford173@gmail.com [trini: Add mach-at91 to the list of consolidations] Signed-off-by: Tom Rini trini@konsulko.com
I don't have much knowledge of ARM caching, but the following patch makes it work again on my Dreamplug.
diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c index d54de53f31..8a065d73ae 100644 --- a/arch/arm/mach-kirkwood/cpu.c +++ b/arch/arm/mach-kirkwood/cpu.c @@ -291,7 +291,6 @@ int arch_misc_init(void) temp |= (1 << 22); writefr_extra_feature_reg(temp);
- icache_enable(); /* Change reset vector to address 0x0 */ temp = get_cr(); set_cr(temp & ~CR_V); diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h index f4d717213c..6348935c68 100644 --- a/include/configs/dreamplug.h +++ b/include/configs/dreamplug.h @@ -79,4 +79,6 @@ #define CONFIG_SYS_ATA_IDE0_OFFSET MV_SATA_PORT0_OFFSET #endif /*CONFIG_MVSATA_IDE*/
+#define CONFIG_SYS_DCACHE_OFF + #endif /* _CONFIG_DREAMPLUG_H */
I'd be grateful if someone could take a look. If you need me to do any testing or provide any more information, please let me know.
I found another issue (documented in the same bug). I'll send a separate email about that.
Regards,
Leigh.
-- [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923379

On Wed, Feb 27, 2019 at 5:32 AM Leigh Brown leigh@solinno.co.uk wrote:
Hello,
Vagrant Cascadian asked for people to test the version of u-boot packaged for Debian Buster. I tested u-boot on my Dreamplug and found it was not working correctly. I raised a bug for Debian[1] but I have also tested with the mainline version of u-boot and found the same issues.
The first issue is that the following commit caused u-boot to no longer be able to access usb storage on the Dreamplug:
commit 93b283d49f933f95f3a6f40762936f454ac655a8 Author: Adam Ford aford173@gmail.com Date: Thu Aug 16 13:23:11 2018 -0500
Sorry about that.
ARM: CPU: arm926ejs: Consolidate cache routines to common file Four different boards had different options for enabling cache that were virtually all the same. This consolidates these common functions into arch/arm/cpu/arm926ejs/cache.c This also has the positive side-effect of enabling cache on the Davinci (da850) boards. Signed-off-by: Adam Ford <aford173@gmail.com> [trini: Add mach-at91 to the list of consolidations] Signed-off-by: Tom Rini <trini@konsulko.com>
I don't have much knowledge of ARM caching, but the following patch makes it work again on my Dreamplug.
I am not that familiar with the ARM caching either, I was just hoping to enable it on the da850 board and compared the various code chunks between ARM 926 boards and noticed a common thread. Tom took it and added some more.
diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c index d54de53f31..8a065d73ae 100644 --- a/arch/arm/mach-kirkwood/cpu.c +++ b/arch/arm/mach-kirkwood/cpu.c @@ -291,7 +291,6 @@ int arch_misc_init(void) temp |= (1 << 22); writefr_extra_feature_reg(temp);
#ifndef CONFIG_SYS_ICACHE_OFF
icache_enable();
#endif
Instead of commenting out that line, try defining CONFIG_SYS_ICACHE_OFF in your header like you did for the CONFIG_SYS_DCACHE_OFF and encapsulate the function call with ifdef's so any other kirkwood processors can enable/disable it independently.
/* Change reset vector to address 0x0 */ temp = get_cr(); set_cr(temp & ~CR_V);
diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h index f4d717213c..6348935c68 100644 --- a/include/configs/dreamplug.h +++ b/include/configs/dreamplug.h @@ -79,4 +79,6 @@ #define CONFIG_SYS_ATA_IDE0_OFFSET MV_SATA_PORT0_OFFSET #endif /*CONFIG_MVSATA_IDE*/
+#define CONFIG_SYS_DCACHE_OFF
#define CONFIG_SYS_ICACHE_OFF
- #endif /* _CONFIG_DREAMPLUG_H */
I'd be grateful if someone could take a look. If you need me to do any testing or provide any more information, please let me know.
I found another issue (documented in the same bug). I'll send a separate email about that.
Regards,
Leigh.
-- [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923379

Hi Adam,
Thanks very much for your response.
On 2019-02-27 12:50, Adam Ford wrote:
On Wed, Feb 27, 2019 at 5:32 AM Leigh Brown leigh@solinno.co.uk wrote:
Hello,
Vagrant Cascadian asked for people to test the version of u-boot packaged for Debian Buster. I tested u-boot on my Dreamplug and found it was not working correctly. I raised a bug for Debian[1] but I have also tested with the mainline version of u-boot and found the same issues.
The first issue is that the following commit caused u-boot to no longer be able to access usb storage on the Dreamplug:
commit 93b283d49f933f95f3a6f40762936f454ac655a8 Author: Adam Ford aford173@gmail.com Date: Thu Aug 16 13:23:11 2018 -0500
Sorry about that.
ARM: CPU: arm926ejs: Consolidate cache routines to common file Four different boards had different options for enabling cache that were virtually all the same. This consolidates these common functions into arch/arm/cpu/arm926ejs/cache.c This also has the positive side-effect of enabling cache on the Davinci (da850) boards. Signed-off-by: Adam Ford <aford173@gmail.com> [trini: Add mach-at91 to the list of consolidations] Signed-off-by: Tom Rini <trini@konsulko.com>
I don't have much knowledge of ARM caching, but the following patch makes it work again on my Dreamplug.
I am not that familiar with the ARM caching either, I was just hoping to enable it on the da850 board and compared the various code chunks between ARM 926 boards and noticed a common thread. Tom took it and added some more.
Okay. Hopefully Tom can comment on my proposed fixes.
diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c index d54de53f31..8a065d73ae 100644 --- a/arch/arm/mach-kirkwood/cpu.c +++ b/arch/arm/mach-kirkwood/cpu.c @@ -291,7 +291,6 @@ int arch_misc_init(void) temp |= (1 << 22); writefr_extra_feature_reg(temp);
#ifndef CONFIG_SYS_ICACHE_OFF
icache_enable();
#endif
Instead of commenting out that line, try defining CONFIG_SYS_ICACHE_OFF in your header like you did for the CONFIG_SYS_DCACHE_OFF and encapsulate the function call with ifdef's so any other kirkwood processors can enable/disable it independently.
The reason I removed that line is because icache_enable() is called from enable_caches() which is called from initr_caches() which is in the list of functions in init_sequence_r[] prior to arch_misc_init(). In other words, it will already have been called by then if CONFIG_SYS_ICACHE_OFF is not set. Does that make sense?
/* Change reset vector to address 0x0 */ temp = get_cr(); set_cr(temp & ~CR_V);
diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h index f4d717213c..6348935c68 100644 --- a/include/configs/dreamplug.h +++ b/include/configs/dreamplug.h @@ -79,4 +79,6 @@ #define CONFIG_SYS_ATA_IDE0_OFFSET MV_SATA_PORT0_OFFSET #endif /*CONFIG_MVSATA_IDE*/
+#define CONFIG_SYS_DCACHE_OFF
#define CONFIG_SYS_ICACHE_OFF
The reason I have not done this is because the Kirkwood arch_misc_init() function was already unconditionally enabling the instruction cache, so we want to retain that behaviour - I think. I hope that makes sense.
- #endif /* _CONFIG_DREAMPLUG_H */
I'd be grateful if someone could take a look. If you need me to do any testing or provide any more information, please let me know.
I found another issue (documented in the same bug). I'll send a separate email about that.
Regards,
Leigh.
-- [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923379
Regards,
Leigh.

On Wed, Feb 27, 2019 at 8:52 AM Leigh Brown leigh@solinno.co.uk wrote:
Hi Adam,
Thanks very much for your response.
On 2019-02-27 12:50, Adam Ford wrote:
On Wed, Feb 27, 2019 at 5:32 AM Leigh Brown leigh@solinno.co.uk wrote:
Hello,
Vagrant Cascadian asked for people to test the version of u-boot packaged for Debian Buster. I tested u-boot on my Dreamplug and found it was not working correctly. I raised a bug for Debian[1] but I have also tested with the mainline version of u-boot and found the same issues.
The first issue is that the following commit caused u-boot to no longer be able to access usb storage on the Dreamplug:
commit 93b283d49f933f95f3a6f40762936f454ac655a8 Author: Adam Ford aford173@gmail.com Date: Thu Aug 16 13:23:11 2018 -0500
Sorry about that.
ARM: CPU: arm926ejs: Consolidate cache routines to common file Four different boards had different options for enabling cache that were virtually all the same. This consolidates these common functions into arch/arm/cpu/arm926ejs/cache.c This also has the positive side-effect of enabling cache on the Davinci (da850) boards. Signed-off-by: Adam Ford <aford173@gmail.com> [trini: Add mach-at91 to the list of consolidations] Signed-off-by: Tom Rini <trini@konsulko.com>
I don't have much knowledge of ARM caching, but the following patch makes it work again on my Dreamplug.
I am not that familiar with the ARM caching either, I was just hoping to enable it on the da850 board and compared the various code chunks between ARM 926 boards and noticed a common thread. Tom took it and added some more.
Okay. Hopefully Tom can comment on my proposed fixes.
diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c index d54de53f31..8a065d73ae 100644 --- a/arch/arm/mach-kirkwood/cpu.c +++ b/arch/arm/mach-kirkwood/cpu.c @@ -291,7 +291,6 @@ int arch_misc_init(void) temp |= (1 << 22); writefr_extra_feature_reg(temp);
#ifndef CONFIG_SYS_ICACHE_OFF
icache_enable();
#endif
Instead of commenting out that line, try defining CONFIG_SYS_ICACHE_OFF in your header like you did for the CONFIG_SYS_DCACHE_OFF and encapsulate the function call with ifdef's so any other kirkwood processors can enable/disable it independently.
The reason I removed that line is because icache_enable() is called from enable_caches() which is called from initr_caches() which is in the list of functions in init_sequence_r[] prior to arch_misc_init(). In other words, it will already have been called by then if CONFIG_SYS_ICACHE_OFF is not set. Does that make sense?
That makes a lot of sense to me.
/* Change reset vector to address 0x0 */ temp = get_cr(); set_cr(temp & ~CR_V);
diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h index f4d717213c..6348935c68 100644 --- a/include/configs/dreamplug.h +++ b/include/configs/dreamplug.h @@ -79,4 +79,6 @@ #define CONFIG_SYS_ATA_IDE0_OFFSET MV_SATA_PORT0_OFFSET #endif /*CONFIG_MVSATA_IDE*/
+#define CONFIG_SYS_DCACHE_OFF
#define CONFIG_SYS_ICACHE_OFF
The reason I have not done this is because the Kirkwood arch_misc_init() function was already unconditionally enabling the instruction cache, so we want to retain that behaviour - I think. I hope that makes sense.
That makes a lot of sense too.
adam
- #endif /* _CONFIG_DREAMPLUG_H */
I'd be grateful if someone could take a look. If you need me to do any testing or provide any more information, please let me know.
I found another issue (documented in the same bug). I'll send a separate email about that.
Regards,
Leigh.
-- [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923379
Regards,
Leigh.

Hi Leigh,
On Thu, Feb 28, 2019 at 1:11 AM Leigh Brown leigh@solinno.co.uk wrote:
Hello,
Vagrant Cascadian asked for people to test the version of u-boot packaged for Debian Buster. I tested u-boot on my Dreamplug and found it was not working correctly. I raised a bug for Debian[1] but I have also tested with the mainline version of u-boot and found the same issues.
The first issue is that the following commit caused u-boot to no longer be able to access usb storage on the Dreamplug:
commit 93b283d49f933f95f3a6f40762936f454ac655a8 Author: Adam Ford aford173@gmail.com Date: Thu Aug 16 13:23:11 2018 -0500
ARM: CPU: arm926ejs: Consolidate cache routines to common file Four different boards had different options for enabling cache that were virtually all the same. This consolidates these common functions into arch/arm/cpu/arm926ejs/cache.c This also has the positive side-effect of enabling cache on the Davinci (da850) boards. Signed-off-by: Adam Ford <aford173@gmail.com> [trini: Add mach-at91 to the list of consolidations] Signed-off-by: Tom Rini <trini@konsulko.com>
I don't have much knowledge of ARM caching, but the following patch makes it work again on my Dreamplug.
diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c index d54de53f31..8a065d73ae 100644 --- a/arch/arm/mach-kirkwood/cpu.c +++ b/arch/arm/mach-kirkwood/cpu.c @@ -291,7 +291,6 @@ int arch_misc_init(void) temp |= (1 << 22); writefr_extra_feature_reg(temp);
icache_enable(); /* Change reset vector to address 0x0 */ temp = get_cr(); set_cr(temp & ~CR_V);
diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h index f4d717213c..6348935c68 100644 --- a/include/configs/dreamplug.h +++ b/include/configs/dreamplug.h @@ -79,4 +79,6 @@ #define CONFIG_SYS_ATA_IDE0_OFFSET MV_SATA_PORT0_OFFSET #endif /*CONFIG_MVSATA_IDE*/
+#define CONFIG_SYS_DCACHE_OFF
- #endif /* _CONFIG_DREAMPLUG_H */
I'd be grateful if someone could take a look. If you need me to do any testing or provide any more information, please let me know.
Thanks for pointing this out. I'd been having a problem like this with the Ethernet interface. I'd been blaming the hardware I was using but your change makes things work for me.
I found another issue (documented in the same bug). I'll send a separate email about that.
Regards,
Leigh.
-- [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923379____________________... U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

On Wed, Feb 27, 2019 at 11:32:16AM +0000, Leigh Brown wrote:
Hello,
Vagrant Cascadian asked for people to test the version of u-boot packaged for Debian Buster. I tested u-boot on my Dreamplug and found it was not working correctly. I raised a bug for Debian[1] but I have also tested with the mainline version of u-boot and found the same issues.
The first issue is that the following commit caused u-boot to no longer be able to access usb storage on the Dreamplug:
commit 93b283d49f933f95f3a6f40762936f454ac655a8 Author: Adam Ford aford173@gmail.com Date: Thu Aug 16 13:23:11 2018 -0500
ARM: CPU: arm926ejs: Consolidate cache routines to common file Four different boards had different options for enabling cache that were virtually all the same. This consolidates these common functions into arch/arm/cpu/arm926ejs/cache.c This also has the positive side-effect of enabling cache on the Davinci (da850) boards. Signed-off-by: Adam Ford <aford173@gmail.com> [trini: Add mach-at91 to the list of consolidations] Signed-off-by: Tom Rini <trini@konsulko.com>
I don't have much knowledge of ARM caching, but the following patch makes it work again on my Dreamplug.
diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c index d54de53f31..8a065d73ae 100644
Thanks for digging into this. Based on the whole thread, yes, this makes sense.
Reviewed-by: Tom Rini trini@konsulko.com
participants (4)
-
Adam Ford
-
Chris Packham
-
Leigh Brown
-
Tom Rini