[v3,8/8] hw/arm/virt: Add gpio-keys node for Poweroff using DT

From: Shannon Zhao <shannon.zhao@linaro.org>

Add a gpio-keys node. This is used for Poweroff for the systems which
use DT not ACPI.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Tested-by: Wei Huang <wei@redhat.com>
---
 hw/arm/virt.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

On 16 November 2015 at 13:23,  <shannon.zhao@linaro.org> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Add a gpio-keys node. This is used for Poweroff for the systems which
> use DT not ACPI.
>
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> Tested-by: Wei Huang <wei@redhat.com>
> ---
>  hw/arm/virt.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index cb1d45b..3394262 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -561,6 +561,7 @@ static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
>
>      pl061_dev = sysbus_create_simple("pl061", base, pic[irq]);
>
> +    uint32_t phandle = qemu_fdt_alloc_phandle(vbi->fdt);
>      nodename = g_strdup_printf("/pl061@%" PRIx64, base);
>      qemu_fdt_add_subnode(vbi->fdt, nodename);
>      qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
> @@ -573,6 +574,20 @@ static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
>                             GIC_FDT_IRQ_FLAGS_LEVEL_HI);
>      qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
>      qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "phandle", phandle);
> +
> +    qemu_fdt_add_subnode(vbi->fdt, "/gpio-keys");
> +    qemu_fdt_setprop_string(vbi->fdt, "/gpio-keys", "compatible", "gpio-keys");
> +    qemu_fdt_setprop_cell(vbi->fdt, "/gpio-keys", "#size-cells", 0);
> +    qemu_fdt_setprop_cell(vbi->fdt, "/gpio-keys", "#address-cells", 1);
> +    qemu_fdt_setprop(vbi->fdt, "/gpio-keys", "autorepeat", NULL, 0);

Why would we want the power button to autorepeat ?

> +
> +    qemu_fdt_add_subnode(vbi->fdt, "/gpio-keys/poweroff");
> +    qemu_fdt_setprop_string(vbi->fdt, "/gpio-keys/poweroff",
> +                            "label", "GPIO Key Poweroff");
> +    qemu_fdt_setprop_cell(vbi->fdt, "/gpio-keys/poweroff", "linux,code", 116);

This could use a comment /* KEY_POWER */ to indicate what 116 is.
(Alternatively, see whether we can #include "standard-headers/linux/input.h"
which #defines KEY_POWER.)

> +    qemu_fdt_setprop_cells(vbi->fdt, "/gpio-keys/poweroff",
> +                           "gpios", phandle, 3, 0);
>
>      /* connect powerdown request */
>      qemu_register_powerdown_notifier(&virt_system_powerdown_notifier);

thanks
-- PMM

On 2015/11/28 1:16, Peter Maydell wrote:
> On 16 November 2015 at 13:23,  <shannon.zhao@linaro.org> wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add a gpio-keys node. This is used for Poweroff for the systems which
>> use DT not ACPI.
>>
>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> Tested-by: Wei Huang <wei@redhat.com>
>> ---
>>  hw/arm/virt.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index cb1d45b..3394262 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -561,6 +561,7 @@ static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
>>
>>      pl061_dev = sysbus_create_simple("pl061", base, pic[irq]);
>>
>> +    uint32_t phandle = qemu_fdt_alloc_phandle(vbi->fdt);
>>      nodename = g_strdup_printf("/pl061@%" PRIx64, base);
>>      qemu_fdt_add_subnode(vbi->fdt, nodename);
>>      qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
>> @@ -573,6 +574,20 @@ static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
>>                             GIC_FDT_IRQ_FLAGS_LEVEL_HI);
>>      qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
>>      qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
>> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "phandle", phandle);
>> +
>> +    qemu_fdt_add_subnode(vbi->fdt, "/gpio-keys");
>> +    qemu_fdt_setprop_string(vbi->fdt, "/gpio-keys", "compatible", "gpio-keys");
>> +    qemu_fdt_setprop_cell(vbi->fdt, "/gpio-keys", "#size-cells", 0);
>> +    qemu_fdt_setprop_cell(vbi->fdt, "/gpio-keys", "#address-cells", 1);
>> +    qemu_fdt_setprop(vbi->fdt, "/gpio-keys", "autorepeat", NULL, 0);
> 
> Why would we want the power button to autorepeat ?
> 
Oh, it doesn't need. Will remove it.

>> +
>> +    qemu_fdt_add_subnode(vbi->fdt, "/gpio-keys/poweroff");
>> +    qemu_fdt_setprop_string(vbi->fdt, "/gpio-keys/poweroff",
>> +                            "label", "GPIO Key Poweroff");
>> +    qemu_fdt_setprop_cell(vbi->fdt, "/gpio-keys/poweroff", "linux,code", 116);
> 
> This could use a comment /* KEY_POWER */ to indicate what 116 is.
> (Alternatively, see whether we can #include "standard-headers/linux/input.h"
> which #defines KEY_POWER.)
> 
Ok, it could include the linux/input.h here.

Thanks,