On Tue, 19 Jan 2021 at 13:47, Maxim Uvarov <maxim.uva...@linaro.org> wrote: > > On Tue, 19 Jan 2021 at 16:07, Peter Maydell <peter.mayd...@linaro.org> wrote: > > > > On Fri, 15 Jan 2021 at 10:11, Maxim Uvarov <maxim.uva...@linaro.org> wrote: > > Remind me why we start with GPIO line number 3 and not 0 ? > > > > Original gpio power key use 3 and 4 (non-secure). I just selected the > same to be consistent.
Those are different GPIO lines on a different PL061 doing a different job. I don't think they need to be the same number. The power keys are on 3 and 4 because pins 0, 1 and 2 were reserved for PCI hotplug, CPU hotplug and memory hotplug. Unless you have some similar reason why you need to reserve pins on the secure PL061, I would just start from 0. > > > + 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 ? > > > gpio-pwr created from the first patch. There are no bindings yet. You can't use bindings you've just made up -- you have to get them accepted into the kernel's official devicetree documentation if the ones already there aren't sufficient, before you can add code to QEMU that generates them. > > 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 > > > These handles are from 'secure memory' where linux does not have > access. But I think we can use that > binding with other compatible. Like compatible = "gpio-poweroff,secure". That's not how you specify that a node is only relevant to the secure world: you set the 'status' property to 'disabled' and the 'secure-status' property to 'okay': https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/secure.txt thanks -- PMM