Re: [PATCH 4/5] hw/sh4/r2d: Use the IEC binary prefix definitions

[BALATON Zoltan]

On Mon, 9 Jan 2023, Philippe Mathieu-Daudé wrote:
> IEC binary prefixes ease code review: the unit is explicit.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/sh4/r2d.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
> index 39fc4f19d9..b3667e9b12 100644
> --- a/hw/sh4/r2d.c
> +++ b/hw/sh4/r2d.c
> @@ -47,10 +47,10 @@
> #define FLASH_BASE 0x00000000
> #define FLASH_SIZE (16 * MiB)
>
> -#define SDRAM_BASE 0x0c000000 /* Physical location of SDRAM: Area 3 */
> -#define SDRAM_SIZE 0x04000000
> +#define SDRAM_BASE          (192 * MiB) /* Physical location of SDRAM: Area 3 
> */
> +#define SDRAM_SIZE          (64 * MiB)

I don't think changing these help as the docs probably have memory map 
with the hex numbers rather than sizes so it's easier to match as it is 
now.

> -#define SM501_VRAM_SIZE 0x800000
> +#define SM501_VRAM_SIZE     (8 * MiB)

This one is OK but since it's only used once in

qdev_prop_set_uint32(dev, "vram-size", SM501_VRAM_SIZE);

you might as well just inline it there and remove the define which is then 
pretty clear and easier to see without needing to look up the define far 
away from its usage.

Regards,
BALATON Zoltan

> #define BOOT_PARAMS_OFFSET 0x0010000
> /* CONFIG_BOOT_LINK_OFFSET of Linux kernel */