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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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