[PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Marek Vasut]

The FlexSPI NOR boot offset does not require any special handling,
the image_offset is correct in either case (0x1000 for FlexSPI NOR
and 0x8000 for SD/eMMC) and the offset of u-boot.itb from the start
of flash.bin is always 0x58000 on MX8MN/MX8MP, which matches the
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000 in case
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x300, which is always the
case on MX8MN/MX8MP.

The CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR is really overloaded in
case of the MX8MN/MX8MP, but fixing that needs additional plumbing.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@denx.de>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
---
 arch/arm/mach-imx/spl_imx_romapi.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-imx/spl_imx_romapi.c b/arch/arm/mach-imx/spl_imx_romapi.c
index d827de375a6..c47f5a6bdb4 100644
--- a/arch/arm/mach-imx/spl_imx_romapi.c
+++ b/arch/arm/mach-imx/spl_imx_romapi.c
@@ -38,14 +38,8 @@ ulong spl_romapi_raw_seekable_read(u32 offset, u32 size, void *buf)
 
 ulong __weak spl_romapi_get_uboot_base(u32 image_offset, u32 rom_bt_dev)
 {
-	u32 offset;
-
-	if (((rom_bt_dev >> 16) & 0xff) ==  BT_DEV_TYPE_FLEXSPINOR)
-		offset = CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512;
-	else
-		offset = image_offset + CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000;
-
-	return offset;
+	return image_offset +
+		(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000);
 }
 
 static int is_boot_from_stream_device(u32 boot)
-- 
2.34.1

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Ye Li]

Hi Marek,

On Wed, 2022-03-09 at 17:09 +0100, Marek Vasut wrote:
> Caution: EXT Email
> 
> The FlexSPI NOR boot offset does not require any special handling,
> the image_offset is correct in either case (0x1000 for FlexSPI NOR
> and 0x8000 for SD/eMMC) and the offset of u-boot.itb from the start
> of flash.bin is always 0x58000 on MX8MN/MX8MP, which matches the
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000 in case
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x300, which is always the
> case on MX8MN/MX8MP.
> 
> The CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR is really overloaded in
> case of the MX8MN/MX8MP, but fixing that needs additional plumbing.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <festevam@denx.de>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  arch/arm/mach-imx/spl_imx_romapi.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/spl_imx_romapi.c b/arch/arm/mach-
> imx/spl_imx_romapi.c
> index d827de375a6..c47f5a6bdb4 100644
> --- a/arch/arm/mach-imx/spl_imx_romapi.c
> +++ b/arch/arm/mach-imx/spl_imx_romapi.c
> @@ -38,14 +38,8 @@ ulong spl_romapi_raw_seekable_read(u32 offset, u32
> size, void *buf)
> 
>  ulong __weak spl_romapi_get_uboot_base(u32 image_offset, u32
> rom_bt_dev)
>  {
> -       u32 offset;
> -
> -       if (((rom_bt_dev >> 16) & 0xff) ==  BT_DEV_TYPE_FLEXSPINOR)
> -               offset = CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR *
> 512;
> -       else
> -               offset = image_offset +
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000;
> -
> -       return offset;
> +       return image_offset +
> +               (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 -
> 0x8000);
>  }

The change is problematic to flexspi.
Actually u-boot.itb is fixed at device offset 0x60000 (= 512 * 0x300)
for flexspi/emmc/sd.The case is the image_offset for emmc/sd may vary according to the
primary boot or secondary boot and the eMMC user partition or boot
partition.

If you changed to "image_offset +
(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000", the address
for flexspi becomes 0x59000 (= 0x1000 + 0x60000 - 0x8000)


Best regards,
Ye Li
> 
>  static int is_boot_from_stream_device(u32 boot)
> --
> 2.34.1
> 

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Marek Vasut]

On 3/21/22 04:35, Ye Li wrote:
> Hi Marek,

Hi,

>> diff --git a/arch/arm/mach-imx/spl_imx_romapi.c b/arch/arm/mach-
>> imx/spl_imx_romapi.c
>> index d827de375a6..c47f5a6bdb4 100644
>> --- a/arch/arm/mach-imx/spl_imx_romapi.c
>> +++ b/arch/arm/mach-imx/spl_imx_romapi.c
>> @@ -38,14 +38,8 @@ ulong spl_romapi_raw_seekable_read(u32 offset, u32
>> size, void *buf)
>>
>>   ulong __weak spl_romapi_get_uboot_base(u32 image_offset, u32
>> rom_bt_dev)
>>   {
>> -       u32 offset;
>> -
>> -       if (((rom_bt_dev >> 16) & 0xff) ==  BT_DEV_TYPE_FLEXSPINOR)
>> -               offset = CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR *
>> 512;
>> -       else
>> -               offset = image_offset +
>> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000;
>> -
>> -       return offset;
>> +       return image_offset +
>> +               (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 -
>> 0x8000);
>>   }
> 
> The change is problematic to flexspi.

Yes, I need this change to get boot from flexspi working on i.MX8MP, 
without this change writing flash.bin to flexspi results in unbootable 
system.

> Actually u-boot.itb is fixed at device offset 0x60000 (= 512 * 0x300)
> for flexspi/emmc/sd.The case is the image_offset for emmc/sd may vary according to the
> primary boot or secondary boot and the eMMC user partition or boot
> partition.
> 
> If you changed to "image_offset +
> (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000", the address
> for flexspi becomes 0x59000 (= 0x1000 + 0x60000 - 0x8000)

This is how I populate the FlexSPI on MX8MP:

dhcp ${loadaddr} 192.168.1.1:flash.bin ; \
\
setexpr sfaddr ${loadaddr} - 0x1000 ; \
\
base ${sfaddr} ; \
mw 0 0 0x400 ; \
mw 0x400 0x42464346 ; \
mw 0x404 0x56010000 ; \
mw 0x40c 00030300 ; \
mw 0x444 0x00020101 ; \
mw 0x450 0x10000000 ; \
mw 0x480 0x0818040b ; \
mw 0x484 0x24043008 ; \
mw 0x5c0 0x100 ; \
mw 0x5c4 0x10000 ; \
base 0 ; \
\
setexpr filesize ${filesize} + 0x1000 ; \
\
sf probe && sf update ${sfaddr} 0 ${filesize}

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Ye Li]

Hi Marek,

On Mon, 2022-03-21 at 15:59 +0100, Marek Vasut wrote:
> Caution: EXT Email
> 
> On 3/21/22 04:35, Ye Li wrote:
> > 
> > Hi Marek,
> Hi,
> 
> > 
> > > 
> > > diff --git a/arch/arm/mach-imx/spl_imx_romapi.c b/arch/arm/mach-
> > > imx/spl_imx_romapi.c
> > > index d827de375a6..c47f5a6bdb4 100644
> > > --- a/arch/arm/mach-imx/spl_imx_romapi.c
> > > +++ b/arch/arm/mach-imx/spl_imx_romapi.c
> > > @@ -38,14 +38,8 @@ ulong spl_romapi_raw_seekable_read(u32 offset,
> > > u32
> > > size, void *buf)
> > > 
> > >   ulong __weak spl_romapi_get_uboot_base(u32 image_offset, u32
> > > rom_bt_dev)
> > >   {
> > > -       u32 offset;
> > > -
> > > -       if (((rom_bt_dev >> 16) & 0xff)
> > > ==  BT_DEV_TYPE_FLEXSPINOR)
> > > -               offset = CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
> > > *
> > > 512;
> > > -       else
> > > -               offset = image_offset +
> > > CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000;
> > > -
> > > -       return offset;
> > > +       return image_offset +
> > > +               (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 -
> > > 0x8000);
> > >   }
> > The change is problematic to flexspi.
> Yes, I need this change to get boot from flexspi working on i.MX8MP,
> without this change writing flash.bin to flexspi results in
> unbootable
> system.
> 

To support boot from flexspi, please try another two changes.
1. Modify board/freescale/imx8mp_evk/imximage-8mp-lpddr4.cfg, set 
   "BOOT_FROM" to "fspi". (This change is unnecessary on 8MP after
switch to binman) 

2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the offset
to 0x5f000.  The previous offset 0x58000 is for SD, calculated by
0x60000 - 0x8000 (32KB image offset).

   		uboot: blob-ext@2 {
			filename = "u-boot.itb";
			offset = <0x5f000>;
		};

Best regards,
Ye Li
> > 
> > Actually u-boot.itb is fixed at device offset 0x60000 (= 512 *
> > 0x300)
> > for flexspi/emmc/sd.The case is the image_offset for emmc/sd may
> > vary according to the
> > primary boot or secondary boot and the eMMC user partition or boot
> > partition.
> > 
> > If you changed to "image_offset +
> > (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000", the
> > address
> > for flexspi becomes 0x59000 (= 0x1000 + 0x60000 - 0x8000)
> This is how I populate the FlexSPI on MX8MP:
> 
> dhcp ${loadaddr} 192.168.1.1:flash.bin ; \
> \
> setexpr sfaddr ${loadaddr} - 0x1000 ; \
> \
> base ${sfaddr} ; \
> mw 0 0 0x400 ; \
> mw 0x400 0x42464346 ; \
> mw 0x404 0x56010000 ; \
> mw 0x40c 00030300 ; \
> mw 0x444 0x00020101 ; \
> mw 0x450 0x10000000 ; \
> mw 0x480 0x0818040b ; \
> mw 0x484 0x24043008 ; \
> mw 0x5c0 0x100 ; \
> mw 0x5c4 0x10000 ; \
> base 0 ; \
> \
> setexpr filesize ${filesize} + 0x1000 ; \
> \
> sf probe && sf update ${sfaddr} 0 ${filesize}

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Marek Vasut]

On 3/23/22 03:42, Ye Li wrote:
> Hi Marek,

Hi,

>>>> diff --git a/arch/arm/mach-imx/spl_imx_romapi.c b/arch/arm/mach-
>>>> imx/spl_imx_romapi.c
>>>> index d827de375a6..c47f5a6bdb4 100644
>>>> --- a/arch/arm/mach-imx/spl_imx_romapi.c
>>>> +++ b/arch/arm/mach-imx/spl_imx_romapi.c
>>>> @@ -38,14 +38,8 @@ ulong spl_romapi_raw_seekable_read(u32 offset,
>>>> u32
>>>> size, void *buf)
>>>>
>>>>    ulong __weak spl_romapi_get_uboot_base(u32 image_offset, u32
>>>> rom_bt_dev)
>>>>    {
>>>> -       u32 offset;
>>>> -
>>>> -       if (((rom_bt_dev >> 16) & 0xff)
>>>> ==  BT_DEV_TYPE_FLEXSPINOR)
>>>> -               offset = CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
>>>> *
>>>> 512;
>>>> -       else
>>>> -               offset = image_offset +
>>>> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000;
>>>> -
>>>> -       return offset;
>>>> +       return image_offset +
>>>> +               (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 -
>>>> 0x8000);
>>>>    }
>>> The change is problematic to flexspi.
>> Yes, I need this change to get boot from flexspi working on i.MX8MP,
>> without this change writing flash.bin to flexspi results in
>> unbootable
>> system.
>>
> 
> To support boot from flexspi, please try another two changes.
> 1. Modify board/freescale/imx8mp_evk/imximage-8mp-lpddr4.cfg, set
>     "BOOT_FROM" to "fspi". (This change is unnecessary on 8MP after
> switch to binman)

I already use binman and I need to be able to boot from both SD and 
FlexSPI, with this patch I can do just that.

> 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the offset
> to 0x5f000.  The previous offset 0x58000 is for SD, calculated by
> 0x60000 - 0x8000 (32KB image offset).
> 
>     		uboot: blob-ext@2 {
> 			filename = "u-boot.itb";
> 			offset = <0x5f000>;
> 		};

But that breaks booting from SD card for me ?

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Ye Li]

Hi Marek,

On Wed, 2022-03-23 at 22:16 +0100, Marek Vasut wrote:
> Caution: EXT Email
> 
> On 3/23/22 03:42, Ye Li wrote:
> > 
> > Hi Marek,
> Hi,
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > diff --git a/arch/arm/mach-imx/spl_imx_romapi.c
> > > > > b/arch/arm/mach-
> > > > > imx/spl_imx_romapi.c
> > > > > index d827de375a6..c47f5a6bdb4 100644
> > > > > --- a/arch/arm/mach-imx/spl_imx_romapi.c
> > > > > +++ b/arch/arm/mach-imx/spl_imx_romapi.c
> > > > > @@ -38,14 +38,8 @@ ulong spl_romapi_raw_seekable_read(u32
> > > > > offset,
> > > > > u32
> > > > > size, void *buf)
> > > > > 
> > > > >    ulong __weak spl_romapi_get_uboot_base(u32 image_offset,
> > > > > u32
> > > > > rom_bt_dev)
> > > > >    {
> > > > > -       u32 offset;
> > > > > -
> > > > > -       if (((rom_bt_dev >> 16) & 0xff)
> > > > > ==  BT_DEV_TYPE_FLEXSPINOR)
> > > > > -               offset =
> > > > > CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
> > > > > *
> > > > > 512;
> > > > > -       else
> > > > > -               offset = image_offset +
> > > > > CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000;
> > > > > -
> > > > > -       return offset;
> > > > > +       return image_offset +
> > > > > +               (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR *
> > > > > 512 -
> > > > > 0x8000);
> > > > >    }
> > > > The change is problematic to flexspi.
> > > Yes, I need this change to get boot from flexspi working on
> > > i.MX8MP,
> > > without this change writing flash.bin to flexspi results in
> > > unbootable
> > > system.
> > > 
> > To support boot from flexspi, please try another two changes.
> > 1. Modify board/freescale/imx8mp_evk/imximage-8mp-lpddr4.cfg, set
> >     "BOOT_FROM" to "fspi". (This change is unnecessary on 8MP after
> > switch to binman)
> I already use binman and I need to be able to boot from both SD and
> FlexSPI, with this patch I can do just that.
> 
> > 
> > 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the
> > offset
> > to 0x5f000.  The previous offset 0x58000 is for SD, calculated by
> > 0x60000 - 0x8000 (32KB image offset).
> > 
> >               uboot: blob-ext@2 {
> >                       filename = "u-boot.itb";
> >                       offset = <0x5f000>;
> >               };
> But that breaks booting from SD card for me ?

Do you want to use one flash.bin for both SD and flexspi? 

When first introduced 8m support by imx8mimage.c, we expected the u-
boot.itb at same device offset (0x60000) on SD/emmc and flexspi. The
imx8mimage will calculate the offset inside the flash.bin automatically
according to different IVT offset. The ROMAPI driver also works
correspondingly.
After using binman, the u-boot.itb offset inside the flash.bin has to
be manually set in this DTS node. To follow the original design, this
offset should be different. That's why I asked to update this dts node
for flexspi.

If you change the ROM API driver, that will break our design. You can
try to overwrite spl_romapi_get_uboot_base for your board only.

Best regards,
Ye Li

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Marek Vasut]

On 3/28/22 08:54, Ye Li wrote:
> Hi Marek,

Hi,

[...]

>>> 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the
>>> offset
>>> to 0x5f000.  The previous offset 0x58000 is for SD, calculated by
>>> 0x60000 - 0x8000 (32KB image offset).
>>>
>>>                uboot: blob-ext@2 {
>>>                        filename = "u-boot.itb";
>>>                        offset = <0x5f000>;
>>>                };
>> But that breaks booting from SD card for me ?
> 
> Do you want to use one flash.bin for both SD and flexspi?

Yes, the board I use can boot from SD/eMMC/FlexSPI. I don't want to 
build multiple confusing "flash.bin" files, one for each boot media.

> When first introduced 8m support by imx8mimage.c, we expected the u-
> boot.itb at same device offset (0x60000) on SD/emmc and flexspi. The
> imx8mimage will calculate the offset inside the flash.bin automatically
> according to different IVT offset. The ROMAPI driver also works
> correspondingly.
> After using binman, the u-boot.itb offset inside the flash.bin has to
> be manually set in this DTS node. To follow the original design, this
> offset should be different. That's why I asked to update this dts node
> for flexspi.

This does imply that there are currently no users that boot from flexspi 
in upstream U-Boot, because such users would have to manually modify 
both arch/arm/dts/imx8m?-u-boot.dtsi and board/*/imximage.cfg to 
generate suitable flash.bin which can be started from FlexSPI.

Also, git grep confirms that there are no users:

u-boot$ git grep BOOT_FROM.*fspi
doc/imx/mkimage/imx8image.txt:BOOT_FROM 
[sd|emmc_fastboot|fspi|nand_4k|nand_8k|nand_16k] [sector_size]

> If you change the ROM API driver, that will break our design. You can
> try to overwrite spl_romapi_get_uboot_base for your board only.

Since there are no users which boot from flexspi upstream, this design 
can still be fixed such that it does not require different flash.bin for 
different boot media, but rather one flash.bin works on all boot media. 
I think that is much better.

[...]

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Ye Li]

On Mon, 2022-03-28 at 16:54 +0200, Marek Vasut wrote:
> Caution: EXT Email
> 
> On 3/28/22 08:54, Ye Li wrote:
> > 
> > Hi Marek,
> Hi,
> 
> [...]
> 
> > 
> > > 
> > > > 
> > > > 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the
> > > > offset
> > > > to 0x5f000.  The previous offset 0x58000 is for SD, calculated
> > > > by
> > > > 0x60000 - 0x8000 (32KB image offset).
> > > > 
> > > >                uboot: blob-ext@2 {
> > > >                        filename = "u-boot.itb";
> > > >                        offset = <0x5f000>;
> > > >                };
> > > But that breaks booting from SD card for me ?
> > Do you want to use one flash.bin for both SD and flexspi?
> Yes, the board I use can boot from SD/eMMC/FlexSPI. I don't want to
> build multiple confusing "flash.bin" files, one for each boot media.
> 
> > 
> > When first introduced 8m support by imx8mimage.c, we expected the
> > u-
> > boot.itb at same device offset (0x60000) on SD/emmc and flexspi.
> > The
> > imx8mimage will calculate the offset inside the flash.bin
> > automatically
> > according to different IVT offset. The ROMAPI driver also works
> > correspondingly.
> > After using binman, the u-boot.itb offset inside the flash.bin has
> > to
> > be manually set in this DTS node. To follow the original design,
> > this
> > offset should be different. That's why I asked to update this dts
> > node
> > for flexspi.
> This does imply that there are currently no users that boot from
> flexspi
> in upstream U-Boot, because such users would have to manually modify
> both arch/arm/dts/imx8m?-u-boot.dtsi and board/*/imximage.cfg to
> generate suitable flash.bin which can be started from FlexSPI.
> 
> Also, git grep confirms that there are no users:
> 
> u-boot$ git grep BOOT_FROM.*fspi
> doc/imx/mkimage/imx8image.txt:BOOT_FROM
> [sd|emmc_fastboot|fspi|nand_4k|nand_8k|nand_16k] [sector_size]
> 
> > 
> > If you change the ROM API driver, that will break our design. You
> > can
> > try to overwrite spl_romapi_get_uboot_base for your board only.
> Since there are no users which boot from flexspi upstream, this
> design
> can still be fixed such that it does not require different flash.bin
> for
> different boot media, but rather one flash.bin works on all boot
> media.
> I think that is much better.

Your flash.bin can work on flexspi because you manually program the
flexspi configurations header.  But once you want to upgrade the
flash.bin, flexspi configurations will also be erased due to the block
size. Then you have to reprogram the configurations with flash.bin.
So most of our customers add the flexspi configurations to flash.bin
head. They don't use so called one image for both SD and flexspi. 

As the spl_romapi_get_uboot_base is defined to weak. It is better to
overwrite this function for your particular usage.

Best regards,
Ye Li
> 
> [...]

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Marek Vasut]

On 3/29/22 04:49, Ye Li wrote:

Hi,

>>> If you change the ROM API driver, that will break our design. You
>>> can
>>> try to overwrite spl_romapi_get_uboot_base for your board only.
>> Since there are no users which boot from flexspi upstream, this
>> design
>> can still be fixed such that it does not require different flash.bin
>> for
>> different boot media, but rather one flash.bin works on all boot
>> media.
>> I think that is much better.
> 
> Your flash.bin can work on flexspi because you manually program the
> flexspi configurations header.

Sure, the header content is described in the MX8MP RM Rev. 1 06/2021 
"6.1.5.3.1 FlexSPI Configuration Block", this has nothing to do with 
this patch.

> But once you want to upgrade the
> flash.bin, flexspi configurations will also be erased due to the block
> size. Then you have to reprogram the configurations with flash.bin.
> So most of our customers add the flexspi configurations to flash.bin
> head. They don't use so called one image for both SD and flexspi.

There are no upstream users of flexspi right now, see above.

> As the spl_romapi_get_uboot_base is defined to weak. It is better to
> overwrite this function for your particular usage.

I would much rather prefer to have one flash.bin which works on both SD 
card and FlexSPI, on all iMX8M, that is far less confusing. And since 
there are no upstream users of flexspi boot so far, this is how it can 
still be implemented, consistently.

If downstream wants to have multiple flash.bin, one for each boot media, 
that's up to downstream.

[...]

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Ye Li]

On Tue, 2022-03-29 at 11:01 +0200, Marek Vasut wrote:
> Caution: EXT Email
> 
> On 3/29/22 04:49, Ye Li wrote:
> 
> Hi,
> 
> > 
> > > 
> > > > 
> > > > If you change the ROM API driver, that will break our design.
> > > > You
> > > > can
> > > > try to overwrite spl_romapi_get_uboot_base for your board only.
> > > Since there are no users which boot from flexspi upstream, this
> > > design
> > > can still be fixed such that it does not require different
> > > flash.bin
> > > for
> > > different boot media, but rather one flash.bin works on all boot
> > > media.
> > > I think that is much better.
> > Your flash.bin can work on flexspi because you manually program the
> > flexspi configurations header.
> Sure, the header content is described in the MX8MP RM Rev. 1 06/2021
> "6.1.5.3.1 FlexSPI Configuration Block", this has nothing to do with
> this patch.
> 
> > 
> > But once you want to upgrade the
> > flash.bin, flexspi configurations will also be erased due to the
> > block
> > size. Then you have to reprogram the configurations with flash.bin.
> > So most of our customers add the flexspi configurations to
> > flash.bin
> > head. They don't use so called one image for both SD and flexspi.
> There are no upstream users of flexspi right now, see above.
> 
> > 
> > As the spl_romapi_get_uboot_base is defined to weak. It is better
> > to
> > overwrite this function for your particular usage.
> I would much rather prefer to have one flash.bin which works on both
> SD
> card and FlexSPI, on all iMX8M, that is far less confusing. And since
> there are no upstream users of flexspi boot so far, this is how it
> can
> still be implemented, consistently.

I can think out 3 drawbacks using this one flash.bin for flexspi:

1. The flexspi configuration header will be erased when you update the
flash.bin to flexspi device. In a common usage, this header will
combine with flash.bin to a final boot image which is not same with SD.
 
2. How can users update u-boot.itb only if using this one flash.bin?
With the same offset of SD, it causes the u-boot.itb locates at a
offset not block aligned.

3. Not all iMX8M can support this one flash.bin.  8MM and 8MQ have
different IVT. Their flexspi IVT can't work for SD/eMMC.

Best regards,
Ye Li
> 
> If downstream wants to have multiple flash.bin, one for each boot
> media,
> that's up to downstream.
> 
> [...]

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Marek Vasut]

On 3/29/22 11:56, Ye Li wrote:

Hi,

>>> But once you want to upgrade the
>>> flash.bin, flexspi configurations will also be erased due to the
>>> block
>>> size. Then you have to reprogram the configurations with flash.bin.
>>> So most of our customers add the flexspi configurations to
>>> flash.bin
>>> head. They don't use so called one image for both SD and flexspi.
>> There are no upstream users of flexspi right now, see above.
>>
>>>
>>> As the spl_romapi_get_uboot_base is defined to weak. It is better
>>> to
>>> overwrite this function for your particular usage.
>> I would much rather prefer to have one flash.bin which works on both
>> SD
>> card and FlexSPI, on all iMX8M, that is far less confusing. And since
>> there are no upstream users of flexspi boot so far, this is how it
>> can
>> still be implemented, consistently.
> 
> I can think out 3 drawbacks using this one flash.bin for flexspi:
> 
> 1. The flexspi configuration header will be erased when you update the
> flash.bin to flexspi device. In a common usage, this header will
> combine with flash.bin to a final boot image which is not same with SD.

This is not correct.

If you need to update only the flash.bin in SPI NOR without rewriting 
the FCFB header, use 'sf update' with 0x1000 start address:
dhcp ${loadaddr} 192.168.1.1:flash.bin && \
   sf update ${loadaddr} 0x1000 ${filesize}

If you need to update both the flash.bin and generate the FCFB, use the 
aforementioned script with sf update using 0x0 start address, i.e.:
dhcp ${loadaddr} 192.168.1.1:flash.bin ; \
\
setexpr sfaddr ${loadaddr} - 0x1000 ; \
\
base ${sfaddr} ; \
mw 0 0 0x400 ; \
mw 0x400 0x42464346 ; \
mw 0x404 0x56010000 ; \
mw 0x40c 00030300 ; \
mw 0x444 0x00020101 ; \
mw 0x450 0x10000000 ; \
mw 0x480 0x0818040b ; \
mw 0x484 0x24043008 ; \
mw 0x5c0 0x100 ; \
mw 0x5c4 0x10000 ; \
base 0 ; \
\
setexpr filesize ${filesize} + 0x1000 ; \
\
sf probe && sf update ${sfaddr} 0 ${filesize}

> 2. How can users update u-boot.itb only if using this one flash.bin?

Write u-boot.itb to offset 0x59000 in SPI NOR:
=> sf update ${loadaddr} 0x59000 ${filesize}

That 0x59000 is ( imx8mp-u-boot.dtsi /imx-boot/blob-ext@2/offset = 
<0x58000>) + (the SPI NOR flash.bin offset = 0x1000) = 0x59000

> With the same offset of SD, it causes the u-boot.itb locates at a
> offset not block aligned.

0x59000 is both 4 kiB and 512 Byte aligned .

> 3. Not all iMX8M can support this one flash.bin.  8MM and 8MQ have
> different IVT. Their flexspi IVT can't work for SD/eMMC.

What's the difference ? Looking at the MX8MM RM rev.3, FlexSPI boot 
looks very much the same.

(we have no MX8MQ/MX8MM boards which boot from FlexSPI either).

...

I still believe it is better (=less confusing for users) to have one 
unified flash.bin for all boot media.

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Fabio Estevam]

Hi Marek,

On Wed, Mar 30, 2022 at 7:27 PM Marek Vasut <marex@denx.de> wrote:

> I still believe it is better (=less confusing for users) to have one
> unified flash.bin for all boot media.

I agree with your approach.

Thanks

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Ye Li]

On Thu, 2022-03-31 at 00:27 +0200, Marek Vasut wrote:
> Caution: EXT Email
> 
> On 3/29/22 11:56, Ye Li wrote:
> 
> Hi,
> 
> > 
> > > 
> > > > 
> > > > But once you want to upgrade the
> > > > flash.bin, flexspi configurations will also be erased due to
> > > > the
> > > > block
> > > > size. Then you have to reprogram the configurations with
> > > > flash.bin.
> > > > So most of our customers add the flexspi configurations to
> > > > flash.bin
> > > > head. They don't use so called one image for both SD and
> > > > flexspi.
> > > There are no upstream users of flexspi right now, see above.
> > > 
> > > > 
> > > > 
> > > > As the spl_romapi_get_uboot_base is defined to weak. It is
> > > > better
> > > > to
> > > > overwrite this function for your particular usage.
> > > I would much rather prefer to have one flash.bin which works on
> > > both
> > > SD
> > > card and FlexSPI, on all iMX8M, that is far less confusing. And
> > > since
> > > there are no upstream users of flexspi boot so far, this is how
> > > it
> > > can
> > > still be implemented, consistently.
> > I can think out 3 drawbacks using this one flash.bin for flexspi:
> > 
> > 1. The flexspi configuration header will be erased when you update
> > the
> > flash.bin to flexspi device. In a common usage, this header will
> > combine with flash.bin to a final boot image which is not same with
> > SD.
> This is not correct.
> 
> If you need to update only the flash.bin in SPI NOR without rewriting
> the FCFB header, use 'sf update' with 0x1000 start address:
> dhcp ${loadaddr} 192.168.1.1:flash.bin && \
>    sf update ${loadaddr} 0x1000 ${filesize}
> 
> If you need to update both the flash.bin and generate the FCFB, use
> the
> aforementioned script with sf update using 0x0 start address, i.e.:
> dhcp ${loadaddr} 192.168.1.1:flash.bin ; \
> \
> setexpr sfaddr ${loadaddr} - 0x1000 ; \
> \
> base ${sfaddr} ; \
> mw 0 0 0x400 ; \
> mw 0x400 0x42464346 ; \
> mw 0x404 0x56010000 ; \
> mw 0x40c 00030300 ; \
> mw 0x444 0x00020101 ; \
> mw 0x450 0x10000000 ; \
> mw 0x480 0x0818040b ; \
> mw 0x484 0x24043008 ; \
> mw 0x5c0 0x100 ; \
> mw 0x5c4 0x10000 ; \
> base 0 ; \
> \
> setexpr filesize ${filesize} + 0x1000 ; \
> \
> sf probe && sf update ${sfaddr} 0 ${filesize}
> 
> > 
> > 2. How can users update u-boot.itb only if using this one
> > flash.bin?
> Write u-boot.itb to offset 0x59000 in SPI NOR:
> => sf update ${loadaddr} 0x59000 ${filesize}
> 
> That 0x59000 is ( imx8mp-u-boot.dtsi /imx-boot/blob-ext@2/offset =
> <0x58000>) + (the SPI NOR flash.bin offset = 0x1000) = 0x59000
> 
> > 
> > With the same offset of SD, it causes the u-boot.itb locates at a
> > offset not block aligned.
> 0x59000 is both 4 kiB and 512 Byte aligned .

4KB is not a erase block size supported by all NOR flash. You should
use 64KB which is default to all.

Same for #1 drawback, 0x1000 offset can't satify the erase block size.
so every time your update flash.bin will need to update flash
configuration header as well, why not combine them together.


> 
> > 
> > 3. Not all iMX8M can support this one flash.bin.  8MM and 8MQ have
> > different IVT. Their flexspi IVT can't work for SD/eMMC.
> What's the difference ? Looking at the MX8MM RM rev.3, FlexSPI boot
> looks very much the same.
> 
No. The IVT structures are same but the base used by the fields in IVT
is changed. So you can't use one image for flexspi and SD, it will fail
to boot.

Another thing is the from imx8mm-u-boot.dtsi. the offset is 0x57c00. it
even does not align with 4KB. 

         binman_uboot: uboot {
             filename = "u-boot.itb";
             offset = <0x57c00>;
             type = "blob-ext";
         };


Best regards,
Ye Li


> (we have no MX8MQ/MX8MM boards which boot from FlexSPI either).
> 
> ...
> 
> I still believe it is better (=less confusing for users) to have one
> unified flash.bin for all boot media.

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Tim Harvey]

On Mon, Mar 28, 2022 at 7:55 AM Marek Vasut <marex@denx.de> wrote:
>
> On 3/28/22 08:54, Ye Li wrote:
> > Hi Marek,
>
> Hi,
>
> [...]
>
> >>> 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the
> >>> offset
> >>> to 0x5f000.  The previous offset 0x58000 is for SD, calculated by
> >>> 0x60000 - 0x8000 (32KB image offset).
> >>>
> >>>                uboot: blob-ext@2 {
> >>>                        filename = "u-boot.itb";
> >>>                        offset = <0x5f000>;
> >>>                };
> >> But that breaks booting from SD card for me ?
> >
> > Do you want to use one flash.bin for both SD and flexspi?
>
> Yes, the board I use can boot from SD/eMMC/FlexSPI. I don't want to
> build multiple confusing "flash.bin" files, one for each boot media.
>
> > When first introduced 8m support by imx8mimage.c, we expected the u-
> > boot.itb at same device offset (0x60000) on SD/emmc and flexspi. The
> > imx8mimage will calculate the offset inside the flash.bin automatically
> > according to different IVT offset. The ROMAPI driver also works
> > correspondingly.
> > After using binman, the u-boot.itb offset inside the flash.bin has to
> > be manually set in this DTS node. To follow the original design, this
> > offset should be different. That's why I asked to update this dts node
> > for flexspi.
>
> This does imply that there are currently no users that boot from flexspi
> in upstream U-Boot, because such users would have to manually modify
> both arch/arm/dts/imx8m?-u-boot.dtsi and board/*/imximage.cfg to
> generate suitable flash.bin which can be started from FlexSPI.
>
> Also, git grep confirms that there are no users:
>
> u-boot$ git grep BOOT_FROM.*fspi
> doc/imx/mkimage/imx8image.txt:BOOT_FROM
> [sd|emmc_fastboot|fspi|nand_4k|nand_8k|nand_16k] [sector_size]
>
> > If you change the ROM API driver, that will break our design. You can
> > try to overwrite spl_romapi_get_uboot_base for your board only.
>
> Since there are no users which boot from flexspi upstream, this design
> can still be fixed such that it does not require different flash.bin for
> different boot media, but rather one flash.bin works on all boot media.
> I think that is much better.
>

Marek,

I'm also a fan of single U-Boot binaries supporting multiple boards
and configurations.

How are you handling env for the various boot devices? I haven't
looked in a while... can multiple env support co-exist now between
MMC/SPI and be selected at runtime?

Best Regards,

Tim

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Marek Vasut]

On 3/31/22 17:09, Tim Harvey wrote:
> On Mon, Mar 28, 2022 at 7:55 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/28/22 08:54, Ye Li wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>> [...]
>>
>>>>> 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the
>>>>> offset
>>>>> to 0x5f000.  The previous offset 0x58000 is for SD, calculated by
>>>>> 0x60000 - 0x8000 (32KB image offset).
>>>>>
>>>>>                 uboot: blob-ext@2 {
>>>>>                         filename = "u-boot.itb";
>>>>>                         offset = <0x5f000>;
>>>>>                 };
>>>> But that breaks booting from SD card for me ?
>>>
>>> Do you want to use one flash.bin for both SD and flexspi?
>>
>> Yes, the board I use can boot from SD/eMMC/FlexSPI. I don't want to
>> build multiple confusing "flash.bin" files, one for each boot media.
>>
>>> When first introduced 8m support by imx8mimage.c, we expected the u-
>>> boot.itb at same device offset (0x60000) on SD/emmc and flexspi. The
>>> imx8mimage will calculate the offset inside the flash.bin automatically
>>> according to different IVT offset. The ROMAPI driver also works
>>> correspondingly.
>>> After using binman, the u-boot.itb offset inside the flash.bin has to
>>> be manually set in this DTS node. To follow the original design, this
>>> offset should be different. That's why I asked to update this dts node
>>> for flexspi.
>>
>> This does imply that there are currently no users that boot from flexspi
>> in upstream U-Boot, because such users would have to manually modify
>> both arch/arm/dts/imx8m?-u-boot.dtsi and board/*/imximage.cfg to
>> generate suitable flash.bin which can be started from FlexSPI.
>>
>> Also, git grep confirms that there are no users:
>>
>> u-boot$ git grep BOOT_FROM.*fspi
>> doc/imx/mkimage/imx8image.txt:BOOT_FROM
>> [sd|emmc_fastboot|fspi|nand_4k|nand_8k|nand_16k] [sector_size]
>>
>>> If you change the ROM API driver, that will break our design. You can
>>> try to overwrite spl_romapi_get_uboot_base for your board only.
>>
>> Since there are no users which boot from flexspi upstream, this design
>> can still be fixed such that it does not require different flash.bin for
>> different boot media, but rather one flash.bin works on all boot media.
>> I think that is much better.
>>
> 
> Marek,
> 
> I'm also a fan of single U-Boot binaries supporting multiple boards
> and configurations.
> 
> How are you handling env for the various boot devices? I haven't
> looked in a while... can multiple env support co-exist now between
> MMC/SPI and be selected at runtime?

Do '$ git grep env_get_location' , you will find board-level examples 
where this function is implemented and it returns ENVL_* based on 
various hardware-specific conditions.

I _think_ there is even imx arch specific implementation for this 
env_get_location() which automatically picks SD/eMMC/FlexSPI for you 
right now.

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Tim Harvey]

On Thu, Mar 31, 2022 at 8:26 AM Marek Vasut <marex@denx.de> wrote:
>
> On 3/31/22 17:09, Tim Harvey wrote:
> > On Mon, Mar 28, 2022 at 7:55 AM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 3/28/22 08:54, Ye Li wrote:
> >>> Hi Marek,
> >>
> >> Hi,
> >>
> >> [...]
> >>
> >>>>> 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the
> >>>>> offset
> >>>>> to 0x5f000.  The previous offset 0x58000 is for SD, calculated by
> >>>>> 0x60000 - 0x8000 (32KB image offset).
> >>>>>
> >>>>>                 uboot: blob-ext@2 {
> >>>>>                         filename = "u-boot.itb";
> >>>>>                         offset = <0x5f000>;
> >>>>>                 };
> >>>> But that breaks booting from SD card for me ?
> >>>
> >>> Do you want to use one flash.bin for both SD and flexspi?
> >>
> >> Yes, the board I use can boot from SD/eMMC/FlexSPI. I don't want to
> >> build multiple confusing "flash.bin" files, one for each boot media.
> >>
> >>> When first introduced 8m support by imx8mimage.c, we expected the u-
> >>> boot.itb at same device offset (0x60000) on SD/emmc and flexspi. The
> >>> imx8mimage will calculate the offset inside the flash.bin automatically
> >>> according to different IVT offset. The ROMAPI driver also works
> >>> correspondingly.
> >>> After using binman, the u-boot.itb offset inside the flash.bin has to
> >>> be manually set in this DTS node. To follow the original design, this
> >>> offset should be different. That's why I asked to update this dts node
> >>> for flexspi.
> >>
> >> This does imply that there are currently no users that boot from flexspi
> >> in upstream U-Boot, because such users would have to manually modify
> >> both arch/arm/dts/imx8m?-u-boot.dtsi and board/*/imximage.cfg to
> >> generate suitable flash.bin which can be started from FlexSPI.
> >>
> >> Also, git grep confirms that there are no users:
> >>
> >> u-boot$ git grep BOOT_FROM.*fspi
> >> doc/imx/mkimage/imx8image.txt:BOOT_FROM
> >> [sd|emmc_fastboot|fspi|nand_4k|nand_8k|nand_16k] [sector_size]
> >>
> >>> If you change the ROM API driver, that will break our design. You can
> >>> try to overwrite spl_romapi_get_uboot_base for your board only.
> >>
> >> Since there are no users which boot from flexspi upstream, this design
> >> can still be fixed such that it does not require different flash.bin for
> >> different boot media, but rather one flash.bin works on all boot media.
> >> I think that is much better.
> >>
> >
> > Marek,
> >
> > I'm also a fan of single U-Boot binaries supporting multiple boards
> > and configurations.
> >
> > How are you handling env for the various boot devices? I haven't
> > looked in a while... can multiple env support co-exist now between
> > MMC/SPI and be selected at runtime?
>
> Do '$ git grep env_get_location' , you will find board-level examples
> where this function is implemented and it returns ENVL_* based on
> various hardware-specific conditions.
>
> I _think_ there is even imx arch specific implementation for this
> env_get_location() which automatically picks SD/eMMC/FlexSPI for you
> right now.

Marek,

I guess where I keep getting hung up is how to configure the ENV for
multiple env drivers as there is only one CONFIG_ENV_OFFSET Kconfig
which means for all devices your going to have the same offset/redund
configuration which is likely not appropriate for mmc/nand/spi for
example. I don't see any way currently to configure those separately.
Unless I'm missing something obvious I suppose get_env_location()
could be augmented to be able to return board-specific env location
config as well.

Best regards,

Tim

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Marek Vasut]

On 3/31/22 18:03, Tim Harvey wrote:
> On Thu, Mar 31, 2022 at 8:26 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/31/22 17:09, Tim Harvey wrote:
>>> On Mon, Mar 28, 2022 at 7:55 AM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 3/28/22 08:54, Ye Li wrote:
>>>>> Hi Marek,
>>>>
>>>> Hi,
>>>>
>>>> [...]
>>>>
>>>>>>> 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the
>>>>>>> offset
>>>>>>> to 0x5f000.  The previous offset 0x58000 is for SD, calculated by
>>>>>>> 0x60000 - 0x8000 (32KB image offset).
>>>>>>>
>>>>>>>                  uboot: blob-ext@2 {
>>>>>>>                          filename = "u-boot.itb";
>>>>>>>                          offset = <0x5f000>;
>>>>>>>                  };
>>>>>> But that breaks booting from SD card for me ?
>>>>>
>>>>> Do you want to use one flash.bin for both SD and flexspi?
>>>>
>>>> Yes, the board I use can boot from SD/eMMC/FlexSPI. I don't want to
>>>> build multiple confusing "flash.bin" files, one for each boot media.
>>>>
>>>>> When first introduced 8m support by imx8mimage.c, we expected the u-
>>>>> boot.itb at same device offset (0x60000) on SD/emmc and flexspi. The
>>>>> imx8mimage will calculate the offset inside the flash.bin automatically
>>>>> according to different IVT offset. The ROMAPI driver also works
>>>>> correspondingly.
>>>>> After using binman, the u-boot.itb offset inside the flash.bin has to
>>>>> be manually set in this DTS node. To follow the original design, this
>>>>> offset should be different. That's why I asked to update this dts node
>>>>> for flexspi.
>>>>
>>>> This does imply that there are currently no users that boot from flexspi
>>>> in upstream U-Boot, because such users would have to manually modify
>>>> both arch/arm/dts/imx8m?-u-boot.dtsi and board/*/imximage.cfg to
>>>> generate suitable flash.bin which can be started from FlexSPI.
>>>>
>>>> Also, git grep confirms that there are no users:
>>>>
>>>> u-boot$ git grep BOOT_FROM.*fspi
>>>> doc/imx/mkimage/imx8image.txt:BOOT_FROM
>>>> [sd|emmc_fastboot|fspi|nand_4k|nand_8k|nand_16k] [sector_size]
>>>>
>>>>> If you change the ROM API driver, that will break our design. You can
>>>>> try to overwrite spl_romapi_get_uboot_base for your board only.
>>>>
>>>> Since there are no users which boot from flexspi upstream, this design
>>>> can still be fixed such that it does not require different flash.bin for
>>>> different boot media, but rather one flash.bin works on all boot media.
>>>> I think that is much better.
>>>>
>>>
>>> Marek,
>>>
>>> I'm also a fan of single U-Boot binaries supporting multiple boards
>>> and configurations.
>>>
>>> How are you handling env for the various boot devices? I haven't
>>> looked in a while... can multiple env support co-exist now between
>>> MMC/SPI and be selected at runtime?
>>
>> Do '$ git grep env_get_location' , you will find board-level examples
>> where this function is implemented and it returns ENVL_* based on
>> various hardware-specific conditions.
>>
>> I _think_ there is even imx arch specific implementation for this
>> env_get_location() which automatically picks SD/eMMC/FlexSPI for you
>> right now.
> 
> Marek,
> 
> I guess where I keep getting hung up is how to configure the ENV for
> multiple env drivers as there is only one CONFIG_ENV_OFFSET Kconfig
> which means for all devices your going to have the same offset/redund
> configuration which is likely not appropriate for mmc/nand/spi for
> example. I don't see any way currently to configure those separately.
> Unless I'm missing something obvious I suppose get_env_location()
> could be augmented to be able to return board-specific env location
> config as well.

e.g. env/mmc.c has the following:

  78         } dt_prop = {
  79                 .offset_redund = "u-boot,mmc-env-offset-redundant",
  80                 .partition = "u-boot,mmc-env-partition",
  81                 .offset = "u-boot,mmc-env-offset",
  82         };

So use that to specify per-env-driver offset. Indeed, the 
CONFIG_ENV*OFFSET is shared by all env driver.

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Tim Harvey]

On Thu, Mar 31, 2022 at 9:41 AM Marek Vasut <marex@denx.de> wrote:
>
> On 3/31/22 18:03, Tim Harvey wrote:
> > On Thu, Mar 31, 2022 at 8:26 AM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 3/31/22 17:09, Tim Harvey wrote:
> >>> On Mon, Mar 28, 2022 at 7:55 AM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> On 3/28/22 08:54, Ye Li wrote:
> >>>>> Hi Marek,
> >>>>
> >>>> Hi,
> >>>>
> >>>> [...]
> >>>>
> >>>>>>> 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the
> >>>>>>> offset
> >>>>>>> to 0x5f000.  The previous offset 0x58000 is for SD, calculated by
> >>>>>>> 0x60000 - 0x8000 (32KB image offset).
> >>>>>>>
> >>>>>>>                  uboot: blob-ext@2 {
> >>>>>>>                          filename = "u-boot.itb";
> >>>>>>>                          offset = <0x5f000>;
> >>>>>>>                  };
> >>>>>> But that breaks booting from SD card for me ?
> >>>>>
> >>>>> Do you want to use one flash.bin for both SD and flexspi?
> >>>>
> >>>> Yes, the board I use can boot from SD/eMMC/FlexSPI. I don't want to
> >>>> build multiple confusing "flash.bin" files, one for each boot media.
> >>>>
> >>>>> When first introduced 8m support by imx8mimage.c, we expected the u-
> >>>>> boot.itb at same device offset (0x60000) on SD/emmc and flexspi. The
> >>>>> imx8mimage will calculate the offset inside the flash.bin automatically
> >>>>> according to different IVT offset. The ROMAPI driver also works
> >>>>> correspondingly.
> >>>>> After using binman, the u-boot.itb offset inside the flash.bin has to
> >>>>> be manually set in this DTS node. To follow the original design, this
> >>>>> offset should be different. That's why I asked to update this dts node
> >>>>> for flexspi.
> >>>>
> >>>> This does imply that there are currently no users that boot from flexspi
> >>>> in upstream U-Boot, because such users would have to manually modify
> >>>> both arch/arm/dts/imx8m?-u-boot.dtsi and board/*/imximage.cfg to
> >>>> generate suitable flash.bin which can be started from FlexSPI.
> >>>>
> >>>> Also, git grep confirms that there are no users:
> >>>>
> >>>> u-boot$ git grep BOOT_FROM.*fspi
> >>>> doc/imx/mkimage/imx8image.txt:BOOT_FROM
> >>>> [sd|emmc_fastboot|fspi|nand_4k|nand_8k|nand_16k] [sector_size]
> >>>>
> >>>>> If you change the ROM API driver, that will break our design. You can
> >>>>> try to overwrite spl_romapi_get_uboot_base for your board only.
> >>>>
> >>>> Since there are no users which boot from flexspi upstream, this design
> >>>> can still be fixed such that it does not require different flash.bin for
> >>>> different boot media, but rather one flash.bin works on all boot media.
> >>>> I think that is much better.
> >>>>
> >>>
> >>> Marek,
> >>>
> >>> I'm also a fan of single U-Boot binaries supporting multiple boards
> >>> and configurations.
> >>>
> >>> How are you handling env for the various boot devices? I haven't
> >>> looked in a while... can multiple env support co-exist now between
> >>> MMC/SPI and be selected at runtime?
> >>
> >> Do '$ git grep env_get_location' , you will find board-level examples
> >> where this function is implemented and it returns ENVL_* based on
> >> various hardware-specific conditions.
> >>
> >> I _think_ there is even imx arch specific implementation for this
> >> env_get_location() which automatically picks SD/eMMC/FlexSPI for you
> >> right now.
> >
> > Marek,
> >
> > I guess where I keep getting hung up is how to configure the ENV for
> > multiple env drivers as there is only one CONFIG_ENV_OFFSET Kconfig
> > which means for all devices your going to have the same offset/redund
> > configuration which is likely not appropriate for mmc/nand/spi for
> > example. I don't see any way currently to configure those separately.
> > Unless I'm missing something obvious I suppose get_env_location()
> > could be augmented to be able to return board-specific env location
> > config as well.
>
> e.g. env/mmc.c has the following:
>
>   78         } dt_prop = {
>   79                 .offset_redund = "u-boot,mmc-env-offset-redundant",
>   80                 .partition = "u-boot,mmc-env-partition",
>   81                 .offset = "u-boot,mmc-env-offset",
>   82         };
>
> So use that to specify per-env-driver offset. Indeed, the
> CONFIG_ENV*OFFSET is shared by all env driver.

Right... but I think the dt overrides there only exist for MMC (I
don't see then in env/nand.c for example)... but I can add that. I've
always wanted to have a single binary for IMX6 nand vs emmc boards and
this has been the ony hangup.

Thanks!

Tim

Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[Marek Vasut]

On 3/31/22 20:02, Tim Harvey wrote:
> On Thu, Mar 31, 2022 at 9:41 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/31/22 18:03, Tim Harvey wrote:
>>> On Thu, Mar 31, 2022 at 8:26 AM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 3/31/22 17:09, Tim Harvey wrote:
>>>>> On Mon, Mar 28, 2022 at 7:55 AM Marek Vasut <marex@denx.de> wrote:
>>>>>>
>>>>>> On 3/28/22 08:54, Ye Li wrote:
>>>>>>> Hi Marek,
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>>>> 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the
>>>>>>>>> offset
>>>>>>>>> to 0x5f000.  The previous offset 0x58000 is for SD, calculated by
>>>>>>>>> 0x60000 - 0x8000 (32KB image offset).
>>>>>>>>>
>>>>>>>>>                   uboot: blob-ext@2 {
>>>>>>>>>                           filename = "u-boot.itb";
>>>>>>>>>                           offset = <0x5f000>;
>>>>>>>>>                   };
>>>>>>>> But that breaks booting from SD card for me ?
>>>>>>>
>>>>>>> Do you want to use one flash.bin for both SD and flexspi?
>>>>>>
>>>>>> Yes, the board I use can boot from SD/eMMC/FlexSPI. I don't want to
>>>>>> build multiple confusing "flash.bin" files, one for each boot media.
>>>>>>
>>>>>>> When first introduced 8m support by imx8mimage.c, we expected the u-
>>>>>>> boot.itb at same device offset (0x60000) on SD/emmc and flexspi. The
>>>>>>> imx8mimage will calculate the offset inside the flash.bin automatically
>>>>>>> according to different IVT offset. The ROMAPI driver also works
>>>>>>> correspondingly.
>>>>>>> After using binman, the u-boot.itb offset inside the flash.bin has to
>>>>>>> be manually set in this DTS node. To follow the original design, this
>>>>>>> offset should be different. That's why I asked to update this dts node
>>>>>>> for flexspi.
>>>>>>
>>>>>> This does imply that there are currently no users that boot from flexspi
>>>>>> in upstream U-Boot, because such users would have to manually modify
>>>>>> both arch/arm/dts/imx8m?-u-boot.dtsi and board/*/imximage.cfg to
>>>>>> generate suitable flash.bin which can be started from FlexSPI.
>>>>>>
>>>>>> Also, git grep confirms that there are no users:
>>>>>>
>>>>>> u-boot$ git grep BOOT_FROM.*fspi
>>>>>> doc/imx/mkimage/imx8image.txt:BOOT_FROM
>>>>>> [sd|emmc_fastboot|fspi|nand_4k|nand_8k|nand_16k] [sector_size]
>>>>>>
>>>>>>> If you change the ROM API driver, that will break our design. You can
>>>>>>> try to overwrite spl_romapi_get_uboot_base for your board only.
>>>>>>
>>>>>> Since there are no users which boot from flexspi upstream, this design
>>>>>> can still be fixed such that it does not require different flash.bin for
>>>>>> different boot media, but rather one flash.bin works on all boot media.
>>>>>> I think that is much better.
>>>>>>
>>>>>
>>>>> Marek,
>>>>>
>>>>> I'm also a fan of single U-Boot binaries supporting multiple boards
>>>>> and configurations.
>>>>>
>>>>> How are you handling env for the various boot devices? I haven't
>>>>> looked in a while... can multiple env support co-exist now between
>>>>> MMC/SPI and be selected at runtime?
>>>>
>>>> Do '$ git grep env_get_location' , you will find board-level examples
>>>> where this function is implemented and it returns ENVL_* based on
>>>> various hardware-specific conditions.
>>>>
>>>> I _think_ there is even imx arch specific implementation for this
>>>> env_get_location() which automatically picks SD/eMMC/FlexSPI for you
>>>> right now.
>>>
>>> Marek,
>>>
>>> I guess where I keep getting hung up is how to configure the ENV for
>>> multiple env drivers as there is only one CONFIG_ENV_OFFSET Kconfig
>>> which means for all devices your going to have the same offset/redund
>>> configuration which is likely not appropriate for mmc/nand/spi for
>>> example. I don't see any way currently to configure those separately.
>>> Unless I'm missing something obvious I suppose get_env_location()
>>> could be augmented to be able to return board-specific env location
>>> config as well.
>>
>> e.g. env/mmc.c has the following:
>>
>>    78         } dt_prop = {
>>    79                 .offset_redund = "u-boot,mmc-env-offset-redundant",
>>    80                 .partition = "u-boot,mmc-env-partition",
>>    81                 .offset = "u-boot,mmc-env-offset",
>>    82         };
>>
>> So use that to specify per-env-driver offset. Indeed, the
>> CONFIG_ENV*OFFSET is shared by all env driver.
> 
> Right... but I think the dt overrides there only exist for MMC (I
> don't see then in env/nand.c for example)... but I can add that. I've
> always wanted to have a single binary for IMX6 nand vs emmc boards and
> this has been the ony hangup.

Then I think, send a patch for env SF. The DT overrides seem to be the 
right approach, it is certainly better than hard-coded Kconfigs.

[PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset

[sbabic]

> The FlexSPI NOR boot offset does not require any special handling,
> the image_offset is correct in either case (0x1000 for FlexSPI NOR
> and 0x8000 for SD/eMMC) and the offset of u-boot.itb from the start
> of flash.bin is always 0x58000 on MX8MN/MX8MP, which matches the
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000 in case
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x300, which is always the
> case on MX8MN/MX8MP.
> The CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR is really overloaded in
> case of the MX8MN/MX8MP, but fixing that needs additional plumbing.
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <festevam@denx.de>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
=====================================================================