On Fri, 15 Jan 2021 at 10:11, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
>
> Add secure pl061 for reset/power down machine from
> the secure world (Arm Trusted Firmware). Connect it
> with gpio-pwr driver.
>
> Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org>
> ---
> hw/arm/Kconfig | 1 +
> hw/arm/virt.c | 50 +++++++++++++++++++++++++++++++++++++++++++
> include/hw/arm/virt.h | 2 ++
> 3 files changed, 53 insertions(+)
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 0a242e4c5d..13cc42dcc8 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -17,6 +17,7 @@ config ARM_VIRT
> select PL011 # UART
> select PL031 # RTC
> select PL061 # GPIO
> + select GPIO_PWR
> select PLATFORM_BUS
> select SMBIOS
> select VIRTIO_MMIO
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 26bb66e8e1..436ae894c9 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -153,6 +153,7 @@ static const MemMapEntry base_memmap[] = {
> [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN },
> [VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN},
> [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
> + [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 },
> [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size
> */
> [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
> @@ -841,6 +842,46 @@ static void create_gpio_keys(const VirtMachineState *vms,
> "gpios", phandle, 3, 0);
> }
>
> +#define ATF_GPIO_POWEROFF 3
> +#define ATF_GPIO_REBOOT 4
These aren't ATF specific, so you could name them SECURE_GPIO_POWEROFF
and SECURE_GPIO_REBOOT.
Remind me why we start with GPIO line number 3 and not 0 ?
> +
> +static void create_gpio_pwr(const VirtMachineState *vms,
> + DeviceState *pl061_dev,
> + uint32_t phandle)
> +{
> + DeviceState *gpio_pwr_dev;
> +
> + /* gpio-pwr */
> + gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL);
> +
> + /* connect secure pl061 to gpio-pwr */
> + qdev_connect_gpio_out(pl061_dev, ATF_GPIO_POWEROFF,
> + qdev_get_gpio_in_named(gpio_pwr_dev, "reset", 0));
> + qdev_connect_gpio_out(pl061_dev, ATF_GPIO_REBOOT,
> + qdev_get_gpio_in_named(gpio_pwr_dev, "shutdown",
> 0));
You've connected the POWEROFF gpio line to 'reset' and the
REBOOT line to 'shutdown'. This looks like it's backwards.
> + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr");
> + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr", "compatible", "gpio-pwr");
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#size-cells", 0);
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#address-cells", 1);
> +
> + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/poweroff");
> + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/poweroff",
> + "label", "GPIO PWR Poweroff");
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/poweroff", "code",
> + ATF_GPIO_POWEROFF);
> + qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/poweroff",
> + "gpios", phandle, 3, 0);
> +
> + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/reboot");
> + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/reboot",
> + "label", "GPIO PWR Reboot");
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/reboot", "code",
> + ATF_GPIO_REBOOT);
> + qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/reboot",
> + "gpios", phandle, 3, 0);
There doesn't seem to be any documented 'gpio-pwr' devicetree
binding. Where does this come from ?
I think the bindings you want to be using are
https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/gpio-restart.txt
https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt
thanks
-- PMM
