[U-Boot] [PATCH v4 10/10] ARM: HYP/non-sec/PSCI: emit DT nodes

[Jon Loeliger]

On Sat, Apr 26, 2014 at 7:17 AM, Marc Zyngier <marc.zyngier at arm.com> wrote:

> diff --git a/arch/arm/cpu/armv7/virt-dt.c b/arch/arm/cpu/armv7/virt-dt.c
> new file mode 100644
> index 0000000..0b0d6a7
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/virt-dt.c

> +
> +static int fdt_psci(void *fdt)
> +{
> +#ifdef CONFIG_ARMV7_PSCI
> +       int nodeoff;
> +       int tmp;
> +
> +       nodeoff = fdt_path_offset(fdt, "/cpus");
> +       if (nodeoff < 0) {
> +               printf("couldn't find /cpus\n");
> +               return nodeoff;
> +       }
> +
> +       /* add 'enable-method = "psci"' to each cpu node */
> +       for (tmp = fdt_first_subnode(fdt, nodeoff);
> +            tmp >= 0;
> +            tmp = fdt_next_subnode(fdt, tmp)) {
> +               const struct fdt_property *prop;
> +               int len;
> +
> +               prop = fdt_get_property(fdt, tmp, "device_type", &len);
> +               if (!prop)
> +                       continue;
> +               if (len < 4)
> +                       continue;
> +               if (strcmp(prop->data, "cpu"))
> +                       continue;
> +
> +               fdt_setprop_string(fdt, tmp, "enable-method", "psci");
> +       }
> +
> +       nodeoff = fdt_path_offset(fdt, "/psci");
> +       if (nodeoff < 0) {
> +               nodeoff = fdt_path_offset(fdt, "/");
> +               if (nodeoff < 0)
> +                       return nodeoff;
> +
> +               nodeoff = fdt_add_subnode(fdt, nodeoff, "psci");
> +               if (nodeoff < 0)
> +                       return nodeoff;
> +       }
> +
> +       tmp = fdt_setprop_string(fdt, nodeoff, "compatible", "arm,psci");
> +       if (tmp)
> +               return tmp;
> +       tmp = fdt_setprop_string(fdt, nodeoff, "method", "smc");
> +       if (tmp)
> +               return tmp;
> +       tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_suspend", ARM_PSCI_FN_CPU_SUSPEND);
> +       if (tmp)
> +               return tmp;
> +       tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_off", ARM_PSCI_FN_CPU_OFF);
> +       if (tmp)
> +               return tmp;
> +       tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_on", ARM_PSCI_FN_CPU_ON);
> +       if (tmp)
> +               return tmp;
> +       tmp = fdt_setprop_u32(fdt, nodeoff, "migrate", ARM_PSCI_FN_MIGRATE);
> +       if (tmp)
> +               return tmp;
> +#endif
> +       return 0;
> +}
>

So, I wonder if it would be better to be a bit more selective or cautious
about adding these nodes and properties.  Specifically, if they are already
present in the device tree itself, perhaps they should be honored and left
alone?

I understand that U-Boot gets to define what it implements, and that if
the secure monitor code doesn't actually implement something, or for
that matter *does* implement it, it makes sense for U-Boot to be able
to state those facts in a device tree.  However, the DTS may also be
stating what it has implemented or willing to honor on the Linux side
as well.  So, yeah, there has to be agreement here.

But who gets to make the final adjustment to the device tree?  U-boot
with this code, or the DTS author who may have hand coded specific
wishes and loaded a specific device tree?

HTH,
jdl