On Fri, May 02 2014 at 9:13:08 pm BST, Jon Loeliger <loeli...@gmail.com> wrote:
> On Sat, Apr 26, 2014 at 7:17 AM, Marc Zyngier <marc.zyng...@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;
>> +}
>>
Hi Jon,
> 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?
Well, we have exactly two possibilities:
- PSCI is provided by the platform's firmware, and we'd better not touch
the DT at all.
- PSCI is provided U-Boot, and we *own* the PSCI related nodes.
I don't think there is an alternative to that, because either U-Boot
or the firmware will install its own secure vectors. The DT manipulation
code just reflect this situation.
> 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?
We could have an environment variable named
"i_know_this_looks_silly_but_nevertheless_please_pretty_please_leave_my_cpu_nodes_alone"?
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
