(I've taken this work over from John so these review comments
are more in the nature of "notes to myself" than anything else ;-)
On 10 May 2013 07:58, John Rigby <john.ri...@linaro.org> wrote:
> If no fdt is provided on command line and the new field
> get_dtb in struct arm_boot_info is set then call it to
> get a device tree blob.
>
> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
> index 7b2b02d..4c56a1b 100644
> --- a/include/hw/arm/arm.h
> +++ b/include/hw/arm/arm.h
> @@ -31,6 +31,10 @@ struct arm_boot_info {
> const char *kernel_cmdline;
> const char *initrd_filename;
> const char *dtb_filename;
> + /* if a board is able to create a dtb without a dtb file then it
> + * sets get_dtb. This will only be used if no dtb file is provided.
> + */
> + void *(*get_dtb)(hwaddr addr, const struct arm_boot_info *binfo, int
> *size);
The general idea here is OK, but it's not clear to me why
we need to pass in the addr to the hook function (certainly
the example in part 3 doesn't need it). I think we can just
move to
void *(*get_dtb)(const struct arm_boot_info *binfo, int *size);
As with load_device_tree(), we should require (and document!)
that the returned blob is in memory freeable with g_free().
NB: we don't actually free the blob in load_dtb(), either
for load_device_tree() or for this; we should (separate patch).
> hwaddr loader_start;
> /* multicore boards that use the default secondary core boot functions
> * need to put the address of the secondary boot code, the boot reg,
> @@ -59,6 +63,8 @@ struct arm_boot_info {
> int is_linux;
> hwaddr initrd_start;
> hwaddr initrd_size;
> + void *dtb_blob;
> + int dtb_blob_size;
These appear to be relics from a previous version -- they're not used
and can be dropped.
thanks
-- PMM
