From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vasily Khoruzhick Date: Thu, 28 Nov 2019 17:06:22 -0800 Subject: [U-Boot] [PATCH 2/2] rockchip: rk3399: rockpro64: enable force power on reset workaround In-Reply-To: References: <20191128061433.1952869-1-anarsoul@gmail.com> <20191128061433.1952869-2-anarsoul@gmail.com> <1ab40490-a232-b5fd-6180-17867aa6a053@rock-chips.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Thu, Nov 28, 2019 at 4:59 PM Kever Yang wrote: > > Hi Vasily, > > On 2019/11/28 下午11:51, Vasily Khoruzhick wrote: > > On Thu, Nov 28, 2019 at 1:23 AM Kever Yang 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 > >>> --- > >>> 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 > >> > >