[U-Boot] [PATCH] ARM: ti: Update layout for MMC and eMMC (env and dfu)

Jean-Jacques Hiblot
2017-03-16T12:23:50Z

On 24/02/2017 20:35, Sam Protsenko wrote:
> On Fri, Feb 24, 2017 at 3:06 PM, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
>>
>> On 24/02/2017 13:59, Sam Protsenko wrote:
>>> On Fri, Feb 24, 2017 at 10:53 AM, Jean-Jacques Hiblot <jjhiblot@ti.com>
>>> wrote:
>>>>
>>>> On 23/02/2017 20:28, Sam Protsenko wrote:
>>>>> On Thu, Feb 23, 2017 at 5:14 PM, Jean-Jacques Hiblot <jjhiblot@ti.com>
>>>>> wrote:
>>>>>>
>>>>>> On 23/02/2017 14:46, Sam Protsenko wrote:
>>>>>>> On Wed, Feb 22, 2017 at 10:27 AM, Jean-Jacques Hiblot
>>>>>>> <jjhiblot@ti.com>
>>>>>>> wrote:
>>>>>>>> The problems with the current DFU layout are:
>>>>>>>> MMC: The space allocated for u-boot is too small for the latest
>>>>>>>> u-boot
>>>>>>>> (>750KB). We need to increase it. eMMC uses a much bigger
>>>>>>>> area
>>>>>>>> (2MB).
>>>>>>>> eMMC: region "u-boot.img.raw" overlaps the environment area and the
>>>>>>>> region
>>>>>>>> "spl-os-image.raw".
>>>>>>>> both: region "spl-os-image.raw" is quite small and can't handle
>>>>>>>> android
>>>>>>>> kernels
>>>>>>>>
>>>>>>>> Fixing this requires growing some regions and moving others.
>>>>>>>> Care has been taken to leave some room for further growth of
>>>>>>>> "spl-os-args.raw".
>>>>>>>> Also the "env" now appears in the dfu so that it's apparent that the
>>>>>>>> region is not free space that can be used to grow "u-boot.img.raw".
>>>>>>>> The total space allocated for those raw binaries is 16MB, of which
>>>>>>>> 13+MB
>>>>>>>> are reserved for the kernel image.
>>>>>>>>
>>>>>>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Sam,
>>>>>>>>
>>>>>>>> This patch goes a bit further in changing the layout of the MMC than
>>>>>>>> yours. However
>>>>>>>> it doesn't update the GPT partitioning. This would still be needed.
>>>>>>>>
>>>>>>> Hi Jean-Jacques,
>>>>>>>
>>>>>>> Got a couple of questions about this change and my patch (changing
>>>>>>> eMMC partitions table), about how to make them play nice together.
>>>>>>>
>>>>>>> 1. How DFU table is connected to eMMC partition table? And why my
>>>>>>> patch was interfering with DFU table?
>>>>>> AFAIK DFU is not related to GPT. I never used the GPT so I can't for
>>>>>> sure
>>>>>> how it fits together.
>>>>>> My guess is that env_mmc.c has no idea that GPT even exists and uses
>>>>>> raw
>>>>>> access to the eMMC. In this case CONFIG_ENV_OFFSET must be defined to
>>>>>> point
>>>>>> at the beginning of your env partition and must be adapted when the GPT
>>>>>> is
>>>>>> updated. The same goes for the DFU. Ideally GPT and DFU should use the
>>>>>> same
>>>>>> layout (at least for MLO, u-boot, kernel and args).
>>>>>>
>>>>>>> 2. Can I change CONFIG_ENV_OFFSET? Because my changes is going to move
>>>>>>> "reserved" partition on eMMC, where U-Boot environment is stored. Or
>>>>>>> it should be in sync with DFU table somehow? If yes -- we should work
>>>>>>> together to find solution that works best for both cases.
>>>>>> CONFIG_ENV_OFFSET is loosely related to DFU: we don't update the
>>>>>> environment
>>>>>> through DFU. However we must make sure that the area used to store the
>>>>>> env
>>>>>> doesn't overlap a another region (MLO, u-boot, kernel, args). The dfu
>>>>>> doesn't define those regions, it's done with #defines like
>>>>>> CONFIG_SYS_MMCSD_RAW_MODE_KERNEL_SECTOR. But DFU exposes them for
>>>>>> "remote"
>>>>>> upgrade.
>>>>>>
>>>>> So if I change eMMC GPT partition table, it's ok for me to update
>>>>> CONFIG_ENV_OFFSET as well, right?
>>>> Yes you need to change it
>>>>> Because in my case CONFIG_ENV_OFFSET
>>>>> is going to be different from the value your patch sets. In case of
>>>>> eMMC table, we're using "reserved" partition to store U-Boot
>>>>> environment, and I want it to be at 1664 KiB, which gives us
>>>>> CONFIG_ENV_OFFSET=0x1A0000. Is it ok if I change it to that value? Or
>>>>> something else must be changed along with it? If yes, then what
>>>>> exactly?
>>>> You need to adapt DFU if you plan to use it. Maybe you can define a new
>>>> layout for DFU, something like dfu_alt_info_emmc_gpt that will reflect
>>>> your
>>>> GPT layout
>>>>
>>>> Anyway IMO it would be good to use the same offset/size for GPT and
>>>> non-GPT
>>>> case for the following regions:
>>>> - MLO
>>>> - u-boot
>>>> - env
>>>>
>>>> For MLO we already use the same: offset 128kB / size 256kB
>>>> For u-boot, you increased the size to 1MB, I suggest that we increase it
>>>> a
>>>> bit more (1.5MB or 2MB) just to make sure that we don't have to increase
>>>> its
>>>> size again next year.
>>>> For the env, I suggest that the size is increased to 256kB to take care
>>>> of
>>>> redundant env.
>>>>
>>> Thank you for detailed explanation. I'm gonna get back to that task in
>>> a few weeks, don't have enough time right now. Once I come up with
>>> some particular patch, I will add you to reviewers, to be sure that
>>> DFU part is correct.
>> Before you switch to another task maybe we can agree on the offset and sizes
>> for MLO, u-boot and env so that I can do the necessary changes on my side
>> (dfu, #defines).
>>
> Sure. I agree with you that we should increase U-Boot partition. I'm
> gonna do next layout for eMMC table:
>
> MLO: start=128K, size=256K
> U-Boot: size=2M
Sorry I took so long to reply.
I'd re-arrange the following 3 elements and make the dtb area bigger. At
the moment DTBs are around 100k but they keep growing.

> DTB: size=128K
> misc: size=128K
> U-Boot environment: size=256K
U-Boot environment: size=256K
DTB: size=256K
misc: size=128K

JJ

> efs: size=16M
> crypto: size=16K
> recovery: size=10M
> boot: size=10M
> system: size=768M
> cache: size=256M
> ipu1: size=1M
> ipu2: size=1M
> userdata: size=the rest of eMMC space
>
> Is it ok with you? Also, we should check if it's ok with Praneeth, as
> he mentioned that some changes to that table may be necessary in the
> future (from Android side).

>
>>>>> Thanks.
>>>>>
>>>>>>> 3. What about my similar patch for AM57x EVM (see [1])? Is it fine, or
>>>>>>> it affects DFU table as well?
>>>>>> I guess it's OK because the env is stored in a FAT partition:
>>>>>> CONFIG_ENV_OFFSET is not used
>>>>>>
>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>> [1] https://patchwork.ozlabs.org/patch/730736/
>>>>>>>
>>>>>>>> tested on DRA7. DFU updates ok for MMC and eMMC
>>>>>>>>
>>>>>>>> Jean-Jacques
>>>>>>>>
>>>>>>>> include/configs/dra7xx_evm.h | 2 +-
>>>>>>>> include/configs/omap5_uevm.h | 2 +-
>>>>>>>> include/configs/ti_armv7_common.h | 7 ++++---
>>>>>>>> include/environment/ti/dfu.h | 14 ++++++++------
>>>>>>>> 4 files changed, 14 insertions(+), 11 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/include/configs/dra7xx_evm.h
>>>>>>>> b/include/configs/dra7xx_evm.h
>>>>>>>> index bce2816..a56c224 100644
>>>>>>>> --- a/include/configs/dra7xx_evm.h
>>>>>>>> +++ b/include/configs/dra7xx_evm.h
>>>>>>>> @@ -28,7 +28,7 @@
>>>>>>>> #define CONFIG_ENV_IS_IN_MMC
>>>>>>>> #define CONFIG_SYS_MMC_ENV_DEV 1 /* SLOT2: eMMC(1)
>>>>>>>> */
>>>>>>>> #define CONFIG_ENV_SIZE (128 << 10)
>>>>>>>> -#define CONFIG_ENV_OFFSET 0xE0000
>>>>>>>> +#define CONFIG_ENV_OFFSET 0x220000
>>>>>>>> #define CONFIG_ENV_OFFSET_REDUND (CONFIG_ENV_OFFSET +
>>>>>>>> CONFIG_ENV_SIZE)
>>>>>>>> #define CONFIG_SYS_REDUNDAND_ENVIRONMENT
>>>>>>>> #endif
>>>>>>>> diff --git a/include/configs/omap5_uevm.h
>>>>>>>> b/include/configs/omap5_uevm.h
>>>>>>>> index 736d804..127776e 100644
>>>>>>>> --- a/include/configs/omap5_uevm.h
>>>>>>>> +++ b/include/configs/omap5_uevm.h
>>>>>>>> @@ -38,7 +38,7 @@
>>>>>>>> #define CONFIG_ENV_IS_IN_MMC
>>>>>>>> #define CONFIG_SYS_MMC_ENV_DEV 1 /* SLOT2: eMMC(1)
>>>>>>>> */
>>>>>>>> #define CONFIG_ENV_SIZE (128 << 10)
>>>>>>>> -#define CONFIG_ENV_OFFSET 0xE0000
>>>>>>>> +#define CONFIG_ENV_OFFSET 0x220000
>>>>>>>> #define CONFIG_ENV_OFFSET_REDUND (CONFIG_ENV_OFFSET +
>>>>>>>> CONFIG_ENV_SIZE)
>>>>>>>> #define CONFIG_SYS_REDUNDAND_ENVIRONMENT
>>>>>>>>
>>>>>>>> diff --git a/include/configs/ti_armv7_common.h
>>>>>>>> b/include/configs/ti_armv7_common.h
>>>>>>>> index 905e1ef..4cfa134 100644
>>>>>>>> --- a/include/configs/ti_armv7_common.h
>>>>>>>> +++ b/include/configs/ti_armv7_common.h
>>>>>>>> @@ -282,9 +282,10 @@
>>>>>>>> #define CONFIG_SPL_FS_LOAD_ARGS_NAME "args"
>>>>>>>>
>>>>>>>> /* RAW SD card / eMMC */
>>>>>>>> -#define CONFIG_SYS_MMCSD_RAW_MODE_KERNEL_SECTOR 0x900 /*
>>>>>>>> address 0x120000 */
>>>>>>>> -#define CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR 0x80 /* address
>>>>>>>> 0x10000 */
>>>>>>>> -#define CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS 0x80 /* 64KiB */
>>>>>>>> +#define CONFIG_SYS_MMCSD_RAW_MODE_KERNEL_SECTOR 0x1500 /*
>>>>>>>> address 0x2A0000 */
>>>>>>>> +#define CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR 0x1300 /* address
>>>>>>>> 0x260000 */
>>>>>>>> +#define CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS 0x100 /* 128KiB */
>>>>>>>> +
>>>>>>>>
>>>>>>>> /* spl export command */
>>>>>>>> #define CONFIG_CMD_SPL
>>>>>>>> diff --git a/include/environment/ti/dfu.h
>>>>>>>> b/include/environment/ti/dfu.h
>>>>>>>> index caf71a3..63e0ab8 100644
>>>>>>>> --- a/include/environment/ti/dfu.h
>>>>>>>> +++ b/include/environment/ti/dfu.h
>>>>>>>> @@ -12,9 +12,10 @@
>>>>>>>> "rootfs part 0 2;" \
>>>>>>>> "MLO fat 0 1;" \
>>>>>>>> "MLO.raw raw 0x100 0x100;" \
>>>>>>>> - "u-boot.img.raw raw 0x300 0x400;" \
>>>>>>>> - "spl-os-args.raw raw 0x80 0x80;" \
>>>>>>>> - "spl-os-image.raw raw 0x900 0x2000;" \
>>>>>>>> + "u-boot.img.raw raw 0x300 0xE00;" \
>>>>>>>> + "u-env.raw raw 0x1100 0x200;" \
>>>>>>>> + "spl-os-args.raw raw 0x1300 0x100;" \
>>>>>>>> + "spl-os-image.raw raw 0x1500 0x6B00;" \
>>>>>>>> "spl-os-args fat 0 1;" \
>>>>>>>> "spl-os-image fat 0 1;" \
>>>>>>>> "u-boot.img fat 0 1;" \
>>>>>>>> @@ -27,9 +28,10 @@
>>>>>>>> "rootfs part 1 2;" \
>>>>>>>> "MLO fat 1 1;" \
>>>>>>>> "MLO.raw raw 0x100 0x100;" \
>>>>>>>> - "u-boot.img.raw raw 0x300 0x1000;" \
>>>>>>>> - "spl-os-args.raw raw 0x80 0x80;" \
>>>>>>>> - "spl-os-image.raw raw 0x900 0x2000;" \
>>>>>>>> + "u-boot.img.raw raw 0x300 0xE00;" \
>>>>>>>> + "u-env.raw raw 0x1100 0x200;" \
>>>>>>>> + "spl-os-args.raw raw 0x1300 0x100;" \
>>>>>>>> + "spl-os-image.raw raw 0x1500 0x6B00;" \
>>>>>>>> "spl-os-args fat 1 1;" \
>>>>>>>> "spl-os-image fat 1 1;" \
>>>>>>>> "u-boot.img fat 1 1;" \
>>>>>>>> --
>>>>>>>> 1.9.1
>>>>>>>>