| Message ID | 20220927100347.176606-6-jean-philippe@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/arm/virt: Fix dt-schema warnings | expand |
On Tue, 27 Sept 2022 at 11:12, Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > > Since the pl061 device can be used as interrupt controller, its node > should contain "interrupt-controller" and "#interrupt-cells" properties. It *can* be, but this PL061 is *not* an interrupt controller. I don't see any reason why we should claim so in the DT. thanks -- PMM
On Tue, Sep 27, 2022 at 6:25 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 27 Sept 2022 at 11:12, Jean-Philippe Brucker > <jean-philippe@linaro.org> wrote: > > > > Since the pl061 device can be used as interrupt controller, its node > > should contain "interrupt-controller" and "#interrupt-cells" properties. > > It *can* be, but this PL061 is *not* an interrupt controller. > I don't see any reason why we should claim so in the DT. Taking another look, it is an interrupt controller. The GPIOs are connected to the 'gpio-keys' node which is interrupt based (there's a polled version too). That binding happens to be pretty lax and allows the GPIO to be specified either with 'gpios' or 'interrupts' property. The Linux PL061 driver happens to work only because it always registers an interrupt controller regardless of having "interrupt-controller" and "#interrupt-cells" properties or not. Rob
On Tue, 29 Nov 2022 at 20:56, Rob Herring <robh@kernel.org> wrote: > > On Tue, Sep 27, 2022 at 6:25 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > > > On Tue, 27 Sept 2022 at 11:12, Jean-Philippe Brucker > > <jean-philippe@linaro.org> wrote: > > > > > > Since the pl061 device can be used as interrupt controller, its node > > > should contain "interrupt-controller" and "#interrupt-cells" properties. > > > > It *can* be, but this PL061 is *not* an interrupt controller. > > I don't see any reason why we should claim so in the DT. > > Taking another look, it is an interrupt controller. The GPIOs are > connected to the 'gpio-keys' node which is interrupt based (there's a > polled version too). That binding happens to be pretty lax and allows > the GPIO to be specified either with 'gpios' or 'interrupts' property. > The Linux PL061 driver happens to work only because it always > registers an interrupt controller regardless of having > "interrupt-controller" and "#interrupt-cells" properties or not. No, it really isn't an interrupt controller. The interrupt controller in this system is the GIC. The PL061 is a GPIO controller. It *has* an interrupt line, which it connects to the GIC, but that doesn't make it an interrupt controller any more than it makes a UART an interrupt controller. It just means you can use the GPIOs in either a polled or an interrupt-driven mode, same as you can use a PL011 UART in either polled or interrupt-driven mode. And the guest knows it can do that because we've told it "this is a PL061" and that's part of the PL061's functionality. The gpio-keys stuff is just "here is a wire which is tracking the state of an emulated power button". This isn't an interrupt, it's just a wire that has some status the guest probably cares about. The second PL061 which this bit of dtb-generation code also applies to happens to currently be being used purely for output GPIOs, so calling that one an interrupt controller makes even less sense. thanks -- PMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 6805c57530..10ce66c722 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1012,6 +1012,8 @@ static void create_gpio_devices(const VirtMachineState *vms, int gpio, qemu_fdt_setprop_cell(ms->fdt, nodename, "clocks", vms->clock_phandle); qemu_fdt_setprop_string(ms->fdt, nodename, "clock-names", "apb_pclk"); qemu_fdt_setprop_cell(ms->fdt, nodename, "phandle", phandle); + qemu_fdt_setprop_cell(ms->fdt, nodename, "#interrupt-cells", 2); + qemu_fdt_setprop(ms->fdt, nodename, "interrupt-controller", NULL, 0); if (gpio != VIRT_GPIO) { /* Mark as not usable by the normal world */
Since the pl061 device can be used as interrupt controller, its node should contain "interrupt-controller" and "#interrupt-cells" properties. Fix the corresponding dt-validate warnings: pl061@9030000: 'interrupt-controller' is a required property From schema: linux/Documentation/devicetree/bindings/gpio/pl061-gpio.yaml pl061@9030000: '#interrupt-cells' is a required property From schema: linux/Documentation/devicetree/bindings/gpio/pl061-gpio.yaml Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> --- hw/arm/virt.c | 2 ++ 1 file changed, 2 insertions(+)