Hi Ley Foon, > -----Original Message----- > From: Tan, Ley Foon <ley.foon....@intel.com> > Sent: Friday, 9 April, 2021 2:08 PM > To: Lim, Elly Siew Chin <elly.siew.chin....@intel.com>; u-boot@lists.denx.de > Cc: Marek Vasut <ma...@denx.de>; See, Chin Liang > <chin.liang....@intel.com>; Simon Goldschmidt > <simon.k.r.goldschm...@gmail.com>; Chee, Tien Fong > <tien.fong.c...@intel.com>; Westergreen, Dalon > <dalon.westergr...@intel.com>; Simon Glass <s...@chromium.org>; Gan, > Yau Wai <yau.wai....@intel.com> > Subject: RE: [v1 10/17] arm: socfpga: Add SDRAM driver helper function for > Intel N5X device > > > > > -----Original Message----- > > From: Lim, Elly Siew Chin <elly.siew.chin....@intel.com> > > Sent: Wednesday, March 31, 2021 10:39 PM > > To: u-boot@lists.denx.de > > Cc: Marek Vasut <ma...@denx.de>; Tan, Ley Foon > > <ley.foon....@intel.com>; See, Chin Liang <chin.liang....@intel.com>; > > Simon Goldschmidt <simon.k.r.goldschm...@gmail.com>; Chee, Tien Fong > > <tien.fong.c...@intel.com>; Westergreen, Dalon > > <dalon.westergr...@intel.com>; Simon Glass <s...@chromium.org>; Gan, > > Yau Wai <yau.wai....@intel.com>; Lim, Elly Siew Chin > > <elly.siew.chin....@intel.com> > > Subject: [v1 10/17] arm: socfpga: Add SDRAM driver helper function for > > Intel N5X device > > > > Add is_ddr_init_skipped function to check if need to skip DDR > > initialization for N5X. This patch is preparation for N5X DDR driver > > support. > > > > Signed-off-by: Siew Chin Lim <elly.siew.chin....@intel.com> > > Signed-off-by: Tien Fong Chee <tien.fong.c...@intel.com> > > --- > > arch/arm/mach-socfpga/include/mach/misc.h | 4 ++ > > arch/arm/mach-socfpga/misc_soc64.c | 67 > > ++++++++++++++++++++++++++++++- > > 2 files changed, 70 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/mach-socfpga/include/mach/misc.h > > b/arch/arm/mach- socfpga/include/mach/misc.h index > > 649d2f6ce2..c41b7c14cd 100644 > > --- a/arch/arm/mach-socfpga/include/mach/misc.h > > +++ b/arch/arm/mach-socfpga/include/mach/misc.h > > @@ -44,6 +44,10 @@ void socfpga_sdram_remap_zero(void); int > > is_fpga_config_ready(void); #endif > > > > +#if IS_ENABLED(CONFIG_TARGET_SOCFPGA_N5X) > > +bool is_ddr_init_skipped(void); > > +#endif > > + > > void do_bridge_reset(int enable, unsigned int mask); void > > socfpga_pl310_clear(void); void socfpga_get_managers_addr(void); diff > > --git a/arch/arm/mach-socfpga/misc_soc64.c b/arch/arm/mach- > > socfpga/misc_soc64.c index 7b973a79e8..d3945e55aa 100644 > > --- a/arch/arm/mach-socfpga/misc_soc64.c > > +++ b/arch/arm/mach-socfpga/misc_soc64.c > > @@ -1,6 +1,6 @@ > > // SPDX-License-Identifier: GPL-2.0 > > /* > > - * Copyright (C) 2016-2018 Intel Corporation <www.intel.com> > > + * Copyright (C) 2016-2021 Intel Corporation <www.intel.com> > > * > > */ > > > > @@ -19,6 +19,13 @@ > > > > DECLARE_GLOBAL_DATA_PTR; > > > > +/* Reset type */ > > +enum reset_type { > > + POR_RESET, > > + WARM_RESET, > > + COLD_RESET > > +}; > > + > > /* > > * FPGA programming support for SoC FPGA Stratix 10 > > */ > > @@ -88,3 +95,61 @@ void do_bridge_reset(int enable, unsigned int mask) > > > > socfpga_bridges_reset(enable); > > } > > + > > +#if IS_ENABLED(CONFIG_TARGET_SOCFPGA_N5X) > > +static bool is_ddr_retention_enabled(u32 boot_scratch_cold0_reg) { > > + return boot_scratch_cold0_reg & > > + ALT_SYSMGR_SCRATCH_REG_0_DDR_RETENTION_MASK; > > +} > > + > > +static bool is_ddr_bitstream_sha_matching(u32 boot_scratch_cold0_reg) > { > > + return boot_scratch_cold0_reg & > > ALT_SYSMGR_SCRATCH_REG_0_DDR_SHA_MASK; > > +} > > + > > +static enum reset_type get_reset_type(u32 boot_scratch_cold0_reg) { > > + return (boot_scratch_cold0_reg & > > + ALT_SYSMGR_SCRATCH_REG_0_DDR_RESET_TYPE_MASK) >> > > + ALT_SYSMGR_SCRATCH_REG_0_DDR_RESET_TYPE_SHIFT; > > +} > > + > > +bool is_ddr_init_skipped(void) > > 1. This function can move to DDR driver. > 2. Change this function return true if need the DDR init, so that doesn't need > to invert checking when call to this function, more readable.
Okay. > > > +{ > > + u32 reg = readl(socfpga_get_sysmgr_addr() + > > + SYSMGR_SOC64_BOOT_SCRATCH_COLD0); > > + > > + if (get_reset_type(reg) == POR_RESET) { > Store reset type in a variable, don't call get_reset_type() multiple times. > > > + debug("%s: POR reset is triggered\n", __func__); > > + debug("%s: DDR init is required\n", __func__); > > + return false; > > + } > > + > > + if (get_reset_type(reg) == WARM_RESET) { > > + debug("%s: Warm reset is triggered\n", __func__); > > + debug("%s: DDR init is skipped\n", __func__); > > + return true; > > + } > > + > > + if (get_reset_type(reg) == COLD_RESET) { > > + debug("%s: Cold reset is triggered\n", __func__); > > + > > + if (is_ddr_retention_enabled(reg)) { > > + debug("%s: DDR retention bit is set\n", __func__); > > + > > + if (is_ddr_bitstream_sha_matching(reg)) { > Combine this IF with parent IF with &&. Okay. > > > + debug("%s: Matching in DDR bistream\n", > > + __func__); > > + debug("%s: DDR init is skipped\n", __func__); > > + return true; > > + } > > + > > + debug("%s: Mismatch in DDR bistream\n", __func__); > > + } > > + } > > + > > + debug("%s: DDR init is required\n", __func__); > > + return false; > > +} > > +#endif > > -- > > 2.13.0 Regards, Tien Fong