Re: [TF-A] [PATCHv7 2/3] arm-virt: refactor gpios creation

On Fri, 15 Jan 2021 at 10:11, Maxim Uvarov maxim.uvarov@linaro.org wrote:

No functional change. Just refactor code to better support secure and normal world gpios.

Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org

@@ -847,21 +873,22 @@ 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);

• if (gpio == VIRT_GPIO) {

•    qemu_fdt_setprop_string(vms->fdt, "/chosen", "stdout-path", nodename);
  

You don't want to set /chosen/stdout-path (that is specific to the uart, it's telling the kernel where it should send its bootup output by default).

• } else {

•    /* 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");
  
  

• 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);
  

•    qemu_fdt_setprop_string(vms->fdt, "/secure-chosen", "stdout-path",
  

•                            nodename);
  

• }

Similarly here you don't want to set /secure-chosen/stdout-path.

Patch looks OK otherwise.

thanks -- PMM