diff mbox series

[09/10] hw/arm/virt: Fix devicetree warnings about the SMMU node

Message ID 20220824155113.286730-10-jean-philippe@linaro.org
State New
Headers show
Series hw/arm/virt: Fix dt-schema warnings | expand

Commit Message

Jean-Philippe Brucker Aug. 24, 2022, 3:51 p.m. UTC 
dt-validate reports three issues in the SMMU device-tree node:

  smmuv3@9050000: $nodename:0: 'smmuv3@9050000' does not match '^iommu@[0-9a-f]*'
  smmuv3@9050000: interrupt-names: 'oneOf' conditional failed, one must be fixed:
  	['eventq', 'priq', 'cmdq-sync', 'gerror'] is too long
  	'combined' was expected
  	'gerror' was expected
  	'gerror' is not one of ['cmdq-sync', 'priq']
  smmuv3@9050000: 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
  From schema: linux/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml

Fix them by:
* changing the node name
* reordering the IRQs
* removing the clock properties which are not expected for the SMMU node

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 hw/arm/virt.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Peter Maydell Aug. 24, 2022, 7:45 p.m. UTC | #1 
On Wed, 24 Aug 2022 at 16:51, Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> dt-validate reports three issues in the SMMU device-tree node:
>
>   smmuv3@9050000: $nodename:0: 'smmuv3@9050000' does not match '^iommu@[0-9a-f]*'
>   smmuv3@9050000: interrupt-names: 'oneOf' conditional failed, one must be fixed:
>         ['eventq', 'priq', 'cmdq-sync', 'gerror'] is too long
>         'combined' was expected
>         'gerror' was expected
>         'gerror' is not one of ['cmdq-sync', 'priq']
>   smmuv3@9050000: 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
>   From schema: linux/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
>
> Fix them by:
> * changing the node name
> * reordering the IRQs
> * removing the clock properties which are not expected for the SMMU node

Why does dt-validate insist on a fixed interrupt order here?

thanks
-- PMM
Jean-Philippe Brucker Sept. 1, 2022, 3:01 p.m. UTC | #2 
On Wed, Aug 24, 2022 at 08:45:05PM +0100, Peter Maydell wrote:
> On Wed, 24 Aug 2022 at 16:51, Jean-Philippe Brucker
> <jean-philippe@linaro.org> wrote:
> >
> > dt-validate reports three issues in the SMMU device-tree node:
> >
> >   smmuv3@9050000: $nodename:0: 'smmuv3@9050000' does not match '^iommu@[0-9a-f]*'
> >   smmuv3@9050000: interrupt-names: 'oneOf' conditional failed, one must be fixed:
> >         ['eventq', 'priq', 'cmdq-sync', 'gerror'] is too long
> >         'combined' was expected
> >         'gerror' was expected
> >         'gerror' is not one of ['cmdq-sync', 'priq']
> >   smmuv3@9050000: 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
> >   From schema: linux/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> >
> > Fix them by:
> > * changing the node name
> > * reordering the IRQs
> > * removing the clock properties which are not expected for the SMMU node
> 
> Why does dt-validate insist on a fixed interrupt order here?

I think the binding can be relaxed, since the driver must always look at
interrupt-names and can't assume a specific order (given that all except
gerror are now optional).

Thanks,
Jean
diff mbox series

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 779eb5ea31..de508d5329 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1329,7 +1329,9 @@  static void create_smmu(const VirtMachineState *vms,
     int i;
     hwaddr base = vms->memmap[VIRT_SMMU].base;
     hwaddr size = vms->memmap[VIRT_SMMU].size;
-    const char irq_names[] = "eventq\0priq\0cmdq-sync\0gerror";
+    uint32_t irq_type = GIC_FDT_IRQ_TYPE_SPI;
+    uint32_t irq_trigger = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI;
+    const char irq_names[] = "eventq\0gerror\0priq\0cmdq-sync";
     DeviceState *dev;
     MachineState *ms = MACHINE(vms);
 
@@ -1348,22 +1350,20 @@  static void create_smmu(const VirtMachineState *vms,
                            qdev_get_gpio_in(vms->gic, irq + i));
     }
 
-    node = g_strdup_printf("/smmuv3@%" PRIx64, base);
+    node = g_strdup_printf("/iommu@%" PRIx64, base);
     qemu_fdt_add_subnode(ms->fdt, node);
     qemu_fdt_setprop(ms->fdt, node, "compatible", compat, sizeof(compat));
     qemu_fdt_setprop_sized_cells(ms->fdt, node, "reg", 2, base, 2, size);
 
     qemu_fdt_setprop_cells(ms->fdt, node, "interrupts",
-            GIC_FDT_IRQ_TYPE_SPI, irq    , GIC_FDT_IRQ_FLAGS_EDGE_LO_HI,
-            GIC_FDT_IRQ_TYPE_SPI, irq + 1, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI,
-            GIC_FDT_IRQ_TYPE_SPI, irq + 2, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI,
-            GIC_FDT_IRQ_TYPE_SPI, irq + 3, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
+            irq_type, irq + SMMU_IRQ_EVTQ,      irq_trigger,
+            irq_type, irq + SMMU_IRQ_GERROR,    irq_trigger,
+            irq_type, irq + SMMU_IRQ_PRIQ,      irq_trigger,
+            irq_type, irq + SMMU_IRQ_CMD_SYNC,  irq_trigger);
 
     qemu_fdt_setprop(ms->fdt, node, "interrupt-names", irq_names,
                      sizeof(irq_names));
 
-    qemu_fdt_setprop_cell(ms->fdt, node, "clocks", vms->clock_phandle);
-    qemu_fdt_setprop_string(ms->fdt, node, "clock-names", "apb_pclk");
     qemu_fdt_setprop(ms->fdt, node, "dma-coherent", NULL, 0);
 
     qemu_fdt_setprop_cell(ms->fdt, node, "#iommu-cells", 1);