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

[Vasily Khoruzhick]

On Thu, Nov 28, 2019 at 4:59 PM Kever Yang <kever.yang at 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 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?

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