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

[Kever Yang]

Hi Vasily,

On 2019/11/28 下午11:51, Vasily Khoruzhick wrote:
> On Thu, Nov 28, 2019 at 1:23 AM Kever Yang <kever.yang at 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.
>
>> 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?

- For reboot, after this patch, the syr83x VSEL is high, what is its 
output voltage?

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


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