[PATCH] hw/riscv/virt: Comment absence of #msi-cells

[PATCH] hw/riscv/virt: Comment absence of #msi-cells

[Andrew Jones]

commit 6df664f87c73 ("Revert "hw/riscv/virt.c: imsics DT: add
'#msi-cells'"") removed #msi-cells. Now that we have a Linux
commit to reference add a comment explaining why it was removed
to avoid it getting added back due to DT validation failures.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 hw/riscv/virt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index cef41c150aaf..4fd9c0edf038 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -552,6 +552,16 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
                           FDT_IMSIC_INT_CELLS);
     qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", NULL, 0);
     qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller", NULL, 0);
+    /*
+     * Per the DT binding, we should also add #msi-cells with a value of zero.
+     * But, Linux which does not include commit db8e81132cf0 ("of/irq: Support
+     * #msi-cells=<0> in of_msi_get_domain") does not properly handle #msi-cells
+     * with a value of zero. For this reason, the property has intentionally
+     * been left out. It's harmless to not add it since the absence of the
+     * property and a value of zero for the property mean the same thing.
+     * However, since the DT binding requires the property, DT validation may
+     * fail.
+     */
     qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
                      imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
     qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,
-- 
2.46.0

Re: [PATCH] hw/riscv/virt: Comment absence of #msi-cells

[Daniel Henrique Barboza]

On 9/25/24 1:39 PM, Andrew Jones wrote:
> commit 6df664f87c73 ("Revert "hw/riscv/virt.c: imsics DT: add
> '#msi-cells'"") removed #msi-cells. Now that we have a Linux
> commit to reference add a comment explaining why it was removed
> to avoid it getting added back due to DT validation failures.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/riscv/virt.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index cef41c150aaf..4fd9c0edf038 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -552,6 +552,16 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
>                             FDT_IMSIC_INT_CELLS);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", NULL, 0);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller", NULL, 0);
> +    /*
> +     * Per the DT binding, we should also add #msi-cells with a value of zero.
> +     * But, Linux which does not include commit db8e81132cf0 ("of/irq: Support
> +     * #msi-cells=<0> in of_msi_get_domain") does not properly handle #msi-cells
> +     * with a value of zero. For this reason, the property has intentionally
> +     * been left out. It's harmless to not add it since the absence of the
> +     * property and a value of zero for the property mean the same thing.
> +     * However, since the DT binding requires the property, DT validation may
> +     * fail.
> +     */
>       qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
>                        imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,