[U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

[U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

[Marek Vasut]

Make sure the ARM ACTLR register has correct configuration, otherwise
the Linux kernel refuses to boot. In particular, the "Write Full Line
of Zeroes" bit must be cleared.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Chin Liang See <chin.liang.see@intel.com>
Cc: Dinh Nguyen <dinguyen@kernel.org>
---
NOTE: This gem was well hidden in the Altera U-Boot fork and is really needed.
      What is not entirely clear to me is WHY ? So why is this needed ?

 arch/arm/mach-socfpga/board.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach-socfpga/board.c
index 189e12a668..ffdabc04fc 100644
--- a/arch/arm/mach-socfpga/board.c
+++ b/arch/arm/mach-socfpga/board.c
@@ -18,7 +18,18 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-void s_init(void) {}
+void s_init(void) {
+	/*
+	 * Preconfigure ACTLR, make sure Write Full Line of Zeroes is disabled.
+	 * This is optional on CycloneV / ArriaV.
+	 * This is mandatory on Arria10, otherwise Linux refuses to boot.
+	 */
+	asm volatile(
+		"mcr p15, 0, %0, c1, c0, 1\n"
+		"isb\n"
+		"dsb\n"
+	::"r"(0x44));
+}
 
 /*
  * Miscellaneous platform dependent initialisations
-- 
2.17.0

[U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

[See, Chin Liang]

On Tue, 2018-05-29 at 18:39 +0200, Marek Vasut wrote:
> Make sure the ARM ACTLR register has correct configuration, otherwise
> the Linux kernel refuses to boot. In particular, the "Write Full Line
> of Zeroes" bit must be cleared.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <chin.liang.see@intel.com>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> ---
> NOTE: This gem was well hidden in the Altera U-Boot fork and is
> really needed.
>       What is not entirely clear to me is WHY ? So why is this needed
> ?

I vaguely recall it's related to HW constrain. And check back the
downstream U-Boot, actually we need to set the bit instead clearing
it. https://github.com/altera-opensource/u-boot-socfpga/blob/socfpga_v2
014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/pl310.c

Thanks
Chin Liang

[...]

[U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

[Marek Vasut]

On 05/31/2018 11:52 AM, See, Chin Liang wrote:
> On Tue, 2018-05-29 at 18:39 +0200, Marek Vasut wrote:
>> Make sure the ARM ACTLR register has correct configuration, otherwise
>> the Linux kernel refuses to boot. In particular, the "Write Full Line
>> of Zeroes" bit must be cleared.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Chin Liang See <chin.liang.see@intel.com>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> ---
>> NOTE: This gem was well hidden in the Altera U-Boot fork and is
>> really needed.
>>       What is not entirely clear to me is WHY ? So why is this needed
>> ?
> 
> I vaguely recall it's related to HW constrain. And check back the
> downstream U-Boot, actually we need to set the bit instead clearing
> it. https://github.com/altera-opensource/u-boot-socfpga/blob/socfpga_v2
> 014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/pl310.c

You are clearing it here:
https://github.com/altera-opensource/u-boot-socfpga/blob/socfpga_v2014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/lowlevel_init.S#L35

The PL310 configuration is a different register. What HW constraint ?
I'd really like to understand this problem.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

[Marek Vasut]

On 05/31/2018 12:00 PM, Marek Vasut wrote:
> On 05/31/2018 11:52 AM, See, Chin Liang wrote:
>> On Tue, 2018-05-29 at 18:39 +0200, Marek Vasut wrote:
>>> Make sure the ARM ACTLR register has correct configuration, otherwise
>>> the Linux kernel refuses to boot. In particular, the "Write Full Line
>>> of Zeroes" bit must be cleared.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>> ---
>>> NOTE: This gem was well hidden in the Altera U-Boot fork and is
>>> really needed.
>>>       What is not entirely clear to me is WHY ? So why is this needed
>>> ?
>>
>> I vaguely recall it's related to HW constrain. And check back the
>> downstream U-Boot, actually we need to set the bit instead clearing
>> it. https://github.com/altera-opensource/u-boot-socfpga/blob/socfpga_v2
>> 014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/pl310.c
> 
> You are clearing it here:
> https://github.com/altera-opensource/u-boot-socfpga/blob/socfpga_v2014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/lowlevel_init.S#L35
> 
> The PL310 configuration is a different register. What HW constraint ?
> I'd really like to understand this problem.

Bump ? I had a report the register even has to be set to 0 , otherwise
there are ethernet problems. What is going on with this register ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

[Marek Vasut]

On 06/06/2018 08:47 PM, Marek Vasut wrote:
> On 05/31/2018 12:00 PM, Marek Vasut wrote:
>> On 05/31/2018 11:52 AM, See, Chin Liang wrote:
>>> On Tue, 2018-05-29 at 18:39 +0200, Marek Vasut wrote:
>>>> Make sure the ARM ACTLR register has correct configuration, otherwise
>>>> the Linux kernel refuses to boot. In particular, the "Write Full Line
>>>> of Zeroes" bit must be cleared.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>> ---
>>>> NOTE: This gem was well hidden in the Altera U-Boot fork and is
>>>> really needed.
>>>>       What is not entirely clear to me is WHY ? So why is this needed
>>>> ?
>>>
>>> I vaguely recall it's related to HW constrain. And check back the
>>> downstream U-Boot, actually we need to set the bit instead clearing
>>> it. https://github.com/altera-opensource/u-boot-socfpga/blob/socfpga_v2
>>> 014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/pl310.c
>>
>> You are clearing it here:
>> https://github.com/altera-opensource/u-boot-socfpga/blob/socfpga_v2014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/lowlevel_init.S#L35
>>
>> The PL310 configuration is a different register. What HW constraint ?
>> I'd really like to understand this problem.
> 
> Bump ? I had a report the register even has to be set to 0 , otherwise
> there are ethernet problems. What is going on with this register ?

Bump again ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

[Marek Vasut]

On 06/23/2018 05:55 AM, Marek Vasut wrote:
> On 06/06/2018 08:47 PM, Marek Vasut wrote:
>> On 05/31/2018 12:00 PM, Marek Vasut wrote:
>>> On 05/31/2018 11:52 AM, See, Chin Liang wrote:
>>>> On Tue, 2018-05-29 at 18:39 +0200, Marek Vasut wrote:
>>>>> Make sure the ARM ACTLR register has correct configuration, otherwise
>>>>> the Linux kernel refuses to boot. In particular, the "Write Full Line
>>>>> of Zeroes" bit must be cleared.
>>>>>
>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>>> ---
>>>>> NOTE: This gem was well hidden in the Altera U-Boot fork and is
>>>>> really needed.
>>>>>       What is not entirely clear to me is WHY ? So why is this needed
>>>>> ?
>>>>
>>>> I vaguely recall it's related to HW constrain. And check back the
>>>> downstream U-Boot, actually we need to set the bit instead clearing
>>>> it. https://github.com/altera-opensource/u-boot-socfpga/blob/socfpga_v2
>>>> 014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/pl310.c
>>>
>>> You are clearing it here:
>>> https://github.com/altera-opensource/u-boot-socfpga/blob/socfpga_v2014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/lowlevel_init.S#L35
>>>
>>> The PL310 configuration is a different register. What HW constraint ?
>>> I'd really like to understand this problem.
>>
>> Bump ? I had a report the register even has to be set to 0 , otherwise
>> there are ethernet problems. What is going on with this register ?
> 
> Bump again ?
> 

Expanding the CC list. I am tempted to stop accepting any socfpga
patches until this is resolved.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

[See, Chin Liang]

On Sat, 2018-06-23 at 06:23 +0200, Marek Vasut wrote:
> On 06/23/2018 05:55 AM, Marek Vasut wrote:
> > 
> > On 06/06/2018 08:47 PM, Marek Vasut wrote:
> > > 
> > > On 05/31/2018 12:00 PM, Marek Vasut wrote:
> > > > 
> > > > On 05/31/2018 11:52 AM, See, Chin Liang wrote:
> > > > > 
> > > > > On Tue, 2018-05-29 at 18:39 +0200, Marek Vasut wrote:
> > > > > > 
> > > > > > Make sure the ARM ACTLR register has correct configuration,
> > > > > > otherwise
> > > > > > the Linux kernel refuses to boot. In particular, the "Write
> > > > > > Full Line
> > > > > > of Zeroes" bit must be cleared.
> > > > > > 
> > > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > > Cc: Chin Liang See <chin.liang.see@intel.com>
> > > > > > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > > > > > ---
> > > > > > NOTE: This gem was well hidden in the Altera U-Boot fork
> > > > > > and is
> > > > > > really needed.
> > > > > >       What is not entirely clear to me is WHY ? So why is
> > > > > > this needed
> > > > > > ?
> > > > > I vaguely recall it's related to HW constrain. And check back
> > > > > the
> > > > > downstream U-Boot, actually we need to set the bit instead
> > > > > clearing
> > > > > it. https://github.com/altera-opensource/u-boot-socfpga/blob/
> > > > > socfpga_v2
> > > > > 014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/pl3
> > > > > 10.c
> > > > You are clearing it here:
> > > > https://github.com/altera-opensource/u-boot-socfpga/blob/socfpg
> > > > a_v2014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/l
> > > > owlevel_init.S#L35
> > > > 
> > > > The PL310 configuration is a different register. What HW
> > > > constraint ?
> > > > I'd really like to understand this problem.
> > > Bump ? I had a report the register even has to be set to 0 ,
> > > otherwise
> > > there are ethernet problems. What is going on with this register
> > > ?
> > Bump again ?
> > 
> Expanding the CC list. I am tempted to stop accepting any socfpga
> patches until this is resolved.
> 

Just realize this is not addressed as Ley Foon talked to me

We checked with HW team that the full line of zeroes is a special setup
of signals between A9 and L2 cache controller. As they recalled that
this signal is not 100% AXI compliance, it may hang the system if L2
cache is disabled while full line of zeroes is set.

In A10, one thing new is we enabled L2 cache in BootROM and this bit is
set. When entering the U-Boot, we are ensuring all cache are disabled.
At this point, we need to ensure full line of zeroes bit is cleared.
Only when U-Boot renable the cache including L2, this bit is set again.

Hope this explains.

Thanks
Chin Liang

[U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

[Marek Vasut]

On 07/03/2018 05:10 PM, See, Chin Liang wrote:
> On Sat, 2018-06-23 at 06:23 +0200, Marek Vasut wrote:
>> On 06/23/2018 05:55 AM, Marek Vasut wrote:
>>>
>>> On 06/06/2018 08:47 PM, Marek Vasut wrote:
>>>>
>>>> On 05/31/2018 12:00 PM, Marek Vasut wrote:
>>>>>
>>>>> On 05/31/2018 11:52 AM, See, Chin Liang wrote:
>>>>>>
>>>>>> On Tue, 2018-05-29 at 18:39 +0200, Marek Vasut wrote:
>>>>>>>
>>>>>>> Make sure the ARM ACTLR register has correct configuration,
>>>>>>> otherwise
>>>>>>> the Linux kernel refuses to boot. In particular, the "Write
>>>>>>> Full Line
>>>>>>> of Zeroes" bit must be cleared.
>>>>>>>
>>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>>>>> ---
>>>>>>> NOTE: This gem was well hidden in the Altera U-Boot fork
>>>>>>> and is
>>>>>>> really needed.
>>>>>>>       What is not entirely clear to me is WHY ? So why is
>>>>>>> this needed
>>>>>>> ?
>>>>>> I vaguely recall it's related to HW constrain. And check back
>>>>>> the
>>>>>> downstream U-Boot, actually we need to set the bit instead
>>>>>> clearing
>>>>>> it. https://github.com/altera-opensource/u-boot-socfpga/blob/
>>>>>> socfpga_v2
>>>>>> 014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/pl3
>>>>>> 10.c
>>>>> You are clearing it here:
>>>>> https://github.com/altera-opensource/u-boot-socfpga/blob/socfpg
>>>>> a_v2014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/l
>>>>> owlevel_init.S#L35
>>>>>
>>>>> The PL310 configuration is a different register. What HW
>>>>> constraint ?
>>>>> I'd really like to understand this problem.
>>>> Bump ? I had a report the register even has to be set to 0 ,
>>>> otherwise
>>>> there are ethernet problems. What is going on with this register
>>>> ?
>>> Bump again ?
>>>
>> Expanding the CC list. I am tempted to stop accepting any socfpga
>> patches until this is resolved.
>>
> 
> Just realize this is not addressed as Ley Foon talked to me

I am real grateful for that.

> We checked with HW team that the full line of zeroes is a special setup
> of signals between A9 and L2 cache controller. As they recalled that
> this signal is not 100% AXI compliance, it may hang the system if L2
> cache is disabled while full line of zeroes is set.
> 
> In A10, one thing new is we enabled L2 cache in BootROM and this bit is
> set. When entering the U-Boot, we are ensuring all cache are disabled.
> At this point, we need to ensure full line of zeroes bit is cleared.
> Only when U-Boot renable the cache including L2, this bit is set again.
> 
> Hope this explains.

I'll have to give it some thought, but I think so.

Is there an errata or some piece of documentation I can refer to about
this behavior ? I'd like to add link to the commit message.

Thanks, really appreciated!

-- 
Best regards,
Marek Vasut