All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
@ 2016-07-28 10:13 Ziyuan Xu
  2016-07-28 10:39 ` Alexander Graf
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Ziyuan Xu @ 2016-07-28 10:13 UTC (permalink / raw)
  To: u-boot

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 <xzy.xu@rock-chips.com>
---

 arch/arm/include/asm/system.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 2bdc0be..12d4ba0 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -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
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-28 10:13 [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A Ziyuan Xu
@ 2016-07-28 10:39 ` Alexander Graf
  2016-07-28 12:03   ` Ziyuan Xu
  2016-07-28 11:03 ` Chen-Yu Tsai
  2016-07-28 22:16 ` Tom Rini
  2 siblings, 1 reply; 16+ messages in thread
From: Alexander Graf @ 2016-07-28 10:39 UTC (permalink / raw)
  To: u-boot

On 07/28/2016 12:13 PM, Ziyuan Xu wrote:
> 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 <xzy.xu@rock-chips.com>

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


Alex

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-28 10:13 [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A Ziyuan Xu
  2016-07-28 10:39 ` Alexander Graf
@ 2016-07-28 11:03 ` Chen-Yu Tsai
  2016-07-28 11:51   ` Ziyuan Xu
  2016-07-28 22:15   ` Tom Rini
  2016-07-28 22:16 ` Tom Rini
  2 siblings, 2 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2016-07-28 11:03 UTC (permalink / raw)
  To: u-boot

Hi,

On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
> 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 <xzy.xu@rock-chips.com>
> ---
>
>  arch/arm/include/asm/system.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> index 2bdc0be..12d4ba0 100644
> --- a/arch/arm/include/asm/system.h
> +++ b/arch/arm/include/asm/system.h
> @@ -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
>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-28 11:03 ` Chen-Yu Tsai
@ 2016-07-28 11:51   ` Ziyuan Xu
  2016-07-28 22:15   ` Tom Rini
  1 sibling, 0 replies; 16+ messages in thread
From: Ziyuan Xu @ 2016-07-28 11:51 UTC (permalink / raw)
  To: u-boot

Hi,

On 2016?07?28? 19:03, Chen-Yu Tsai wrote:
> Hi,
>
> On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
>> 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 <xzy.xu@rock-chips.com>
>> ---
>>
>>   arch/arm/include/asm/system.h | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
>> index 2bdc0be..12d4ba0 100644
>> --- a/arch/arm/include/asm/system.h
>> +++ b/arch/arm/include/asm/system.h
>> @@ -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
>>
> 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.:-)
>
> You'll see they also support ARMv6's CP15 ISB/DSB.
>
> Regards
> ChenYu
>
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-28 10:39 ` Alexander Graf
@ 2016-07-28 12:03   ` Ziyuan Xu
  2016-07-28 12:41     ` Sandy Patterson
  0 siblings, 1 reply; 16+ messages in thread
From: Ziyuan Xu @ 2016-07-28 12:03 UTC (permalink / raw)
  To: u-boot

Hi Alexander,

On 2016?07?28? 18:39, Alexander Graf wrote:
> On 07/28/2016 12:13 PM, Ziyuan Xu wrote:
>> 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 <xzy.xu@rock-chips.com>
>
> 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.

>
>
> Alex
>
>
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-28 12:03   ` Ziyuan Xu
@ 2016-07-28 12:41     ` Sandy Patterson
  0 siblings, 0 replies; 16+ messages in thread
From: Sandy Patterson @ 2016-07-28 12:41 UTC (permalink / raw)
  To: u-boot

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

On Thu, Jul 28, 2016 at 8:03 AM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:

> Hi Alexander,
>
> On 2016?07?28? 18:39, Alexander Graf wrote:
>
>> On 07/28/2016 12:13 PM, Ziyuan Xu wrote:
>>
>>> 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 <xzy.xu@rock-chips.com>
>>>
>>
>> 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.
>
>
>>
>> Alex
>>
>>
>>
>>
>>
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-28 11:03 ` Chen-Yu Tsai
  2016-07-28 11:51   ` Ziyuan Xu
@ 2016-07-28 22:15   ` Tom Rini
  2016-07-28 23:34     ` Ziyuan Xu
  1 sibling, 1 reply; 16+ messages in thread
From: Tom Rini @ 2016-07-28 22:15 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 28, 2016 at 07:03:17PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
> > 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 <xzy.xu@rock-chips.com>
> > ---
> >
> >  arch/arm/include/asm/system.h | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> > index 2bdc0be..12d4ba0 100644
> > --- a/arch/arm/include/asm/system.h
> > +++ b/arch/arm/include/asm/system.h
> > @@ -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
> >
> 
> 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!

-- 
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>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-28 10:13 [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A Ziyuan Xu
  2016-07-28 10:39 ` Alexander Graf
  2016-07-28 11:03 ` Chen-Yu Tsai
@ 2016-07-28 22:16 ` Tom Rini
  2 siblings, 0 replies; 16+ messages in thread
From: Tom Rini @ 2016-07-28 22:16 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 28, 2016 at 06:13:26PM +0800, Ziyuan Xu wrote:
`
> 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).

Good catch!

-- 
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>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-28 22:15   ` Tom Rini
@ 2016-07-28 23:34     ` Ziyuan Xu
  2016-07-29  0:34       ` Tom Rini
  0 siblings, 1 reply; 16+ messages in thread
From: Ziyuan Xu @ 2016-07-28 23:34 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On 2016?07?29? 06:15, Tom Rini wrote:
> On Thu, Jul 28, 2016 at 07:03:17PM +0800, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
>>> 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 <xzy.xu@rock-chips.com>
>>> ---
>>>
>>>   arch/arm/include/asm/system.h | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
>>> index 2bdc0be..12d4ba0 100644
>>> --- a/arch/arm/include/asm/system.h
>>> +++ b/arch/arm/include/asm/system.h
>>> @@ -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
>>>
>> 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!

If I understand correctly, I can change into as bellow:
#define isb() ISB
How about it?
>
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-28 23:34     ` Ziyuan Xu
@ 2016-07-29  0:34       ` Tom Rini
  2016-07-29  1:06         ` Ziyuan Xu
  0 siblings, 1 reply; 16+ messages in thread
From: Tom Rini @ 2016-07-29  0:34 UTC (permalink / raw)
  To: u-boot

On Fri, Jul 29, 2016 at 07:34:09AM +0800, Ziyuan Xu wrote:
> Hi Tom,
> 
> On 2016?07?29? 06:15, Tom Rini wrote:
> >On Thu, Jul 28, 2016 at 07:03:17PM +0800, Chen-Yu Tsai wrote:
> >>Hi,
> >>
> >>On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
> >>>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 <xzy.xu@rock-chips.com>
> >>>---
> >>>
> >>>  arch/arm/include/asm/system.h | 6 ++++--
> >>>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>>diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> >>>index 2bdc0be..12d4ba0 100644
> >>>--- a/arch/arm/include/asm/system.h
> >>>+++ b/arch/arm/include/asm/system.h
> >>>@@ -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
> >>>
> >>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!
> 
> 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.

-- 
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>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-29  0:34       ` Tom Rini
@ 2016-07-29  1:06         ` Ziyuan Xu
  2016-07-29  1:12           ` Tom Rini
  0 siblings, 1 reply; 16+ messages in thread
From: Ziyuan Xu @ 2016-07-29  1:06 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On 2016?07?29? 08:34, Tom Rini wrote:
> On Fri, Jul 29, 2016 at 07:34:09AM +0800, Ziyuan Xu wrote:
>> Hi Tom,
>>
>> On 2016?07?29? 06:15, Tom Rini wrote:
>>> On Thu, Jul 28, 2016 at 07:03:17PM +0800, Chen-Yu Tsai wrote:
>>>> Hi,
>>>>
>>>> On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
>>>>> 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 <xzy.xu@rock-chips.com>
>>>>> ---
>>>>>
>>>>>   arch/arm/include/asm/system.h | 6 ++++--
>>>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
>>>>> index 2bdc0be..12d4ba0 100644
>>>>> --- a/arch/arm/include/asm/system.h
>>>>> +++ b/arch/arm/include/asm/system.h
>>>>> @@ -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
>>>>>
>>>> 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!
>> 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?:-)

>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-29  1:06         ` Ziyuan Xu
@ 2016-07-29  1:12           ` Tom Rini
  2016-07-29  1:30             ` Ziyuan Xu
  2016-07-31  3:59             ` Ziyuan Xu
  0 siblings, 2 replies; 16+ messages in thread
From: Tom Rini @ 2016-07-29  1:12 UTC (permalink / raw)
  To: u-boot

On Fri, Jul 29, 2016 at 09:06:29AM +0800, Ziyuan Xu wrote:
> Hi Tom,
> 
> On 2016?07?29? 08:34, Tom Rini wrote:
> >On Fri, Jul 29, 2016 at 07:34:09AM +0800, Ziyuan Xu wrote:
> >>Hi Tom,
> >>
> >>On 2016?07?29? 06:15, Tom Rini wrote:
> >>>On Thu, Jul 28, 2016 at 07:03:17PM +0800, Chen-Yu Tsai wrote:
> >>>>Hi,
> >>>>
> >>>>On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
> >>>>>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 <xzy.xu@rock-chips.com>
> >>>>>---
> >>>>>
> >>>>>  arch/arm/include/asm/system.h | 6 ++++--
> >>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>>>>
> >>>>>diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> >>>>>index 2bdc0be..12d4ba0 100644
> >>>>>--- a/arch/arm/include/asm/system.h
> >>>>>+++ b/arch/arm/include/asm/system.h
> >>>>>@@ -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
> >>>>>
> >>>>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!
> >>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?:-)

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.

-- 
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>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-29  1:12           ` Tom Rini
@ 2016-07-29  1:30             ` Ziyuan Xu
  2016-07-31  3:59             ` Ziyuan Xu
  1 sibling, 0 replies; 16+ messages in thread
From: Ziyuan Xu @ 2016-07-29  1:30 UTC (permalink / raw)
  To: u-boot



On 2016?07?29? 09:12, Tom Rini wrote:
> On Fri, Jul 29, 2016 at 09:06:29AM +0800, Ziyuan Xu wrote:
>> Hi Tom,
>>
>> On 2016?07?29? 08:34, Tom Rini wrote:
>>> On Fri, Jul 29, 2016 at 07:34:09AM +0800, Ziyuan Xu wrote:
>>>> Hi Tom,
>>>>
>>>> On 2016?07?29? 06:15, Tom Rini wrote:
>>>>> On Thu, Jul 28, 2016 at 07:03:17PM +0800, Chen-Yu Tsai wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
>>>>>>> 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 <xzy.xu@rock-chips.com>
>>>>>>> ---
>>>>>>>
>>>>>>>   arch/arm/include/asm/system.h | 6 ++++--
>>>>>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
>>>>>>> index 2bdc0be..12d4ba0 100644
>>>>>>> --- a/arch/arm/include/asm/system.h
>>>>>>> +++ b/arch/arm/include/asm/system.h
>>>>>>> @@ -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
>>>>>>>
>>>>>> 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!
>>>> 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?:-)
> 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.:-)

>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-29  1:12           ` Tom Rini
  2016-07-29  1:30             ` Ziyuan Xu
@ 2016-07-31  3:59             ` Ziyuan Xu
  2016-07-31 14:27               ` Tom Rini
  1 sibling, 1 reply; 16+ messages in thread
From: Ziyuan Xu @ 2016-07-31  3:59 UTC (permalink / raw)
  To: u-boot

Hi Tom,


On 2016?07?29? 09:12, Tom Rini wrote:
> On Fri, Jul 29, 2016 at 09:06:29AM +0800, Ziyuan Xu wrote:
>> Hi Tom,
>>
>> On 2016?07?29? 08:34, Tom Rini wrote:
>>> On Fri, Jul 29, 2016 at 07:34:09AM +0800, Ziyuan Xu wrote:
>>>> Hi Tom,
>>>>
>>>> On 2016?07?29? 06:15, Tom Rini wrote:
>>>>> On Thu, Jul 28, 2016 at 07:03:17PM +0800, Chen-Yu Tsai wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
>>>>>>> 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 <xzy.xu@rock-chips.com>
>>>>>>> ---
>>>>>>>
>>>>>>>   arch/arm/include/asm/system.h | 6 ++++--
>>>>>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
>>>>>>> index 2bdc0be..12d4ba0 100644
>>>>>>> --- a/arch/arm/include/asm/system.h
>>>>>>> +++ b/arch/arm/include/asm/system.h
>>>>>>> @@ -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
>>>>>>>
>>>>>> 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!
>>>> 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?:-)
> 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?

>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-31  3:59             ` Ziyuan Xu
@ 2016-07-31 14:27               ` Tom Rini
  2016-08-01  1:07                 ` Ziyuan Xu
  0 siblings, 1 reply; 16+ messages in thread
From: Tom Rini @ 2016-07-31 14:27 UTC (permalink / raw)
  To: u-boot

On Sun, Jul 31, 2016 at 11:59:19AM +0800, Ziyuan Xu wrote:
> Hi Tom,
> 
> 
> On 2016?07?29? 09:12, Tom Rini wrote:
> >On Fri, Jul 29, 2016 at 09:06:29AM +0800, Ziyuan Xu wrote:
> >>Hi Tom,
> >>
> >>On 2016?07?29? 08:34, Tom Rini wrote:
> >>>On Fri, Jul 29, 2016 at 07:34:09AM +0800, Ziyuan Xu wrote:
> >>>>Hi Tom,
> >>>>
> >>>>On 2016?07?29? 06:15, Tom Rini wrote:
> >>>>>On Thu, Jul 28, 2016 at 07:03:17PM +0800, Chen-Yu Tsai wrote:
> >>>>>>Hi,
> >>>>>>
> >>>>>>On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
> >>>>>>>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 <xzy.xu@rock-chips.com>
> >>>>>>>---
> >>>>>>>
> >>>>>>>  arch/arm/include/asm/system.h | 6 ++++--
> >>>>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>>diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> >>>>>>>index 2bdc0be..12d4ba0 100644
> >>>>>>>--- a/arch/arm/include/asm/system.h
> >>>>>>>+++ b/arch/arm/include/asm/system.h
> >>>>>>>@@ -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
> >>>>>>>
> >>>>>>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!
> >>>>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?:-)
> >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!

-- 
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>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A
  2016-07-31 14:27               ` Tom Rini
@ 2016-08-01  1:07                 ` Ziyuan Xu
  0 siblings, 0 replies; 16+ messages in thread
From: Ziyuan Xu @ 2016-08-01  1:07 UTC (permalink / raw)
  To: u-boot



On 2016?07?31? 22:27, Tom Rini wrote:
> On Sun, Jul 31, 2016 at 11:59:19AM +0800, Ziyuan Xu wrote:
>> Hi Tom,
>>
>>
>> On 2016?07?29? 09:12, Tom Rini wrote:
>>> On Fri, Jul 29, 2016 at 09:06:29AM +0800, Ziyuan Xu wrote:
>>>> Hi Tom,
>>>>
>>>> On 2016?07?29? 08:34, Tom Rini wrote:
>>>>> On Fri, Jul 29, 2016 at 07:34:09AM +0800, Ziyuan Xu wrote:
>>>>>> Hi Tom,
>>>>>>
>>>>>> On 2016?07?29? 06:15, Tom Rini wrote:
>>>>>>> On Thu, Jul 28, 2016 at 07:03:17PM +0800, Chen-Yu Tsai wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On Thu, Jul 28, 2016 at 6:13 PM, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
>>>>>>>>> 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 <xzy.xu@rock-chips.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>>   arch/arm/include/asm/system.h | 6 ++++--
>>>>>>>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
>>>>>>>>> index 2bdc0be..12d4ba0 100644
>>>>>>>>> --- a/arch/arm/include/asm/system.h
>>>>>>>>> +++ b/arch/arm/include/asm/system.h
>>>>>>>>> @@ -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
>>>>>>>>>
>>>>>>>> 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!
>>>>>> 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?:-)
>>> 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!
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
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-08-01  1:07 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-28 10:13 [U-Boot] [PATCH] arm: re-implement proper ISB instruction for ARMv7-A Ziyuan Xu
2016-07-28 10:39 ` Alexander Graf
2016-07-28 12:03   ` Ziyuan Xu
2016-07-28 12:41     ` Sandy Patterson
2016-07-28 11:03 ` Chen-Yu Tsai
2016-07-28 11:51   ` Ziyuan Xu
2016-07-28 22:15   ` Tom Rini
2016-07-28 23:34     ` Ziyuan Xu
2016-07-29  0:34       ` Tom Rini
2016-07-29  1:06         ` Ziyuan Xu
2016-07-29  1:12           ` Tom Rini
2016-07-29  1:30             ` Ziyuan Xu
2016-07-31  3:59             ` Ziyuan Xu
2016-07-31 14:27               ` Tom Rini
2016-08-01  1:07                 ` Ziyuan Xu
2016-07-28 22:16 ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.