[PATCH 0/3] arch: arm: imx: spi: Add QSPI boot support to the IMX8 platforms

[PATCH 0/3] arch: arm: imx: spi: Add QSPI boot support to the IMX8 platforms

[Joacim Zetterling]

This patch series adds QSPI boot option to the IMX8 platforms.
For the moment only the IMX8MN platform is supported.

This has been tested with the support of the Flex SPI/QSPI
driver on an IMX8MN-EVK board configured for QSPI boot
(SW1:0110) and on an IMX8MN custom board with a QSPI flash.

Joacim Zetterling (3):
  arch: arm: imx: spi: Add QSPI boot support to the IMX8 platforms
  scripts: imx: spi: Add QSPI boot support to IMX build image script
  arch: arm: imx: spi: Add QSPI boot option to the IMX8MN-EVK

 .../bb-cfg-param-imx8mn-evk.bin               | Bin 0 -> 512 bytes
 .../flash-header-imx8mn-evk.imxcfg            |   5 +
 arch/arm/boards/nxp-imx8mn-evk/lowlevel.c     |   3 +
 arch/arm/mach-imx/Makefile                    |   1 +
 arch/arm/mach-imx/include/mach/imx-header.h   |   2 +
 arch/arm/mach-imx/include/mach/xload.h        |   1 +
 arch/arm/mach-imx/xload-qspi.c                | 145 ++++++++++++++++++
 scripts/imx/imx-image.c                       |  25 +++
 scripts/imx/imx.c                             |  67 +++++++-
 9 files changed, 248 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boards/nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin
 create mode 100644 arch/arm/mach-imx/xload-qspi.c

-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/3] arch: arm: imx: spi: Add QSPI boot support to the IMX8 platforms

[Joacim Zetterling]

The IMX8 platforms does only support boot from a SD or an EMMC
device. With this functionality also a boot from a QSPI device
is possible. For the moment only the IMX8MN platform is supported.

Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
---
 arch/arm/mach-imx/Makefile             |   1 +
 arch/arm/mach-imx/include/mach/xload.h |   1 +
 arch/arm/mach-imx/xload-qspi.c         | 145 +++++++++++++++++++++++++
 3 files changed, 147 insertions(+)
 create mode 100644 arch/arm/mach-imx/xload-qspi.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 2cafcd77e00d..d844196422df 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -29,3 +29,4 @@ obj-$(CONFIG_BAREBOX_UPDATE_IMX_EXTERNAL_NAND) += imx-bbu-external-nand.o
 obj-$(CONFIG_RESET_IMX_SRC) += src.o
 lwl-y += cpu_init.o
 pbl-y += xload-spi.o xload-common.o xload-imx-nand.o xload-gpmi-nand.o
+pbl-y += xload-qspi.o
diff --git a/arch/arm/mach-imx/include/mach/xload.h b/arch/arm/mach-imx/include/mach/xload.h
index 03ec23ebbdb0..1876f3a4bd18 100644
--- a/arch/arm/mach-imx/include/mach/xload.h
+++ b/arch/arm/mach-imx/include/mach/xload.h
@@ -11,6 +11,7 @@ int imx6_nand_start_image(void);
 int imx7_esdhc_start_image(int instance);
 int imx8m_esdhc_load_image(int instance, bool start);
 int imx8mn_esdhc_load_image(int instance, bool start);
+int imx8mn_qspi_start_image(int instance, bool start);
 int imx8mp_esdhc_load_image(int instance, bool start);
 
 int imx_image_size(void);
diff --git a/arch/arm/mach-imx/xload-qspi.c b/arch/arm/mach-imx/xload-qspi.c
new file mode 100644
index 000000000000..c8305f15ee0a
--- /dev/null
+++ b/arch/arm/mach-imx/xload-qspi.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <common.h>
+#include <asm-generic/sections.h>
+#include <asm/cache.h>
+#include <mach/xload.h>
+#include <mach/imx8mn-regs.h>
+#include <mach/imx-header.h>
+
+#include <io.h>
+#include <regmap.h>
+#include <linux/sizes.h>
+
+#include <mach/atf.h>
+
+#define HDR_SIZE		512
+
+#define IMX8M_QSPI_MMAP		0x8000000
+
+
+static int check_ivt_header_v2(struct imx_flash_header_v2 **header_pointer,
+			    void *buf, u32 offset, u32 ivt_offset)
+{
+	int i, header_count = 1;
+	struct imx_flash_header_v2 *hdr;
+
+	for (i = 0; i < header_count; i++) {
+		hdr = buf + offset + ivt_offset;
+
+		if (!is_imx_flash_header_v2(hdr)) {
+			pr_debug("IVT header not found in QSPI. "
+				 "Found tag: 0x%02x length: 0x%04x "
+				 "version: %02x\n",
+				 hdr->header.tag, hdr->header.length,
+				 hdr->header.version);
+			return -EINVAL;
+		}
+
+		if (IS_ENABLED(CONFIG_ARCH_IMX8MQ) &&
+		    hdr->boot_data.plugin & PLUGIN_HDMI_IMAGE) {
+			/*
+			 * In images that include signed HDMI
+			 * firmware, first v2 header would be
+			 * dedicated to that and would not contain any
+			 * useful for us information. In order for us
+			 * to pull the rest of the bootloader image
+			 * in, we need to re-read header from SD/MMC,
+			 * this time skipping anything HDMI firmware
+			 * related.
+			 */
+			offset += hdr->boot_data.size + hdr->header.length;
+			header_count++;
+		}
+	}
+
+	*header_pointer = hdr;
+	return 0;
+}
+
+static int
+imx8m_qspi_load_image(void __iomem *ahb_addr, ptrdiff_t address,
+		 ptrdiff_t entry, u32 offset, u32 ivt_offset, bool start)
+{
+	void *buf = (void *)address;
+	struct imx_flash_header_v2 *hdr = NULL;
+	int ret, len;
+	void __noreturn (*bb)(void);
+	unsigned int ofs;
+
+	len = imx_image_size();
+
+	/* Read out the data directly from the AHB buffer. */
+	memcpy(buf, __io_virt((void *)ahb_addr), 0x2000);
+
+	ret = check_ivt_header_v2(&hdr, buf, offset, ivt_offset);
+	if (ret)
+		return ret;
+
+	pr_debug("Check ok, loading image\n");
+
+	ofs = offset + hdr->entry - hdr->boot_data.start;
+
+	if (entry != address) {
+		/*
+		 * Passing entry different from address is interpreted
+		 * as a request to place the image such that its entry
+		 * point would be exactly at 'entry', that is:
+		 *
+		 *     buf + ofs = entry
+		 *
+		 * solving the above for 'buf' gives us the
+		 * adjustment that needs to be made:
+		 *
+		 *     buf = entry - ofs
+		 *
+		 */
+		if (WARN_ON(entry - ofs < address)) {
+			/*
+			 * We want to make sure we won't try to place
+			 * the start of the image before the beginning
+			 * of the memory buffer we were given in
+			 * address.
+			 */
+			return -EINVAL;
+		}
+
+		buf = (void *)(entry - ofs);
+	}
+
+	/* Read out the data directly from the AHB buffer. */
+	memcpy(buf, __io_virt((void *)ahb_addr), len + SZ_4K);
+
+	pr_debug("Image loaded successfully\n");
+
+	if (!start)
+		return 0;
+
+	bb = buf + ofs;
+
+	sync_caches_for_execution();
+
+	bb();
+}
+
+/**
+ * imx8mn_qspi_start_image - Load and optionally start an image from the
+ * FlexSPI controller.
+ * @instance: The FlexSPI controller instance (0)
+ * @start: Whether to directly start the loaded image
+ *
+ * This uses imx8m_qspi_load_image() to load an image from QSPI. It is assumed
+ * that the image is the currently running barebox image (This information
+ * is used to calculate the length of the image).
+ * The image is started afterwards.
+ *
+ * Return: If successful, this function does not return (if directly started)
+ * or 0. A negative error code is returned when this function fails.
+ */
+int imx8mn_qspi_start_image(int instance, bool start)
+{
+	void __iomem *ahb_addr = IOMEM(IMX8M_QSPI_MMAP);
+
+	return imx8m_qspi_load_image(ahb_addr, MX8M_DDR_CSD1_BASE_ADDR,
+				MX8M_ATF_BL33_BASE_ADDR, SZ_4K, 0, start);
+}
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] arch: arm: imx: spi: Add QSPI boot support to the IMX8 platforms

[Ahmad Fatoum]

Hello Joacim,

On 08.03.22 17:07, Joacim Zetterling wrote:
> The IMX8 platforms does only support boot from a SD or an EMMC
> device. With this functionality also a boot from a QSPI device
> is possible. For the moment only the IMX8MN platform is supported.

Did you consider using the return to boot ROM feature of the i.MX8MN
to implement this? Added benefit is that you can use this for
USB boot (SDPS) as well. I have been successfully using Uwe's patches
for the i.MX8MP[1] locally on the i.MX8MN for USB boot.

Although seeing that the QSPI is memory-mapped, it's probably much
less effort to just use it directly.

[1]: https://lore.barebox.org/barebox/20210813152245.15841-2-u.kleine-koenig@pengutronix.de/

Cheers,
Ahmad

> 
> Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
> ---
>  arch/arm/mach-imx/Makefile             |   1 +
>  arch/arm/mach-imx/include/mach/xload.h |   1 +
>  arch/arm/mach-imx/xload-qspi.c         | 145 +++++++++++++++++++++++++
>  3 files changed, 147 insertions(+)
>  create mode 100644 arch/arm/mach-imx/xload-qspi.c
> 
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index 2cafcd77e00d..d844196422df 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> @@ -29,3 +29,4 @@ obj-$(CONFIG_BAREBOX_UPDATE_IMX_EXTERNAL_NAND) += imx-bbu-external-nand.o
>  obj-$(CONFIG_RESET_IMX_SRC) += src.o
>  lwl-y += cpu_init.o
>  pbl-y += xload-spi.o xload-common.o xload-imx-nand.o xload-gpmi-nand.o
> +pbl-y += xload-qspi.o
> diff --git a/arch/arm/mach-imx/include/mach/xload.h b/arch/arm/mach-imx/include/mach/xload.h
> index 03ec23ebbdb0..1876f3a4bd18 100644
> --- a/arch/arm/mach-imx/include/mach/xload.h
> +++ b/arch/arm/mach-imx/include/mach/xload.h
> @@ -11,6 +11,7 @@ int imx6_nand_start_image(void);
>  int imx7_esdhc_start_image(int instance);
>  int imx8m_esdhc_load_image(int instance, bool start);
>  int imx8mn_esdhc_load_image(int instance, bool start);
> +int imx8mn_qspi_start_image(int instance, bool start);
>  int imx8mp_esdhc_load_image(int instance, bool start);
>  
>  int imx_image_size(void);
> diff --git a/arch/arm/mach-imx/xload-qspi.c b/arch/arm/mach-imx/xload-qspi.c
> new file mode 100644
> index 000000000000..c8305f15ee0a
> --- /dev/null
> +++ b/arch/arm/mach-imx/xload-qspi.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <common.h>
> +#include <asm-generic/sections.h>
> +#include <asm/cache.h>
> +#include <mach/xload.h>
> +#include <mach/imx8mn-regs.h>
> +#include <mach/imx-header.h>
> +
> +#include <io.h>
> +#include <regmap.h>
> +#include <linux/sizes.h>
> +
> +#include <mach/atf.h>
> +
> +#define HDR_SIZE		512
> +
> +#define IMX8M_QSPI_MMAP		0x8000000
> +
> +
> +static int check_ivt_header_v2(struct imx_flash_header_v2 **header_pointer,
> +			    void *buf, u32 offset, u32 ivt_offset)
> +{
> +	int i, header_count = 1;
> +	struct imx_flash_header_v2 *hdr;
> +
> +	for (i = 0; i < header_count; i++) {
> +		hdr = buf + offset + ivt_offset;
> +
> +		if (!is_imx_flash_header_v2(hdr)) {
> +			pr_debug("IVT header not found in QSPI. "
> +				 "Found tag: 0x%02x length: 0x%04x "
> +				 "version: %02x\n",
> +				 hdr->header.tag, hdr->header.length,
> +				 hdr->header.version);
> +			return -EINVAL;
> +		}
> +
> +		if (IS_ENABLED(CONFIG_ARCH_IMX8MQ) &&
> +		    hdr->boot_data.plugin & PLUGIN_HDMI_IMAGE) {
> +			/*
> +			 * In images that include signed HDMI
> +			 * firmware, first v2 header would be
> +			 * dedicated to that and would not contain any
> +			 * useful for us information. In order for us
> +			 * to pull the rest of the bootloader image
> +			 * in, we need to re-read header from SD/MMC,
> +			 * this time skipping anything HDMI firmware
> +			 * related.
> +			 */
> +			offset += hdr->boot_data.size + hdr->header.length;
> +			header_count++;
> +		}
> +	}
> +
> +	*header_pointer = hdr;
> +	return 0;
> +}
> +
> +static int
> +imx8m_qspi_load_image(void __iomem *ahb_addr, ptrdiff_t address,
> +		 ptrdiff_t entry, u32 offset, u32 ivt_offset, bool start)
> +{
> +	void *buf = (void *)address;
> +	struct imx_flash_header_v2 *hdr = NULL;
> +	int ret, len;
> +	void __noreturn (*bb)(void);
> +	unsigned int ofs;
> +
> +	len = imx_image_size();
> +
> +	/* Read out the data directly from the AHB buffer. */
> +	memcpy(buf, __io_virt((void *)ahb_addr), 0x2000);
> +
> +	ret = check_ivt_header_v2(&hdr, buf, offset, ivt_offset);
> +	if (ret)
> +		return ret;
> +
> +	pr_debug("Check ok, loading image\n");
> +
> +	ofs = offset + hdr->entry - hdr->boot_data.start;
> +
> +	if (entry != address) {
> +		/*
> +		 * Passing entry different from address is interpreted
> +		 * as a request to place the image such that its entry
> +		 * point would be exactly at 'entry', that is:
> +		 *
> +		 *     buf + ofs = entry
> +		 *
> +		 * solving the above for 'buf' gives us the
> +		 * adjustment that needs to be made:
> +		 *
> +		 *     buf = entry - ofs
> +		 *
> +		 */
> +		if (WARN_ON(entry - ofs < address)) {
> +			/*
> +			 * We want to make sure we won't try to place
> +			 * the start of the image before the beginning
> +			 * of the memory buffer we were given in
> +			 * address.
> +			 */
> +			return -EINVAL;
> +		}
> +
> +		buf = (void *)(entry - ofs);
> +	}
> +
> +	/* Read out the data directly from the AHB buffer. */
> +	memcpy(buf, __io_virt((void *)ahb_addr), len + SZ_4K);

Why the SZ_4K?

> +
> +	pr_debug("Image loaded successfully\n");
> +
> +	if (!start)
> +		return 0;
> +
> +	bb = buf + ofs;
> +
> +	sync_caches_for_execution();
> +
> +	bb();
> +}
> +
> +/**
> + * imx8mn_qspi_start_image - Load and optionally start an image from the
> + * FlexSPI controller.
> + * @instance: The FlexSPI controller instance (0)
> + * @start: Whether to directly start the loaded image
> + *
> + * This uses imx8m_qspi_load_image() to load an image from QSPI. It is assumed
> + * that the image is the currently running barebox image (This information
> + * is used to calculate the length of the image).
> + * The image is started afterwards.
> + *
> + * Return: If successful, this function does not return (if directly started)
> + * or 0. A negative error code is returned when this function fails.
> + */
> +int imx8mn_qspi_start_image(int instance, bool start)
> +{
> +	void __iomem *ahb_addr = IOMEM(IMX8M_QSPI_MMAP);
> +
> +	return imx8m_qspi_load_image(ahb_addr, MX8M_DDR_CSD1_BASE_ADDR,
> +				MX8M_ATF_BL33_BASE_ADDR, SZ_4K, 0, start);
> +}


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] scripts: arch: imx: Add QSPI boot support to IMX build image script

[Joacim Zetterling]

This functionality extend the IMX image build script with a support
for generating QSPI boot images.

A QSPI boot image need device configuration parameters located at
an offset of 0x400 in the IVT section. The configuration parameters
are stored in a 512 byte bin file which will be included in the
final boot image. The boot image parameters comes from the board
flash header imxcfg file.

The QSPI configuration parameters are described in the reference
manual for the specific target.

Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
---
 arch/arm/mach-imx/include/mach/imx-header.h |  2 +
 scripts/imx/imx-image.c                     | 25 ++++++++
 scripts/imx/imx.c                           | 67 ++++++++++++++++++++-
 3 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/include/mach/imx-header.h b/arch/arm/mach-imx/include/mach/imx-header.h
index 8e968e6efba3..a58deb8d864e 100644
--- a/arch/arm/mach-imx/include/mach/imx-header.h
+++ b/arch/arm/mach-imx/include/mach/imx-header.h
@@ -118,6 +118,8 @@ struct config_data {
 	char *signed_hdmi_firmware_file;
 	int encrypt_image;
 	size_t dek_size;
+	uint32_t bb_cfg_ofs;
+	char *bb_cfg_file;
 };
 
 #define MAX_RECORDS_DCD_V2 1024
diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
index 439912a805dc..b8ae63ad9400 100644
--- a/scripts/imx/imx-image.c
+++ b/scripts/imx/imx-image.c
@@ -951,6 +951,31 @@ int main(int argc, char *argv[])
 			exit(1);
 		}
 
+		/*
+		* The boot ROM expects a 512-byte configuration parameters area for
+		* some devices (like the FlexSPI NOR flash) to be present an defined
+		* offset in the image ivt section (0x400 for the FlexSPI NOR).
+		*/
+		if (data.bb_cfg_file) {
+			size_t bb_cfg_file_size = 512;
+			char *bb_cfg;
+
+			bb_cfg = calloc(512, sizeof(char));
+			if (!bb_cfg)
+				exit(1);
+
+			bb_cfg = read_file(data.bb_cfg_file, &bb_cfg_file_size);
+
+			if (lseek(outfd, data.bb_cfg_ofs, SEEK_SET) < 0) {
+				perror("lseek");
+				exit(1);
+			}
+
+			xwrite(outfd, bb_cfg, bb_cfg_file_size);
+
+			free(bb_cfg);
+		}
+
 		if (lseek(outfd, data.header_gap, SEEK_SET) < 0) {
 			perror("lseek");
 			exit(1);
diff --git a/scripts/imx/imx.c b/scripts/imx/imx.c
index 87560ad27de1..a242e6d76a47 100644
--- a/scripts/imx/imx.c
+++ b/scripts/imx/imx.c
@@ -215,6 +215,16 @@ static int do_dcdofs_error(struct config_data *data, int argc, char *argv[])
 	return -EINVAL;
 }
 
+static int do_header_gap(struct config_data *data, int argc, char *argv[])
+{
+	if (argc < 2)
+		return -EINVAL;
+
+	data->header_gap = strtoul(argv[1], NULL, 0);
+
+	return 0;
+}
+
 struct soc_type {
 	char *name;
 	int header_version;
@@ -223,6 +233,7 @@ struct soc_type {
 	uint32_t first_opcode;
 };
 
+#define SZ_4K	(4 * 1024)
 #define SZ_32K	(32 * 1024)
 
 static struct soc_type socs[] = {
@@ -580,7 +591,6 @@ do_signed_hdmi_firmware(struct config_data *data, int argc, char *argv[])
 	const char *file;
 	int len;
 
-
 	if (argc != 2) {
 		fprintf(stderr, "usage: signed_hdmi_firmware <file>\n");
 		return -EINVAL;
@@ -609,6 +619,52 @@ do_signed_hdmi_firmware(struct config_data *data, int argc, char *argv[])
 	return 0;
 }
 
+static int do_bb_cfg_ofs(struct config_data *data, int argc, char *argv[])
+{
+	if (argc < 2)
+		return -EINVAL;
+
+	data->bb_cfg_ofs = strtoul(argv[1], NULL, 0);
+
+	return 0;
+}
+
+static int do_bb_cfg_file(struct config_data *data, int argc, char *argv[])
+{
+	const char *file;
+	int len;
+
+	if (argc != 2) {
+		fprintf(stderr, "usage: bb_cfg_file <file>\n");
+		return -EINVAL;
+	}
+
+	if ((data->cpu_type != IMX_CPU_IMX8MM) && 
+		(data->cpu_type != IMX_CPU_IMX8MN) && 
+		(data->cpu_type != IMX_CPU_IMX8MP) &&
+		(data->cpu_type != IMX_CPU_IMX8MQ)) {
+		fprintf(stderr,
+			"Warning: The configuration param command is "
+			"only supported i.MX8 SoCs\n");
+		return 0;
+	}
+
+	file = argv[1];
+
+	if (*file == '"')
+		file++;
+
+	data->bb_cfg_file = strdup(file);
+	if (!data->bb_cfg_file)
+		return -ENOMEM;
+
+	len = strlen(data->bb_cfg_file);
+	if (data->bb_cfg_file[len - 1] == '"')
+		data->bb_cfg_file[len - 1] = 0;
+
+	return 0;
+}
+
 struct command cmds[] = {
 	{
 		.name = "wm",
@@ -634,6 +690,9 @@ struct command cmds[] = {
 	}, {
 		.name = "dcdofs",
 		.parse = do_dcdofs_error,
+	}, {
+		.name = "header_gap",
+		.parse = do_header_gap,
 	}, {
 		.name = "soc",
 		.parse = do_soc,
@@ -667,6 +726,12 @@ struct command cmds[] = {
 	}, {
 		.name = "signed_hdmi_firmware",
 		.parse = do_signed_hdmi_firmware,
+	}, {
+		.name = "bb_cfg_ofs",
+		.parse = do_bb_cfg_ofs,
+	}, {
+		.name = "bb_cfg_file",
+		.parse = do_bb_cfg_file,
 	},
 };
 
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] scripts: arch: imx: Add QSPI boot support to IMX build image script

[Ahmad Fatoum]

On 08.03.22 17:08, Joacim Zetterling wrote:
> This functionality extend the IMX image build script with a support
> for generating QSPI boot images.
> 
> A QSPI boot image need device configuration parameters located at
> an offset of 0x400 in the IVT section. The configuration parameters
> are stored in a 512 byte bin file which will be included in the
> final boot image. The boot image parameters comes from the board
> flash header imxcfg file.
> 
> The QSPI configuration parameters are described in the reference
> manual for the specific target.
> 
> Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>

Does this mean we can't have the same barebox image for both SD/eMMC
and QSPI-NOR?

Does filetype correctly detect this format? If not, you should
add a check there to allow barebox update handlers to bail out
when installing an incompatible image.

Cheers,
Ahmad

> ---
>  arch/arm/mach-imx/include/mach/imx-header.h |  2 +
>  scripts/imx/imx-image.c                     | 25 ++++++++
>  scripts/imx/imx.c                           | 67 ++++++++++++++++++++-
>  3 files changed, 93 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/include/mach/imx-header.h b/arch/arm/mach-imx/include/mach/imx-header.h
> index 8e968e6efba3..a58deb8d864e 100644
> --- a/arch/arm/mach-imx/include/mach/imx-header.h
> +++ b/arch/arm/mach-imx/include/mach/imx-header.h
> @@ -118,6 +118,8 @@ struct config_data {
>  	char *signed_hdmi_firmware_file;
>  	int encrypt_image;
>  	size_t dek_size;
> +	uint32_t bb_cfg_ofs;
> +	char *bb_cfg_file;
>  };
>  
>  #define MAX_RECORDS_DCD_V2 1024
> diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
> index 439912a805dc..b8ae63ad9400 100644
> --- a/scripts/imx/imx-image.c
> +++ b/scripts/imx/imx-image.c
> @@ -951,6 +951,31 @@ int main(int argc, char *argv[])
>  			exit(1);
>  		}
>  
> +		/*
> +		* The boot ROM expects a 512-byte configuration parameters area for
> +		* some devices (like the FlexSPI NOR flash) to be present an defined
> +		* offset in the image ivt section (0x400 for the FlexSPI NOR).
> +		*/
> +		if (data.bb_cfg_file) {
> +			size_t bb_cfg_file_size = 512;
> +			char *bb_cfg;
> +
> +			bb_cfg = calloc(512, sizeof(char));
> +			if (!bb_cfg)
> +				exit(1);
> +
> +			bb_cfg = read_file(data.bb_cfg_file, &bb_cfg_file_size);
> +
> +			if (lseek(outfd, data.bb_cfg_ofs, SEEK_SET) < 0) {
> +				perror("lseek");
> +				exit(1);
> +			}
> +
> +			xwrite(outfd, bb_cfg, bb_cfg_file_size);
> +
> +			free(bb_cfg);
> +		}
> +
>  		if (lseek(outfd, data.header_gap, SEEK_SET) < 0) {
>  			perror("lseek");
>  			exit(1);
> diff --git a/scripts/imx/imx.c b/scripts/imx/imx.c
> index 87560ad27de1..a242e6d76a47 100644
> --- a/scripts/imx/imx.c
> +++ b/scripts/imx/imx.c
> @@ -215,6 +215,16 @@ static int do_dcdofs_error(struct config_data *data, int argc, char *argv[])
>  	return -EINVAL;
>  }
>  
> +static int do_header_gap(struct config_data *data, int argc, char *argv[])
> +{
> +	if (argc < 2)
> +		return -EINVAL;
> +
> +	data->header_gap = strtoul(argv[1], NULL, 0);
> +
> +	return 0;
> +}
> +
>  struct soc_type {
>  	char *name;
>  	int header_version;
> @@ -223,6 +233,7 @@ struct soc_type {
>  	uint32_t first_opcode;
>  };
>  
> +#define SZ_4K	(4 * 1024)
>  #define SZ_32K	(32 * 1024)
>  
>  static struct soc_type socs[] = {
> @@ -580,7 +591,6 @@ do_signed_hdmi_firmware(struct config_data *data, int argc, char *argv[])
>  	const char *file;
>  	int len;
>  
> -
>  	if (argc != 2) {
>  		fprintf(stderr, "usage: signed_hdmi_firmware <file>\n");
>  		return -EINVAL;
> @@ -609,6 +619,52 @@ do_signed_hdmi_firmware(struct config_data *data, int argc, char *argv[])
>  	return 0;
>  }
>  
> +static int do_bb_cfg_ofs(struct config_data *data, int argc, char *argv[])
> +{
> +	if (argc < 2)
> +		return -EINVAL;
> +
> +	data->bb_cfg_ofs = strtoul(argv[1], NULL, 0);
> +
> +	return 0;
> +}
> +
> +static int do_bb_cfg_file(struct config_data *data, int argc, char *argv[])
> +{
> +	const char *file;
> +	int len;
> +
> +	if (argc != 2) {
> +		fprintf(stderr, "usage: bb_cfg_file <file>\n");
> +		return -EINVAL;
> +	}
> +
> +	if ((data->cpu_type != IMX_CPU_IMX8MM) && 
> +		(data->cpu_type != IMX_CPU_IMX8MN) && 
> +		(data->cpu_type != IMX_CPU_IMX8MP) &&
> +		(data->cpu_type != IMX_CPU_IMX8MQ)) {
> +		fprintf(stderr,
> +			"Warning: The configuration param command is "
> +			"only supported i.MX8 SoCs\n");
> +		return 0;
> +	}
> +
> +	file = argv[1];
> +
> +	if (*file == '"')
> +		file++;
> +
> +	data->bb_cfg_file = strdup(file);
> +	if (!data->bb_cfg_file)
> +		return -ENOMEM;
> +
> +	len = strlen(data->bb_cfg_file);
> +	if (data->bb_cfg_file[len - 1] == '"')
> +		data->bb_cfg_file[len - 1] = 0;
> +
> +	return 0;
> +}
> +
>  struct command cmds[] = {
>  	{
>  		.name = "wm",
> @@ -634,6 +690,9 @@ struct command cmds[] = {
>  	}, {
>  		.name = "dcdofs",
>  		.parse = do_dcdofs_error,
> +	}, {
> +		.name = "header_gap",
> +		.parse = do_header_gap,
>  	}, {
>  		.name = "soc",
>  		.parse = do_soc,
> @@ -667,6 +726,12 @@ struct command cmds[] = {
>  	}, {
>  		.name = "signed_hdmi_firmware",
>  		.parse = do_signed_hdmi_firmware,
> +	}, {
> +		.name = "bb_cfg_ofs",
> +		.parse = do_bb_cfg_ofs,
> +	}, {
> +		.name = "bb_cfg_file",
> +		.parse = do_bb_cfg_file,
>  	},
>  };
>  


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] scripts: arch: imx: Add QSPI boot support to IMX build image script

[Sascha Hauer]

Hi Joacim,

On Tue, Mar 08, 2022 at 05:08:00PM +0100, Joacim Zetterling wrote:
> This functionality extend the IMX image build script with a support
> for generating QSPI boot images.
> 
> A QSPI boot image need device configuration parameters located at
> an offset of 0x400 in the IVT section. The configuration parameters
> are stored in a 512 byte bin file which will be included in the
> final boot image. The boot image parameters comes from the board
> flash header imxcfg file.
> 
> The QSPI configuration parameters are described in the reference
> manual for the specific target.
> 
> Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
> ---
>  arch/arm/mach-imx/include/mach/imx-header.h |  2 +
>  scripts/imx/imx-image.c                     | 25 ++++++++
>  scripts/imx/imx.c                           | 67 ++++++++++++++++++++-
>  3 files changed, 93 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/include/mach/imx-header.h b/arch/arm/mach-imx/include/mach/imx-header.h
> index 8e968e6efba3..a58deb8d864e 100644
> --- a/arch/arm/mach-imx/include/mach/imx-header.h
> +++ b/arch/arm/mach-imx/include/mach/imx-header.h
> @@ -118,6 +118,8 @@ struct config_data {
>  	char *signed_hdmi_firmware_file;
>  	int encrypt_image;
>  	size_t dek_size;
> +	uint32_t bb_cfg_ofs;
> +	char *bb_cfg_file;
>  };
>  
>  #define MAX_RECORDS_DCD_V2 1024
> diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
> index 439912a805dc..b8ae63ad9400 100644
> --- a/scripts/imx/imx-image.c
> +++ b/scripts/imx/imx-image.c
> @@ -951,6 +951,31 @@ int main(int argc, char *argv[])
>  			exit(1);
>  		}
>  
> +		/*
> +		* The boot ROM expects a 512-byte configuration parameters area for
> +		* some devices (like the FlexSPI NOR flash) to be present an defined
> +		* offset in the image ivt section (0x400 for the FlexSPI NOR).
> +		*/
> +		if (data.bb_cfg_file) {
> +			size_t bb_cfg_file_size = 512;
> +			char *bb_cfg;
> +
> +			bb_cfg = calloc(512, sizeof(char));
> +			if (!bb_cfg)
> +				exit(1);

The buffer you are allocating here is not used...
> +
> +			bb_cfg = read_file(data.bb_cfg_file, &bb_cfg_file_size);

...the pointer is directly overwritten with the buffer that read_file returns.

You should check the return value of read_file.

Anyway, specifying filenames in config files doesn't work properly. When
you are building out of tree then imx-image will look for the file
relative to $(objtree), but the file really is relative to $(srctree).

In other tools we usually pass filenames via a command line option, see
for example:

images/Makefile.mvebu:43:BINHDR_start_lenovo_ix4_300d.pblb.mvebu1img = $(board)/lenovo-ix4-300d/binary.0

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] arch: arm: imx: spi: Add QSPI boot option to the IMX8MN-EVK

[Joacim Zetterling]

Add lowlevel board functionality to be able to boot from
a QSPI device.

Also added configuration parameters for a QSPI device to be
included in a final QSPI boot image.
All configuration for a QSPI boot image are controlled within
the board flash header configuration file.

Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
---
 .../nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin    | Bin 0 -> 512 bytes
 .../nxp-imx8mn-evk/flash-header-imx8mn-evk.imxcfg |   5 +++++
 arch/arm/boards/nxp-imx8mn-evk/lowlevel.c         |   3 +++
 3 files changed, 8 insertions(+)
 create mode 100644 arch/arm/boards/nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin

diff --git a/arch/arm/boards/nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin b/arch/arm/boards/nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin
new file mode 100644
index 0000000000000000000000000000000000000000..b39e85b8442f27660eb8d797233cdcc02759bc0e
GIT binary patch
literal 512
zcmZ>Bc5`B2U<_jb0cK_}MJmC_$b_s#0GUIG&CMdg!C}CnLP*`HJb@5k1j&O50t!*3
E0ni%)6aWAK

literal 0
HcmV?d00001

diff --git a/arch/arm/boards/nxp-imx8mn-evk/flash-header-imx8mn-evk.imxcfg b/arch/arm/boards/nxp-imx8mn-evk/flash-header-imx8mn-evk.imxcfg
index 27a2138e4322..232ce5f59bdf 100644
--- a/arch/arm/boards/nxp-imx8mn-evk/flash-header-imx8mn-evk.imxcfg
+++ b/arch/arm/boards/nxp-imx8mn-evk/flash-header-imx8mn-evk.imxcfg
@@ -5,3 +5,8 @@ soc imx8mn
 loadaddr 0x912000
 max_load_size 0x3f000
 ivtofs 0x0
+
+# Uncomment the following for QSPI boot
+# header_gap 0x1000
+# bb_cfg_ofs 0x400
+# bb_cfg_file "arch/arm/boards/nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin"
diff --git a/arch/arm/boards/nxp-imx8mn-evk/lowlevel.c b/arch/arm/boards/nxp-imx8mn-evk/lowlevel.c
index de53213ebc77..c8a43e48c085 100644
--- a/arch/arm/boards/nxp-imx8mn-evk/lowlevel.c
+++ b/arch/arm/boards/nxp-imx8mn-evk/lowlevel.c
@@ -170,6 +170,9 @@ static void start_atf(void)
 	case BOOTSOURCE_MMC:
 		imx8mn_esdhc_load_image(instance, false);
 		break;
+	case BOOTSOURCE_SPI:
+		imx8mn_qspi_start_image(0, false);
+		break;
 	default:
 		printf("Unhandled bootsource BOOTSOURCE_%d\n", src);
 		hang();
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/3] arch: arm: imx: spi: Add QSPI boot option to the IMX8MN-EVK

[Sascha Hauer]

On Tue, Mar 08, 2022 at 05:08:01PM +0100, Joacim Zetterling wrote:
> Add lowlevel board functionality to be able to boot from
> a QSPI device.
> 
> Also added configuration parameters for a QSPI device to be
> included in a final QSPI boot image.
> All configuration for a QSPI boot image are controlled within
> the board flash header configuration file.
> 
> Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
> ---
>  .../nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin    | Bin 0 -> 512 bytes
>  .../nxp-imx8mn-evk/flash-header-imx8mn-evk.imxcfg |   5 +++++
>  arch/arm/boards/nxp-imx8mn-evk/lowlevel.c         |   3 +++
>  3 files changed, 8 insertions(+)
>  create mode 100644 arch/arm/boards/nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin
> 
> diff --git a/arch/arm/boards/nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin b/arch/arm/boards/nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin

Looking into the file the content doesn't look very complicated. Can't
we just create it from parameters in the imx-image tool itself?

That would save us from adding another input binary file.

> new file mode 100644
> index 0000000000000000000000000000000000000000..b39e85b8442f27660eb8d797233cdcc02759bc0e
> GIT binary patch
> literal 512
> zcmZ>Bc5`B2U<_jb0cK_}MJmC_$b_s#0GUIG&CMdg!C}CnLP*`HJb@5k1j&O50t!*3
> E0ni%)6aWAK
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/arch/arm/boards/nxp-imx8mn-evk/flash-header-imx8mn-evk.imxcfg b/arch/arm/boards/nxp-imx8mn-evk/flash-header-imx8mn-evk.imxcfg
> index 27a2138e4322..232ce5f59bdf 100644
> --- a/arch/arm/boards/nxp-imx8mn-evk/flash-header-imx8mn-evk.imxcfg
> +++ b/arch/arm/boards/nxp-imx8mn-evk/flash-header-imx8mn-evk.imxcfg
> @@ -5,3 +5,8 @@ soc imx8mn
>  loadaddr 0x912000
>  max_load_size 0x3f000
>  ivtofs 0x0
> +
> +# Uncomment the following for QSPI boot
> +# header_gap 0x1000
> +# bb_cfg_ofs 0x400
> +# bb_cfg_file "arch/arm/boards/nxp-imx8mn-evk/bb-cfg-param-imx8mn-evk.bin"

It would be nice to just create both images, then the user would not
have to change the sourcecode.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox