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

[Jean-Jacques Hiblot <jjhiblot@ti.com>]

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.


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