Skip to contentSkip to conversation

arm: re-implement proper ISB instruction for ARMv7-A

PATCH New Ziyuan Xu 5 participants 16 messages Jul 28, 2016 – Aug 1, 2016 Arm (32-bit)

lore
Share & tools
lore

Ziyuan Xu

author
#0 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

 arch/arm/include/asm/system.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +4 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
  */
 void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
 #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
 
 #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
 #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
 #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
 #define wfi()
 #endif
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Quoted 14 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Quoted 5 lines

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

Perfect! So with this, EFI support can still be in and things work fine?

Alex

author
#2 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Hi Alexander,

Quoted 16 lines · Alexander Graf
Quoted 15 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Quoted 7 lines
Quoted 5 lines

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

Perfect! So with this, EFI support can still be in and things work fine?

I had not test EFI feature, in fact, I have no experience about it. Any progress I will promptly inform you.

Quoted 7 lines

Alex

#3 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Ziyuan,

I tested this patch and it works for me with the current u-boot.git/master.

Re the EFI support. The problem was that when you compile in efi_runtime.c it breaks booting the kernel. Probably rubbing the caching system the wrong way. With this patch we are able to boot the kernel with the EFI parts compiled in. I don't believe anyone has tested the functionality of EFI wtih rk3288.

Sandy Patterson

Quoted 44 lines · Ziyuan Xu

Hi Alexander,

Quoted 17 lines · Alexander Graf
Quoted 15 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Quoted 9 lines
Quoted 6 lines

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

Perfect! So with this, EFI support can still be in and things work fine?

I had not test EFI feature, in fact, I have no experience about it. Any progress I will promptly inform you.

Quoted 7 lines

Alex

#4 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

In reply to Ziyuan Xu

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

 arch/arm/include/asm/system.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
  */
 void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
 #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
 #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
 #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
 #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
 #define wfi()
 #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

You'll see they also support ARMv6's CP15 ISB/DSB.

Regards
ChenYu

author
#5 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Hi,

Quoted 39 lines · Chen-Yu Tsai

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

  arch/arm/include/asm/system.h | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
   */
  void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
  #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
Quoted 12 lines

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
  #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
  #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
  #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
  #define wfi()
  #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

I know just what you mean.
arch/arm/include/asm/barriers.h defined ISB macro. If I only want to fix the issue which I hit on rk3288 board, I just use ISB in arch/arm/include/asm/system.h::set_cr() instead of isb(). But isb() had been invoked in some places, IMHO, this patch is more apposite.:-)

Quoted 8 lines

You'll see they also support ARMv6's CP15 ISB/DSB.

Regards
ChenYu

#6 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

In reply to Chen-Yu Tsai
Quoted 39 lines · Chen-Yu Tsai

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

 arch/arm/include/asm/system.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
  */
 void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
 #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
Quoted 13 lines

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
 #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
 #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
 #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
 #define wfi()
 #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

Please fix arch/arm/include/asm/system.h to use the macros found in barriers.h rather than have their own versions. Thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160728/ec9d4b9f/attachment.sig>
author
#7 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Hi Tom,

Quoted 52 lines · Tom Rini
Quoted 39 lines · Chen-Yu Tsai

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

  arch/arm/include/asm/system.h | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
   */
  void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
  #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
Quoted 12 lines

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
  #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
  #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
  #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
  #define wfi()
  #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

Quoted 2 lines

Please fix arch/arm/include/asm/system.h to use the macros found in barriers.h rather than have their own versions. Thanks!

If I understand correctly, I can change into as bellow:
#define isb() ISB
How about it?

Quoted 7 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#8 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Quoted 55 lines · Ziyuan Xu

Hi Tom,

Quoted 52 lines · Tom Rini
Quoted 51 lines · Chen-Yu Tsai

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

 arch/arm/include/asm/system.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
  */
 void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
 #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
 #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
 #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
 #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
 #define wfi()
 #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

Quoted 6 lines
Quoted 2 lines

Please fix arch/arm/include/asm/system.h to use the macros found in barriers.h rather than have their own versions. Thanks!

If I understand correctly, I can change into as bellow:
#define isb() ISB
How about it?

Well, I'd rather not have ISB and isb, just ISB, which means we might have to fix other places too. If that starts looking too huge, we can do this in steps and just do what you suggested for now and for next release move everything over.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160728/33977d6f/attachment.sig>
author
#9 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Hi Tom,

Quoted 58 lines · Tom Rini
Quoted 57 lines · Ziyuan Xu

Hi Tom,

Quoted 54 lines · Tom Rini
Quoted 39 lines · Chen-Yu Tsai

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

  arch/arm/include/asm/system.h | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
   */
  void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
  #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
Quoted 12 lines

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
  #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
  #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
  #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
  #define wfi()
  #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

Please fix arch/arm/include/asm/system.h to use the macros found in barriers.h rather than have their own versions. Thanks!

Quoted 7 lines
Quoted 3 lines

If I understand correctly, I can change into as bellow:
#define isb() ISB
How about it?

Well, I'd rather not have ISB and isb, just ISB, which means we might have to fix other places too. If that starts looking too huge, we can do this in steps and just do what you suggested for now and for next release move everything over.

As I mentioned before, arch/arm/include/asm/barriers.h defined ISB macro. If I only want to fix the issue which I hit on rk3288 board, I just use ISB in arch/arm/include/asm/system.h::set_cr() instead of isb(). But isb() had been invoked in some places.

I can't verify integrallty after all revision, it involve some boards and feature. But this does fix for rk3288, if you agree with me, could you apply it provisionally?:-)

Quoted 1 line
#10 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Quoted 68 lines · Ziyuan Xu

Hi Tom,

Quoted 58 lines · Tom Rini
Quoted 57 lines · Ziyuan Xu

Hi Tom,

Quoted 54 lines · Tom Rini
Quoted 39 lines · Chen-Yu Tsai

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

 arch/arm/include/asm/system.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
  */
 void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
 #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
Quoted 12 lines

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
 #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
 #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
 #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
 #define wfi()
 #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

Please fix arch/arm/include/asm/system.h to use the macros found in barriers.h rather than have their own versions. Thanks!

Quoted 7 lines
Quoted 3 lines

If I understand correctly, I can change into as bellow:
#define isb() ISB
How about it?

Well, I'd rather not have ISB and isb, just ISB, which means we might have to fix other places too. If that starts looking too huge, we can do this in steps and just do what you suggested for now and for next release move everything over.

Quoted 8 lines

As I mentioned before, arch/arm/include/asm/barriers.h defined ISB macro. If I only want to fix the issue which I hit on rk3288 board, I just use ISB in arch/arm/include/asm/system.h::set_cr() instead of isb(). But isb() had been invoked in some places.

I can't verify integrallty after all revision, it involve some boards and feature. But this does fix for rk3288, if you agree with me, could you apply it provisionally?:-)

I would really like to try and fix the other possibly latent issues that we have by not calling a real ISB. Please try moving towards all places that need an isb calling the correct one from barriers.h and giving it a spin on the hardware you have available.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160728/2e837f41/attachment.sig>
author
#11 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Quoted 74 lines · Tom Rini
Quoted 73 lines · Ziyuan Xu

Hi Tom,

Quoted 58 lines · Tom Rini
Quoted 57 lines · Ziyuan Xu

Hi Tom,

Quoted 54 lines · Tom Rini
Quoted 39 lines · Chen-Yu Tsai

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

  arch/arm/include/asm/system.h | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
   */
  void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
  #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
Quoted 12 lines

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
  #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
  #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
  #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
  #define wfi()
  #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

Please fix arch/arm/include/asm/system.h to use the macros found in barriers.h rather than have their own versions. Thanks!

Quoted 7 lines
Quoted 3 lines

If I understand correctly, I can change into as bellow:
#define isb() ISB
How about it?

Well, I'd rather not have ISB and isb, just ISB, which means we might have to fix other places too. If that starts looking too huge, we can do this in steps and just do what you suggested for now and for next release move everything over.

As I mentioned before, arch/arm/include/asm/barriers.h defined ISB macro. If I only want to fix the issue which I hit on rk3288 board, I just use ISB in arch/arm/include/asm/system.h::set_cr() instead of isb(). But isb() had been invoked in some places.

Quoted 7 lines
Quoted 3 lines

I can't verify integrallty after all revision, it involve some boards and feature. But this does fix for rk3288, if you agree with me, could you apply it provisionally?:-)

I would really like to try and fix the other possibly latent issues that we have by not calling a real ISB. Please try moving towards all places that need an isb calling the correct one from barriers.h and giving it a spin on the hardware you have available.

Okay, I will confirm it on my boards.:-)

Quoted 1 line
author
#12 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

In reply to Tom Rini

Hi Tom,

Quoted 74 lines · Tom Rini
Quoted 73 lines · Ziyuan Xu

Hi Tom,

Quoted 58 lines · Tom Rini
Quoted 57 lines · Ziyuan Xu

Hi Tom,

Quoted 54 lines · Tom Rini
Quoted 39 lines · Chen-Yu Tsai

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

  arch/arm/include/asm/system.h | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
   */
  void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
  #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
Quoted 12 lines

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
  #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
  #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
  #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
  #define wfi()
  #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

Please fix arch/arm/include/asm/system.h to use the macros found in barriers.h rather than have their own versions. Thanks!

Quoted 7 lines
Quoted 3 lines

If I understand correctly, I can change into as bellow:
#define isb() ISB
How about it?

Well, I'd rather not have ISB and isb, just ISB, which means we might have to fix other places too. If that starts looking too huge, we can do this in steps and just do what you suggested for now and for next release move everything over.

As I mentioned before, arch/arm/include/asm/barriers.h defined ISB macro. If I only want to fix the issue which I hit on rk3288 board, I just use ISB in arch/arm/include/asm/system.h::set_cr() instead of isb(). But isb() had been invoked in some places.

Quoted 7 lines
Quoted 3 lines

I can't verify integrallty after all revision, it involve some boards and feature. But this does fix for rk3288, if you agree with me, could you apply it provisionally?:-)

I would really like to try and fix the other possibly latent issues that we have by not calling a real ISB. Please try moving towards all places that need an isb calling the correct one from barriers.h and giving it a spin on the hardware you have available.

I used ISB instead of isb(), everything works sane on my rockchip rk3288 boards.:-)
Will you send a patch to fix it and other possibly latent issues? Or apply this temporarily?

Quoted 1 line
#13 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Quoted 81 lines · Ziyuan Xu

Hi Tom,

Quoted 77 lines · Tom Rini
Quoted 76 lines · Ziyuan Xu

Hi Tom,

Quoted 58 lines · Tom Rini
Quoted 57 lines · Ziyuan Xu

Hi Tom,

Quoted 54 lines · Tom Rini
Quoted 39 lines · Chen-Yu Tsai

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

 arch/arm/include/asm/system.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
  */
 void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
 #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
Quoted 12 lines

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
 #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
 #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
 #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
 #define wfi()
 #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

Please fix arch/arm/include/asm/system.h to use the macros found in barriers.h rather than have their own versions. Thanks!

Quoted 7 lines
Quoted 3 lines

If I understand correctly, I can change into as bellow:
#define isb() ISB
How about it?

Well, I'd rather not have ISB and isb, just ISB, which means we might have to fix other places too. If that starts looking too huge, we can do this in steps and just do what you suggested for now and for next release move everything over.

As I mentioned before, arch/arm/include/asm/barriers.h defined ISB macro. If I only want to fix the issue which I hit on rk3288 board, I just use ISB in arch/arm/include/asm/system.h::set_cr() instead of isb(). But isb() had been invoked in some places.

I can't verify integrallty after all revision, it involve some boards and feature. But this does fix for rk3288, if you agree with me, could you apply it provisionally?:-)

Quoted 8 lines
Quoted 4 lines

I would really like to try and fix the other possibly latent issues that we have by not calling a real ISB. Please try moving towards all places that need an isb calling the correct one from barriers.h and giving it a spin on the hardware you have available.

I used ISB instead of isb(), everything works sane on my rockchip rk3288 boards.:-)
Will you send a patch to fix it and other possibly latent issues? Or apply this temporarily?

Please send a patch of all of the changes you did, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160731/c879eec1/attachment.sig>
author
#14 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

Quoted 86 lines · Tom Rini
Quoted 85 lines · Ziyuan Xu

Hi Tom,

Quoted 81 lines · Tom Rini
Quoted 68 lines · Ziyuan Xu

Hi Tom,

Quoted 58 lines · Tom Rini
Quoted 57 lines · Ziyuan Xu

Hi Tom,

Quoted 54 lines · Tom Rini
Quoted 39 lines · Chen-Yu Tsai

Hi,

Quoted 36 lines · Ziyuan Xu

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Signed-off-by Ziyuan Xu <[email protected]>

---

  arch/arm/include/asm/system.h | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +0 -2
index 2bdc0be..12d4ba0 100644
@@ -227,13 +227,15 @@ void __noreturn psci_system_reset(bool smc);
   */
  void save_boot_params_ret(void);
 
-#define isb() __asm__ __volatile__ ("" : : : "memory")
-
  #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
Quoted 12 lines

arch/arm/include/asm/system.h void __noreturn psci_system_reset(bool smc); · line 241

Quoted 10 lines
arch/arm/include/asm/system.h +4 -0
  #ifdef __ARM_ARCH_7A__
+#define isb() __asm__ __volatile__ ("isb" : : : "memory")
+
  #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
  #else
+#define isb() __asm__ __volatile__ ("" : : : "memory")
+
  #define wfi()
  #endif

arch/arm/include/asm/barriers.h already has a proper set of ISB/DSB macros. Please consider using those instead.

Please fix arch/arm/include/asm/system.h to use the macros found in barriers.h rather than have their own versions. Thanks!

Quoted 7 lines
Quoted 3 lines

If I understand correctly, I can change into as bellow:
#define isb() ISB
How about it?

Well, I'd rather not have ISB and isb, just ISB, which means we might have to fix other places too. If that starts looking too huge, we can do this in steps and just do what you suggested for now and for next release move everything over.

Quoted 8 lines

As I mentioned before, arch/arm/include/asm/barriers.h defined ISB macro. If I only want to fix the issue which I hit on rk3288 board, I just use ISB in arch/arm/include/asm/system.h::set_cr() instead of isb(). But isb() had been invoked in some places.

I can't verify integrallty after all revision, it involve some boards and feature. But this does fix for rk3288, if you agree with me, could you apply it provisionally?:-)

I would really like to try and fix the other possibly latent issues that we have by not calling a real ISB. Please try moving towards all places that need an isb calling the correct one from barriers.h and giving it a spin on the hardware you have available.

Quoted 5 lines
Quoted 4 lines

I used ISB instead of isb(), everything works sane on my rockchip rk3288 boards.:-)
Will you send a patch to fix it and other possibly latent issues? Or apply this temporarily?

Please send a patch of all of the changes you did, thanks!

Tom, it's impracticable only use ISB for all situation instead of isb(). There're two isb() invocation in drivers/ddr/fsl/fsl_ddr_gen4.c, and not only used on ARM architecture board, but also powerpc. See below:

arch/powerpc/include/asm/config_mpc85xx.h:769:#define CONFIG_SYS_FSL_DDRC_GEN4 arch/powerpc/include/asm/config_mpc85xx.h:822:#define CONFIG_SYS_FSL_DDRC_GEN4 arch/powerpc/include/asm/config_mpc85xx.h:955:
!defined(CONFIG_SYS_FSL_DDRC_GEN4) arch/arm/include/asm/arch-ls102xa/config.h:100:#define CONFIG_SYS_FSL_DDRC_GEN4 arch/arm/include/asm/arch-fsl-layerscape/config.h:13:#define CONFIG_SYS_FSL_DDRC_GEN4

If you insist on ISB only, we have to differentiate arch via CONFIG_SYS_ARCH, seems twisted.
How about it?
#define isb() ISB

Quoted 1 line
#15 lore raw

PATCH 1/1 arm: re-implement proper ISB instruction for ARMv7-A

In reply to Ziyuan Xu

On Thu, Jul 28, 2016 at 06:13:26PM +0800, Ziyuan Xu wrote:
`

Quoted 14 lines

For ARMv7-A architecture, the valid ISB instruction is asm volatile("isb").

This patch fixes the U-Boot was stuck in invalidate_dcache_all() before booting linux kernel, which occurred on rk3288-base development board such as evb-rk3288, rock2-rk3288. And something output via console like:

=> bootz 0x2000000
0x02000000
ramdisk start = 0x00000000, ramdisk end = 0x00000000 Continuing to boot without FDT
Initial value for argc=3
Final value for argc=3
using: ATAGS

Quoted 3 lines

Starting kernel ...

Linux kernel exactly the same way(see arch/arm/include/asm/barrier.h).

Good catch!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160728/bfbdfb17/attachment.sig>