| Message ID | 1462020342-4851-1-git-send-email-marex@denx.de |
|---|---|
| State | Accepted |
| Commit | 29b59353fe7cd62c74960b76e7b56bbc368429d2 |
| Delegated to: | Stefan Roese |
| Headers | show |
On 04/30/2016 02:45 PM, Marek Vasut wrote: > For reason unknown, recently, the DDR init code writers are really fond > of hiding some small floating point operating deep in their creations. > This patch removes one from the Marvell A38x code. > > Instead of returning size of chip as float from ddr3_get_device_size() > in GiB units, return it as int in MiB units. Since this would interfere > with the huge switch code in ddr3_calc_mem_cs_size(), rework the code > to match the change. > > Before this patch, the cs_mem_size variable could have these values: > ( { 16, 32 } x { 8, 16 } x { 0.01, 0.5, 1, 2, 4, 8 } ) / 8 = > { 0.000000, 0.001250, 0.002500, 0.005000, 0.062500, 0.125000, > 0.250000, 0.500000, 1.000000, 2.000000, 4.000000, } > The switch code checked for a subset of the resulting RAM sizes, which > is in range 128 MiB ... 2048 MiB. > > With this patch, the cs_mem_size variable can have these values: > ( { 16, 32 } x { 8, 16 } x { 0, 512, 1024, 2048, 4096, 8192 } ) / 8 = > { 0, 64, 128, 256, 512, 1024, 2048, 4096 } > To retain previous behavior, filter out 0 MiB (invalid size), 64 MiB > and 4096 MiB options. > > Removing the floating point stuff also saves 1.5k from text segment: > clearfog : spl/u-boot-spl:all -1592 spl/u-boot-spl:text -1592 > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Dirk Eibach <dirk.eibach@gdsys.cc> > Cc: Stefan Roese <sr@denx.de> Bump? > --- > drivers/ddr/marvell/a38x/ddr3_init.c | 29 ++++++++++------------------- > 1 file changed, 10 insertions(+), 19 deletions(-) > > diff --git a/drivers/ddr/marvell/a38x/ddr3_init.c b/drivers/ddr/marvell/a38x/ddr3_init.c > index ee05f57..55baad4 100644 > --- a/drivers/ddr/marvell/a38x/ddr3_init.c > +++ b/drivers/ddr/marvell/a38x/ddr3_init.c > @@ -678,7 +678,7 @@ u32 ddr3_get_device_width(u32 cs) > return (device_width == 0) ? 8 : 16; > } > > -float ddr3_get_device_size(u32 cs) > +static int ddr3_get_device_size(u32 cs) > { > u32 device_size_low, device_size_high, device_size; > u32 data, cs_low_offset, cs_high_offset; > @@ -695,15 +695,15 @@ float ddr3_get_device_size(u32 cs) > > switch (device_size) { > case 0: > - return 2; > + return 2048; > case 2: > - return 0.5; > + return 512; > case 3: > - return 1; > + return 1024; > case 4: > - return 4; > + return 4096; > case 5: > - return 8; > + return 8192; > case 1: > default: > DEBUG_INIT_C("Error: Wrong device size of Cs: ", cs, 1); > @@ -711,13 +711,13 @@ float ddr3_get_device_size(u32 cs) > * Small value will give wrong emem size in > * ddr3_calc_mem_cs_size > */ > - return 0.01; > + return 0; > } > } > > int ddr3_calc_mem_cs_size(u32 cs, u32 *cs_size) > { > - float cs_mem_size; > + int cs_mem_size; > > /* Calculate in GiB */ > cs_mem_size = ((ddr3_get_bus_width() / ddr3_get_device_width(cs)) * > @@ -731,21 +731,12 @@ int ddr3_calc_mem_cs_size(u32 cs, u32 *cs_size) > */ > cs_mem_size *= DDR_CONTROLLER_BUS_WIDTH_MULTIPLIER; > > - if (cs_mem_size == 0.125) { > - *cs_size = 128 << 20; > - } else if (cs_mem_size == 0.25) { > - *cs_size = 256 << 20; > - } else if (cs_mem_size == 0.5) { > - *cs_size = 512 << 20; > - } else if (cs_mem_size == 1) { > - *cs_size = 1 << 30; > - } else if (cs_mem_size == 2) { > - *cs_size = 2 << 30; > - } else { > + if (!cs_mem_size || (cs_mem_size == 64) || (cs_mem_size == 4096)) { > DEBUG_INIT_C("Error: Wrong Memory size of Cs: ", cs, 1); > return MV_BAD_VALUE; > } > > + *cs_size = cs_mem_size << 20; > return MV_OK; > } > >
On 30.04.2016 14:45, Marek Vasut wrote: > For reason unknown, recently, the DDR init code writers are really fond > of hiding some small floating point operating deep in their creations. > This patch removes one from the Marvell A38x code. > > Instead of returning size of chip as float from ddr3_get_device_size() > in GiB units, return it as int in MiB units. Since this would interfere > with the huge switch code in ddr3_calc_mem_cs_size(), rework the code > to match the change. > > Before this patch, the cs_mem_size variable could have these values: > ( { 16, 32 } x { 8, 16 } x { 0.01, 0.5, 1, 2, 4, 8 } ) / 8 = > { 0.000000, 0.001250, 0.002500, 0.005000, 0.062500, 0.125000, > 0.250000, 0.500000, 1.000000, 2.000000, 4.000000, } > The switch code checked for a subset of the resulting RAM sizes, which > is in range 128 MiB ... 2048 MiB. > > With this patch, the cs_mem_size variable can have these values: > ( { 16, 32 } x { 8, 16 } x { 0, 512, 1024, 2048, 4096, 8192 } ) / 8 = > { 0, 64, 128, 256, 512, 1024, 2048, 4096 } > To retain previous behavior, filter out 0 MiB (invalid size), 64 MiB > and 4096 MiB options. > > Removing the floating point stuff also saves 1.5k from text segment: > clearfog : spl/u-boot-spl:all -1592 spl/u-boot-spl:text -1592 > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Dirk Eibach <dirk.eibach@gdsys.cc> > Cc: Stefan Roese <sr@denx.de> Marek, thanks for working on this. Applied to u-boot-marvell/master Thanks, Stefan
diff --git a/drivers/ddr/marvell/a38x/ddr3_init.c b/drivers/ddr/marvell/a38x/ddr3_init.c index ee05f57..55baad4 100644 --- a/drivers/ddr/marvell/a38x/ddr3_init.c +++ b/drivers/ddr/marvell/a38x/ddr3_init.c @@ -678,7 +678,7 @@ u32 ddr3_get_device_width(u32 cs) return (device_width == 0) ? 8 : 16; } -float ddr3_get_device_size(u32 cs) +static int ddr3_get_device_size(u32 cs) { u32 device_size_low, device_size_high, device_size; u32 data, cs_low_offset, cs_high_offset; @@ -695,15 +695,15 @@ float ddr3_get_device_size(u32 cs) switch (device_size) { case 0: - return 2; + return 2048; case 2: - return 0.5; + return 512; case 3: - return 1; + return 1024; case 4: - return 4; + return 4096; case 5: - return 8; + return 8192; case 1: default: DEBUG_INIT_C("Error: Wrong device size of Cs: ", cs, 1); @@ -711,13 +711,13 @@ float ddr3_get_device_size(u32 cs) * Small value will give wrong emem size in * ddr3_calc_mem_cs_size */ - return 0.01; + return 0; } } int ddr3_calc_mem_cs_size(u32 cs, u32 *cs_size) { - float cs_mem_size; + int cs_mem_size; /* Calculate in GiB */ cs_mem_size = ((ddr3_get_bus_width() / ddr3_get_device_width(cs)) * @@ -731,21 +731,12 @@ int ddr3_calc_mem_cs_size(u32 cs, u32 *cs_size) */ cs_mem_size *= DDR_CONTROLLER_BUS_WIDTH_MULTIPLIER; - if (cs_mem_size == 0.125) { - *cs_size = 128 << 20; - } else if (cs_mem_size == 0.25) { - *cs_size = 256 << 20; - } else if (cs_mem_size == 0.5) { - *cs_size = 512 << 20; - } else if (cs_mem_size == 1) { - *cs_size = 1 << 30; - } else if (cs_mem_size == 2) { - *cs_size = 2 << 30; - } else { + if (!cs_mem_size || (cs_mem_size == 64) || (cs_mem_size == 4096)) { DEBUG_INIT_C("Error: Wrong Memory size of Cs: ", cs, 1); return MV_BAD_VALUE; } + *cs_size = cs_mem_size << 20; return MV_OK; }
For reason unknown, recently, the DDR init code writers are really fond of hiding some small floating point operating deep in their creations. This patch removes one from the Marvell A38x code. Instead of returning size of chip as float from ddr3_get_device_size() in GiB units, return it as int in MiB units. Since this would interfere with the huge switch code in ddr3_calc_mem_cs_size(), rework the code to match the change. Before this patch, the cs_mem_size variable could have these values: ( { 16, 32 } x { 8, 16 } x { 0.01, 0.5, 1, 2, 4, 8 } ) / 8 = { 0.000000, 0.001250, 0.002500, 0.005000, 0.062500, 0.125000, 0.250000, 0.500000, 1.000000, 2.000000, 4.000000, } The switch code checked for a subset of the resulting RAM sizes, which is in range 128 MiB ... 2048 MiB. With this patch, the cs_mem_size variable can have these values: ( { 16, 32 } x { 8, 16 } x { 0, 512, 1024, 2048, 4096, 8192 } ) / 8 = { 0, 64, 128, 256, 512, 1024, 2048, 4096 } To retain previous behavior, filter out 0 MiB (invalid size), 64 MiB and 4096 MiB options. Removing the floating point stuff also saves 1.5k from text segment: clearfog : spl/u-boot-spl:all -1592 spl/u-boot-spl:text -1592 Signed-off-by: Marek Vasut <marex@denx.de> Cc: Dirk Eibach <dirk.eibach@gdsys.cc> Cc: Stefan Roese <sr@denx.de> --- drivers/ddr/marvell/a38x/ddr3_init.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-)