Re: [PATCH 9/9] hw/riscv/virt.c: remove 'long' casts in fmt strings

[Daniel Henrique Barboza <dbarboza@ventanamicro.com>]

On 4/29/25 9:40 AM, Daniel Henrique Barboza wrote:
> Joel,
> 
> I'll make these changes in this patch to be consistent with what we've
> been discussing:
> 
> - change addr to hwaddr
> - use HWADDR_PRIx instead of PRIx64
> 
> i.e. this diff:
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 1eae84db15..0020d8f404 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -303,12 +303,13 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
>   static void create_fdt_socket_memory(RISCVVirtState *s, int socket)
>   {
>       g_autofree char *mem_name = NULL;
> -    uint64_t addr, size;
> +    hwaddr addr;
> +    uint64_t size;
>       MachineState *ms = MACHINE(s);
> 
>       addr = s->memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
>       size = riscv_socket_mem_size(ms, socket);
> -    mem_name = g_strdup_printf("/memory@%"PRIx64, addr);
> +    mem_name = g_strdup_printf("/memory@%"HWADDR_PRIx, addr);
>       qemu_fdt_add_subnode(ms->fdt, mem_name);
>       qemu_fdt_setprop_cells(ms->fdt, mem_name, "reg",
>           addr >> 32, addr, size >> 32, size);
> 
> 
> I did this change and applied your "hw/riscv/virt: device tree reg cleanups" series on top
> of it, and there are no conflicts. No change needed in your side.

It seems I was wrong. The v2 will conflict with your patch 03. I think a rebase from
your series can't be avoided ...

Daniel

> 
> 
> Thanks,
> 
> Daniel
> 
> 
> 
> On 4/24/25 6:41 AM, Joel Stanley wrote:
>> On Wed, 23 Apr 2025 at 20:39, Daniel Henrique Barboza
>> <dbarboza@ventanamicro.com> wrote:
>>>
>>> We can avoid the 'long' casts by using PRIx64 and HWADDR_PRIx on the fmt
>>> strings for uint64_t and hwaddr types.
>>>
>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>>> ---
>>>   hw/riscv/virt.c | 29 +++++++++++++++--------------
>>>   1 file changed, 15 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>>> index 036a0a9bfb..075c035f25 100644
>>> --- a/hw/riscv/virt.c
>>> +++ b/hw/riscv/virt.c
>>> @@ -309,7 +309,7 @@ static void create_fdt_socket_memory(RISCVVirtState *s, int socket)
>>>
>>>       addr = s->memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
>>>       size = riscv_socket_mem_size(ms, socket);
>>> -    mem_name = g_strdup_printf("/memory@%lx", (long)addr);
>>> +    mem_name = g_strdup_printf("/memory@%"PRIx64, addr);
>>
>> I wondered why this wasn't a HWADDR_PRIx.
>>
>> addr (and NodeInfo::node_mem?) could be a hwaddr? That would make
>> everything more consistent.
>>
>> Cheers,
>>
>> Joel
>