[U-Boot] [PATCH 2/2] rockchip: rk3399: rockpro64: enable force power on reset workaround

Kever Yang
2019-12-02T03:03:46Z

Hi Huge Cole-Baker,

On 2019/12/1 下午10:12, Hugh Cole-Baker wrote:
> Hi Vasily,
>
>> On 29 Nov 2019, at 01:06, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
>>
>> On Thu, Nov 28, 2019 at 4:59 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>>> Hi Vasily,
>>>
>>> On 2019/11/28 下午11:51, Vasily Khoruzhick wrote:
>>>> On Thu, Nov 28, 2019 at 1:23 AM Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>> Hi Vasily,
>>>>>
>>>>> I think this should not be needed, see comments below.
>>>> Hi Kever,
>>>>
>>>> I've spent 2 weeks of my evenings debugging this issue but
>>> I can understand you work pretty hard on make it work, it's not so easy
>>> to identify the root cause
>>>
>>> some times, thanks very much for working on this.
>>>
>>>> unfortunately I don't have a proper fix. This is the only solution
>>>> that makes my rockpro64 reboot reliably with mainline u-boot and ATF.
>>>> See my comments below.
> I also had a problem where Linux would hang or panic after rebooting, with
> mainline u-boot and ATF on a rockpro64. This patch does fix the issue for me,
> I have tested it by performing 10 reboots from Linux in a row and I've seen
> no hangs or panics.
>
> I noticed the Armbian project have recently included a patch to ATF [1] which
> switches all power domains on before ATF performs a soft reset. I have also
> tested using u-boot mainline, without any patches to u-boot, but including ATF
> patched with your reset fix [2] and the Armbian power domains patch [1]. This
> also fixes the same hanging on reboot issue for me without modifications to
> u-boot, I've also tested 10 reboots in a row with this ATF and seen no hangs.
>
> So this u-boot patch may not be needed if ATF is patched instead to switch
> power domains on before soft reset.
>
> FWIW, when I was able to see panic messages from Linux when it panicked on
> boot, the call trace always seemed to include rockchip_pd_power_off() [3].

Thanks very much for your detail info about this issue,  I have sync
with my colleague,
rockchip bl31.elf in rkbin do open all the power domain before reboot,
so I believe this would be a
better solution in ATF for it can resolve issue on all the rk3399 boards
instead of add one by one in U-Boot.
Does any one of you would like to upstream the patch in ATF?

Thanks,

- Kever

>
> [1] https://github.com/armbian/build/blob/master/patch/atf/atf-rk3399/switch-power-domains-on-before-reset.patch
> [2] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2512
> [3] https://gist.github.com/sigmaris/c0e155c8cb0a325d84f549185f9a568c
>
>>>>> Hi Philipp, Klaus and Christoph:
>>>>>
>>>>> Could you help to check why do you need below patch for your board?
>>>>>
>>>>> ae0d33a729 rockchip: rk3399-puma: add code to allow forcing a power-on reset
>>>>>
>>>>>
>>>>> I think we don't need this workaround for rk3399 CPU_B voltage
>>>>> supply, and
>>>>> here is what I got:
>>>>> - rockchip use cru glb_rst_1 for reboot in kernel;
>>>>> - the glb_rst_1 will reset all the GPIO logic to default state;
>>>>> - the cpu_b voltage supplier syr83x have a VSEL connect to rk3399, which
>>>>> may be
>>>>> a pull up/down IO,
>>>>> - the syr83x output with the hardware default state of the VSEL(with
>>>>> RK3399 default IO output)
>>>>> should be normal output(1.0V), and another state output for
>>>>> suspend(disabled),
>>>>> - In order to make the syr83x works as expected, the kernel setting of
>>>>> syr83x should be correct,
>>>>> check property:
>>>>> fcs,suspend-voltage-selector = <1>;
>>>>> This is correct for rockpro64(vsel connect to a gpio with default
>>>>> pull down) on upstream,
>>>>> but I don't have a puma schematic, so I don't know if this is
>>>>> correct for puma.
>>>>> - With correct setting in syr83x, the cpu_b should always have power
>>>>> supply after reboot/reset with
>>>>> cru glb_rst_1 reg.
>>>>> So no workaround is needed in U-Boot, please correct me if anything is
>>>>> missing.
>>>> I already tried re-initializing SYR83x, see [1] (and thus fixed couple
>>>> of the bugs in FAN53555 driver which has been broken since it was
>>>> merged into u-boot) but it doesn't help with reboot issue on
>>>> RockPro64. I checked VSEL gpio status and it's identical on cold boot
>>>> and on soft reboot, so I doubt SYR83X settings are related since I
>>>> checked regulators settings and they're correct.
>>> Could you help to provide some more info?
>>>
>>> - For code boot, syr83x VSEL is low, and what is its output voltage?
>>>
>>> - For reboot, syr83x VSEL is low, what is its output voltage?
>> I'll double check later tonight.
>>
>>> - For reboot, after this patch, the syr83x VSEL is high, what is its
>>> output voltage?
>> This patch doesn't touch VSEL. It sets OTP_OUT to 1, see [1]
>> "Over-Temperature Protection" and thus forces board reset.
>>
>> OTP_OUT is GPIO1 A6.
>>
>> VSEL (or rather CPU_B_SLEEP_H) is different pin, it's wired to GPIO1 C1
>>
>> http://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf
>>
>>> I think the setting of syr83x is wrong if the output voltage is not the
>>> same for code boot and reboot.
>>>
>>>> I tried to boot with CPUFREQ disabled - that didn't help, linux hangs
>>>> when booted after soft reset.
>>>>
>>>> Also tried to boot with big cluster disabled - that didn't help either.
>>> This is strange, this patch only affects the big cluster CPU, if big
>>> cluster is disabled, then there is
>>>
>>> no reason this patch can work. Have you double check the big cluster is
>>> truly _DISABLED_?
>> See explanation above.
>>
>>> Thanks,
>>>
>>> - Kever
>>>
>>>> So could you merge this patch please unless someone else wants to work
>>>> on this issue?
>>>>
>>>> Thanks,
>>>> Vasily.
>>>>
>>>> [1] https://github.com/anarsoul/u-boot-pine64/commit/7a50e58f09c68efe08f0b9912805fb9b3c985751
>>>>
>>>>> Thanks,
>>>>> - Kever
>>>>> On 2019/11/28 下午2:14, Vasily Khoruzhick wrote:
>>>>>> Rockpro64 doesn't boot reliably after soft reset, so let's force power on
>>>>>> reset by asserting sysreset pin if we detected soft reset.
>>>>>>
>>>>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>>>>> ---
>>>>>> arch/arm/dts/rk3399-rockpro64-u-boot.dtsi | 8 ++++++++
>>>>>> configs/rockpro64-rk3399_defconfig | 1 +
>>>>>> 2 files changed, 9 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
>>>>>> index 4648513ea9..bb94bcf7be 100644
>>>>>> --- a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
>>>>>> +++ b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
>>>>>> @@ -6,11 +6,19 @@
>>>>>> #include "rk3399-u-boot.dtsi"
>>>>>> #include "rk3399-sdram-lpddr4-100.dtsi"
>>>>>> / {
>>>>>> + config {
>>>>>> + sysreset-gpio = <&gpio1 RK_PA6 GPIO_ACTIVE_HIGH>;
>>>>>> + };
>>>>>> +
>>>>>> chosen {
>>>>>> u-boot,spl-boot-order = "same-as-spl", &sdmmc, &sdhci;
>>>>>> };
>>>>>> };
>>>>>>
>>>>>> +&gpio1 {
>>>>>> + u-boot,dm-pre-reloc;
>>>>>> +};
>>>>>> +
>>>>>> &vdd_center {
>>>>>> regulator-min-microvolt = <950000>;
>>>>>> regulator-max-microvolt = <950000>;
>>>>>> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
>>>>>> index 49e27c91cb..d153ac5485 100644
>>>>>> --- a/configs/rockpro64-rk3399_defconfig
>>>>>> +++ b/configs/rockpro64-rk3399_defconfig
>>>>>> @@ -1,6 +1,7 @@
>>>>>> CONFIG_ARM=y
>>>>>> CONFIG_ARCH_ROCKCHIP=y
>>>>>> CONFIG_SYS_TEXT_BASE=0x00200000
>>>>>> +CONFIG_SPL_GPIO_SUPPORT=y
>>>>>> CONFIG_ROCKCHIP_RK3399=y
>>>>>> CONFIG_ENV_OFFSET=0x3F8000
>>>>>> CONFIG_TARGET_ROCKPRO64_RK3399=y
>>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot