| Message ID | 1552379474-12867-2-git-send-email-ley.foon.tan@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | Update Stratix 10 SDRAM driver | expand |
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; > } >
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
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 ? [...]
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
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.
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
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.
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
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; }
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(-)