[v1 10/17] arm: socfpga: Add SDRAM driver helper function for Intel N5X device

[Chee, Tien Fong]

Hi Ley Foon,

> -----Original Message-----
> From: Tan, Ley Foon <ley.foon.tan at intel.com>
> Sent: Friday, 9 April, 2021 2:08 PM
> To: Lim, Elly Siew Chin <elly.siew.chin.lim at intel.com>; u-boot at lists.denx.de
> Cc: Marek Vasut <marex at denx.de>; See, Chin Liang
> <chin.liang.see at intel.com>; Simon Goldschmidt
> <simon.k.r.goldschmidt at gmail.com>; Chee, Tien Fong
> <tien.fong.chee at intel.com>; Westergreen, Dalon
> <dalon.westergreen at intel.com>; Simon Glass <sjg at chromium.org>; Gan,
> Yau Wai <yau.wai.gan at 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.lim at intel.com>
> > Sent: Wednesday, March 31, 2021 10:39 PM
> > To: u-boot at lists.denx.de
> > Cc: Marek Vasut <marex at denx.de>; Tan, Ley Foon
> > <ley.foon.tan at intel.com>; See, Chin Liang <chin.liang.see at intel.com>;
> > Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>; Chee, Tien Fong
> > <tien.fong.chee at intel.com>; Westergreen, Dalon
> > <dalon.westergreen at intel.com>; Simon Glass <sjg at chromium.org>; Gan,
> > Yau Wai <yau.wai.gan at intel.com>; Lim, Elly Siew Chin
> > <elly.siew.chin.lim at 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.lim at intel.com>
> > Signed-off-by: Tien Fong Chee <tien.fong.chee at 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