[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCHv8 2/3] arm-virt: refactor gpios creation
From: |
Andrew Jones |
Subject: |
Re: [PATCHv8 2/3] arm-virt: refactor gpios creation |
Date: |
Fri, 22 Jan 2021 09:07:12 +0100 |
On Wed, Jan 20, 2021 at 12:27:47PM +0300, Maxim Uvarov wrote:
> No functional change. Just refactor code to better
> support secure and normal world gpios.
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
> hw/arm/virt.c | 64 ++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 96985917d3..c427ce5f81 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -820,17 +820,43 @@ static void virt_powerdown_req(Notifier *n, void
> *opaque)
> }
> }
>
> -static void create_gpio(const VirtMachineState *vms)
> +static void create_gpio_keys(const VirtMachineState *vms,
> + DeviceState *pl061_dev,
> + uint32_t phandle)
> +{
> + gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> + qdev_get_gpio_in(pl061_dev, 3));
> +
> + qemu_fdt_add_subnode(vms->fdt, "/gpio-keys");
> + qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible",
> "gpio-keys");
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0);
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1);
> +
> + qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff");
> + qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff",
> + "label", "GPIO Key Poweroff");
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code",
> + KEY_POWER);
> + qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
> + "gpios", phandle, 3, 0);
> +}
> +
> +static void create_gpio_devices(const VirtMachineState *vms, int gpio,
> + MemoryRegion *mem)
> {
> 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];
> + hwaddr base = vms->memmap[gpio].base;
> + hwaddr size = vms->memmap[gpio].size;
> + int irq = vms->irqmap[gpio];
> const char compat[] = "arm,pl061\0arm,primecell";
> + SysBusDevice *s;
>
> - pl061_dev = sysbus_create_simple("pl061", base,
> - qdev_get_gpio_in(vms->gic, irq));
> + pl061_dev = qdev_new("pl061");
> + s = SYS_BUS_DEVICE(pl061_dev);
> + sysbus_realize_and_unref(s, &error_fatal);
> + memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0));
> + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
>
> uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt);
> nodename = g_strdup_printf("/pl061@%" PRIx64, base);
> @@ -847,21 +873,17 @@ static void create_gpio(const VirtMachineState *vms)
> qemu_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk");
> qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle);
>
> - gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> - qdev_get_gpio_in(pl061_dev, 3));
> - qemu_fdt_add_subnode(vms->fdt, "/gpio-keys");
> - qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible",
> "gpio-keys");
> - qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0);
> - qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1);
> -
> - qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff");
> - qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff",
> - "label", "GPIO Key Poweroff");
> - qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code",
> - KEY_POWER);
> - qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
> - "gpios", phandle, 3, 0);
> + if (gpio != VIRT_GPIO) {
> + /* Mark as not usable by the normal world */
> + qemu_fdt_setprop_string(vms->fdt, nodename, "status", "disabled");
> + qemu_fdt_setprop_string(vms->fdt, nodename, "secure-status", "okay");
> + }
nit: The above if-block could/should have waited until the next patch to
be added.
> g_free(nodename);
> +
> + /* Child gpio devices */
> + if (gpio == VIRT_GPIO) {
Same nit as above, the next patch is where we should start conditionally
doing stuff based on the gpio type.
> + create_gpio_keys(vms, pl061_dev, phandle);
> + }
> }
>
> static void create_virtio_devices(const VirtMachineState *vms)
> @@ -1990,7 +2012,7 @@ static void machvirt_init(MachineState *machine)
> if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) {
> vms->acpi_dev = create_acpi_ged(vms);
> } else {
> - create_gpio(vms);
> + create_gpio_devices(vms, VIRT_GPIO, sysmem);
> }
>
> /* connect powerdown request */
> --
> 2.17.1
>
>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Thanks,
drew