[U-Boot] [PATCH 00/10] Update Stratix 10 SDRAM driver

[U-Boot] [PATCH 00/10] Update Stratix 10 SDRAM driver

[Ley Foon Tan]

This patchset update Stratix 10 SDRAM driver to support:
- Multi-banks memory (Patch [1-8])
  - Stratix 10 support up to 2 memory banks:
	Bank 0: Address 0, size 2GB
	Bank 1: Address 0x100000000, size 124GB
- Add warm reset boot checking function, to use in patch [10] (Patch[9])
- Add ECC memory scrubbing support (Patch [10])
  - Use cache enabled + "DC ZVA" instruction to clear memory to zeros

Ley Foon Tan (10):
  ddr: altera: stratix10: Move SDRAM size check to SDRAM driver
  arm: socfpga: Add sdram_s10.h to sdram.h
  ddr: altera: s10: Add multiple memory banks support
  arm: socfpga: Update dram_init_banksize() for Stratix10
  board: altera: Stratix10: Add board_get_usable_ram_top()
  board: altera: Stratix10: Add ft_board_setup()
  arm: socfpga: Enable OF_BOARD_SETUP for Stratix 10
  configs: stratix10: Change CONFIG_NR_DRAM_BANKS to 2
  arm: socfpga: stratix10: Add cpu_has_been_warmreset()
  ddr: altera: Stratix10: Add ECC memory scrubbing

 arch/arm/mach-socfpga/Kconfig                 |   1 +
 arch/arm/mach-socfpga/board.c                 |  16 +-
 .../include/mach/reset_manager_s10.h          |   3 +
 arch/arm/mach-socfpga/include/mach/sdram.h    |   2 +
 .../arm/mach-socfpga/include/mach/sdram_s10.h |  10 +
 arch/arm/mach-socfpga/reset_manager_s10.c     |   9 +
 arch/arm/mach-socfpga/spl_s10.c               |  11 --
 board/altera/stratix10-socdk/socfpga.c        |  37 ++++
 configs/socfpga_stratix10_defconfig           |   2 +-
 drivers/ddr/altera/sdram_s10.c                | 178 ++++++++++++++++++
 10 files changed, 256 insertions(+), 13 deletions(-)

-- 
2.19.0

[U-Boot] [PATCH 01/10] ddr: altera: stratix10: Move SDRAM size check to SDRAM driver

[Ley Foon Tan]

Move SDRAM size check to SDRAM driver. sdram_calculate_size()
is called in SDRAM initialization already, avoid calling
twice in size check function.

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 arch/arm/mach-socfpga/spl_s10.c | 11 -----------
 drivers/ddr/altera/sdram_s10.c  | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
index a3db20a819..a141ffe82a 100644
--- a/arch/arm/mach-socfpga/spl_s10.c
+++ b/arch/arm/mach-socfpga/spl_s10.c
@@ -181,17 +181,6 @@ void board_init_f(ulong dummy)
 		hang();
 	}
 
-	gd->ram_size = sdram_calculate_size();
-	printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
-
-	/* Sanity check ensure correct SDRAM size specified */
-	debug("DDR: Running SDRAM size sanity check\n");
-	if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
-		puts("DDR: SDRAM size check failed!\n");
-		hang();
-	}
-	debug("DDR: SDRAM size check passed!\n");
-
 	mbox_init();
 
 #ifdef CONFIG_CADENCE_QSPI
diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
index a48567c109..8895813440 100644
--- a/drivers/ddr/altera/sdram_s10.c
+++ b/drivers/ddr/altera/sdram_s10.c
@@ -134,6 +134,17 @@ static int poll_hmc_clock_status(void)
 				 SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
 }
 
+static void sdram_size_check(void)
+{
+	/* Sanity check ensure correct SDRAM size specified */
+	debug("DDR: Running SDRAM size sanity check\n");
+	if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
+		puts("DDR: SDRAM size check failed!\n");
+		hang();
+	}
+	debug("DDR: SDRAM size check passed!\n");
+}
+
 /**
  * sdram_mmr_init_full() - Function to initialize SDRAM MMR
  *
@@ -339,6 +350,8 @@ int sdram_mmr_init_full(unsigned int unused)
 	else
 		gd->ram_size = size;
 
+	printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
+
 	/* Enable or disable the SDRAM ECC */
 	if (CTRLCFG1_CFG_CTRL_EN_ECC(ctrlcfg1)) {
 		setbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1,
@@ -361,6 +374,8 @@ int sdram_mmr_init_full(unsigned int unused)
 			      DDR_HMC_ECCCTL2_AWB_EN_SET_MSK));
 	}
 
+	sdram_size_check();
+
 	debug("DDR: HMC init success\n");
 	return 0;
 }
-- 
2.19.0

[U-Boot] [PATCH 01/10] ddr: altera: stratix10: Move SDRAM size check to SDRAM driver

[Marek Vasut]

On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> Move SDRAM size check to SDRAM driver. sdram_calculate_size()
> is called in SDRAM initialization already, avoid calling
> twice in size check function.
> 
> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> ---
>  arch/arm/mach-socfpga/spl_s10.c | 11 -----------
>  drivers/ddr/altera/sdram_s10.c  | 15 +++++++++++++++
>  2 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
> index a3db20a819..a141ffe82a 100644
> --- a/arch/arm/mach-socfpga/spl_s10.c
> +++ b/arch/arm/mach-socfpga/spl_s10.c
> @@ -181,17 +181,6 @@ void board_init_f(ulong dummy)
>  		hang();
>  	}
>  
> -	gd->ram_size = sdram_calculate_size();
> -	printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
> -
> -	/* Sanity check ensure correct SDRAM size specified */
> -	debug("DDR: Running SDRAM size sanity check\n");
> -	if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
> -		puts("DDR: SDRAM size check failed!\n");
> -		hang();
> -	}
> -	debug("DDR: SDRAM size check passed!\n");
> -
>  	mbox_init();
>  
>  #ifdef CONFIG_CADENCE_QSPI
> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
> index a48567c109..8895813440 100644
> --- a/drivers/ddr/altera/sdram_s10.c
> +++ b/drivers/ddr/altera/sdram_s10.c
> @@ -134,6 +134,17 @@ static int poll_hmc_clock_status(void)
>  				 SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
>  }
>  
> +static void sdram_size_check(void)
> +{
> +	/* Sanity check ensure correct SDRAM size specified */
> +	debug("DDR: Running SDRAM size sanity check\n");
> +	if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
> +		puts("DDR: SDRAM size check failed!\n");
> +		hang();
> +	}
> +	debug("DDR: SDRAM size check passed!\n");
> +}
> +
>  /**
>   * sdram_mmr_init_full() - Function to initialize SDRAM MMR
>   *
> @@ -339,6 +350,8 @@ int sdram_mmr_init_full(unsigned int unused)
>  	else
>  		gd->ram_size = size;
>  
> +	printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));

Is the type cast needed?

>  	/* Enable or disable the SDRAM ECC */
>  	if (CTRLCFG1_CFG_CTRL_EN_ECC(ctrlcfg1)) {
>  		setbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1,
> @@ -361,6 +374,8 @@ int sdram_mmr_init_full(unsigned int unused)
>  			      DDR_HMC_ECCCTL2_AWB_EN_SET_MSK));
>  	}
>  
> +	sdram_size_check();
> +
>  	debug("DDR: HMC init success\n");
>  	return 0;
>  }
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 01/10] ddr: altera: stratix10: Move SDRAM size check to SDRAM driver

[Ley Foon Tan]

On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>
> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> > Move SDRAM size check to SDRAM driver. sdram_calculate_size()
> > is called in SDRAM initialization already, avoid calling
> > twice in size check function.
> >
> > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > ---
> >  arch/arm/mach-socfpga/spl_s10.c | 11 -----------
> >  drivers/ddr/altera/sdram_s10.c  | 15 +++++++++++++++
> >  2 files changed, 15 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
> > index a3db20a819..a141ffe82a 100644
> > --- a/arch/arm/mach-socfpga/spl_s10.c
> > +++ b/arch/arm/mach-socfpga/spl_s10.c
> > @@ -181,17 +181,6 @@ void board_init_f(ulong dummy)
> >               hang();
> >       }
> >
> > -     gd->ram_size = sdram_calculate_size();
> > -     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
> > -
> > -     /* Sanity check ensure correct SDRAM size specified */
> > -     debug("DDR: Running SDRAM size sanity check\n");
> > -     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
> > -             puts("DDR: SDRAM size check failed!\n");
> > -             hang();
> > -     }
> > -     debug("DDR: SDRAM size check passed!\n");
> > -
> >       mbox_init();
> >
> >  #ifdef CONFIG_CADENCE_QSPI
> > diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
> > index a48567c109..8895813440 100644
> > --- a/drivers/ddr/altera/sdram_s10.c
> > +++ b/drivers/ddr/altera/sdram_s10.c
> > @@ -134,6 +134,17 @@ static int poll_hmc_clock_status(void)
> >                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
> >  }
> >
> > +static void sdram_size_check(void)
> > +{
> > +     /* Sanity check ensure correct SDRAM size specified */
> > +     debug("DDR: Running SDRAM size sanity check\n");
> > +     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
> > +             puts("DDR: SDRAM size check failed!\n");
> > +             hang();
> > +     }
> > +     debug("DDR: SDRAM size check passed!\n");
> > +}
> > +
> >  /**
> >   * sdram_mmr_init_full() - Function to initialize SDRAM MMR
> >   *
> > @@ -339,6 +350,8 @@ int sdram_mmr_init_full(unsigned int unused)
> >       else
> >               gd->ram_size = size;
> >
> > +     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
>
> Is the type cast needed?
Yes, otherwise there is warning.
>
> >       /* Enable or disable the SDRAM ECC */
> >       if (CTRLCFG1_CFG_CTRL_EN_ECC(ctrlcfg1)) {
> >               setbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1,
> > @@ -361,6 +374,8 @@ int sdram_mmr_init_full(unsigned int unused)
> >                             DDR_HMC_ECCCTL2_AWB_EN_SET_MSK));
> >       }
> >
> > +     sdram_size_check();
> > +
> >       debug("DDR: HMC init success\n");
> >       return 0;
> >  }
> >
>
Regards
Ley Foon

[U-Boot] [PATCH 01/10] ddr: altera: stratix10: Move SDRAM size check to SDRAM driver

[Marek Vasut]

On 3/19/19 4:26 AM, Ley Foon Tan wrote:
> On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
>>> Move SDRAM size check to SDRAM driver. sdram_calculate_size()
>>> is called in SDRAM initialization already, avoid calling
>>> twice in size check function.
>>>
>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>> ---
>>>  arch/arm/mach-socfpga/spl_s10.c | 11 -----------
>>>  drivers/ddr/altera/sdram_s10.c  | 15 +++++++++++++++
>>>  2 files changed, 15 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
>>> index a3db20a819..a141ffe82a 100644
>>> --- a/arch/arm/mach-socfpga/spl_s10.c
>>> +++ b/arch/arm/mach-socfpga/spl_s10.c
>>> @@ -181,17 +181,6 @@ void board_init_f(ulong dummy)
>>>               hang();
>>>       }
>>>
>>> -     gd->ram_size = sdram_calculate_size();
>>> -     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
>>> -
>>> -     /* Sanity check ensure correct SDRAM size specified */
>>> -     debug("DDR: Running SDRAM size sanity check\n");
>>> -     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
>>> -             puts("DDR: SDRAM size check failed!\n");
>>> -             hang();
>>> -     }
>>> -     debug("DDR: SDRAM size check passed!\n");
>>> -
>>>       mbox_init();
>>>
>>>  #ifdef CONFIG_CADENCE_QSPI
>>> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
>>> index a48567c109..8895813440 100644
>>> --- a/drivers/ddr/altera/sdram_s10.c
>>> +++ b/drivers/ddr/altera/sdram_s10.c
>>> @@ -134,6 +134,17 @@ static int poll_hmc_clock_status(void)
>>>                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
>>>  }
>>>
>>> +static void sdram_size_check(void)
>>> +{
>>> +     /* Sanity check ensure correct SDRAM size specified */
>>> +     debug("DDR: Running SDRAM size sanity check\n");
>>> +     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
>>> +             puts("DDR: SDRAM size check failed!\n");
>>> +             hang();
>>> +     }
>>> +     debug("DDR: SDRAM size check passed!\n");
>>> +}
>>> +
>>>  /**
>>>   * sdram_mmr_init_full() - Function to initialize SDRAM MMR
>>>   *
>>> @@ -339,6 +350,8 @@ int sdram_mmr_init_full(unsigned int unused)
>>>       else
>>>               gd->ram_size = size;
>>>
>>> +     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
>>
>> Is the type cast needed?
> Yes, otherwise there is warning.

Maybe the warning is justified and needs to be fixed instead of hidden ?

[...]

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 01/10] ddr: altera: stratix10: Move SDRAM size check to SDRAM driver

[Ley Foon Tan]

On Tue, Mar 19, 2019 at 5:39 PM Marek Vasut <marex@denx.de> wrote:
>
> On 3/19/19 4:26 AM, Ley Foon Tan wrote:
> > On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> >>> Move SDRAM size check to SDRAM driver. sdram_calculate_size()
> >>> is called in SDRAM initialization already, avoid calling
> >>> twice in size check function.
> >>>
> >>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> >>> ---
> >>>  arch/arm/mach-socfpga/spl_s10.c | 11 -----------
> >>>  drivers/ddr/altera/sdram_s10.c  | 15 +++++++++++++++
> >>>  2 files changed, 15 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
> >>> index a3db20a819..a141ffe82a 100644
> >>> --- a/arch/arm/mach-socfpga/spl_s10.c
> >>> +++ b/arch/arm/mach-socfpga/spl_s10.c
> >>> @@ -181,17 +181,6 @@ void board_init_f(ulong dummy)
> >>>               hang();
> >>>       }
> >>>
> >>> -     gd->ram_size = sdram_calculate_size();
> >>> -     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
> >>> -
> >>> -     /* Sanity check ensure correct SDRAM size specified */
> >>> -     debug("DDR: Running SDRAM size sanity check\n");
> >>> -     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
> >>> -             puts("DDR: SDRAM size check failed!\n");
> >>> -             hang();
> >>> -     }
> >>> -     debug("DDR: SDRAM size check passed!\n");
> >>> -
> >>>       mbox_init();
> >>>
> >>>  #ifdef CONFIG_CADENCE_QSPI
> >>> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
> >>> index a48567c109..8895813440 100644
> >>> --- a/drivers/ddr/altera/sdram_s10.c
> >>> +++ b/drivers/ddr/altera/sdram_s10.c
> >>> @@ -134,6 +134,17 @@ static int poll_hmc_clock_status(void)
> >>>                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
> >>>  }
> >>>
> >>> +static void sdram_size_check(void)
> >>> +{
> >>> +     /* Sanity check ensure correct SDRAM size specified */
> >>> +     debug("DDR: Running SDRAM size sanity check\n");
> >>> +     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
> >>> +             puts("DDR: SDRAM size check failed!\n");
> >>> +             hang();
> >>> +     }
> >>> +     debug("DDR: SDRAM size check passed!\n");
> >>> +}
> >>> +
> >>>  /**
> >>>   * sdram_mmr_init_full() - Function to initialize SDRAM MMR
> >>>   *
> >>> @@ -339,6 +350,8 @@ int sdram_mmr_init_full(unsigned int unused)
> >>>       else
> >>>               gd->ram_size = size;
> >>>
> >>> +     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
> >>
> >> Is the type cast needed?
> > Yes, otherwise there is warning.
>
> Maybe the warning is justified and needs to be fixed instead of hidden ?
>

drivers/ddr/altera/sdram_s10.c:461:16: warning: format ‘%d’ expects
argument of type ‘int’, but argument 2 has type ‘phys_size_t {aka
volatile long long unsigned int}’ [-Wformat=]
  printf("DDR: %d MiB\n", gd->ram_size >> 20);
               ~^         ~~~~~~~~~~~~~~~~~~

Regards
Ley Foon

[U-Boot] [PATCH 01/10] ddr: altera: stratix10: Move SDRAM size check to SDRAM driver

[Marek Vasut]

On 3/19/19 10:46 AM, Ley Foon Tan wrote:
> On Tue, Mar 19, 2019 at 5:39 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/19/19 4:26 AM, Ley Foon Tan wrote:
>>> On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
>>>>> Move SDRAM size check to SDRAM driver. sdram_calculate_size()
>>>>> is called in SDRAM initialization already, avoid calling
>>>>> twice in size check function.
>>>>>
>>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>>>> ---
>>>>>  arch/arm/mach-socfpga/spl_s10.c | 11 -----------
>>>>>  drivers/ddr/altera/sdram_s10.c  | 15 +++++++++++++++
>>>>>  2 files changed, 15 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
>>>>> index a3db20a819..a141ffe82a 100644
>>>>> --- a/arch/arm/mach-socfpga/spl_s10.c
>>>>> +++ b/arch/arm/mach-socfpga/spl_s10.c
>>>>> @@ -181,17 +181,6 @@ void board_init_f(ulong dummy)
>>>>>               hang();
>>>>>       }
>>>>>
>>>>> -     gd->ram_size = sdram_calculate_size();
>>>>> -     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
>>>>> -
>>>>> -     /* Sanity check ensure correct SDRAM size specified */
>>>>> -     debug("DDR: Running SDRAM size sanity check\n");
>>>>> -     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
>>>>> -             puts("DDR: SDRAM size check failed!\n");
>>>>> -             hang();
>>>>> -     }
>>>>> -     debug("DDR: SDRAM size check passed!\n");
>>>>> -
>>>>>       mbox_init();
>>>>>
>>>>>  #ifdef CONFIG_CADENCE_QSPI
>>>>> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
>>>>> index a48567c109..8895813440 100644
>>>>> --- a/drivers/ddr/altera/sdram_s10.c
>>>>> +++ b/drivers/ddr/altera/sdram_s10.c
>>>>> @@ -134,6 +134,17 @@ static int poll_hmc_clock_status(void)
>>>>>                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
>>>>>  }
>>>>>
>>>>> +static void sdram_size_check(void)
>>>>> +{
>>>>> +     /* Sanity check ensure correct SDRAM size specified */
>>>>> +     debug("DDR: Running SDRAM size sanity check\n");
>>>>> +     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
>>>>> +             puts("DDR: SDRAM size check failed!\n");
>>>>> +             hang();
>>>>> +     }
>>>>> +     debug("DDR: SDRAM size check passed!\n");
>>>>> +}
>>>>> +
>>>>>  /**
>>>>>   * sdram_mmr_init_full() - Function to initialize SDRAM MMR
>>>>>   *
>>>>> @@ -339,6 +350,8 @@ int sdram_mmr_init_full(unsigned int unused)
>>>>>       else
>>>>>               gd->ram_size = size;
>>>>>
>>>>> +     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
>>>>
>>>> Is the type cast needed?
>>> Yes, otherwise there is warning.
>>
>> Maybe the warning is justified and needs to be fixed instead of hidden ?
>>
> 
> drivers/ddr/altera/sdram_s10.c:461:16: warning: format ‘%d’ expects
> argument of type ‘int’, but argument 2 has type ‘phys_size_t {aka
> volatile long long unsigned int}’ [-Wformat=]
>   printf("DDR: %d MiB\n", gd->ram_size >> 20);
>                ~^         ~~~~~~~~~~~~~~~~~~

That's %lld then.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 01/10] ddr: altera: stratix10: Move SDRAM size check to SDRAM driver

[Ley Foon Tan]

On Tue, Mar 19, 2019 at 5:47 PM Marek Vasut <marex@denx.de> wrote:
>
> On 3/19/19 10:46 AM, Ley Foon Tan wrote:
> > On Tue, Mar 19, 2019 at 5:39 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 3/19/19 4:26 AM, Ley Foon Tan wrote:
> >>> On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> >>>>> Move SDRAM size check to SDRAM driver. sdram_calculate_size()
> >>>>> is called in SDRAM initialization already, avoid calling
> >>>>> twice in size check function.
> >>>>>
> >>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> >>>>> ---
> >>>>>  arch/arm/mach-socfpga/spl_s10.c | 11 -----------
> >>>>>  drivers/ddr/altera/sdram_s10.c  | 15 +++++++++++++++
> >>>>>  2 files changed, 15 insertions(+), 11 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
> >>>>> index a3db20a819..a141ffe82a 100644
> >>>>> --- a/arch/arm/mach-socfpga/spl_s10.c
> >>>>> +++ b/arch/arm/mach-socfpga/spl_s10.c
> >>>>> @@ -181,17 +181,6 @@ void board_init_f(ulong dummy)
> >>>>>               hang();
> >>>>>       }
> >>>>>
> >>>>> -     gd->ram_size = sdram_calculate_size();
> >>>>> -     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
> >>>>> -
> >>>>> -     /* Sanity check ensure correct SDRAM size specified */
> >>>>> -     debug("DDR: Running SDRAM size sanity check\n");
> >>>>> -     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
> >>>>> -             puts("DDR: SDRAM size check failed!\n");
> >>>>> -             hang();
> >>>>> -     }
> >>>>> -     debug("DDR: SDRAM size check passed!\n");
> >>>>> -
> >>>>>       mbox_init();
> >>>>>
> >>>>>  #ifdef CONFIG_CADENCE_QSPI
> >>>>> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
> >>>>> index a48567c109..8895813440 100644
> >>>>> --- a/drivers/ddr/altera/sdram_s10.c
> >>>>> +++ b/drivers/ddr/altera/sdram_s10.c
> >>>>> @@ -134,6 +134,17 @@ static int poll_hmc_clock_status(void)
> >>>>>                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
> >>>>>  }
> >>>>>
> >>>>> +static void sdram_size_check(void)
> >>>>> +{
> >>>>> +     /* Sanity check ensure correct SDRAM size specified */
> >>>>> +     debug("DDR: Running SDRAM size sanity check\n");
> >>>>> +     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
> >>>>> +             puts("DDR: SDRAM size check failed!\n");
> >>>>> +             hang();
> >>>>> +     }
> >>>>> +     debug("DDR: SDRAM size check passed!\n");
> >>>>> +}
> >>>>> +
> >>>>>  /**
> >>>>>   * sdram_mmr_init_full() - Function to initialize SDRAM MMR
> >>>>>   *
> >>>>> @@ -339,6 +350,8 @@ int sdram_mmr_init_full(unsigned int unused)
> >>>>>       else
> >>>>>               gd->ram_size = size;
> >>>>>
> >>>>> +     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
> >>>>
> >>>> Is the type cast needed?
> >>> Yes, otherwise there is warning.
> >>
> >> Maybe the warning is justified and needs to be fixed instead of hidden ?
> >>
> >
> > drivers/ddr/altera/sdram_s10.c:461:16: warning: format ‘%d’ expects
> > argument of type ‘int’, but argument 2 has type ‘phys_size_t {aka
> > volatile long long unsigned int}’ [-Wformat=]
> >   printf("DDR: %d MiB\n", gd->ram_size >> 20);
> >                ~^         ~~~~~~~~~~~~~~~~~~
>
> That's %lld then.
Same as %llx, tiny printf in SPL doesn't support %ll.

Regards
Ley Foon

[U-Boot] [PATCH 01/10] ddr: altera: stratix10: Move SDRAM size check to SDRAM driver

[Marek Vasut]

On 3/20/19 2:30 AM, Ley Foon Tan wrote:
> On Tue, Mar 19, 2019 at 5:47 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/19/19 10:46 AM, Ley Foon Tan wrote:
>>> On Tue, Mar 19, 2019 at 5:39 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 3/19/19 4:26 AM, Ley Foon Tan wrote:
>>>>> On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>>>>>>
>>>>>> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
>>>>>>> Move SDRAM size check to SDRAM driver. sdram_calculate_size()
>>>>>>> is called in SDRAM initialization already, avoid calling
>>>>>>> twice in size check function.
>>>>>>>
>>>>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>>>>>> ---
>>>>>>>  arch/arm/mach-socfpga/spl_s10.c | 11 -----------
>>>>>>>  drivers/ddr/altera/sdram_s10.c  | 15 +++++++++++++++
>>>>>>>  2 files changed, 15 insertions(+), 11 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
>>>>>>> index a3db20a819..a141ffe82a 100644
>>>>>>> --- a/arch/arm/mach-socfpga/spl_s10.c
>>>>>>> +++ b/arch/arm/mach-socfpga/spl_s10.c
>>>>>>> @@ -181,17 +181,6 @@ void board_init_f(ulong dummy)
>>>>>>>               hang();
>>>>>>>       }
>>>>>>>
>>>>>>> -     gd->ram_size = sdram_calculate_size();
>>>>>>> -     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
>>>>>>> -
>>>>>>> -     /* Sanity check ensure correct SDRAM size specified */
>>>>>>> -     debug("DDR: Running SDRAM size sanity check\n");
>>>>>>> -     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
>>>>>>> -             puts("DDR: SDRAM size check failed!\n");
>>>>>>> -             hang();
>>>>>>> -     }
>>>>>>> -     debug("DDR: SDRAM size check passed!\n");
>>>>>>> -
>>>>>>>       mbox_init();
>>>>>>>
>>>>>>>  #ifdef CONFIG_CADENCE_QSPI
>>>>>>> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
>>>>>>> index a48567c109..8895813440 100644
>>>>>>> --- a/drivers/ddr/altera/sdram_s10.c
>>>>>>> +++ b/drivers/ddr/altera/sdram_s10.c
>>>>>>> @@ -134,6 +134,17 @@ static int poll_hmc_clock_status(void)
>>>>>>>                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
>>>>>>>  }
>>>>>>>
>>>>>>> +static void sdram_size_check(void)
>>>>>>> +{
>>>>>>> +     /* Sanity check ensure correct SDRAM size specified */
>>>>>>> +     debug("DDR: Running SDRAM size sanity check\n");
>>>>>>> +     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
>>>>>>> +             puts("DDR: SDRAM size check failed!\n");
>>>>>>> +             hang();
>>>>>>> +     }
>>>>>>> +     debug("DDR: SDRAM size check passed!\n");
>>>>>>> +}
>>>>>>> +
>>>>>>>  /**
>>>>>>>   * sdram_mmr_init_full() - Function to initialize SDRAM MMR
>>>>>>>   *
>>>>>>> @@ -339,6 +350,8 @@ int sdram_mmr_init_full(unsigned int unused)
>>>>>>>       else
>>>>>>>               gd->ram_size = size;
>>>>>>>
>>>>>>> +     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
>>>>>>
>>>>>> Is the type cast needed?
>>>>> Yes, otherwise there is warning.
>>>>
>>>> Maybe the warning is justified and needs to be fixed instead of hidden ?
>>>>
>>>
>>> drivers/ddr/altera/sdram_s10.c:461:16: warning: format ‘%d’ expects
>>> argument of type ‘int’, but argument 2 has type ‘phys_size_t {aka
>>> volatile long long unsigned int}’ [-Wformat=]
>>>   printf("DDR: %d MiB\n", gd->ram_size >> 20);
>>>                ~^         ~~~~~~~~~~~~~~~~~~
>>
>> That's %lld then.
> Same as %llx, tiny printf in SPL doesn't support %ll.

That shouldn't be hard to add, and it fixes a typecast which might hide
bugs.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 01/10] ddr: altera: stratix10: Move SDRAM size check to SDRAM driver

[Ley Foon Tan]

On Wed, Mar 20, 2019 at 6:05 PM Marek Vasut <marex@denx.de> wrote:
>
> On 3/20/19 2:30 AM, Ley Foon Tan wrote:
> > On Tue, Mar 19, 2019 at 5:47 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 3/19/19 10:46 AM, Ley Foon Tan wrote:
> >>> On Tue, Mar 19, 2019 at 5:39 PM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> On 3/19/19 4:26 AM, Ley Foon Tan wrote:
> >>>>> On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
> >>>>>>
> >>>>>> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> >>>>>>> Move SDRAM size check to SDRAM driver. sdram_calculate_size()
> >>>>>>> is called in SDRAM initialization already, avoid calling
> >>>>>>> twice in size check function.
> >>>>>>>
> >>>>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> >>>>>>> ---
> >>>>>>>  arch/arm/mach-socfpga/spl_s10.c | 11 -----------
> >>>>>>>  drivers/ddr/altera/sdram_s10.c  | 15 +++++++++++++++
> >>>>>>>  2 files changed, 15 insertions(+), 11 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
> >>>>>>> index a3db20a819..a141ffe82a 100644
> >>>>>>> --- a/arch/arm/mach-socfpga/spl_s10.c
> >>>>>>> +++ b/arch/arm/mach-socfpga/spl_s10.c
> >>>>>>> @@ -181,17 +181,6 @@ void board_init_f(ulong dummy)
> >>>>>>>               hang();
> >>>>>>>       }
> >>>>>>>
> >>>>>>> -     gd->ram_size = sdram_calculate_size();
> >>>>>>> -     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
> >>>>>>> -
> >>>>>>> -     /* Sanity check ensure correct SDRAM size specified */
> >>>>>>> -     debug("DDR: Running SDRAM size sanity check\n");
> >>>>>>> -     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
> >>>>>>> -             puts("DDR: SDRAM size check failed!\n");
> >>>>>>> -             hang();
> >>>>>>> -     }
> >>>>>>> -     debug("DDR: SDRAM size check passed!\n");
> >>>>>>> -
> >>>>>>>       mbox_init();
> >>>>>>>
> >>>>>>>  #ifdef CONFIG_CADENCE_QSPI
> >>>>>>> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
> >>>>>>> index a48567c109..8895813440 100644
> >>>>>>> --- a/drivers/ddr/altera/sdram_s10.c
> >>>>>>> +++ b/drivers/ddr/altera/sdram_s10.c
> >>>>>>> @@ -134,6 +134,17 @@ static int poll_hmc_clock_status(void)
> >>>>>>>                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
> >>>>>>>  }
> >>>>>>>
> >>>>>>> +static void sdram_size_check(void)
> >>>>>>> +{
> >>>>>>> +     /* Sanity check ensure correct SDRAM size specified */
> >>>>>>> +     debug("DDR: Running SDRAM size sanity check\n");
> >>>>>>> +     if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
> >>>>>>> +             puts("DDR: SDRAM size check failed!\n");
> >>>>>>> +             hang();
> >>>>>>> +     }
> >>>>>>> +     debug("DDR: SDRAM size check passed!\n");
> >>>>>>> +}
> >>>>>>> +
> >>>>>>>  /**
> >>>>>>>   * sdram_mmr_init_full() - Function to initialize SDRAM MMR
> >>>>>>>   *
> >>>>>>> @@ -339,6 +350,8 @@ int sdram_mmr_init_full(unsigned int unused)
> >>>>>>>       else
> >>>>>>>               gd->ram_size = size;
> >>>>>>>
> >>>>>>> +     printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
> >>>>>>
> >>>>>> Is the type cast needed?
> >>>>> Yes, otherwise there is warning.
> >>>>
> >>>> Maybe the warning is justified and needs to be fixed instead of hidden ?
> >>>>
> >>>
> >>> drivers/ddr/altera/sdram_s10.c:461:16: warning: format ‘%d’ expects
> >>> argument of type ‘int’, but argument 2 has type ‘phys_size_t {aka
> >>> volatile long long unsigned int}’ [-Wformat=]
> >>>   printf("DDR: %d MiB\n", gd->ram_size >> 20);
> >>>                ~^         ~~~~~~~~~~~~~~~~~~
> >>
> >> That's %lld then.
> > Same as %llx, tiny printf in SPL doesn't support %ll.
>
> That shouldn't be hard to add, and it fixes a typecast which might hide
> bugs.
>
I will send separate patch to add %ll support in tiny printf.

Regards
Ley Foon

[U-Boot] [PATCH 02/10] arm: socfpga: Add sdram_s10.h to sdram.h

[Ley Foon Tan]

Includes Stratix10 SDRAM header file.

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 arch/arm/mach-socfpga/include/mach/sdram.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-socfpga/include/mach/sdram.h b/arch/arm/mach-socfpga/include/mach/sdram.h
index 79cb9e6064..2c1f55a87a 100644
--- a/arch/arm/mach-socfpga/include/mach/sdram.h
+++ b/arch/arm/mach-socfpga/include/mach/sdram.h
@@ -11,6 +11,8 @@
 #include <asm/arch/sdram_gen5.h>
 #elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
 #include <asm/arch/sdram_arria10.h>
+#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10)
+#include <asm/arch/sdram_s10.h>
 #endif
 
 #endif
-- 
2.19.0

[U-Boot] [PATCH 03/10] ddr: altera: s10: Add multiple memory banks support

[Ley Foon Tan]

Setup bank start address and size based on total SDRAM
memory size calculated from hardware. Update sdram_size_check()
to support multiple banks.

Stratix10 supports up to 2 memory banks.

Bank 0: Address 0, size 2GB
Bank 1: Address 0x100000000, size 124GB

Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 .../arm/mach-socfpga/include/mach/sdram_s10.h |  1 +
 drivers/ddr/altera/sdram_s10.c                | 93 ++++++++++++++++++-
 2 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-socfpga/include/mach/sdram_s10.h b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
index ca68594445..89e355010d 100644
--- a/arch/arm/mach-socfpga/include/mach/sdram_s10.h
+++ b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
@@ -10,6 +10,7 @@
 phys_size_t sdram_calculate_size(void);
 int sdram_mmr_init_full(unsigned int sdr_phy_reg);
 int sdram_calibration_full(void);
+void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
 
 #define DDR_TWR				15
 #define DDR_READ_LATENCY_DELAY		40
diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
index 8895813440..ae4e5ea2fd 100644
--- a/drivers/ddr/altera/sdram_s10.c
+++ b/drivers/ddr/altera/sdram_s10.c
@@ -13,6 +13,7 @@
 #include <asm/arch/sdram_s10.h>
 #include <asm/arch/system_manager.h>
 #include <asm/arch/reset_manager.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -134,17 +135,100 @@ static int poll_hmc_clock_status(void)
 				 SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
 }
 
-static void sdram_size_check(void)
+static void sdram_size_check(phys_addr_t bank_start[], phys_size_t bank_size[])
 {
+	phys_size_t total_ram_check = 0;
+	phys_size_t ram_check = 0;
+	int bank;
+
 	/* Sanity check ensure correct SDRAM size specified */
 	debug("DDR: Running SDRAM size sanity check\n");
-	if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
+
+	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
+		while (ram_check < bank_size[bank]) {
+			ram_check += get_ram_size((void *)(bank_start[bank]
+							   + ram_check),
+							   (phys_size_t)SZ_1G);
+		}
+		total_ram_check += ram_check;
+		ram_check = 0;
+	}
+
+	if (total_ram_check != gd->ram_size) {
 		puts("DDR: SDRAM size check failed!\n");
 		hang();
 	}
+
 	debug("DDR: SDRAM size check passed!\n");
 }
 
+void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[])
+{
+	phys_size_t ram_alias;
+
+	bank_addr[0] = CONFIG_SYS_SDRAM_BASE;
+	bank_size[0] = min((phys_size_t)SZ_2G, (phys_size_t)gd->ram_size);
+
+	if (CONFIG_NR_DRAM_BANKS > 1) {
+		if (gd->ram_size > ((phys_size_t)SZ_2G * (phys_size_t)32)) {
+			/* With memories > 64GB it is not possible to
+			 * access the entire memory. You cannot avoid
+			 * the 2GB IO hole from 2GB to 4GB in the HPS
+			 * memory space. The dram banks must be configured
+			 * as :
+			 * dram[0].start = 0x0
+			 * dram[0].size = SZ_2GB
+			 * dram[1].start = SZ_2GB << 1 (or 4GB)
+			 * dram[1].size = gd->ram_size - SZ_2G
+			 */
+			bank_addr[1] = (phys_addr_t)SZ_2G +
+				(phys_addr_t)SZ_2G;
+			bank_size[1] = gd->ram_size - SZ_2G;
+
+			/* reduce ram size as useable space is smaller.
+			 * We cannot avoid the IO hole.
+			 */
+			gd->ram_size -= SZ_2G;
+
+		} else if (gd->ram_size > SZ_2G) {
+			/* We can use that the DRAM address space
+			 * is repeatedly aliased in the overall
+			 * 128GB of address space as the SDRAM NOC
+			 * ignores unused upper address bits.
+			 *
+			 * This means, a 4GB DDR is replicated every
+			 * 4GB in the address, and a 16GB DDR every
+			 * 16GB.
+			 *
+			 * So, we can access the entire memory, for
+			 * the 4GB case:
+			 * dram[0].start = 0x0
+			 * dram[0].size = SZ_2GB
+			 * dram[1].start = SZ_2GB << 1 + SZ_2GB (or 6GB)
+			 * dram[1].size = gd->ram_size - SZ_2G
+			 *
+			 * more interestingly, the 3GB case would be the same.
+			 */
+
+			/* Now we need to find the appropriate alias
+			 * location.
+			 */
+			ram_alias = SZ_2G;
+			while (ram_alias < gd->ram_size)
+				ram_alias = ram_alias << 1;
+
+			bank_addr[1] = ram_alias + SZ_2G;
+			bank_size[1] = gd->ram_size - SZ_2G;
+		} else {
+			/* Here the map is simple as all memory fits in the
+			 * lower 2GB window.
+			 */
+			bank_addr[1] = 0;
+			bank_size[1] = 0;
+		}
+	}
+}
+
 /**
  * sdram_mmr_init_full() - Function to initialize SDRAM MMR
  *
@@ -155,6 +239,8 @@ int sdram_mmr_init_full(unsigned int unused)
 	u32 update_value, io48_value, ddrioctl;
 	u32 i;
 	int ret;
+	phys_addr_t bank_start[CONFIG_NR_DRAM_BANKS];
+	phys_size_t bank_size[CONFIG_NR_DRAM_BANKS];
 
 	/* Enable access to DDR from CPU master */
 	clrbits_le32(CCU_REG_ADDR(CCU_CPU0_MPRT_ADBASE_DDRREG),
@@ -351,6 +437,7 @@ int sdram_mmr_init_full(unsigned int unused)
 		gd->ram_size = size;
 
 	printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
+	setup_memory_banks(bank_start, bank_size);
 
 	/* Enable or disable the SDRAM ECC */
 	if (CTRLCFG1_CFG_CTRL_EN_ECC(ctrlcfg1)) {
@@ -374,7 +461,7 @@ int sdram_mmr_init_full(unsigned int unused)
 			      DDR_HMC_ECCCTL2_AWB_EN_SET_MSK));
 	}
 
-	sdram_size_check();
+	sdram_size_check(bank_start, bank_size);
 
 	debug("DDR: HMC init success\n");
 	return 0;
-- 
2.19.0

[U-Boot] [PATCH 03/10] ddr: altera: s10: Add multiple memory banks support

[Marek Vasut]

On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> Setup bank start address and size based on total SDRAM
> memory size calculated from hardware. Update sdram_size_check()
> to support multiple banks.
> 
> Stratix10 supports up to 2 memory banks.
> 
> Bank 0: Address 0, size 2GB
> Bank 1: Address 0x100000000, size 124GB
> 
> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>

Shouldn't Quartus generate some sort of DT which gives you all the DRAM
layout information ? Then you won't need any of this
CONFIG_NR_DRAM_BANKS stuff, but you would be able to extract such info
from DT and apply the sanity check only on DRAM you know exists.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 03/10] ddr: altera: s10: Add multiple memory banks support

[Westergreen, Dalon]

On Tue, 2019-03-12 at 11:44 +0100, Marek Vasut wrote:
> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> > Setup bank start address and size based on total SDRAM
> > memory size calculated from hardware. Update sdram_size_check()
> > to support multiple banks.
> > 
> > Stratix10 supports up to 2 memory banks.
> > 
> > Bank 0: Address 0, size 2GB
> > Bank 1: Address 0x100000000, size 124GB
> > 
> > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> 
> Shouldn't Quartus generate some sort of DT which gives you all the DRAM
> layout information ? Then you won't need any of this
> CONFIG_NR_DRAM_BANKS stuff, but you would be able to extract such info
> from DT and apply the sanity check only on DRAM you know exists.
> 
Yes, I think this can be much cleaner and the dts should provide the bank
information rather than creating the banks from the determined dram size.

The determined dram size can just be a check to validate the bank configuration
in the dts.

so

diff --git a/arch/arm/dts/socfpga_stratix10_socdk.dts
b/arch/arm/dts/socfpga_stratix10_socdk.dts
index 6e8ddcd9f4..94c71bb0ed 100644
--- a/arch/arm/dts/socfpga_stratix10_socdk.dts
+++ b/arch/arm/dts/socfpga_stratix10_socdk.dts
@@ -36,7 +36,9 @@

        memory {
                device_type = "memory";
-               reg = <0 0 0 0x80000000>; /* 2GB */
+               /* 4GB */
+               reg = < 0 0x00000000 0 0x80000000
+                       1 0x80000000 0 0x80000000>;
                u-boot,dm-pre-reloc;
        };
 };

--dalon



-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3282 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190312/9315965d/attachment.bin>

[U-Boot] [PATCH 03/10] ddr: altera: s10: Add multiple memory banks support

[Ley Foon Tan]

On Tue, 2019-03-12 at 14:05 +0000, Westergreen, Dalon wrote:
> On Tue, 2019-03-12 at 11:44 +0100, Marek Vasut wrote:
> > 
> > On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> > > 
> > > Setup bank start address and size based on total SDRAM
> > > memory size calculated from hardware. Update sdram_size_check()
> > > to support multiple banks.
> > > 
> > > Stratix10 supports up to 2 memory banks.
> > > 
> > > Bank 0: Address 0, size 2GB
> > > Bank 1: Address 0x100000000, size 124GB
> > > 
> > > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > Shouldn't Quartus generate some sort of DT which gives you all the
> > DRAM
> > layout information ? Then you won't need any of this
> > CONFIG_NR_DRAM_BANKS stuff, but you would be able to extract such
> > info
> > from DT and apply the sanity check only on DRAM you know exists.
> > 
> Yes, I think this can be much cleaner and the dts should provide the
> bank
> information rather than creating the banks from the determined dram
> size.
> 
> The determined dram size can just be a check to validate the bank
> configuration
> in the dts.

Okay.Then user needs update memory node in dts for different SDRAM
size.

Regards
Ley Foon
> 
> so
> 
> diff --git a/arch/arm/dts/socfpga_stratix10_socdk.dts
> b/arch/arm/dts/socfpga_stratix10_socdk.dts
> index 6e8ddcd9f4..94c71bb0ed 100644
> --- a/arch/arm/dts/socfpga_stratix10_socdk.dts
> +++ b/arch/arm/dts/socfpga_stratix10_socdk.dts
> @@ -36,7 +36,9 @@
> 
>         memory {
>                 device_type = "memory";
> -               reg = <0 0 0 0x80000000>; /* 2GB */
> +               /* 4GB */
> +               reg = < 0 0x00000000 0 0x80000000
> +                       1 0x80000000 0 0x80000000>;
>                 u-boot,dm-pre-reloc;
>         };
>  };
> 
> --dalon
> 
> 
> 

[U-Boot] [PATCH 03/10] ddr: altera: s10: Add multiple memory banks support

[Marek Vasut]

On 3/13/19 8:28 AM, Ley Foon Tan wrote:
> On Tue, 2019-03-12 at 14:05 +0000, Westergreen, Dalon wrote:
>> On Tue, 2019-03-12 at 11:44 +0100, Marek Vasut wrote:
>>>
>>> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
>>>>
>>>> Setup bank start address and size based on total SDRAM
>>>> memory size calculated from hardware. Update sdram_size_check()
>>>> to support multiple banks.
>>>>
>>>> Stratix10 supports up to 2 memory banks.
>>>>
>>>> Bank 0: Address 0, size 2GB
>>>> Bank 1: Address 0x100000000, size 124GB
>>>>
>>>> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>> Shouldn't Quartus generate some sort of DT which gives you all the
>>> DRAM
>>> layout information ? Then you won't need any of this
>>> CONFIG_NR_DRAM_BANKS stuff, but you would be able to extract such
>>> info
>>> from DT and apply the sanity check only on DRAM you know exists.
>>>
>> Yes, I think this can be much cleaner and the dts should provide the
>> bank
>> information rather than creating the banks from the determined dram
>> size.
>>
>> The determined dram size can just be a check to validate the bank
>> configuration
>> in the dts.
> 
> Okay.Then user needs update memory node in dts for different SDRAM
> size.

The DT should describe hardware correctly, so yes.

> Regards
> Ley Foon
>>
>> so
>>
>> diff --git a/arch/arm/dts/socfpga_stratix10_socdk.dts
>> b/arch/arm/dts/socfpga_stratix10_socdk.dts
>> index 6e8ddcd9f4..94c71bb0ed 100644
>> --- a/arch/arm/dts/socfpga_stratix10_socdk.dts
>> +++ b/arch/arm/dts/socfpga_stratix10_socdk.dts
>> @@ -36,7 +36,9 @@
>>
>>         memory {
>>                 device_type = "memory";
>> -               reg = <0 0 0 0x80000000>; /* 2GB */
>> +               /* 4GB */
>> +               reg = < 0 0x00000000 0 0x80000000
>> +                       1 0x80000000 0 0x80000000>;
>>                 u-boot,dm-pre-reloc;
>>         };
>>  };
>>
>> --dalon
>>
>>
>>


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 04/10] arm: socfpga: Update dram_init_banksize() for Stratix10

[Ley Foon Tan]

Setup bi_dram struct based on returned from setup_memory_banks().

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 arch/arm/mach-socfpga/board.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach-socfpga/board.c
index 7c8c05cc31..a0e9917e47 100644
--- a/arch/arm/mach-socfpga/board.c
+++ b/arch/arm/mach-socfpga/board.c
@@ -12,6 +12,7 @@
 #include <asm/arch/clock_manager.h>
 #include <asm/arch/misc.h>
 #include <asm/io.h>
+#include <asm/arch/sdram.h>
 
 #include <usb.h>
 #include <usb/dwc2_udc.h>
@@ -48,8 +49,21 @@ int board_init(void)
 
 int dram_init_banksize(void)
 {
-	fdtdec_setup_memory_banksize();
+#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10) && defined(CONFIG_ALTERA_SDRAM)
+	phys_addr_t bank_start[CONFIG_NR_DRAM_BANKS];
+	phys_size_t bank_size[CONFIG_NR_DRAM_BANKS];
+	int bank;
+
+	gd->ram_size = sdram_calculate_size();
+	setup_memory_banks(bank_start, bank_size);
 
+	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
+		gd->bd->bi_dram[bank].start = bank_start[bank];
+		gd->bd->bi_dram[bank].size = bank_size[bank];
+	}
+#else
+	fdtdec_setup_memory_banksize();
+#endif
 	return 0;
 }
 
-- 
2.19.0

[U-Boot] [PATCH 04/10] arm: socfpga: Update dram_init_banksize() for Stratix10

[Marek Vasut]

On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> Setup bi_dram struct based on returned from setup_memory_banks().
> 
> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> ---
>  arch/arm/mach-socfpga/board.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach-socfpga/board.c
> index 7c8c05cc31..a0e9917e47 100644
> --- a/arch/arm/mach-socfpga/board.c
> +++ b/arch/arm/mach-socfpga/board.c
> @@ -12,6 +12,7 @@
>  #include <asm/arch/clock_manager.h>
>  #include <asm/arch/misc.h>
>  #include <asm/io.h>
> +#include <asm/arch/sdram.h>
>  
>  #include <usb.h>
>  #include <usb/dwc2_udc.h>
> @@ -48,8 +49,21 @@ int board_init(void)
>  
>  int dram_init_banksize(void)
>  {
> -	fdtdec_setup_memory_banksize();
> +#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10) && defined(CONFIG_ALTERA_SDRAM)
> +	phys_addr_t bank_start[CONFIG_NR_DRAM_BANKS];
> +	phys_size_t bank_size[CONFIG_NR_DRAM_BANKS];
> +	int bank;
> +
> +	gd->ram_size = sdram_calculate_size();
> +	setup_memory_banks(bank_start, bank_size);
>  
> +	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> +		gd->bd->bi_dram[bank].start = bank_start[bank];
> +		gd->bd->bi_dram[bank].size = bank_size[bank];
> +	}
> +#else
> +	fdtdec_setup_memory_banksize();
> +#endif
>  	return 0;
>  }

Split this out into separate file if this really needs to be different,
but can't you somehow use fdtdec_setup_memory_size() on S10 and then
apply sanity checking instead ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 04/10] arm: socfpga: Update dram_init_banksize() for Stratix10

[Westergreen, Dalon]

On Tue, 2019-03-12 at 11:45 +0100, Marek Vasut wrote:
> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> > Setup bi_dram struct based on returned from setup_memory_banks().
> > 
> > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > ---
> >  arch/arm/mach-socfpga/board.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mach-socfpga/board.c b/arch/arm/mach-socfpga/board.c
> > index 7c8c05cc31..a0e9917e47 100644
> > --- a/arch/arm/mach-socfpga/board.c
> > +++ b/arch/arm/mach-socfpga/board.c
> > @@ -12,6 +12,7 @@
> >  #include <asm/arch/clock_manager.h>
> >  #include <asm/arch/misc.h>
> >  #include <asm/io.h>
> > +#include <asm/arch/sdram.h>
> >  
> >  #include <usb.h>
> >  #include <usb/dwc2_udc.h>
> > @@ -48,8 +49,21 @@ int board_init(void)
> >  
> >  int dram_init_banksize(void)
> >  {
> > -	fdtdec_setup_memory_banksize();
> > +#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10) &&
> > defined(CONFIG_ALTERA_SDRAM)
> > +	phys_addr_t bank_start[CONFIG_NR_DRAM_BANKS];
> > +	phys_size_t bank_size[CONFIG_NR_DRAM_BANKS];
> > +	int bank;
> > +
> > +	gd->ram_size = sdram_calculate_size();
> > +	setup_memory_banks(bank_start, bank_size);
> >  
> > +	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> > +		gd->bd->bi_dram[bank].start = bank_start[bank];
> > +		gd->bd->bi_dram[bank].size = bank_size[bank];
> > +	}
> > +#else
> > +	fdtdec_setup_memory_banksize();
> > +#endif
> >  	return 0;
> >  }
> 
> Split this out into separate file if this really needs to be different,
> but can't you somehow use fdtdec_setup_memory_size() on S10 and then
> apply sanity checking instead ?
> 
yes, this can be done with fdtdec_setup_memory_size.

--dalon

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3282 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190312/e8c39d02/attachment.bin>

[U-Boot] [PATCH 05/10] board: altera: Stratix10: Add board_get_usable_ram_top()

[Ley Foon Tan]

Add board_get_usable_ram_top() function. Limit maximum usable
ram top to 2GB.

Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 board/altera/stratix10-socdk/socfpga.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/board/altera/stratix10-socdk/socfpga.c b/board/altera/stratix10-socdk/socfpga.c
index 043fc543f1..99c10d313c 100644
--- a/board/altera/stratix10-socdk/socfpga.c
+++ b/board/altera/stratix10-socdk/socfpga.c
@@ -5,3 +5,15 @@
  */
 
 #include <common.h>
+#include <asm/arch/sdram.h>
+#include <linux/sizes.h>
+
+#ifdef CONFIG_ALTERA_SDRAM
+ulong board_get_usable_ram_top(ulong total_size)
+{
+	if (sdram_calculate_size() > SZ_2G)
+		return SZ_2G;
+	else
+		return sdram_calculate_size();
+}
+#endif
-- 
2.19.0

[U-Boot] [PATCH 05/10] board: altera: Stratix10: Add board_get_usable_ram_top()

[Marek Vasut]

On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> Add board_get_usable_ram_top() function. Limit maximum usable
> ram top to 2GB.

Why ? There are ARM64 platforms which can access the entire DRAM range
just fine, what's the problem ?

> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> ---
>  board/altera/stratix10-socdk/socfpga.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/board/altera/stratix10-socdk/socfpga.c b/board/altera/stratix10-socdk/socfpga.c
> index 043fc543f1..99c10d313c 100644
> --- a/board/altera/stratix10-socdk/socfpga.c
> +++ b/board/altera/stratix10-socdk/socfpga.c
> @@ -5,3 +5,15 @@
>   */
>  
>  #include <common.h>
> +#include <asm/arch/sdram.h>
> +#include <linux/sizes.h>
> +
> +#ifdef CONFIG_ALTERA_SDRAM
> +ulong board_get_usable_ram_top(ulong total_size)
> +{
> +	if (sdram_calculate_size() > SZ_2G)
> +		return SZ_2G;
> +	else
> +		return sdram_calculate_size();
> +}
> +#endif
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 05/10] board: altera: Stratix10: Add board_get_usable_ram_top()

[Westergreen, Dalon]

On Tue, 2019-03-12 at 11:46 +0100, Marek Vasut wrote:
> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> > Add board_get_usable_ram_top() function. Limit maximum usable
> > ram top to 2GB.
> 
> Why ? There are ARM64 platforms which can access the entire DRAM range
> just fine, what's the problem ?
> 

The issue is the gap in memory between 2GB and 4GB.  There is some trickery
you can use to gain access to the memory in that range, but in general, you
dont have access.  I believe just setting the banks up in the dts will
resolve this.

--dalon

> > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > ---
> >  board/altera/stratix10-socdk/socfpga.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/board/altera/stratix10-socdk/socfpga.c
> > b/board/altera/stratix10-socdk/socfpga.c
> > index 043fc543f1..99c10d313c 100644
> > --- a/board/altera/stratix10-socdk/socfpga.c
> > +++ b/board/altera/stratix10-socdk/socfpga.c
> > @@ -5,3 +5,15 @@
> >   */
> >  
> >  #include <common.h>
> > +#include <asm/arch/sdram.h>
> > +#include <linux/sizes.h>
> > +
> > +#ifdef CONFIG_ALTERA_SDRAM
> > +ulong board_get_usable_ram_top(ulong total_size)
> > +{
> > +	if (sdram_calculate_size() > SZ_2G)
> > +		return SZ_2G;
> > +	else
> > +		return sdram_calculate_size();
> > +}
> > +#endif
> > 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3282 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190312/7396aa26/attachment.bin>

[U-Boot] [PATCH 05/10] board: altera: Stratix10: Add board_get_usable_ram_top()

[Marek Vasut]

On 3/12/19 3:33 PM, Westergreen, Dalon wrote:
> On Tue, 2019-03-12 at 11:46 +0100, Marek Vasut wrote:
>> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
>>> Add board_get_usable_ram_top() function. Limit maximum usable
>>> ram top to 2GB.
>>
>> Why ? There are ARM64 platforms which can access the entire DRAM range
>> just fine, what's the problem ?
>>
> 
> The issue is the gap in memory between 2GB and 4GB.  There is some trickery
> you can use to gain access to the memory in that range, but in general, you
> dont have access.  I believe just setting the banks up in the dts will
> resolve this.

E.g. the R8A779{5..9}* platforms also have gaps in memory and it all
works fine . So unless there's some other specialty, there should be no
problem.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 05/10] board: altera: Stratix10: Add board_get_usable_ram_top()

[Ley Foon Tan]

On Tue, Mar 12, 2019 at 10:40 PM Marek Vasut <marex@denx.de> wrote:
>
> On 3/12/19 3:33 PM, Westergreen, Dalon wrote:
> > On Tue, 2019-03-12 at 11:46 +0100, Marek Vasut wrote:
> >> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> >>> Add board_get_usable_ram_top() function. Limit maximum usable
> >>> ram top to 2GB.
> >>
> >> Why ? There are ARM64 platforms which can access the entire DRAM range
> >> just fine, what's the problem ?
> >>
> >
> > The issue is the gap in memory between 2GB and 4GB.  There is some trickery
> > you can use to gain access to the memory in that range, but in general, you
> > dont have access.  I believe just setting the banks up in the dts will
> > resolve this.
>
> E.g. the R8A779{5..9}* platforms also have gaps in memory and it all
> works fine . So unless there's some other specialty, there should be no
> problem.
>
Yes, we don't need this.

Regards
Ley Foon

[U-Boot] [PATCH 06/10] board: altera: Stratix10: Add ft_board_setup()

[Ley Foon Tan]

Add ft_board_setup() function to setup memory banks before
boot to Linux.

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 board/altera/stratix10-socdk/socfpga.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/board/altera/stratix10-socdk/socfpga.c b/board/altera/stratix10-socdk/socfpga.c
index 99c10d313c..ca6e0e9085 100644
--- a/board/altera/stratix10-socdk/socfpga.c
+++ b/board/altera/stratix10-socdk/socfpga.c
@@ -5,6 +5,7 @@
  */
 
 #include <common.h>
+#include <fdt_support.h>
 #include <asm/arch/sdram.h>
 #include <linux/sizes.h>
 
@@ -17,3 +18,27 @@ ulong board_get_usable_ram_top(ulong total_size)
 		return sdram_calculate_size();
 }
 #endif
+
+#ifdef CONFIG_OF_BOARD_SETUP
+int ft_board_setup(void *blob, bd_t *bd)
+{
+	int ret = 0;
+#ifdef CONFIG_OF_LIBFDT
+	int bank;
+	int actual_bank = 0;
+	u64 start[CONFIG_NR_DRAM_BANKS];
+	u64 size[CONFIG_NR_DRAM_BANKS];
+
+	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
+		if (bd->bi_dram[bank].size) {
+			start[actual_bank] = bd->bi_dram[bank].start;
+			size[actual_bank++] = bd->bi_dram[bank].size;
+		}
+	}
+
+	ret = fdt_fixup_memory_banks(blob, start, size, actual_bank);
+#endif /* CONFIG_OF_LIBFDT */
+
+	return ret;
+}
+#endif
-- 
2.19.0

[U-Boot] [PATCH 06/10] board: altera: Stratix10: Add ft_board_setup()

[Marek Vasut]

On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> Add ft_board_setup() function to setup memory banks before
> boot to Linux.

Shouldn't bootm/booti be doing just that already ?

> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> ---
>  board/altera/stratix10-socdk/socfpga.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/board/altera/stratix10-socdk/socfpga.c b/board/altera/stratix10-socdk/socfpga.c
> index 99c10d313c..ca6e0e9085 100644
> --- a/board/altera/stratix10-socdk/socfpga.c
> +++ b/board/altera/stratix10-socdk/socfpga.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <common.h>
> +#include <fdt_support.h>
>  #include <asm/arch/sdram.h>
>  #include <linux/sizes.h>
>  
> @@ -17,3 +18,27 @@ ulong board_get_usable_ram_top(ulong total_size)
>  		return sdram_calculate_size();
>  }
>  #endif
> +
> +#ifdef CONFIG_OF_BOARD_SETUP
> +int ft_board_setup(void *blob, bd_t *bd)
> +{
> +	int ret = 0;
> +#ifdef CONFIG_OF_LIBFDT
> +	int bank;
> +	int actual_bank = 0;
> +	u64 start[CONFIG_NR_DRAM_BANKS];
> +	u64 size[CONFIG_NR_DRAM_BANKS];
> +
> +	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> +		if (bd->bi_dram[bank].size) {
> +			start[actual_bank] = bd->bi_dram[bank].start;
> +			size[actual_bank++] = bd->bi_dram[bank].size;
> +		}
> +	}
> +
> +	ret = fdt_fixup_memory_banks(blob, start, size, actual_bank);
> +#endif /* CONFIG_OF_LIBFDT */
> +
> +	return ret;
> +}
> +#endif
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 06/10] board: altera: Stratix10: Add ft_board_setup()

[Ley Foon Tan]

On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>
> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> > Add ft_board_setup() function to setup memory banks before
> > boot to Linux.
>
> Shouldn't bootm/booti be doing just that already ?
Don't need this if we use fdtdec_setup_memory_banksize().

>
> > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > ---
> >  board/altera/stratix10-socdk/socfpga.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/board/altera/stratix10-socdk/socfpga.c b/board/altera/stratix10-socdk/socfpga.c
> > index 99c10d313c..ca6e0e9085 100644
> > --- a/board/altera/stratix10-socdk/socfpga.c
> > +++ b/board/altera/stratix10-socdk/socfpga.c
> > @@ -5,6 +5,7 @@
> >   */
> >
> >  #include <common.h>
> > +#include <fdt_support.h>
> >  #include <asm/arch/sdram.h>
> >  #include <linux/sizes.h>
> >
> > @@ -17,3 +18,27 @@ ulong board_get_usable_ram_top(ulong total_size)
> >               return sdram_calculate_size();
> >  }
> >  #endif
> > +
> > +#ifdef CONFIG_OF_BOARD_SETUP
> > +int ft_board_setup(void *blob, bd_t *bd)
> > +{
> > +     int ret = 0;
> > +#ifdef CONFIG_OF_LIBFDT
> > +     int bank;
> > +     int actual_bank = 0;
> > +     u64 start[CONFIG_NR_DRAM_BANKS];
> > +     u64 size[CONFIG_NR_DRAM_BANKS];
> > +
> > +     for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> > +             if (bd->bi_dram[bank].size) {
> > +                     start[actual_bank] = bd->bi_dram[bank].start;
> > +                     size[actual_bank++] = bd->bi_dram[bank].size;
> > +             }
> > +     }
> > +
> > +     ret = fdt_fixup_memory_banks(blob, start, size, actual_bank);
> > +#endif /* CONFIG_OF_LIBFDT */
> > +
> > +     return ret;
> > +}
> > +#endif
> >
Regards
Ley Foon

[U-Boot] [PATCH 07/10] arm: socfpga: Enable OF_BOARD_SETUP for Stratix 10

[Ley Foon Tan]

Enable OF_BOARD_SETUP for Stratix 10.

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 arch/arm/mach-socfpga/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/Kconfig
index 5e87371f8c..990e46fe06 100644
--- a/arch/arm/mach-socfpga/Kconfig
+++ b/arch/arm/mach-socfpga/Kconfig
@@ -36,6 +36,7 @@ config TARGET_SOCFPGA_STRATIX10
 	select ARMV8_SET_SMPEN
 	select ARMV8_SPIN_TABLE
 	select FPGA_STRATIX10
+	select OF_BOARD_SETUP
 
 choice
 	prompt "Altera SOCFPGA board select"
-- 
2.19.0

[U-Boot] [PATCH 08/10] configs: stratix10: Change CONFIG_NR_DRAM_BANKS to 2

[Ley Foon Tan]

Stratix 10 SoC support 2 DRAM banks.

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 configs/socfpga_stratix10_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig
index 9e6d582ee3..4a14ea039e 100644
--- a/configs/socfpga_stratix10_defconfig
+++ b/configs/socfpga_stratix10_defconfig
@@ -6,7 +6,7 @@ CONFIG_TARGET_SOCFPGA_STRATIX10_SOCDK=y
 CONFIG_SPL=y
 CONFIG_IDENT_STRING="socfpga_stratix10"
 CONFIG_SPL_FS_FAT=y
-CONFIG_NR_DRAM_BANKS=1
+CONFIG_NR_DRAM_BANKS=2
 CONFIG_BOOTDELAY=5
 CONFIG_SPL_SPI_LOAD=y
 CONFIG_HUSH_PARSER=y
-- 
2.19.0

[U-Boot] [PATCH 09/10] arm: socfpga: stratix10: Add cpu_has_been_warmreset()

[Ley Foon Tan]

Add helper function cpu_has_been_warmreset() to check
if CPU is from warm reset boot.

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 arch/arm/mach-socfpga/include/mach/reset_manager_s10.h | 3 +++
 arch/arm/mach-socfpga/reset_manager_s10.c              | 9 +++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/mach-socfpga/include/mach/reset_manager_s10.h b/arch/arm/mach-socfpga/include/mach/reset_manager_s10.h
index 31b73edabe..e186296791 100644
--- a/arch/arm/mach-socfpga/include/mach/reset_manager_s10.h
+++ b/arch/arm/mach-socfpga/include/mach/reset_manager_s10.h
@@ -9,6 +9,7 @@
 
 void reset_cpu(ulong addr);
 void reset_deassert_peripherals_handoff(void);
+int cpu_has_been_warmreset(void);
 
 void socfpga_bridges_reset(int enable);
 
@@ -47,6 +48,8 @@ struct socfpga_reset_manager {
 #define RSTMGR_MPUMODRST_CORE0		0
 #define RSTMGR_PER0MODRST_OCP_MASK	0x0020bf00
 #define RSTMGR_BRGMODRST_DDRSCH_MASK	0X00000040
+/* Watchdogs and MPU warm reset mask */
+#define RSTMGR_L4WD_MPU_WARMRESET_MASK	0x000F0F00
 
 /*
  * Define a reset identifier, from which a permodrst bank ID
diff --git a/arch/arm/mach-socfpga/reset_manager_s10.c b/arch/arm/mach-socfpga/reset_manager_s10.c
index f176c38495..f8dd787cc6 100644
--- a/arch/arm/mach-socfpga/reset_manager_s10.c
+++ b/arch/arm/mach-socfpga/reset_manager_s10.c
@@ -103,3 +103,12 @@ void reset_deassert_peripherals_handoff(void)
 	writel(~RSTMGR_PER0MODRST_OCP_MASK, &reset_manager_base->per0modrst);
 	writel(0, &reset_manager_base->per0modrst);
 }
+
+/*
+ * Return non-zero if the CPU has been warm reset
+ */
+int cpu_has_been_warmreset(void)
+{
+	return readl(&reset_manager_base->status) &
+		RSTMGR_L4WD_MPU_WARMRESET_MASK;
+}
-- 
2.19.0

[U-Boot] [PATCH 10/10] ddr: altera: Stratix10: Add ECC memory scrubbing

[Ley Foon Tan]

Scrub memory content if ECC is enabled and it is not
from warm reset boot.

Enable icache and dcache before scrub memory
and use "DC ZVA" instruction to clear memory
to zeros. This instruction writes a cache line
at a time and it can prevent false ECC error
trigger if write cache line partially.

Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 .../arm/mach-socfpga/include/mach/sdram_s10.h |  9 +++
 drivers/ddr/altera/sdram_s10.c                | 76 +++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/arch/arm/mach-socfpga/include/mach/sdram_s10.h b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
index 89e355010d..354f80bfce 100644
--- a/arch/arm/mach-socfpga/include/mach/sdram_s10.h
+++ b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
@@ -23,6 +23,7 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
 #define ECCCTRL1			0x100
 #define ECCCTRL2			0x104
 #define ERRINTEN			0x110
+#define ERRINTENS			0x114
 #define INTMODE				0x11c
 #define INTSTAT				0x120
 #define AUTOWB_CORRADDR			0x138
@@ -53,6 +54,10 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
 #define DDR_HMC_SEQ2CORE_INT_RESP_MASK		BIT(3)
 #define DDR_HMC_HPSINTFCSEL_ENABLE_MASK		0x001f1f1f
 
+#define	DDR_HMC_ERRINTEN_INTMASK				\
+		(DDR_HMC_ERRINTEN_SERRINTEN_EN_SET_MSK |	\
+		 DDR_HMC_ERRINTEN_DERRINTEN_EN_SET_MSK)
+
 /* NOC DDR scheduler */
 #define DDR_SCH_ID_COREID		0
 #define DDR_SCH_ID_REVID		0x4
@@ -181,4 +186,8 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
 #define CALTIMING9_CFG_4_ACT_TO_ACT(x)			\
 	(((x) >> 0) & 0xFF)
 
+/* Firewall DDR scheduler MPFE */
+#define FW_HMC_ADAPTOR_REG_ADDR			0xf8020004
+#define FW_HMC_ADAPTOR_MPU_MASK			BIT(0)
+
 #endif /* _SDRAM_S10_H_ */
diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
index ae4e5ea2fd..2c691d3bee 100644
--- a/drivers/ddr/altera/sdram_s10.c
+++ b/drivers/ddr/altera/sdram_s10.c
@@ -22,6 +22,8 @@ static const struct socfpga_system_manager *sysmgr_regs =
 
 #define DDR_CONFIG(A, B, C, R)	(((A) << 24) | ((B) << 16) | ((C) << 8) | (R))
 
+#define PGTABLE_OFF	0x4000
+
 /* The followring are the supported configurations */
 u32 ddr_config[] = {
 	/* DDR_CONFIG(Address order,Bank,Column,Row) */
@@ -135,6 +137,71 @@ static int poll_hmc_clock_status(void)
 				 SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
 }
 
+static void sdram_clear_mem(phys_addr_t addr, phys_size_t size)
+{
+	phys_size_t i;
+
+	if (addr % CONFIG_SYS_CACHELINE_SIZE) {
+		printf("DDR: address 0x%lx not cacheline size aligned.\n",
+		       (ulong)addr);
+		hang();
+	}
+
+	if (size % CONFIG_SYS_CACHELINE_SIZE) {
+		printf("DDR: size 0x%lx not multiple of cacheline size\n",
+		       (ulong)size);
+		hang();
+	}
+
+	/* Use DC ZVA instruction to clear memory to zeros by a cache line */
+	for (i = 0; i < size; i = i + CONFIG_SYS_CACHELINE_SIZE) {
+		asm("dc zva, %0"
+		     :
+		     : "r"(addr));
+		addr += CONFIG_SYS_CACHELINE_SIZE;
+	}
+}
+
+static void sdram_init_ecc_bits(phys_addr_t *bank_start, phys_size_t *bank_size)
+{
+	phys_size_t size, size_init;
+	phys_addr_t start_addr;
+	int bank;
+	unsigned int start = get_timer(0);
+
+	icache_enable();
+
+	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
+		start_addr = bank_start[bank];
+		size = bank_size[bank];
+
+		if (bank == 0) {
+			/* Initialize small block for page table */
+			memset((void *)start_addr, 0,
+			       PGTABLE_SIZE + PGTABLE_OFF);
+			gd->arch.tlb_addr = start_addr + PGTABLE_OFF;
+			gd->arch.tlb_size = PGTABLE_SIZE;
+			start_addr += PGTABLE_SIZE + PGTABLE_OFF;
+			size -= (PGTABLE_OFF + PGTABLE_SIZE);
+			dcache_enable();
+		}
+
+		while (size) {
+			size_init = min((phys_addr_t)SZ_1G, (phys_addr_t)size);
+			sdram_clear_mem(start_addr, size_init);
+			size -= size_init;
+			start_addr += size_init;
+			WATCHDOG_RESET();
+		}
+	}
+
+	dcache_disable();
+	icache_disable();
+
+	printf("SDRAM-ECC: Initialized success with %d ms\n",
+	       (unsigned int)get_timer(start));
+}
+
 static void sdram_size_check(phys_addr_t bank_start[], phys_size_t bank_size[])
 {
 	phys_size_t total_ram_check = 0;
@@ -451,6 +518,15 @@ int sdram_mmr_init_full(unsigned int unused)
 		setbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL2,
 			     (DDR_HMC_ECCCTL2_RMW_EN_SET_MSK |
 			      DDR_HMC_ECCCTL2_AWB_EN_SET_MSK));
+		writel(DDR_HMC_ERRINTEN_INTMASK,
+		       SOCFPGA_SDR_ADDRESS + ERRINTENS);
+
+		/* Enable non-secure writes to HMC Adapter for SDRAM ECC */
+		writel(FW_HMC_ADAPTOR_MPU_MASK, FW_HMC_ADAPTOR_REG_ADDR);
+
+		/* Initialize memory content if not from warm reset */
+		if (!cpu_has_been_warmreset())
+			sdram_init_ecc_bits(bank_start, bank_size);
 	} else {
 		clrbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1,
 			     (DDR_HMC_ECCCTL_AWB_CNT_RST_SET_MSK |
-- 
2.19.0

[U-Boot] [PATCH 10/10] ddr: altera: Stratix10: Add ECC memory scrubbing

[Marek Vasut]

On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> Scrub memory content if ECC is enabled and it is not
> from warm reset boot.
> 
> Enable icache and dcache before scrub memory
> and use "DC ZVA" instruction to clear memory
> to zeros. This instruction writes a cache line
> at a time and it can prevent false ECC error
> trigger if write cache line partially.
> 
> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> ---
>  .../arm/mach-socfpga/include/mach/sdram_s10.h |  9 +++
>  drivers/ddr/altera/sdram_s10.c                | 76 +++++++++++++++++++
>  2 files changed, 85 insertions(+)
> 
> diff --git a/arch/arm/mach-socfpga/include/mach/sdram_s10.h b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> index 89e355010d..354f80bfce 100644
> --- a/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> +++ b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> @@ -23,6 +23,7 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>  #define ECCCTRL1			0x100
>  #define ECCCTRL2			0x104
>  #define ERRINTEN			0x110
> +#define ERRINTENS			0x114
>  #define INTMODE				0x11c
>  #define INTSTAT				0x120
>  #define AUTOWB_CORRADDR			0x138
> @@ -53,6 +54,10 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>  #define DDR_HMC_SEQ2CORE_INT_RESP_MASK		BIT(3)
>  #define DDR_HMC_HPSINTFCSEL_ENABLE_MASK		0x001f1f1f
>  
> +#define	DDR_HMC_ERRINTEN_INTMASK				\
> +		(DDR_HMC_ERRINTEN_SERRINTEN_EN_SET_MSK |	\
> +		 DDR_HMC_ERRINTEN_DERRINTEN_EN_SET_MSK)
> +
>  /* NOC DDR scheduler */
>  #define DDR_SCH_ID_COREID		0
>  #define DDR_SCH_ID_REVID		0x4
> @@ -181,4 +186,8 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>  #define CALTIMING9_CFG_4_ACT_TO_ACT(x)			\
>  	(((x) >> 0) & 0xFF)
>  
> +/* Firewall DDR scheduler MPFE */
> +#define FW_HMC_ADAPTOR_REG_ADDR			0xf8020004
> +#define FW_HMC_ADAPTOR_MPU_MASK			BIT(0)
> +
>  #endif /* _SDRAM_S10_H_ */
> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
> index ae4e5ea2fd..2c691d3bee 100644
> --- a/drivers/ddr/altera/sdram_s10.c
> +++ b/drivers/ddr/altera/sdram_s10.c
> @@ -22,6 +22,8 @@ static const struct socfpga_system_manager *sysmgr_regs =
>  
>  #define DDR_CONFIG(A, B, C, R)	(((A) << 24) | ((B) << 16) | ((C) << 8) | (R))
>  
> +#define PGTABLE_OFF	0x4000
> +
>  /* The followring are the supported configurations */
>  u32 ddr_config[] = {
>  	/* DDR_CONFIG(Address order,Bank,Column,Row) */
> @@ -135,6 +137,71 @@ static int poll_hmc_clock_status(void)
>  				 SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
>  }
>  
> +static void sdram_clear_mem(phys_addr_t addr, phys_size_t size)
> +{
> +	phys_size_t i;
> +
> +	if (addr % CONFIG_SYS_CACHELINE_SIZE) {
> +		printf("DDR: address 0x%lx not cacheline size aligned.\n",
> +		       (ulong)addr);

Is the cast needed ?

> +		hang();
> +	}
> +
> +	if (size % CONFIG_SYS_CACHELINE_SIZE) {
> +		printf("DDR: size 0x%lx not multiple of cacheline size\n",
> +		       (ulong)size);
> +		hang();
> +	}
> +
> +	/* Use DC ZVA instruction to clear memory to zeros by a cache line */
> +	for (i = 0; i < size; i = i + CONFIG_SYS_CACHELINE_SIZE) {
> +		asm("dc zva, %0"
> +		     :
> +		     : "r"(addr));

Should be asm volatile, so the compiler won't move this around.
Also, you want memory clobber here I think ?

> +		addr += CONFIG_SYS_CACHELINE_SIZE;
> +	}
> +}
> +
> +static void sdram_init_ecc_bits(phys_addr_t *bank_start, phys_size_t *bank_size)
> +{
> +	phys_size_t size, size_init;
> +	phys_addr_t start_addr;
> +	int bank;
> +	unsigned int start = get_timer(0);
> +
> +	icache_enable();
> +
> +	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> +		start_addr = bank_start[bank];
> +		size = bank_size[bank];
> +
> +		if (bank == 0) {
> +			/* Initialize small block for page table */
> +			memset((void *)start_addr, 0,
> +			       PGTABLE_SIZE + PGTABLE_OFF);
> +			gd->arch.tlb_addr = start_addr + PGTABLE_OFF;
> +			gd->arch.tlb_size = PGTABLE_SIZE;
> +			start_addr += PGTABLE_SIZE + PGTABLE_OFF;
> +			size -= (PGTABLE_OFF + PGTABLE_SIZE);
> +			dcache_enable();

If it's only done for bank0, pull this code out of the loop ?

> +		}
> +
> +		while (size) {
> +			size_init = min((phys_addr_t)SZ_1G, (phys_addr_t)size);
> +			sdram_clear_mem(start_addr, size_init);
> +			size -= size_init;
> +			start_addr += size_init;
> +			WATCHDOG_RESET();
> +		}
> +	}
> +
> +	dcache_disable();
> +	icache_disable();
> +
> +	printf("SDRAM-ECC: Initialized success with %d ms\n",
> +	       (unsigned int)get_timer(start));
> +}
> +
>  static void sdram_size_check(phys_addr_t bank_start[], phys_size_t bank_size[])
>  {
>  	phys_size_t total_ram_check = 0;
> @@ -451,6 +518,15 @@ int sdram_mmr_init_full(unsigned int unused)
>  		setbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL2,
>  			     (DDR_HMC_ECCCTL2_RMW_EN_SET_MSK |
>  			      DDR_HMC_ECCCTL2_AWB_EN_SET_MSK));
> +		writel(DDR_HMC_ERRINTEN_INTMASK,
> +		       SOCFPGA_SDR_ADDRESS + ERRINTENS);
> +
> +		/* Enable non-secure writes to HMC Adapter for SDRAM ECC */
> +		writel(FW_HMC_ADAPTOR_MPU_MASK, FW_HMC_ADAPTOR_REG_ADDR);
> +
> +		/* Initialize memory content if not from warm reset */
> +		if (!cpu_has_been_warmreset())
> +			sdram_init_ecc_bits(bank_start, bank_size);
>  	} else {
>  		clrbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1,
>  			     (DDR_HMC_ECCCTL_AWB_CNT_RST_SET_MSK |
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 10/10] ddr: altera: Stratix10: Add ECC memory scrubbing

[Ley Foon Tan]

On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>
> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> > Scrub memory content if ECC is enabled and it is not
> > from warm reset boot.
> >
> > Enable icache and dcache before scrub memory
> > and use "DC ZVA" instruction to clear memory
> > to zeros. This instruction writes a cache line
> > at a time and it can prevent false ECC error
> > trigger if write cache line partially.
> >
> > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > ---
> >  .../arm/mach-socfpga/include/mach/sdram_s10.h |  9 +++
> >  drivers/ddr/altera/sdram_s10.c                | 76 +++++++++++++++++++
> >  2 files changed, 85 insertions(+)
> >
> > diff --git a/arch/arm/mach-socfpga/include/mach/sdram_s10.h b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> > index 89e355010d..354f80bfce 100644
> > --- a/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> > +++ b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> > @@ -23,6 +23,7 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
> >  #define ECCCTRL1                     0x100
> >  #define ECCCTRL2                     0x104
> >  #define ERRINTEN                     0x110
> > +#define ERRINTENS                    0x114
> >  #define INTMODE                              0x11c
> >  #define INTSTAT                              0x120
> >  #define AUTOWB_CORRADDR                      0x138
> > @@ -53,6 +54,10 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
> >  #define DDR_HMC_SEQ2CORE_INT_RESP_MASK               BIT(3)
> >  #define DDR_HMC_HPSINTFCSEL_ENABLE_MASK              0x001f1f1f
> >
> > +#define      DDR_HMC_ERRINTEN_INTMASK                                \
> > +             (DDR_HMC_ERRINTEN_SERRINTEN_EN_SET_MSK |        \
> > +              DDR_HMC_ERRINTEN_DERRINTEN_EN_SET_MSK)
> > +
> >  /* NOC DDR scheduler */
> >  #define DDR_SCH_ID_COREID            0
> >  #define DDR_SCH_ID_REVID             0x4
> > @@ -181,4 +186,8 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
> >  #define CALTIMING9_CFG_4_ACT_TO_ACT(x)                       \
> >       (((x) >> 0) & 0xFF)
> >
> > +/* Firewall DDR scheduler MPFE */
> > +#define FW_HMC_ADAPTOR_REG_ADDR                      0xf8020004
> > +#define FW_HMC_ADAPTOR_MPU_MASK                      BIT(0)
> > +
> >  #endif /* _SDRAM_S10_H_ */
> > diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
> > index ae4e5ea2fd..2c691d3bee 100644
> > --- a/drivers/ddr/altera/sdram_s10.c
> > +++ b/drivers/ddr/altera/sdram_s10.c
> > @@ -22,6 +22,8 @@ static const struct socfpga_system_manager *sysmgr_regs =
> >
> >  #define DDR_CONFIG(A, B, C, R)       (((A) << 24) | ((B) << 16) | ((C) << 8) | (R))
> >
> > +#define PGTABLE_OFF  0x4000
> > +
> >  /* The followring are the supported configurations */
> >  u32 ddr_config[] = {
> >       /* DDR_CONFIG(Address order,Bank,Column,Row) */
> > @@ -135,6 +137,71 @@ static int poll_hmc_clock_status(void)
> >                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
> >  }
> >
> > +static void sdram_clear_mem(phys_addr_t addr, phys_size_t size)
> > +{
> > +     phys_size_t i;
> > +
> > +     if (addr % CONFIG_SYS_CACHELINE_SIZE) {
> > +             printf("DDR: address 0x%lx not cacheline size aligned.\n",
> > +                    (ulong)addr);
>
> Is the cast needed ?
Yes, SPL doesn't support %llx, we need cast to ulong %lx.
>
> > +             hang();
> > +     }
> > +
> > +     if (size % CONFIG_SYS_CACHELINE_SIZE) {
> > +             printf("DDR: size 0x%lx not multiple of cacheline size\n",
> > +                    (ulong)size);
> > +             hang();
> > +     }
> > +
> > +     /* Use DC ZVA instruction to clear memory to zeros by a cache line */
> > +     for (i = 0; i < size; i = i + CONFIG_SYS_CACHELINE_SIZE) {
> > +             asm("dc zva, %0"
> > +                  :
> > +                  : "r"(addr));
>
> Should be asm volatile, so the compiler won't move this around.
Okay.
> Also, you want memory clobber here I think ?
Yes, will add "memory" clobber here.
>
> > +             addr += CONFIG_SYS_CACHELINE_SIZE;
> > +     }
> > +}
> > +
> > +static void sdram_init_ecc_bits(phys_addr_t *bank_start, phys_size_t *bank_size)
> > +{
> > +     phys_size_t size, size_init;
> > +     phys_addr_t start_addr;
> > +     int bank;
> > +     unsigned int start = get_timer(0);
> > +
> > +     icache_enable();
> > +
> > +     for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> > +             start_addr = bank_start[bank];
> > +             size = bank_size[bank];
> > +
> > +             if (bank == 0) {
> > +                     /* Initialize small block for page table */
> > +                     memset((void *)start_addr, 0,
> > +                            PGTABLE_SIZE + PGTABLE_OFF);
> > +                     gd->arch.tlb_addr = start_addr + PGTABLE_OFF;
> > +                     gd->arch.tlb_size = PGTABLE_SIZE;
> > +                     start_addr += PGTABLE_SIZE + PGTABLE_OFF;
> > +                     size -= (PGTABLE_OFF + PGTABLE_SIZE);
> > +                     dcache_enable();
>
> If it's only done for bank0, pull this code out of the loop ?
Okay.
>
> > +             }
> > +
> > +             while (size) {
> > +                     size_init = min((phys_addr_t)SZ_1G, (phys_addr_t)size);
> > +                     sdram_clear_mem(start_addr, size_init);
> > +                     size -= size_init;
> > +                     start_addr += size_init;
> > +                     WATCHDOG_RESET();
> > +             }
> > +     }
> > +
> > +     dcache_disable();
> > +     icache_disable();
> > +
> > +     printf("SDRAM-ECC: Initialized success with %d ms\n",
> > +            (unsigned int)get_timer(start));
> > +}
> > +
> >  static void sdram_size_check(phys_addr_t bank_start[], phys_size_t bank_size[])
> >  {
> >       phys_size_t total_ram_check = 0;
> > @@ -451,6 +518,15 @@ int sdram_mmr_init_full(unsigned int unused)
> >               setbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL2,
> >                            (DDR_HMC_ECCCTL2_RMW_EN_SET_MSK |
> >                             DDR_HMC_ECCCTL2_AWB_EN_SET_MSK));
> > +             writel(DDR_HMC_ERRINTEN_INTMASK,
> > +                    SOCFPGA_SDR_ADDRESS + ERRINTENS);
> > +
> > +             /* Enable non-secure writes to HMC Adapter for SDRAM ECC */
> > +             writel(FW_HMC_ADAPTOR_MPU_MASK, FW_HMC_ADAPTOR_REG_ADDR);
> > +
> > +             /* Initialize memory content if not from warm reset */
> > +             if (!cpu_has_been_warmreset())
> > +                     sdram_init_ecc_bits(bank_start, bank_size);
> >       } else {
> >               clrbits_le32(SOCFPGA_SDR_ADDRESS + ECCCTRL1,
> >                            (DDR_HMC_ECCCTL_AWB_CNT_RST_SET_MSK |
> >
>
>
> --
> Best regards,
> Marek Vasut

[U-Boot] [PATCH 10/10] ddr: altera: Stratix10: Add ECC memory scrubbing

[Marek Vasut]

On 3/19/19 4:14 AM, Ley Foon Tan wrote:
> On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
>>> Scrub memory content if ECC is enabled and it is not
>>> from warm reset boot.
>>>
>>> Enable icache and dcache before scrub memory
>>> and use "DC ZVA" instruction to clear memory
>>> to zeros. This instruction writes a cache line
>>> at a time and it can prevent false ECC error
>>> trigger if write cache line partially.
>>>
>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>> ---
>>>  .../arm/mach-socfpga/include/mach/sdram_s10.h |  9 +++
>>>  drivers/ddr/altera/sdram_s10.c                | 76 +++++++++++++++++++
>>>  2 files changed, 85 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-socfpga/include/mach/sdram_s10.h b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
>>> index 89e355010d..354f80bfce 100644
>>> --- a/arch/arm/mach-socfpga/include/mach/sdram_s10.h
>>> +++ b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
>>> @@ -23,6 +23,7 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>>>  #define ECCCTRL1                     0x100
>>>  #define ECCCTRL2                     0x104
>>>  #define ERRINTEN                     0x110
>>> +#define ERRINTENS                    0x114
>>>  #define INTMODE                              0x11c
>>>  #define INTSTAT                              0x120
>>>  #define AUTOWB_CORRADDR                      0x138
>>> @@ -53,6 +54,10 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>>>  #define DDR_HMC_SEQ2CORE_INT_RESP_MASK               BIT(3)
>>>  #define DDR_HMC_HPSINTFCSEL_ENABLE_MASK              0x001f1f1f
>>>
>>> +#define      DDR_HMC_ERRINTEN_INTMASK                                \
>>> +             (DDR_HMC_ERRINTEN_SERRINTEN_EN_SET_MSK |        \
>>> +              DDR_HMC_ERRINTEN_DERRINTEN_EN_SET_MSK)
>>> +
>>>  /* NOC DDR scheduler */
>>>  #define DDR_SCH_ID_COREID            0
>>>  #define DDR_SCH_ID_REVID             0x4
>>> @@ -181,4 +186,8 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>>>  #define CALTIMING9_CFG_4_ACT_TO_ACT(x)                       \
>>>       (((x) >> 0) & 0xFF)
>>>
>>> +/* Firewall DDR scheduler MPFE */
>>> +#define FW_HMC_ADAPTOR_REG_ADDR                      0xf8020004
>>> +#define FW_HMC_ADAPTOR_MPU_MASK                      BIT(0)
>>> +
>>>  #endif /* _SDRAM_S10_H_ */
>>> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
>>> index ae4e5ea2fd..2c691d3bee 100644
>>> --- a/drivers/ddr/altera/sdram_s10.c
>>> +++ b/drivers/ddr/altera/sdram_s10.c
>>> @@ -22,6 +22,8 @@ static const struct socfpga_system_manager *sysmgr_regs =
>>>
>>>  #define DDR_CONFIG(A, B, C, R)       (((A) << 24) | ((B) << 16) | ((C) << 8) | (R))
>>>
>>> +#define PGTABLE_OFF  0x4000
>>> +
>>>  /* The followring are the supported configurations */
>>>  u32 ddr_config[] = {
>>>       /* DDR_CONFIG(Address order,Bank,Column,Row) */
>>> @@ -135,6 +137,71 @@ static int poll_hmc_clock_status(void)
>>>                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
>>>  }
>>>
>>> +static void sdram_clear_mem(phys_addr_t addr, phys_size_t size)
>>> +{
>>> +     phys_size_t i;
>>> +
>>> +     if (addr % CONFIG_SYS_CACHELINE_SIZE) {
>>> +             printf("DDR: address 0x%lx not cacheline size aligned.\n",
>>> +                    (ulong)addr);
>>
>> Is the cast needed ?
> Yes, SPL doesn't support %llx, we need cast to ulong %lx.

But that doesn't work for 64bit addresses ?
Isn't that limitation of tiny printf implementation instead of SPL ?

[...]

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 10/10] ddr: altera: Stratix10: Add ECC memory scrubbing

[Ley Foon Tan]

On Tue, Mar 19, 2019 at 5:39 PM Marek Vasut <marex@denx.de> wrote:
>
> On 3/19/19 4:14 AM, Ley Foon Tan wrote:
> > On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> >>> Scrub memory content if ECC is enabled and it is not
> >>> from warm reset boot.
> >>>
> >>> Enable icache and dcache before scrub memory
> >>> and use "DC ZVA" instruction to clear memory
> >>> to zeros. This instruction writes a cache line
> >>> at a time and it can prevent false ECC error
> >>> trigger if write cache line partially.
> >>>
> >>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> >>> ---
> >>>  .../arm/mach-socfpga/include/mach/sdram_s10.h |  9 +++
> >>>  drivers/ddr/altera/sdram_s10.c                | 76 +++++++++++++++++++
> >>>  2 files changed, 85 insertions(+)
> >>>
> >>> diff --git a/arch/arm/mach-socfpga/include/mach/sdram_s10.h b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> >>> index 89e355010d..354f80bfce 100644
> >>> --- a/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> >>> +++ b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
> >>> @@ -23,6 +23,7 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
> >>>  #define ECCCTRL1                     0x100
> >>>  #define ECCCTRL2                     0x104
> >>>  #define ERRINTEN                     0x110
> >>> +#define ERRINTENS                    0x114
> >>>  #define INTMODE                              0x11c
> >>>  #define INTSTAT                              0x120
> >>>  #define AUTOWB_CORRADDR                      0x138
> >>> @@ -53,6 +54,10 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
> >>>  #define DDR_HMC_SEQ2CORE_INT_RESP_MASK               BIT(3)
> >>>  #define DDR_HMC_HPSINTFCSEL_ENABLE_MASK              0x001f1f1f
> >>>
> >>> +#define      DDR_HMC_ERRINTEN_INTMASK                                \
> >>> +             (DDR_HMC_ERRINTEN_SERRINTEN_EN_SET_MSK |        \
> >>> +              DDR_HMC_ERRINTEN_DERRINTEN_EN_SET_MSK)
> >>> +
> >>>  /* NOC DDR scheduler */
> >>>  #define DDR_SCH_ID_COREID            0
> >>>  #define DDR_SCH_ID_REVID             0x4
> >>> @@ -181,4 +186,8 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
> >>>  #define CALTIMING9_CFG_4_ACT_TO_ACT(x)                       \
> >>>       (((x) >> 0) & 0xFF)
> >>>
> >>> +/* Firewall DDR scheduler MPFE */
> >>> +#define FW_HMC_ADAPTOR_REG_ADDR                      0xf8020004
> >>> +#define FW_HMC_ADAPTOR_MPU_MASK                      BIT(0)
> >>> +
> >>>  #endif /* _SDRAM_S10_H_ */
> >>> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
> >>> index ae4e5ea2fd..2c691d3bee 100644
> >>> --- a/drivers/ddr/altera/sdram_s10.c
> >>> +++ b/drivers/ddr/altera/sdram_s10.c
> >>> @@ -22,6 +22,8 @@ static const struct socfpga_system_manager *sysmgr_regs =
> >>>
> >>>  #define DDR_CONFIG(A, B, C, R)       (((A) << 24) | ((B) << 16) | ((C) << 8) | (R))
> >>>
> >>> +#define PGTABLE_OFF  0x4000
> >>> +
> >>>  /* The followring are the supported configurations */
> >>>  u32 ddr_config[] = {
> >>>       /* DDR_CONFIG(Address order,Bank,Column,Row) */
> >>> @@ -135,6 +137,71 @@ static int poll_hmc_clock_status(void)
> >>>                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
> >>>  }
> >>>
> >>> +static void sdram_clear_mem(phys_addr_t addr, phys_size_t size)
> >>> +{
> >>> +     phys_size_t i;
> >>> +
> >>> +     if (addr % CONFIG_SYS_CACHELINE_SIZE) {
> >>> +             printf("DDR: address 0x%lx not cacheline size aligned.\n",
> >>> +                    (ulong)addr);
> >>
> >> Is the cast needed ?
> > Yes, SPL doesn't support %llx, we need cast to ulong %lx.
>
> But that doesn't work for 64bit addresses ?
> Isn't that limitation of tiny printf implementation instead of SPL ?
>
%lx still work for 64 bit address.

Yes, I think it is limitation of  tiny printf in SPL.

Regards
Ley Foon

[U-Boot] [PATCH 10/10] ddr: altera: Stratix10: Add ECC memory scrubbing

[Marek Vasut]

On 3/19/19 10:49 AM, Ley Foon Tan wrote:
> On Tue, Mar 19, 2019 at 5:39 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/19/19 4:14 AM, Ley Foon Tan wrote:
>>> On Tue, Mar 12, 2019 at 7:03 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
>>>>> Scrub memory content if ECC is enabled and it is not
>>>>> from warm reset boot.
>>>>>
>>>>> Enable icache and dcache before scrub memory
>>>>> and use "DC ZVA" instruction to clear memory
>>>>> to zeros. This instruction writes a cache line
>>>>> at a time and it can prevent false ECC error
>>>>> trigger if write cache line partially.
>>>>>
>>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>>>> ---
>>>>>  .../arm/mach-socfpga/include/mach/sdram_s10.h |  9 +++
>>>>>  drivers/ddr/altera/sdram_s10.c                | 76 +++++++++++++++++++
>>>>>  2 files changed, 85 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/mach-socfpga/include/mach/sdram_s10.h b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
>>>>> index 89e355010d..354f80bfce 100644
>>>>> --- a/arch/arm/mach-socfpga/include/mach/sdram_s10.h
>>>>> +++ b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
>>>>> @@ -23,6 +23,7 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>>>>>  #define ECCCTRL1                     0x100
>>>>>  #define ECCCTRL2                     0x104
>>>>>  #define ERRINTEN                     0x110
>>>>> +#define ERRINTENS                    0x114
>>>>>  #define INTMODE                              0x11c
>>>>>  #define INTSTAT                              0x120
>>>>>  #define AUTOWB_CORRADDR                      0x138
>>>>> @@ -53,6 +54,10 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>>>>>  #define DDR_HMC_SEQ2CORE_INT_RESP_MASK               BIT(3)
>>>>>  #define DDR_HMC_HPSINTFCSEL_ENABLE_MASK              0x001f1f1f
>>>>>
>>>>> +#define      DDR_HMC_ERRINTEN_INTMASK                                \
>>>>> +             (DDR_HMC_ERRINTEN_SERRINTEN_EN_SET_MSK |        \
>>>>> +              DDR_HMC_ERRINTEN_DERRINTEN_EN_SET_MSK)
>>>>> +
>>>>>  /* NOC DDR scheduler */
>>>>>  #define DDR_SCH_ID_COREID            0
>>>>>  #define DDR_SCH_ID_REVID             0x4
>>>>> @@ -181,4 +186,8 @@ void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
>>>>>  #define CALTIMING9_CFG_4_ACT_TO_ACT(x)                       \
>>>>>       (((x) >> 0) & 0xFF)
>>>>>
>>>>> +/* Firewall DDR scheduler MPFE */
>>>>> +#define FW_HMC_ADAPTOR_REG_ADDR                      0xf8020004
>>>>> +#define FW_HMC_ADAPTOR_MPU_MASK                      BIT(0)
>>>>> +
>>>>>  #endif /* _SDRAM_S10_H_ */
>>>>> diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
>>>>> index ae4e5ea2fd..2c691d3bee 100644
>>>>> --- a/drivers/ddr/altera/sdram_s10.c
>>>>> +++ b/drivers/ddr/altera/sdram_s10.c
>>>>> @@ -22,6 +22,8 @@ static const struct socfpga_system_manager *sysmgr_regs =
>>>>>
>>>>>  #define DDR_CONFIG(A, B, C, R)       (((A) << 24) | ((B) << 16) | ((C) << 8) | (R))
>>>>>
>>>>> +#define PGTABLE_OFF  0x4000
>>>>> +
>>>>>  /* The followring are the supported configurations */
>>>>>  u32 ddr_config[] = {
>>>>>       /* DDR_CONFIG(Address order,Bank,Column,Row) */
>>>>> @@ -135,6 +137,71 @@ static int poll_hmc_clock_status(void)
>>>>>                                SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
>>>>>  }
>>>>>
>>>>> +static void sdram_clear_mem(phys_addr_t addr, phys_size_t size)
>>>>> +{
>>>>> +     phys_size_t i;
>>>>> +
>>>>> +     if (addr % CONFIG_SYS_CACHELINE_SIZE) {
>>>>> +             printf("DDR: address 0x%lx not cacheline size aligned.\n",
>>>>> +                    (ulong)addr);
>>>>
>>>> Is the cast needed ?
>>> Yes, SPL doesn't support %llx, we need cast to ulong %lx.
>>
>> But that doesn't work for 64bit addresses ?
>> Isn't that limitation of tiny printf implementation instead of SPL ?
>>
> %lx still work for 64 bit address.
> 
> Yes, I think it is limitation of  tiny printf in SPL.

Can the tiny printf be fixed ? I think it should be easy.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 00/10] Update Stratix 10 SDRAM driver

[Simon Goldschmidt]

On Tue, Mar 12, 2019 at 9:31 AM Ley Foon Tan <ley.foon.tan@intel.com> wrote:
>
> This patchset update Stratix 10 SDRAM driver to support:
> - Multi-banks memory (Patch [1-8])
>   - Stratix 10 support up to 2 memory banks:
>         Bank 0: Address 0, size 2GB
>         Bank 1: Address 0x100000000, size 124GB
> - Add warm reset boot checking function, to use in patch [10] (Patch[9])
> - Add ECC memory scrubbing support (Patch [10])
>   - Use cache enabled + "DC ZVA" instruction to clear memory to zeros

When working on this, would it make sense to move the S10 SDRAM driver
to DM, using UCLASS_RAM, like I did for gen5 (see Marek's 'next' branch:
http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=4796a7d5a7fd965c02cf290edec17acb2c9af766)

That might allow us to combine more of the code in mach-socfpga again
in the future...

Regards,
Simon

>
> Ley Foon Tan (10):
>   ddr: altera: stratix10: Move SDRAM size check to SDRAM driver
>   arm: socfpga: Add sdram_s10.h to sdram.h
>   ddr: altera: s10: Add multiple memory banks support
>   arm: socfpga: Update dram_init_banksize() for Stratix10
>   board: altera: Stratix10: Add board_get_usable_ram_top()
>   board: altera: Stratix10: Add ft_board_setup()
>   arm: socfpga: Enable OF_BOARD_SETUP for Stratix 10
>   configs: stratix10: Change CONFIG_NR_DRAM_BANKS to 2
>   arm: socfpga: stratix10: Add cpu_has_been_warmreset()
>   ddr: altera: Stratix10: Add ECC memory scrubbing
>
>  arch/arm/mach-socfpga/Kconfig                 |   1 +
>  arch/arm/mach-socfpga/board.c                 |  16 +-
>  .../include/mach/reset_manager_s10.h          |   3 +
>  arch/arm/mach-socfpga/include/mach/sdram.h    |   2 +
>  .../arm/mach-socfpga/include/mach/sdram_s10.h |  10 +
>  arch/arm/mach-socfpga/reset_manager_s10.c     |   9 +
>  arch/arm/mach-socfpga/spl_s10.c               |  11 --
>  board/altera/stratix10-socdk/socfpga.c        |  37 ++++
>  configs/socfpga_stratix10_defconfig           |   2 +-
>  drivers/ddr/altera/sdram_s10.c                | 178 ++++++++++++++++++
>  10 files changed, 256 insertions(+), 13 deletions(-)
>
> --
> 2.19.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 00/10] Update Stratix 10 SDRAM driver

[Ley Foon Tan]

On Tue, 2019-03-12 at 12:08 +0100, Simon Goldschmidt wrote:
> On Tue, Mar 12, 2019 at 9:31 AM Ley Foon Tan <ley.foon.tan@intel.com>
> wrote:
> > 
> > 
> > This patchset update Stratix 10 SDRAM driver to support:
> > - Multi-banks memory (Patch [1-8])
> >   - Stratix 10 support up to 2 memory banks:
> >         Bank 0: Address 0, size 2GB
> >         Bank 1: Address 0x100000000, size 124GB
> > - Add warm reset boot checking function, to use in patch [10]
> > (Patch[9])
> > - Add ECC memory scrubbing support (Patch [10])
> >   - Use cache enabled + "DC ZVA" instruction to clear memory to
> > zeros
> When working on this, would it make sense to move the S10 SDRAM
> driver
> to DM, using UCLASS_RAM, like I did for gen5 (see Marek's 'next'
> branch:
> http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=4796a7d5a7
> fd965c02cf290edec17acb2c9af766)
> 
> That might allow us to combine more of the code in mach-socfpga again
> in the future...
Sure, I will look into this. But will be in separate series.

Thanks.

Regards
Ley Foon
> 
> Regards,
> Simon
> 
> > 
> > 
> > Ley Foon Tan (10):
> >   ddr: altera: stratix10: Move SDRAM size check to SDRAM driver
> >   arm: socfpga: Add sdram_s10.h to sdram.h
> >   ddr: altera: s10: Add multiple memory banks support
> >   arm: socfpga: Update dram_init_banksize() for Stratix10
> >   board: altera: Stratix10: Add board_get_usable_ram_top()
> >   board: altera: Stratix10: Add ft_board_setup()
> >   arm: socfpga: Enable OF_BOARD_SETUP for Stratix 10
> >   configs: stratix10: Change CONFIG_NR_DRAM_BANKS to 2
> >   arm: socfpga: stratix10: Add cpu_has_been_warmreset()
> >   ddr: altera: Stratix10: Add ECC memory scrubbing
> > 
> >  arch/arm/mach-socfpga/Kconfig                 |   1 +
> >  arch/arm/mach-socfpga/board.c                 |  16 +-
> >  .../include/mach/reset_manager_s10.h          |   3 +
> >  arch/arm/mach-socfpga/include/mach/sdram.h    |   2 +
> >  .../arm/mach-socfpga/include/mach/sdram_s10.h |  10 +
> >  arch/arm/mach-socfpga/reset_manager_s10.c     |   9 +
> >  arch/arm/mach-socfpga/spl_s10.c               |  11 --
> >  board/altera/stratix10-socdk/socfpga.c        |  37 ++++
> >  configs/socfpga_stratix10_defconfig           |   2 +-
> >  drivers/ddr/altera/sdram_s10.c                | 178
> > ++++++++++++++++++
> >  10 files changed, 256 insertions(+), 13 deletions(-)
> > 
> > --
> > 2.19.0
> > 
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 00/10] Update Stratix 10 SDRAM driver

[Simon Goldschmidt]

On Thu, Mar 14, 2019 at 9:21 AM Ley Foon Tan <ley.foon.tan@intel.com> wrote:
>
> On Tue, 2019-03-12 at 12:08 +0100, Simon Goldschmidt wrote:
> > On Tue, Mar 12, 2019 at 9:31 AM Ley Foon Tan <ley.foon.tan@intel.com>
> > wrote:
> > >
> > >
> > > This patchset update Stratix 10 SDRAM driver to support:
> > > - Multi-banks memory (Patch [1-8])
> > >   - Stratix 10 support up to 2 memory banks:
> > >         Bank 0: Address 0, size 2GB
> > >         Bank 1: Address 0x100000000, size 124GB
> > > - Add warm reset boot checking function, to use in patch [10]
> > > (Patch[9])
> > > - Add ECC memory scrubbing support (Patch [10])
> > >   - Use cache enabled + "DC ZVA" instruction to clear memory to
> > > zeros
> > When working on this, would it make sense to move the S10 SDRAM
> > driver
> > to DM, using UCLASS_RAM, like I did for gen5 (see Marek's 'next'
> > branch:
> > http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=4796a7d5a7
> > fd965c02cf290edec17acb2c9af766)
> >
> > That might allow us to combine more of the code in mach-socfpga again
> > in the future...
> Sure, I will look into this. But will be in separate series.

Yes, it's a separate issue. It just came to mind when reading this series.

Regards,
Simon

>
> Thanks.
>
> Regards
> Ley Foon
> >
> > Regards,
> > Simon
> >
> > >
> > >
> > > Ley Foon Tan (10):
> > >   ddr: altera: stratix10: Move SDRAM size check to SDRAM driver
> > >   arm: socfpga: Add sdram_s10.h to sdram.h
> > >   ddr: altera: s10: Add multiple memory banks support
> > >   arm: socfpga: Update dram_init_banksize() for Stratix10
> > >   board: altera: Stratix10: Add board_get_usable_ram_top()
> > >   board: altera: Stratix10: Add ft_board_setup()
> > >   arm: socfpga: Enable OF_BOARD_SETUP for Stratix 10
> > >   configs: stratix10: Change CONFIG_NR_DRAM_BANKS to 2
> > >   arm: socfpga: stratix10: Add cpu_has_been_warmreset()
> > >   ddr: altera: Stratix10: Add ECC memory scrubbing
> > >
> > >  arch/arm/mach-socfpga/Kconfig                 |   1 +
> > >  arch/arm/mach-socfpga/board.c                 |  16 +-
> > >  .../include/mach/reset_manager_s10.h          |   3 +
> > >  arch/arm/mach-socfpga/include/mach/sdram.h    |   2 +
> > >  .../arm/mach-socfpga/include/mach/sdram_s10.h |  10 +
> > >  arch/arm/mach-socfpga/reset_manager_s10.c     |   9 +
> > >  arch/arm/mach-socfpga/spl_s10.c               |  11 --
> > >  board/altera/stratix10-socdk/socfpga.c        |  37 ++++
> > >  configs/socfpga_stratix10_defconfig           |   2 +-
> > >  drivers/ddr/altera/sdram_s10.c                | 178
> > > ++++++++++++++++++
> > >  10 files changed, 256 insertions(+), 13 deletions(-)
> > >
> > > --
> > > 2.19.0