[U-Boot] [PATCH v3 1/2] common: Add support for Android DT image

Sam Protsenko semen.protsenko at linaro.org
Thu Jun 7 15:55:23 UTC 2018


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).

> 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.

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

> 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