On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
> Move the code to create the DT node for the PL061 GPIO controller from
> hw/arm/virt.c to the PL061 driver, so it can be reused.
>
> While at it, make the created node comply with the PL061 Device Tree
> bindings:
> - Use generic node name "gpio" instead of "pl061",
I'd split this patch in 2 (first one being fixing missing properties).
> - Add missing "#interrupt-cells" and "interrupt-controller"
> properties.
>
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>
> ---
> v2:
> - New.
> ---
> hw/arm/virt.c | 20 +++-----------------
> hw/gpio/pl061.c | 42 +++++++++++++++++++++++++++++++++++++++++
> include/hw/gpio/pl061.h | 7 +++++++
> 3 files changed, 52 insertions(+), 17 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 7dc96abf72cf2b9a..c88c8850fbe00bdb 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -40,6 +40,7 @@
> #include "hw/arm/primecell.h"
> #include "hw/arm/virt.h"
> #include "hw/block/flash.h"
> +#include "hw/gpio/pl061.h"
> #include "hw/vfio/vfio-calxeda-xgmac.h"
> #include "hw/vfio/vfio-amd-xgbe.h"
> #include "hw/display/ramfb.h"
> @@ -807,30 +808,16 @@ static void virt_powerdown_req(Notifier *n, void
> *opaque)
>
> static void create_gpio(const VirtMachineState *vms)
> {
> - char *nodename;
> DeviceState *pl061_dev;
> hwaddr base = vms->memmap[VIRT_GPIO].base;
> hwaddr size = vms->memmap[VIRT_GPIO].size;
> int irq = vms->irqmap[VIRT_GPIO];
> - const char compat[] = "arm,pl061\0arm,primecell";
>
> pl061_dev = sysbus_create_simple("pl061", base,
> qdev_get_gpio_in(vms->gic, irq));
>
> - uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt);
> - nodename = g_strdup_printf("/pl061@%" PRIx64, base);
> - qemu_fdt_add_subnode(vms->fdt, nodename);
> - qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
> - 2, base, 2, size);
> - qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat,
> sizeof(compat));
> - qemu_fdt_setprop_cell(vms->fdt, nodename, "#gpio-cells", 2);
> - qemu_fdt_setprop(vms->fdt, nodename, "gpio-controller", NULL, 0);
> - qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
> - GIC_FDT_IRQ_TYPE_SPI, irq,
> - GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> - qemu_fdt_setprop_cell(vms->fdt, nodename, "clocks", vms->clock_phandle);
> - qemu_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk");
> - qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle);
> + uint32_t phandle = pl061_create_fdt(vms->fdt, "", 2, base, size, irq,
> + vms->clock_phandle);
>
> gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> qdev_get_gpio_in(pl061_dev, 3));
> @@ -846,7 +833,6 @@ static void create_gpio(const VirtMachineState *vms)
> KEY_POWER);
> qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
> "gpios", phandle, 3, 0);
> - g_free(nodename);
> }
>
> static void create_virtio_devices(const VirtMachineState *vms)
> diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
> index e776c09e474216ef..74ba733a8a5e8ca5 100644
> --- a/hw/gpio/pl061.c
> +++ b/hw/gpio/pl061.c
> @@ -9,12 +9,14 @@
> */
>
> #include "qemu/osdep.h"
> +#include "hw/arm/fdt.h"
> #include "hw/gpio/pl061.h"
> #include "hw/irq.h"
> #include "hw/sysbus.h"
> #include "migration/vmstate.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
> +#include "sysemu/device_tree.h"
>
> //#define DEBUG_PL061 1
>
> @@ -397,3 +399,43 @@ static void pl061_register_types(void)
> }
>
> type_init(pl061_register_types)
> +
> +/*
> + * pl061_create_fdt: Create a DT node for a PL061 GPIO controller
> + * @fdt: device tree blob
> + * @parent: name of the parent node
> + * @n_cells: value of #address-cells and #size-cells
> + * @base: base address of the controller's register block
> + * @size: size of the controller's register block
> + * @irq: interrupt number
> + * @clock: phandle of the apb-pclk clock
> + *
> + * Return value: a phandle referring to the created DT node.
> + *
> + * See the DT Binding Documentation in the Linux kernel source tree:
> + * Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
> + */
> +uint32_t pl061_create_fdt(void *fdt, const char *parent, unsigned int
> n_cells,
> + hwaddr base, hwaddr size, int irq, uint32_t clock)
> +{
> + char *nodename = g_strdup_printf("%s/gpio@%" PRIx64, parent, base);
> + static const char compat[] = "arm,pl061\0arm,primecell";
> + uint32_t phandle = qemu_fdt_alloc_phandle(fdt);
> +
> + qemu_fdt_add_subnode(fdt, nodename);
> + qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", n_cells, base,
> n_cells,
> + size);
> + qemu_fdt_setprop(fdt, nodename, "compatible", compat, sizeof(compat));
> + qemu_fdt_setprop_cell(fdt, nodename, "#gpio-cells", 2);
> + qemu_fdt_setprop(fdt, nodename, "gpio-controller", NULL, 0);
> + qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 2);
> + qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
> + qemu_fdt_setprop_cells(fdt, nodename, "interrupts", GIC_FDT_IRQ_TYPE_SPI,
> + irq, GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> + qemu_fdt_setprop_cell(fdt, nodename, "clocks", clock);
> + qemu_fdt_setprop_string(fdt, nodename, "clock-names", "apb_pclk");
> + qemu_fdt_setprop_cell(fdt, nodename, "phandle", phandle);
> + g_free(nodename);
> +
> + return phandle;
> +}
> diff --git a/include/hw/gpio/pl061.h b/include/hw/gpio/pl061.h
> index 78cc40c52679dc4e..f98c6e24e0e68662 100644
> --- a/include/hw/gpio/pl061.h
> +++ b/include/hw/gpio/pl061.h
> @@ -11,6 +11,13 @@
> #ifndef PL061_GPIO_H
> #define PL061_GPIO_H
>
> +#include <stdint.h>
> +
> +#include "exec/hwaddr.h"
> +
> #define TYPE_PL061 "pl061"
>
> +uint32_t pl061_create_fdt(void *fdt, const char *parent, unsigned int
> n_cells,
> + hwaddr addr, hwaddr size, int irq, uint32_t clock);
> +
> #endif /* PL061_GPIO_H */
>
