Hi Marek,
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
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.
Which makes me ask "is this something we could do for imximage as well?"
is more flexible, as the FSPI header properties can be configured
from within the board-specific DT instead of being hard-coded in
mkimage at build time.
Signed-off-by: Marek Vasut <[email protected]>
---
Cc: "NXP i.MX U-Boot Team" <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Quentin Schulz <[email protected]>
Cc: Simon Glass <[email protected]>
Cc: Stefano Babic <[email protected]>
Cc: Tom Rini <[email protected]>
Cc: Wojciech Dubowik <[email protected]>
Cc: Yannic Moog <[email protected]>
Cc: [email protected]
---
tools/binman/etype/nxp_imx8mimage.py | 93 +++++++++++++++++--
tools/binman/ftest.py | 23 ++---
tools/binman/test/vendor/nxp_imx8m_fspi.dts | 1 -
.../nxp_imx8m_fspi_fail_columnadresswidth.dts | 19 ++++
...dts => nxp_imx8m_fspi_fail_devicetype.dts} | 3 +-
.../nxp_imx8m_fspi_fail_flashpadtype.dts | 19 ++++
.../nxp_imx8m_fspi_fail_readsampleclksrc.dts | 19 ++++
.../nxp_imx8m_fspi_fail_serialclkfreq.dts | 19 ++++
.../test/vendor/nxp_imx8m_fspi_pass.dts | 1 -
9 files changed, 176 insertions(+), 21 deletions(-)
create mode 100644
tools/binman/test/vendor/nxp_imx8m_fspi_fail_columnadresswidth.dts
rename tools/binman/test/vendor/{nxp_imx8m_fspi_fail.dts =>
nxp_imx8m_fspi_fail_devicetype.dts} (83%)
create mode 100644
tools/binman/test/vendor/nxp_imx8m_fspi_fail_flashpadtype.dts
create mode 100644
tools/binman/test/vendor/nxp_imx8m_fspi_fail_readsampleclksrc.dts
create mode 100644
tools/binman/test/vendor/nxp_imx8m_fspi_fail_serialclkfreq.dts
diff --git a/tools/binman/etype/nxp_imx8mimage.py
b/tools/binman/etype/nxp_imx8mimage.py
index 25c43438a87..47cd4481acd 100644
--- a/tools/binman/etype/nxp_imx8mimage.py
+++ 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.
+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?
Please also mention that all nxp,fspi-* arguments are only available if
nxp,boot-from == "fspi".
Please document which is the default if the argument is omitted.
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).
- nxp,loader-address - loader address (SPL text base)
- nxp,rom-version - BootROM version ('2' for i.MX8M Nano and Plus)
- - nxp,fspi-header-filename - FSPI header file name
(CONFIG_FSPI_CONF_FILE).
- Used only if 'nxp,boot-from == "fspi"' .
"""
def __init__(self, section, etype, node):
@@ -37,9 +53,30 @@ class Entry_nxp_imx8mimage(Entry_mkimage):
def ReadNode(self):
super().ReadNode()
self.boot_from = fdt_util.GetString(self._node, 'nxp,boot-from')
- self.fspi_header = fdt_util.GetString(self._node,
'nxp,fspi-header-filename', 'fspi_header.bin')
+ self.fspi_columnadresswidth = fdt_util.GetInt(self._node,
'nxp,fspi-columnadresswidth', 0)
+ self.fspi_controllermisc_diffclk = fdt_util.GetBool(self._node,
'nxp,fspi-controllermisc-diffclk')
+ self.fspi_controllermisc_wordaddr = fdt_util.GetBool(self._node,
'nxp,fspi-controllermisc-wordaddr')
+ self.fspi_controllermisc_safecfg = fdt_util.GetBool(self._node,
'nxp,fspi-controllermisc-safecfg')
+ self.fspi_controllermisc_padovr = fdt_util.GetBool(self._node,
'nxp,fspi-controllermisc-padovr')
+ self.fspi_controllermisc_ddrmode = fdt_util.GetBool(self._node,
'nxp,fspi-controllermisc-ddrmode')
+ self.fspi_devicetype = fdt_util.GetInt(self._node,
'nxp,fspi-devicetype', 1)
+ self.fspi_flasha1size = fdt_util.GetInt(self._node,
'nxp,fspi-flasha1size', 0x10000000)
+ self.fspi_flashpadtype = fdt_util.GetInt(self._node,
'nxp,fspi-flashpadtype', 1)
+ self.fspi_lutcustomseq = fdt_util.GetBool(self._node,
'nxp,fspi-lutcustomseq')
+ self.fspi_readsampleclksrc = fdt_util.GetInt(self._node,
'nxp,fspi-readsampleclksrc', 0)
+ self.fspi_serialclkfreq = fdt_util.GetInt(self._node,
'nxp,fspi-serialclkfreq', 2)
self.loader_address = fdt_util.GetInt(self._node,
'nxp,loader-address')
self.rom_version = fdt_util.GetInt(self._node, 'nxp,rom-version')
+ if not self.fspi_columnadresswidth in [ 0, 3, 12, 13 ]:
+ raise ValueError('nxp,fspi-columnadresswidth can be 0, 3, 12, 13
only.')
+ if not self.fspi_devicetype in [ 1, 2 ]:
+ raise ValueError('nxp,fspi-devicetype can be 1, 2 only.')
+ if not self.fspi_flashpadtype in [ 1, 2, 4, 8 ]:
+ raise ValueError('nxp,fspi-flashpadtype can be 1, 2, 4, 8 only.')
+ if not self.fspi_readsampleclksrc in [ 0, 1, 3 ]:
+ raise ValueError('nxp,fspi-readsampleclksrc can be 0, 1, 3 only.')
+ if not self.fspi_serialclkfreq in [ 1, 2, 3, 4, 5, 6, 7, 8 ]:
+ raise ValueError('nxp,fspi-serialclkfreq can be 1..8 only.')
self.ReadEntries()
def BuildSectionData(self, required):
@@ -59,10 +96,52 @@ class Entry_nxp_imx8mimage(Entry_mkimage):
if self.mkimage.run_cmd(*args) is not None:
outdata = tools.read_file(output_fname)
if self.boot_from == 'fspi':
- spidata = tools.read_file(os.path.join(tools.get_output_dir(),
self.fspi_header))
- if len(spidata) != 448:
- raise ValueError("FSPI header is not 448 Bytes long")
- spidata += tools.get_bytes(0, 0x1000 - len(spidata))
+ # 0x00 ... Tag
+ spidata = struct.pack('<I', 0x42464346)
+ # 0x04 ... Version
+ spidata += struct.pack('<I', 0x56010000)
+ # 0x08 ... Reserved
+ spidata += struct.pack('<I', 0)
+ # 0x0c ... readSampleClkSrc (LSByte at 0x0c), dataHoldTime,
+ # dataSetupTime, columnAdressWidth (MSByte at 0x0f)
+ spidata += struct.pack('<I', 0x00030300 |
+ (self.fspi_columnadresswidth << 24) |
+ self.fspi_readsampleclksrc)
+
+ # 0x10..0x3f ... Padding
+ spidata += tools.get_bytes(0, 0x30)
+
+ # 0x40 ... controllerMiscOption
+ spidata += struct.pack('<I',
+ ((1 << 0) if self.fspi_controllermisc_diffclk else 0) |
+ ((1 << 3) if self.fspi_controllermisc_wordaddr else 0) |
+ ((1 << 4) if self.fspi_controllermisc_safecfg else 0) |
+ ((1 << 5) if self.fspi_controllermisc_padovr else 0) |
+ ((1 << 6) if self.fspi_controllermisc_ddrmode else 0))
+
+ # 0x44 ... deviceType (LSByte at 0x44), sflashPadType,
+ # serialClkFreq, lutCustomSeqEnable (MSByte at 0x47)
+ spidata += struct.pack('<I',
+ ((1 << 24) if self.fspi_lutcustomseq else 0) |
+ (self.fspi_serialclkfreq << 16) |
+ (self.fspi_flashpadtype << 8) |
+ self.fspi_devicetype)
+
+ # 0x48..0x4f ... Padding
+ spidata += tools.get_bytes(0, 0x8)
+
+ # 0x50 ... flashA1Size
+ spidata += struct.pack('<I', self.fspi_flasha1size)
+
+ # 0x54..0x7f ... Padding
+ spidata += tools.get_bytes(0, 0x2c)
+
+ # 0x80 ... lookupTable
+ spidata += struct.pack('<I', 0x0818040b)
+ spidata += struct.pack('<I', 0x24043008)
+
+ # 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...
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. 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. 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?).
Cheers,
Quentin
