Hi Peter,
On 01/25/2016 03:05 PM, Peter Maydell wrote:
> On 18 January 2016 at 15:16, Eric Auger <eric.au...@linaro.org> wrote:
>> Some passthrough'ed devices depend on clock nodes. Those need to be
>> generated in the guest device tree. This patch introduces some helpers
>> to build a clock node from information retrieved in the host device tree.
>>
>> - copy_properties_from_host copies properties from a host device tree
>> node to a guest device tree node
>> - fdt_build_clock_node builds a guest clock node and checks the host
>> fellow clock is a fixed one.
>>
>> fdt_build_clock_node will become static as soon as it gets used. A
>> dummy pre-declaration is needed for compilation of this patch.
>>
>> Signed-off-by: Eric Auger <eric.au...@linaro.org>
>
>> +
>> +/**
>> + * copy_properties_from_host
>> + *
>> + * copies properties listed in an array from host device tree to
>> + * guest device tree. If a non optional property is not found, the
>> + * function self-asserts. An optional property is ignored if not found
>
> This is just "asserts", not "self-asserts".
ok
>
>> + * in the host device tree.
>> + * @props: array of HostProperty to copy
>> + * @nb_props: number of properties in the array
>> + * @host_dt: host device tree blob
>> + * @guest_dt: guest device tree blob
>> + * @node_path: host dt node path where the property is supposed to be
>> + found
>> + * @nodename: guest node name the properties should be added to
>> + */
>
>> +/**
>> + * fdt_build_clock_node
>> + *
>> + * Build a guest clock node, used as a dependency from a passthrough'ed
>> + * device. Most information are retrieved from the host clock node.
>> + * Also check the host clock is a fixed one.
>> + *
>> + * @host_fdt: host device tree blob from which info are retrieved
>> + * @guest_fdt: guest device tree blob where the clock node is added
>> + * @host_phandle: phandle of the clock in host device tree
>> + * @guest_phandle: phandle to assign to the guest node
>> + */
>> +void fdt_build_clock_node(void *host_fdt, void *guest_fdt,
>> + uint32_t host_phandle,
>> + uint32_t guest_phandle);
>> +void fdt_build_clock_node(void *host_fdt, void *guest_fdt,
>> + uint32_t host_phandle,
>> + uint32_t guest_phandle)
>> +{
>> + char *node_path = NULL;
>> + char *nodename;
>> + const void *r;
>> + int ret, node_offset, prop_len, path_len = 16;
>> +
>> + node_offset = fdt_node_offset_by_phandle(host_fdt, host_phandle);
>> + if (node_offset <= 0) {
>> + error_setg(&error_fatal,
>> + "not able to locate clock handle %d in host device tree",
>> + host_phandle);
>
> Don't we want to return here, rather than continuing with the
> rest of the function? ("goto err;" with an err: label before
> the g_free() at the bottom of the function probably best since
> some of the error paths need to free that.)
due to the &error_fatal, we are going to exit here. I thought it is not
worth going further if we can't find the clock node.
Best Regards
Eric
>
>> + }
>> + node_path = g_malloc(path_len);
>> + while ((ret = fdt_get_path(host_fdt, node_offset, node_path, path_len))
>> + == -FDT_ERR_NOSPACE) {
>> + path_len += 16;
>> + node_path = g_realloc(node_path, path_len);
>> + }
>> + if (ret < 0) {
>> + error_setg(&error_fatal,
>> + "not able to retrieve node path for clock handle %d",
>> + host_phandle);
>> + }
>> +
>> + r = qemu_fdt_getprop(host_fdt, node_path, "compatible", &prop_len,
>> + &error_fatal);
>> + if (strcmp(r, "fixed-clock")) {
>> + error_setg(&error_fatal,
>> + "clock handle %d is not a fixed clock", host_phandle);
>> + }
>> +
>> + nodename = strrchr(node_path, '/');
>> + qemu_fdt_add_subnode(guest_fdt, nodename);
>> +
>> + copy_properties_from_host(clock_copied_properties,
>> + ARRAY_SIZE(clock_copied_properties),
>> + host_fdt, guest_fdt,
>> + node_path, nodename);
>> +
>> + qemu_fdt_setprop_cell(guest_fdt, nodename, "phandle", guest_phandle);
>> +
>> + g_free(node_path);
>> +}
>> +
>> /* Device Specific Code */
>
> thanks
> -- PMM
>
