[U-Boot] [PATCH v3 1/2] common: Add support for Android DT image
Andrew F. Davis
afd at ti.com
Thu Jun 7 16:23:21 UTC 2018
On 06/07/2018 10:55 AM, Sam Protsenko wrote:
> On 7 June 2018 at 17:50, Andrew F. Davis <afd at ti.com> wrote:
>> On 06/06/2018 09:06 AM, Sam Protsenko wrote:
>>> Android documentation recommends new image format for storing DTB/DTBO
>>> files: [1]. To support that format, this patch adds helper functions for
>>> Android DTB/DTBO format. In image-android-dt.* files you can find helper
>>> functions to work with Android DT image format, such us routines for:
>>> - printing the dump of image structure
>>> - getting the address and size of desired dtb/dtbo file
>>>
>>> This patch uses dt_table.h file, that was added in 643cefa4d848 ("Import
>>> Android's dt_table.h for DT image format") by Alex Deymo.
>>>
>>> [1] https://source.android.com/devices/architecture/dto/partitions
>>>
>>
>>
>> Hmmm, I was CC'd for v2 but not for v3 here, I hope this didn't have
>> anything to do with my objections to this image format [0]..
>>
>
> Sorry, just forgot to add you to the list when doing "git send-email".
> Objections are very welcomed, though in this case I think are not
> related, as I'm not touching TI boards in this series. So please stick
> to actual objections to code quality rather than your personal
> preferences (which I actually share).
>
Code quality is not the only metric for which one can object to a patch..
>> Anyway let me re-iterate my issue for everyone following along at home:
>> we have no need for this image format. We can do all this and more with FIT.
>>
>
> Once you can show me the mail from related Google representatives (or
> similar Android documentation) saying that they are dropping DT image
> format in favor of FIT image (or that FIT format is allowed for that),
> I will remove this code gladly, as I agree with you that FIT approach
> is slightly better (due to configurations). Before this, I'm convinced
> that this is the *only* official way to boot FDTs for Android right
> now, see [1] for details.
>
> [1] https://source.android.com/devices/architecture/dto/partitions
>
>> If Google wants to go off and create some new format then they are free
>> to use it in their bootloader, but if they are going to leverage
>> community projects like U-Boot then they should use existing standards.
>>
>> Mandating (this will not be a "recommends" for long) new formats should
>> not be accepted so easily.
>>
>
> Personally, I don't see a big deal for adding one slim format. Btw,
> they might have some explanation why FIT format isn't applicable for
> Android (e.g. for A/B support or because of some other Project Treble
> requirements). Anyway, I'm just providing a working solution for
> Android requirements [1]. In case Google mandates using only DT
> format, I don't see the other way around.
>
If they have some explanation than you need to provide it as you are the
one upstreaming this. My guess is they just didn't know about FIT at the
time so made this format.
> Bottom line: let's put aside personal preferences, and merge this,
> just because of [1]. This way we have *flexibility* to embrace each of
> formats really fast:
> - for now we can use FIT format
> - if someone wants to use U-Boot for AOSP supported boards, they can
> use DT format
> - if Google agrees on FIT, anyone can switch to FIT
>
Merging this implies approval, then Google can say "Hey, why use FIT
when our format is also an upstream supported format?". And they would
be right. I'm not sure we want to split the supported boot images like that.
As before, when people use it, they can upstream it, right now no
upstream board uses this, for me "just in case" is not a valid reason
for this to be here before we internally (TI/Linaro/Google) settle this.
Even then, U-Boot is a community project, none of these companies get to
mandate what gets into upstream.
Andrew
>> Andrew
>>
>> [0] https://patches.linaro.org/cover/133492/#417053
>>
>>
>>> Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
>>> ---
>>> Changes in v3:
>>> - dropped include/dt_image.h (was sent in another patch by Alex Deymo)
>>> - rebased on top of last master
>>>
>>> common/image-android-dt.c | 157 +++++++++++++++++++++++++++++++++++++
>>> include/image-android-dt.h | 21 +++++
>>> 2 files changed, 178 insertions(+)
>>> create mode 100644 common/image-android-dt.c
>>> create mode 100644 include/image-android-dt.h
>>>
>>> diff --git a/common/image-android-dt.c b/common/image-android-dt.c
>>> new file mode 100644
>>> index 0000000000..9b7683faab
>>> --- /dev/null
>>> +++ b/common/image-android-dt.c
>>> @@ -0,0 +1,157 @@
>>> +/*
>>> + * (C) Copyright 2018 Linaro Ltd.
>>> + * Sam Protsenko <semen.protsenko at linaro.org>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#include <image-android-dt.h>
>>> +#include <dt_table.h>
>>> +#include <common.h>
>>> +#include <linux/libfdt.h>
>>> +#include <mapmem.h>
>>> +
>>> +/**
>>> + * Check if image header is correct.
>>> + *
>>> + * @param hdr_addr Start address of DT image
>>> + * @return true if header is correct or false if header is incorrect
>>> + */
>>> +bool android_dt_check_header(ulong hdr_addr)
>>> +{
>>> + const struct dt_table_header *hdr;
>>> + u32 magic;
>>> +
>>> + hdr = map_sysmem(hdr_addr, sizeof(*hdr));
>>> + magic = fdt32_to_cpu(hdr->magic);
>>> + unmap_sysmem(hdr);
>>> +
>>> + return magic == DT_TABLE_MAGIC;
>>> +}
>>> +
>>> +/**
>>> + * Get the address of FDT (dtb or dtbo) in memory by its index in image.
>>> + *
>>> + * @param hdr_addr Start address of DT image
>>> + * @param index Index of desired FDT in image (starting from 0)
>>> + * @param[out] addr If not NULL, will contain address to specified FDT
>>> + * @param[out] size If not NULL, will contain size of specified FDT
>>> + *
>>> + * @return true on success or false on error
>>> + */
>>> +bool android_dt_get_fdt_by_index(ulong hdr_addr, u32 index, ulong *addr,
>>> + u32 *size)
>>> +{
>>> + const struct dt_table_header *hdr;
>>> + const struct dt_table_entry *e;
>>> + u32 entry_count, entries_offset, entry_size;
>>> + ulong e_addr;
>>> + u32 dt_offset, dt_size;
>>> +
>>> + hdr = map_sysmem(hdr_addr, sizeof(*hdr));
>>> + entry_count = fdt32_to_cpu(hdr->dt_entry_count);
>>> + entries_offset = fdt32_to_cpu(hdr->dt_entries_offset);
>>> + entry_size = fdt32_to_cpu(hdr->dt_entry_size);
>>> + unmap_sysmem(hdr);
>>> +
>>> + if (index > entry_count) {
>>> + printf("Error: index > dt_entry_count (%u > %u)\n", index,
>>> + entry_count);
>>> + return false;
>>> + }
>>> +
>>> + e_addr = hdr_addr + entries_offset + index * entry_size;
>>> + e = map_sysmem(e_addr, sizeof(*e));
>>> + dt_offset = fdt32_to_cpu(e->dt_offset);
>>> + dt_size = fdt32_to_cpu(e->dt_size);
>>> + unmap_sysmem(e);
>>> +
>>> + if (addr)
>>> + *addr = hdr_addr + dt_offset;
>>> + if (size)
>>> + *size = dt_size;
>>> +
>>> + return true;
>>> +}
>>> +
>>> +#if !defined(CONFIG_SPL_BUILD)
>>> +static void android_dt_print_fdt_info(const struct fdt_header *fdt)
>>> +{
>>> + u32 fdt_size;
>>> + int root_node_off;
>>> + const char *compatible = NULL;
>>> +
>>> + fdt_size = fdt_totalsize(fdt);
>>> + root_node_off = fdt_path_offset(fdt, "/");
>>> + if (root_node_off < 0) {
>>> + printf("Error: Root node not found\n");
>>> + } else {
>>> + compatible = fdt_getprop(fdt, root_node_off, "compatible",
>>> + NULL);
>>> + }
>>> +
>>> + printf(" (FDT)size = %d\n", fdt_size);
>>> + printf(" (FDT)compatible = %s\n",
>>> + compatible ? compatible : "(unknown)");
>>> +}
>>> +
>>> +/**
>>> + * Print information about DT image structure.
>>> + *
>>> + * @param hdr_addr Start address of DT image
>>> + */
>>> +void android_dt_print_contents(ulong hdr_addr)
>>> +{
>>> + const struct dt_table_header *hdr;
>>> + u32 entry_count, entries_offset, entry_size;
>>> + u32 i;
>>> +
>>> + hdr = map_sysmem(hdr_addr, sizeof(*hdr));
>>> + entry_count = fdt32_to_cpu(hdr->dt_entry_count);
>>> + entries_offset = fdt32_to_cpu(hdr->dt_entries_offset);
>>> + entry_size = fdt32_to_cpu(hdr->dt_entry_size);
>>> +
>>> + /* Print image header info */
>>> + printf("dt_table_header:\n");
>>> + printf(" magic = %08x\n", fdt32_to_cpu(hdr->magic));
>>> + printf(" total_size = %d\n", fdt32_to_cpu(hdr->total_size));
>>> + printf(" header_size = %d\n", fdt32_to_cpu(hdr->header_size));
>>> + printf(" dt_entry_size = %d\n", entry_size);
>>> + printf(" dt_entry_count = %d\n", entry_count);
>>> + printf(" dt_entries_offset = %d\n", entries_offset);
>>> + printf(" page_size = %d\n", fdt32_to_cpu(hdr->page_size));
>>> + printf(" reserved[0] = %08x\n", fdt32_to_cpu(hdr->reserved[0]));
>>> +
>>> + unmap_sysmem(hdr);
>>> +
>>> + /* Print image entries info */
>>> + for (i = 0; i < entry_count; ++i) {
>>> + const ulong e_addr = hdr_addr + entries_offset + i * entry_size;
>>> + const struct dt_table_entry *e;
>>> + const struct fdt_header *fdt;
>>> + u32 dt_offset, dt_size;
>>> + u32 j;
>>> +
>>> + e = map_sysmem(e_addr, sizeof(*e));
>>> + dt_offset = fdt32_to_cpu(e->dt_offset);
>>> + dt_size = fdt32_to_cpu(e->dt_size);
>>> +
>>> + printf("dt_table_entry[%d]:\n", i);
>>> + printf(" dt_size = %d\n", dt_size);
>>> + printf(" dt_offset = %d\n", dt_offset);
>>> + printf(" id = %08x\n", fdt32_to_cpu(e->id));
>>> + printf(" rev = %08x\n", fdt32_to_cpu(e->rev));
>>> + for (j = 0; j < 4; ++j) {
>>> + printf(" custom[%d] = %08x\n", j,
>>> + fdt32_to_cpu(e->custom[j]));
>>> + }
>>> +
>>> + unmap_sysmem(e);
>>> +
>>> + /* Print FDT info for this entry */
>>> + fdt = map_sysmem(hdr_addr + dt_offset, sizeof(*fdt));
>>> + android_dt_print_fdt_info(fdt);
>>> + unmap_sysmem(fdt);
>>> + }
>>> +}
>>> +#endif
>>> diff --git a/include/image-android-dt.h b/include/image-android-dt.h
>>> new file mode 100644
>>> index 0000000000..08b810d461
>>> --- /dev/null
>>> +++ b/include/image-android-dt.h
>>> @@ -0,0 +1,21 @@
>>> +/*
>>> + * (C) Copyright 2018 Linaro Ltd.
>>> + * Sam Protsenko <semen.protsenko at linaro.org>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0+
>>> + */
>>> +
>>> +#ifndef IMAGE_ANDROID_DT_H
>>> +#define IMAGE_ANDROID_DT_H
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +bool android_dt_check_header(ulong hdr_addr);
>>> +bool android_dt_get_fdt_by_index(ulong hdr_addr, u32 index, ulong *addr,
>>> + u32 *size);
>>> +
>>> +#if !defined(CONFIG_SPL_BUILD)
>>> +void android_dt_print_contents(ulong hdr_addr);
>>> +#endif
>>> +
>>> +#endif /* IMAGE_ANDROID_DT_H */
>>>
More information about the U-Boot
mailing list