Problems booting exynos5420 with >1 CPU

[Doug Anderson]

Abhilash,



On Fri, Jun 6, 2014 at 11:12 AM, Abhilash Kesavan
<kesavan.abhilash at gmail.com> wrote:
> Hi Doug,
>
> On Fri, Jun 6, 2014 at 11:32 PM, Doug Anderson <dianders at google.com> wrote:
>> Abhilash,
>>
>> On Fri, Jun 6, 2014 at 10:36 AM, Abhilash Kesavan
>> <kesavan.abhilash at gmail.com> wrote:
>>> Hi Doug,
>>>
>>> The first change in the kernel (clearing an iRAM location) is needed
>>> because of an unnecessary change that we are carrying in the Chrome
>>> U-boot. There is no reason for us to have the workaround in the
>>> mainline kernel. Rather, we should remove the check from our u-boot.
>>> However AFAIR a clean-up patch that I had posted internally was not
>>> accepted as we had frozen the SPL at the time.
>>
>> Ah, is that this one, or a different one?
>>
>> https://chromium-review.googlesource.com/#/c/66049/
> Yes, this along with a kernel side change.

Can we safely take this one without the kernel-side one?


>> If we land that patch now it won't help since nobody is going to be
>> updating their read-only firmware.  We'll need to put code somewhere
>> that fixes it.
> We just carry the workaround fix locally until we migrate to mainline
> u-boot for 5420 where the unnecessay check will not be present.

I think there are people out there who want to run a mainline kernel
on existing Chromebook 2 hardware and don't want to rewrite their RO
firmware.  We need a solution for those people.


>>> The second change is to enable snoops for boot cluster. Internally, we
>>> were disabling the snoops for both the clusters at power off and
>>> enabling it in power_up_setup and power_up. However, I dropped the
>>> approach due to problems pointed out by Nicolas here
>>> http://www.spinics.net/lists/arm-kernel/msg324091.html related to
>>> cpuidle. Hence, we turn it on at the u-boot.
>>
>> I think I followed all that.  What you're saying is that our kernel
>> dynamically enables and disables snoops as needed, but Nicolas pointed
>> out that it was unsafe (though apparently we're not seeing problems in
>> our usage).
> We did not see any problems as CPUIdle was one of the problematic
> scenarios which we have not got enabled.

Ah, makes sense!



I'm still trying to figure out all of this code, but we'll also need
to make sure whatever solution we come up with handles suspend/resume
properly.  I know SRAM is lost across suspend/resume so someone
(either the SPL from read-only memory or the kernel) must be
recreating the SRAM structures after S2R...