Re: [PATCH v2 04/11] hw/arm/aspeed: Use the IEC binary prefix definitions

[Cédric Le Goater]

On 1/18/23 07:53, Joel Stanley wrote:
> On Fri, 30 Dec 2022 at 11:35, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > IEC binary prefixes ease code review: the unit is explicit.
>
> I strongly prefer the existing code; it tells you the size without
> having to do maths.

you mean that it matches better with the address space representation
in the code and the 'info mtree' output ? If so, I agree. We can keep
this patch out, it is not fundamental.

The hex representation of values has its advantages compared to the
macros because hex is generally what you get in debug outputs and
it is easier to compare and manipulate.  Some Linux dev feel the
same.

C.

> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Reviewed-by: Peter Delevoryas <peter@pjd.dev>
> > ---
> >   hw/arm/aspeed_ast10x0.c | 3 ++-
> >   hw/arm/aspeed_ast2600.c | 3 ++-
> >   hw/arm/aspeed_soc.c     | 4 ++--
> >   3 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> > index 122b3fd3f3..3500294df7 100644
> > --- a/hw/arm/aspeed_ast10x0.c
> > +++ b/hw/arm/aspeed_ast10x0.c
> > @@ -10,6 +10,7 @@
> >    */
> >
> >   #include "qemu/osdep.h"
> > +#include "qemu/units.h"
> >   #include "qapi/error.h"
> >   #include "exec/address-spaces.h"
> >   #include "sysemu/sysemu.h"
> > @@ -348,7 +349,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass 
> > *klass, void *data)
> >       sc->name = "ast1030-a1";
> >       sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
> >       sc->silicon_rev = AST1030_A1_SILICON_REV;
> > -    sc->sram_size = 0xc0000;
> > +    sc->sram_size = 768 * KiB;
> >       sc->spis_num = 2;
> >       sc->ehcis_num = 0;
> >       sc->wdts_num = 4;
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > index a79e05ddbd..72df72a540 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -8,6 +8,7 @@
> >    */
> >
> >   #include "qemu/osdep.h"
> > +#include "qemu/units.h"
> >   #include "qapi/error.h"
> >   #include "hw/misc/unimp.h"
> >   #include "hw/arm/aspeed_soc.h"
> > @@ -619,7 +620,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, 
> > void *data)
> >       sc->name         = "ast2600-a3";
> >       sc->cpu_type     = ARM_CPU_TYPE_NAME("cortex-a7");
> >       sc->silicon_rev  = AST2600_A3_SILICON_REV;
> > -    sc->sram_size    = 0x16400;
> > +    sc->sram_size    = 89 * KiB;
> >       sc->spis_num     = 2;
> >       sc->ehcis_num    = 2;
> >       sc->wdts_num     = 4;
> > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> > index 2c0924d311..677342c9ed 100644
> > --- a/hw/arm/aspeed_soc.c
> > +++ b/hw/arm/aspeed_soc.c
> > @@ -517,7 +517,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass *oc, 
> > void *data)
> >       sc->name         = "ast2400-a1";
> >       sc->cpu_type     = ARM_CPU_TYPE_NAME("arm926");
> >       sc->silicon_rev  = AST2400_A1_SILICON_REV;
> > -    sc->sram_size    = 0x8000;
> > +    sc->sram_size    = 32 * KiB;
> >       sc->spis_num     = 1;
> >       sc->ehcis_num    = 1;
> >       sc->wdts_num     = 2;
> > @@ -544,7 +544,7 @@ static void aspeed_soc_ast2500_class_init(ObjectClass *oc, 
> > void *data)
> >       sc->name         = "ast2500-a1";
> >       sc->cpu_type     = ARM_CPU_TYPE_NAME("arm1176");
> >       sc->silicon_rev  = AST2500_A1_SILICON_REV;
> > -    sc->sram_size    = 0x9000;
> > +    sc->sram_size    = 36 * KiB;
> >       sc->spis_num     = 2;
> >       sc->ehcis_num    = 2;
> >       sc->wdts_num     = 3;
> > --
> > 2.38.1