[PATCH] binman: imx8mimage: Generate FSPI header in binman instead of mkimage

[Marek Vasut]

On 6/26/26 1:14 PM, Quentin Schulz wrote:

Hello Quentin,

> On 6/10/26 6:17 PM, Marek Vasut wrote:
>> Stop depending on the current mkimage method of generating the FSPI
>> header, instead generate the FSPI header within binman itself. This
> 
> Soooooo, do we still need the mkimage version or can we remove it?

I believe it is part of ABI, so we cannot remove it.

> I 
> generally would be against it since "users may make use of it" but since 
> you would need to rebuild (according to this commit log) it to be able 
> to compile what you want for your device, essentially making the host 
> tool dependent on the target you're building for, I guess it could be 
> removed? What do you think? I'm eyeing tools/imx8mimage.c and tools/ 
> imximage.c specifically.

Whatever the tool generates right now is a generic SPI NOR boot header, 
for all configurations that are in the tree that use this header. I 
think we should keep this, but only to retain backward compatibility and 
not break users who might depend on it.

> Which makes me ask "is this something we could do for imximage as well?"

What does "this" refer to -- drop something, or, add some header 
support, or something else entirely ?

[...]

>> +++ b/tools/binman/etype/nxp_imx8mimage.py
>> @@ -8,6 +8,7 @@
>>   #
>>   import os
> 
> You remove the only user of this module later in the diff, so this can 
> be removed as well.

Fixed in V2, thanks.

>> +import struct
>>   from collections import OrderedDict
>> @@ -23,10 +24,25 @@ class Entry_nxp_imx8mimage(Entry_mkimage):
>>       Properties / Entry arguments:
>>           - nxp,boot-from - device to boot from (e.g. 'sd')
>> +        - nxp,fspi-columnadresswidth - FSPI column address width
>> +            (3 - HyperFlash, 12/13 - Serial NAND, 0 - Otherwise)
>> +        - nxp,fspi-controllermisc-diffclk - FSPI differential clock 
>> enable
>> +        - nxp,fspi-controllermisc-wordaddr - FSPI word addressable 
>> enable
>> +        - nxp,fspi-controllermisc-safecfg - FSPI safe configuration 
>> frequency
>> +        - nxp,fspi-controllermisc-padovr - FSPI pad setting override
>> +        - nxp,fspi-controllermisc-ddrmode - FSPI DDR mode
>> +        - nxp,fspi-lutcustomseq - FSPI use LUT sequence parameters
>> +        - nxp,fspi-devicetype - FSPI device type
>> +            (1 - SPI NOR, 2 - Serial NAND)
>> +        - nxp,fspi-flashpadtype - FSPI flash pad type
>> +            (1 - Single pad, 2 - Dual pads, 4 - Quad pads, 8 - Octal 
>> pads)
>> +        - nxp,fspi-readsampleclksrc - FSPI clock source
>> +            (0 - Internal loopback, 1 - loopback from DQS pad, 3 - 
>> Flash provided DQS).
>> +        - nxp,fspi-serialclkfreq - FSPI clock frequency
>> +            (1 - 30 MHz, 2 - 50 MHz, 3 - 60 MHz, 4 - 75 MHz, 5 - 80 MHz,
>> +             6 - 100 MHz, 7 - 133 MHz, 8 - 166 MHz).
> 
> OK, so... I have not played with i.MX8M but I would be absolutely 
> clueless on how to configure this. Is there some literature, readme, 
> docs we could point at?

Yes, there is the SoC datasheet if you ever need to change the defaults 
(I haven't done that yet).

> Please also mention that all nxp,fspi-* arguments are only available if 
> nxp,boot-from == "fspi".

Fixed in V2, thanks.

> Please document which is the default if the argument is omitted.

Fixed in V2, thanks.

> Finally, I don't think it makes sense to tell the user "say 1 if you 
> want 30MHz, 2 for 50MHz", why can't the user specify the frequency 
> directly and we do the job for them of translating it to something to 
> put in the binary? (same remark for all arguments).

Does a big switch-case bring up any improvement in this case ?

>> +                # 0x88..0xfff ... Padding (end of FSPI block is 
>> 0x1bf, align to 4k)
>> +                spidata += tools.get_bytes(0, 0x1000 - 0xa8)
> 
> I'll let people with an i.MX8M review the implementation but I do have 
> some feedback to provide on the logic. This essentially removes the 
> ability to use an externally-provided fspi_header.bin. So you should fix 
> in the same commit arch/arm/dts/imx8mm-u-boot.dtsi and arch/arm/dts/ 
> imx8mn-u-boot.dtsi to migrate them to the new mechanism...

Not in the same commit, but the fspi-header.bin references should be 
dropped eventually.

> Which likely 
> means going through all board defconfigs and figuring out which values 
> they use for compiling the mkimage that is used to generate this file 
> and then put it in a board-specific -u-boot.dtsi.

They all use the same values, the default values that are also used in 
this implementation.

> The other issue is 
> downstream which will now be forced to migrate to it. I guess it could 
> be fine but another option could be "specify the binary file to use or 
> all those arguments instead" and keep current logic and simply extend it 
> to support the new arguments.

Let's not make this more complicated than it already is, and let's ditch 
the old fspi-header.bin mechanism entirely. Note that the 
fspi-header.bin handling was broken for some 2 years now, so I don't 
think anyone depends on it.

> Otherwise, maybe checking the argument 
> exists in the DTB and fail hard and tell the user to migrate to the new 
> format with multiple arguments (what happens if we remove support for 
> nxp,fspi-header-filename but it's still present in the DTS and the user 
> hasn't migrated to the new arguments? does it fail the build or silently 
> build something that they cannot boot?).

It generates the header using default values, which seems to be used by 
most users anyway.