From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40372) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UnCaO-0001Rk-2D for qemu-devel@nongnu.org; Thu, 13 Jun 2013 14:49:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UnCaM-0007jg-TR for qemu-devel@nongnu.org; Thu, 13 Jun 2013 14:49:28 -0400 Received: from mail-lb0-f173.google.com ([209.85.217.173]:62406) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UnCaM-0007jS-NJ for qemu-devel@nongnu.org; Thu, 13 Jun 2013 14:49:26 -0400 Received: by mail-lb0-f173.google.com with SMTP id v1so5261235lbd.4 for ; Thu, 13 Jun 2013 11:49:25 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1368169118-9528-2-git-send-email-john.rigby@linaro.org> References: <1368169118-9528-1-git-send-email-john.rigby@linaro.org> <1368169118-9528-2-git-send-email-john.rigby@linaro.org> From: Peter Maydell Date: Thu, 13 Jun 2013 19:49:05 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v3 1/3] ARM: Allow boards to provide an fdt blob List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org (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 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