[PATCH 00/12] imx-usb-loader support for i.MX8MP

[PATCH 00/12] imx-usb-loader support for i.MX8MP

[Sascha Hauer]

The motivation for this series is to add imx-usb-loader support for
i.MX8MP, but there are some by-catches as well.

We recently introduced imx8mm_load_and_start_image_via_tfa() to simplify
the board code. This series adds the same for i.MX8MP and integrates
USB support in that function, so that all new boards using it get USB
support for free.

Also there are several cleanups and fixes for imx-usb-loader.

Sascha

Sascha Hauer (11):
  ARM: i.MX8M: Add romapi support
  ARM: i.MX8MP: Add common code to load image and jump to it via TF-A
  ARM: i.MX8MP-EVK: Use imx8mp_load_and_start_image_via_tfa()
  imx-usb-loader: Factor out common code to function
  imx-usb-loader: rename mxs functions
  imx-usb-loader: Add i.MX8MP support
  imx-usb-loader: drop some casting
  imx-usb-loader: Fix first stage length
  imx-usb-loader: simplify read_memory()
  imx-usb-loader: verify correct image length
  imx-usb-loader: drop some unnecessary casting

Uwe Kleine-König (1):
  ARM: i.MX8MM: Prepare loading only piggydata in imx-usb-loader

 arch/arm/boards/nxp-imx8mp-evk/lowlevel.c |  34 +----
 arch/arm/mach-imx/Makefile                |   2 +-
 arch/arm/mach-imx/atf.c                   |  68 ++++++++-
 arch/arm/mach-imx/include/mach/romapi.h   |  37 +++++
 arch/arm/mach-imx/romapi.c                |  44 ++++++
 include/asm-generic/sections.h            |   1 +
 include/soc/imx8m.h                       |   1 +
 scripts/imx/imx-usb-loader.c              | 167 +++++++++++-----------
 8 files changed, 233 insertions(+), 121 deletions(-)
 create mode 100644 arch/arm/mach-imx/include/mach/romapi.h
 create mode 100644 arch/arm/mach-imx/romapi.c

-- 
2.30.2

[PATCH 01/12] ARM: i.MX8MM: Prepare loading only piggydata in imx-usb-loader

[Sascha Hauer]

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

For two-staged loading via USB we traditionally send the PBL twice. The
first PBL is loaded to OCRAM and executed. Then the full barebox image
including the PBL is sent again and received here. We might change that
in the future in imx-usb-loader so that the PBL is sent only once and we
only receive the rest of the image here. To prepare that step check if
we get a full barebox image or piggydata only. When it's piggydata only
move t to the place where it would be if it would have been a full image.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-imx/atf.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-imx/atf.c b/arch/arm/mach-imx/atf.c
index 709620e5df..ad3800519f 100644
--- a/arch/arm/mach-imx/atf.c
+++ b/arch/arm/mach-imx/atf.c
@@ -76,6 +76,7 @@ void imx8mm_load_and_start_image_via_tfa(void)
 	const u8 *bl31;
 	enum bootsource src;
 	int instance;
+	void *bl33 = (void *)MX8M_ATF_BL33_BASE_ADDR;
 
 	imx8mm_get_boot_source(&src, &instance);
 	switch (src) {
@@ -83,7 +84,29 @@ void imx8mm_load_and_start_image_via_tfa(void)
 		imx8m_esdhc_load_image(instance, false);
 		break;
 	case BOOTSOURCE_SERIAL:
-		imx8mm_barebox_load_usb((void *)MX8M_ATF_BL33_BASE_ADDR);
+		/*
+		 * Traditionally imx-usb-loader sends the PBL twice. The first
+		 * PBL is loaded to OCRAM and executed. Then the full barebox
+		 * image including the PBL is sent again and received here. We
+		 * might change that in the future in imx-usb-loader so that the
+		 * PBL is sent only once and we only receive the rest of the
+		 * image here. To prepare that step we check if we get a full
+		 * barebox image or piggydata only. When it's piggydata only move
+		 * it to the place where it would be if it would have been a
+		 * full image.
+		 */
+		imx8mm_barebox_load_usb(bl33);
+
+		if (!strcmp("barebox", bl33 + 0x20)) {
+			/* Found the barebox marker, so this is a PBL + piggydata */
+			pr_debug("received PBL + piggydata\n");
+		} else {
+			/* no barebox marker, so this is piggydata only */
+			pr_debug("received piggydata\n");
+			memmove(bl33 + barebox_pbl_size, bl33,
+				barebox_image_size - barebox_pbl_size);
+		}
+
 		break;
 	default:
 		printf("Unhandled bootsource BOOTSOURCE_%d\n", src);
@@ -98,8 +121,7 @@ void imx8mm_load_and_start_image_via_tfa(void)
 	 * for the piggy data, so we need to ensure that we are running
 	 * the same code in DRAM.
 	 */
-	memcpy((void *)MX8M_ATF_BL33_BASE_ADDR,
-	       __image_start, barebox_pbl_size);
+	memcpy(bl33, __image_start, barebox_pbl_size);
 
 	get_builtin_firmware(imx8mm_bl31_bin, &bl31, &bl31_size);
 
-- 
2.30.2

[PATCH 02/12] ARM: i.MX8M: Add romapi support

[Sascha Hauer]

The i.MX8MP and i.MX8MN have a C API to the bootrom to chainload images
for example via USB. This patch adds support for the API based on the
corresponding U-Boot code.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-imx/Makefile              |  2 +-
 arch/arm/mach-imx/include/mach/romapi.h | 37 +++++++++++++++++++++
 arch/arm/mach-imx/romapi.c              | 44 +++++++++++++++++++++++++
 include/asm-generic/sections.h          |  1 +
 4 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-imx/include/mach/romapi.h
 create mode 100644 arch/arm/mach-imx/romapi.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 2cafcd77e0..fa5133a229 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -18,7 +18,7 @@ lwl-$(CONFIG_ARCH_IMX6) += imx6-mmdc.o
 obj-$(CONFIG_ARCH_IMX7) += imx7.o
 obj-$(CONFIG_ARCH_VF610) += vf610.o
 obj-pbl-$(CONFIG_ARCH_IMX8M) += imx8m.o
-lwl-$(CONFIG_ARCH_IMX8M) += atf.o
+lwl-$(CONFIG_ARCH_IMX8M) += atf.o romapi.o
 obj-$(CONFIG_IMX_IIM)	+= iim.o
 obj-$(CONFIG_NAND_IMX) += nand.o
 lwl-$(CONFIG_ARCH_IMX_EXTERNAL_BOOT_NAND) += external-nand-boot.o
diff --git a/arch/arm/mach-imx/include/mach/romapi.h b/arch/arm/mach-imx/include/mach/romapi.h
new file mode 100644
index 0000000000..8022fc411e
--- /dev/null
+++ b/arch/arm/mach-imx/include/mach/romapi.h
@@ -0,0 +1,37 @@
+#ifndef __MACH_IMX_ROMAPI_H
+#define __MACH_IMX_ROMAPI_H
+
+struct rom_api {
+	u16 ver;
+	u16 tag;
+	u32 reserved1;
+	u32 (*download_image)(u8 *dest, u32 offset, u32 size,  u32 xor);
+	u32 (*query_boot_infor)(u32 info_type, u32 *info, u32 xor);
+};
+
+enum boot_dev_type_e {
+	BT_DEV_TYPE_SD = 1,
+	BT_DEV_TYPE_MMC = 2,
+	BT_DEV_TYPE_NAND = 3,
+	BT_DEV_TYPE_FLEXSPINOR = 4,
+	BT_DEV_TYPE_SPI_NOR = 6,
+
+	BT_DEV_TYPE_USB = 0xE,
+	BT_DEV_TYPE_MEM_DEV = 0xF,
+
+	BT_DEV_TYPE_INVALID = 0xFF
+};
+
+#define QUERY_ROM_VER		1
+#define QUERY_BT_DEV		2
+#define QUERY_PAGE_SZ		3
+#define QUERY_IVT_OFF		4
+#define QUERY_BT_STAGE		5
+#define QUERY_IMG_OFF		6
+
+#define ROM_API_OKAY		0xF0
+
+int imx8mp_bootrom_load_image(void);
+int imx8mn_bootrom_load_image(void);
+
+#endif /* __MACH_IMX_ROMAPI_H */
diff --git a/arch/arm/mach-imx/romapi.c b/arch/arm/mach-imx/romapi.c
new file mode 100644
index 0000000000..f7d421d737
--- /dev/null
+++ b/arch/arm/mach-imx/romapi.c
@@ -0,0 +1,44 @@
+#include <common.h>
+#include <asm/sections.h>
+#include <mach/romapi.h>
+#include <mach/atf.h>
+
+static int imx8m_bootrom_load(struct rom_api *rom_api, void *adr, size_t size)
+{
+	while (size) {
+		size_t chunksize = min(size, (size_t)1024);
+		int ret;
+
+		ret = rom_api->download_image(adr, 0, chunksize,
+					      (uintptr_t)adr ^ chunksize);
+		if (ret != ROM_API_OKAY) {
+			pr_err("Failed to load piggy data (ret = %x)\n", ret);
+			return -EIO;
+		}
+
+		adr += chunksize;
+		size -= chunksize;
+	}
+
+	return 0;
+}
+
+/* read piggydata via a bootrom callback and place it behind our copy in SDRAM */
+static int imx8m_bootrom_load_image(struct rom_api *rom_api)
+{
+	return imx8m_bootrom_load(rom_api,
+				  (void *)MX8M_ATF_BL33_BASE_ADDR + barebox_pbl_size,
+				  __piggydata_end - __piggydata_start);
+}
+
+int imx8mp_bootrom_load_image(void)
+{
+	struct rom_api *rom_api = (void *)0x980;
+
+	return imx8m_bootrom_load_image(rom_api);
+}
+
+int imx8mn_bootrom_load_image(void)
+{
+	return imx8mp_bootrom_load_image();
+}
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 0b2ed5615b..e54123de0e 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -11,6 +11,7 @@ extern char _end[];
 extern char __image_start[];
 extern char __image_end[];
 extern char __piggydata_start[];
+extern char __piggydata_end[];
 extern void *_barebox_image_size;
 extern void *_barebox_bare_init_size;
 extern void *_barebox_pbl_size;
-- 
2.30.2

[PATCH 03/12] ARM: i.MX8MP: Add common code to load image and jump to it via TF-A

[Sascha Hauer]

Add imx8mp_load_and_start_image_via_tfa() to load barebox proper image
and jump to it via TF-A.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-imx/atf.c | 40 ++++++++++++++++++++++++++++++++++++++++
 include/soc/imx8m.h     |  1 +
 2 files changed, 41 insertions(+)

diff --git a/arch/arm/mach-imx/atf.c b/arch/arm/mach-imx/atf.c
index ad3800519f..d9aaee1bc6 100644
--- a/arch/arm/mach-imx/atf.c
+++ b/arch/arm/mach-imx/atf.c
@@ -6,6 +6,7 @@
 #include <mach/atf.h>
 #include <mach/generic.h>
 #include <mach/xload.h>
+#include <mach/romapi.h>
 #include <soc/imx8m.h>
 #include <soc/fsl/fsl_udc.h>
 
@@ -129,3 +130,42 @@ void imx8mm_load_and_start_image_via_tfa(void)
 
 	/* not reached */
 }
+
+void imx8mp_load_and_start_image_via_tfa(void)
+{
+	size_t bl31_size;
+	const u8 *bl31;
+	enum bootsource src;
+	int instance;
+
+	imx8mp_get_boot_source(&src, &instance);
+	switch (src) {
+	case BOOTSOURCE_MMC:
+		imx8mp_esdhc_load_image(instance, false);
+		break;
+	case BOOTSOURCE_SERIAL:
+		imx8mp_bootrom_load_image();
+		break;
+	default:
+		printf("Unhandled bootsource BOOTSOURCE_%d\n", src);
+		hang();
+	}
+
+
+	/*
+	 * On completion the TF-A will jump to MX8M_ATF_BL33_BASE_ADDR
+	 * in EL2. Copy the image there, but replace the PBL part of
+	 * that image with ourselves. On a high assurance boot only the
+	 * currently running code is validated and contains the checksum
+	 * for the piggy data, so we need to ensure that we are running
+	 * the same code in DRAM.
+	 */
+	memcpy((void *)MX8M_ATF_BL33_BASE_ADDR,
+	       __image_start, barebox_pbl_size);
+
+	get_builtin_firmware(imx8mp_bl31_bin, &bl31, &bl31_size);
+
+	imx8mp_atf_load_bl31(bl31, bl31_size);
+
+	/* not reached */
+}
diff --git a/include/soc/imx8m.h b/include/soc/imx8m.h
index 03b9b59b0b..15f7a4c2c0 100644
--- a/include/soc/imx8m.h
+++ b/include/soc/imx8m.h
@@ -4,5 +4,6 @@
 #define __MACH_IMX8M_H__
 
 void imx8mm_load_and_start_image_via_tfa(void);
+void imx8mp_load_and_start_image_via_tfa(void);
 
 #endif
-- 
2.30.2

[PATCH 04/12] ARM: i.MX8MP-EVK: Use imx8mp_load_and_start_image_via_tfa()

[Sascha Hauer]

Use recently introduced imx8mp_load_and_start_image_via_tfa() instead
of open coding it. With this we gain image loading via USB for the
i.MX8MP-EVK board.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/nxp-imx8mp-evk/lowlevel.c | 34 ++---------------------
 1 file changed, 2 insertions(+), 32 deletions(-)

diff --git a/arch/arm/boards/nxp-imx8mp-evk/lowlevel.c b/arch/arm/boards/nxp-imx8mp-evk/lowlevel.c
index 3306392e9e..df4925f533 100644
--- a/arch/arm/boards/nxp-imx8mp-evk/lowlevel.c
+++ b/arch/arm/boards/nxp-imx8mp-evk/lowlevel.c
@@ -22,6 +22,7 @@
 #include <mfd/pca9450.h>
 #include <soc/imx8m/ddr.h>
 #include <soc/fsl/fsl_udc.h>
+#include <soc/imx8m.h>
 
 extern char __dtb_z_imx8mp_evk_start[];
 
@@ -104,11 +105,6 @@ extern struct dram_timing_info imx8mp_evk_dram_timing;
 
 static void start_atf(void)
 {
-	size_t bl31_size;
-	const u8 *bl31;
-	enum bootsource src;
-	int instance;
-
 	/*
 	 * If we are in EL3 we are running for the first time and need to
 	 * initialize the DRAM and run TF-A (BL31). The TF-A will then jump
@@ -121,33 +117,7 @@ static void start_atf(void)
 
 	imx8mp_ddr_init(&imx8mp_evk_dram_timing, DRAM_TYPE_LPDDR4);
 
-	imx8mp_get_boot_source(&src, &instance);
-	switch (src) {
-	case BOOTSOURCE_MMC:
-		imx8mp_esdhc_load_image(instance, false);
-		break;
-	default:
-		printf("Unhandled bootsource BOOTSOURCE_%d\n", src);
-		hang();
-	}
-
-
-	/*
-	 * On completion the TF-A will jump to MX8M_ATF_BL33_BASE_ADDR
-	 * in EL2. Copy the image there, but replace the PBL part of
-	 * that image with ourselves. On a high assurance boot only the
-	 * currently running code is validated and contains the checksum
-	 * for the piggy data, so we need to ensure that we are running
-	 * the same code in DRAM.
-	 */
-	memcpy((void *)MX8M_ATF_BL33_BASE_ADDR,
-	       __image_start, barebox_pbl_size);
-
-	get_builtin_firmware(imx8mp_bl31_bin, &bl31, &bl31_size);
-
-	imx8mp_atf_load_bl31(bl31, bl31_size);
-
-	/* not reached */
+	imx8mp_load_and_start_image_via_tfa();
 }
 
 /*
-- 
2.30.2

[PATCH 05/12] imx-usb-loader: Factor out common code to function

[Sascha Hauer]

The code to send a buffer straight to an endpoint is implemented twice.
Factor out the code to an extra function.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 69 +++++++++++++++---------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 69df9bbe92..1d19ea0a81 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -713,6 +713,31 @@ static int modify_memory(unsigned addr, unsigned val, int width, int set_bits, i
 	return write_memory(addr, val, 4);
 }
 
+static int send_buf(void *buf, unsigned len)
+{
+	void *p = buf;
+	int cnt = len;
+	int err;
+
+	while (1) {
+		int now = get_min(cnt, mach_id->max_transfer);
+
+		if (!now)
+			break;
+
+		err = transfer(2, p, now, &now);
+		if (err) {
+			printf("dl_command err=%i, last_trans=%i\n", err, now);
+			return err;
+		}
+
+		p += now;
+		cnt -= now;
+	}
+
+	return 0;
+}
+
 static int load_file(void *buf, unsigned len, unsigned dladdr,
 		     unsigned char type, bool mode_barebox)
 {
@@ -729,8 +754,6 @@ static int load_file(void *buf, unsigned len, unsigned dladdr,
 	int retry = 0;
 	unsigned transfer_size = 0;
 	unsigned char tmp[64];
-	void *p;
-	int cnt;
 
 	len = ALIGN(len, 4);
 
@@ -760,24 +783,9 @@ static int load_file(void *buf, unsigned len, unsigned dladdr,
 					err, last_trans, tmp[0], tmp[1], tmp[2], tmp[3]);
 	}
 
-	p = buf;
-	cnt = len;
-
-	while (1) {
-		int now = get_min(cnt, mach_id->max_transfer);
-
-		if (!now)
-			break;
-
-		err = transfer(2, p, now, &now);
-		if (err) {
-			printf("dl_command err=%i, last_trans=%i\n", err, last_trans);
-			return err;
-		}
-
-		p += now;
-		cnt -= now;
-	}
+	err = send_buf(buf, len);
+	if (err)
+		return err;
 
 	if (mode_barebox)
 		return transfer_size;
@@ -1447,8 +1455,6 @@ static int mxs_load_file(libusb_device_handle *dev, uint8_t *data, int size)
 {
 	static struct mxs_command dl_command;
 	int last_trans, err;
-	void *p;
-	int cnt;
 
 	dl_command.sign = htonl(0x424c5443); /* Signature: BLTC */
 	dl_command.tag = htonl(0x1);
@@ -1466,24 +1472,7 @@ static int mxs_load_file(libusb_device_handle *dev, uint8_t *data, int size)
 		return err;
 	}
 
-	p = data;
-	cnt = size;
-
-	while (1) {
-		int now = get_min(cnt, mach_id->max_transfer);
-
-		if (!now)
-			break;
-
-		err = transfer(2, p, now, &now);
-		if (err) {
-			printf("dl_command err=%i, last_trans=%i\n", err, now);
-			return err;
-		}
-
-		p += now;
-		cnt -= now;
-	}
+	err = send_buf(data, size);
 
 	return err;
 }
-- 
2.30.2

[PATCH 06/12] imx-usb-loader: rename mxs functions

[Sascha Hauer]

mxs_load_file() actually uploads a buffer, so rename accordingly.
mxs_work() doesn't have a meaningful name at all, so rename to what
it actually does, mxs_load_file().

While at it remove the unused libusb_device_handle * argument.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 1d19ea0a81..5f7dc3bc0b 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -1450,8 +1450,7 @@ static int write_mem(const struct config_data *data, uint32_t addr,
 	return modify_memory(addr, val, width, set_bits, clear_bits);
 }
 
-/* MXS section */
-static int mxs_load_file(libusb_device_handle *dev, uint8_t *data, int size)
+static int mxs_load_buf(uint8_t *data, int size)
 {
 	static struct mxs_command dl_command;
 	int last_trans, err;
@@ -1477,7 +1476,7 @@ static int mxs_load_file(libusb_device_handle *dev, uint8_t *data, int size)
 	return err;
 }
 
-static int mxs_work(struct usb_work *curr)
+static int mxs_load_file(struct usb_work *curr)
 {
 	size_t fsize = 0;
 	unsigned char *buf = NULL;
@@ -1486,9 +1485,8 @@ static int mxs_work(struct usb_work *curr)
 	if (!buf)
 		return -errno;
 
-	return mxs_load_file(usb_dev_handle, buf, fsize);
+	return mxs_load_buf(buf, fsize);
 }
-/* end of mxs section */
 
 static int parse_initfile(const char *filename)
 {
@@ -1608,7 +1606,7 @@ int main(int argc, char *argv[])
 	}
 
 	if (mach_id->dev_type == DEV_MXS) {
-		ret = mxs_work(&w);
+		ret = mxs_load_file(&w);
 		goto out;
 	}
 
-- 
2.30.2

[PATCH 07/12] imx-usb-loader: Add i.MX8MP support

[Sascha Hauer]

For the i.MX8MP NXP dropped the SDP protocol used on other SoCs.
Instead the image is just sent straight to the device.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 37 +++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 5f7dc3bc0b..974ff741bf 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -74,6 +74,7 @@ struct mach_id {
 #define DEV_IMX		0
 #define DEV_MXS		1
 	unsigned char dev_type;
+	unsigned char hid_endpoint;
 };
 
 struct usb_work {
@@ -180,6 +181,14 @@ static const struct mach_id imx_ids[] = {
 		.header_type = HDR_MX53,
 		.mode = MODE_HID,
 		.max_transfer = 1024,
+	}, {
+		.vid = 0x1fc9,
+		.pid = 0x0146,
+		.name = "i.MX8MP",
+		.header_type = HDR_MX53,
+		.max_transfer = 1020,
+		.mode = MODE_HID,
+		.hid_endpoint = 1,
 	}, {
 		.vid = 0x1fc9,
 		.pid = 0x012b,
@@ -470,15 +479,22 @@ static int transfer(int report, unsigned char *p, unsigned cnt, int *last_trans)
 
 		if (report < 3) {
 			memcpy(&tmp[1], p, cnt);
-			err = libusb_control_transfer(usb_dev_handle,
-					CTRL_OUT,
-					HID_SET_REPORT,
-					(HID_REPORT_TYPE_OUTPUT << 8) | report,
-					0,
-					tmp, cnt + 1, 1000);
-			*last_trans = (err > 0) ? err - 1 : 0;
-			if (err > 0)
-				err = 0;
+			if (mach_id->hid_endpoint) {
+				int trans;
+				err = libusb_interrupt_transfer(usb_dev_handle,
+						mach_id->hid_endpoint, tmp, cnt + 1, &trans, 1000);
+				*last_trans = trans - 1;
+			} else {
+				err = libusb_control_transfer(usb_dev_handle,
+						CTRL_OUT,
+						HID_SET_REPORT,
+						(HID_REPORT_TYPE_OUTPUT << 8) | report,
+						0,
+						tmp, cnt + 1, 1000);
+				*last_trans = (err > 0) ? err - 1 : 0;
+				if (err > 0)
+					err = 0;
+			}
 		} else {
 			*last_trans = 0;
 			memset(&tmp[1], 0, cnt);
@@ -1353,6 +1369,9 @@ static int do_irom_download(struct usb_work *curr, int verify)
 
 	header_offset = ret;
 
+	if (mach_id->hid_endpoint)
+		return send_file(buf + header_offset, fsize - header_offset);
+
 	if (plugin && (!curr->plug)) {
 		printf("Only plugin header found\n");
 		ret = -1;
-- 
2.30.2

[PATCH 08/12] imx-usb-loader: drop some casting

[Sascha Hauer]

Instead of casting 'bd' to struct imx_boot_data * multiple times use
an extra variable for it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 974ff741bf..b2c835225c 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -1258,19 +1258,22 @@ static int get_dl_start(const unsigned char *p, const unsigned char *file_start,
 	}
 	case HDR_MX53:
 	{
-		unsigned char *bd;
+		unsigned char *_bd;
 		struct imx_flash_header_v2 *hdr = (struct imx_flash_header_v2 *)p;
+		struct imx_boot_data *bd;
 
 		*header_addr = hdr->self;
-		bd = hdr->boot_data_ptr + cvt_dest_to_src;
-		if ((bd < file_start) || ((bd + 4) > file_end)) {
+		_bd = hdr->boot_data_ptr + cvt_dest_to_src;
+		if ((_bd < file_start) || ((_bd + 4) > file_end)) {
 			printf("bad boot_data_ptr %08x\n", hdr->boot_data_ptr);
 			return -1;
 		}
 
-		*firststage_len = ((struct imx_boot_data *)bd)->size;
-		*plugin = ((struct imx_boot_data *)bd)->plugin;
-		((struct imx_boot_data *)bd)->plugin = 0;
+		bd = (void *)_bd;
+
+		*firststage_len = bd->size;
+		*plugin = bd->plugin;
+		bd->plugin = 0;
 
 		break;
 	}
-- 
2.30.2

[PATCH 09/12] imx-usb-loader: Fix first stage length

[Sascha Hauer]

On most SoCs the IVT header is not at the beginning of the image but at
offset 0x400. As we skip the leading 0x400 bytes while uploading we can
substract that offset from the image size we upload. This shouldn't make
a difference now, but in future we might change the two-staged upload
process: Now we upload the PBL once and afterwards the full image
incuding the same PBL again. It would be more elegant to upload the PBL
only once and only the rest of the image afterwards. If we do so, it it
important to exactly upload the first stage so that barebox on the other
end receives exactly the second stage at the offset it expects it to be.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index b2c835225c..f4f13d5cc7 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -1271,7 +1271,7 @@ static int get_dl_start(const unsigned char *p, const unsigned char *file_start,
 
 		bd = (void *)_bd;
 
-		*firststage_len = bd->size;
+		*firststage_len = bd->size - (hdr->self - bd->start);
 		*plugin = bd->plugin;
 		bd->plugin = 0;
 
-- 
2.30.2

[PATCH 10/12] imx-usb-loader: simplify read_memory()

[Sascha Hauer]

In read_memory() we transfer at maximum 64 bytes, so the returned number
of actually sent bytes will never be greater than 64 and we can drop the
corresponding checks. Then it is checked if there are really 64 bytes
transferred. We don't do this elsewhere, so drop it here as well.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index f4f13d5cc7..44ad7e3f0f 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -619,17 +619,10 @@ static int read_memory(unsigned addr, void *dest, unsigned cnt)
 					err, last_trans, tmp[0], tmp[1], tmp[2], tmp[3], cnt, rem);
 			break;
 		}
-		if ((last_trans > rem) || (last_trans > 64)) {
-			if ((last_trans == 64) && (rem < 64)) {
-				/* Last transfer is expected to be too large for HID */
-			} else {
-				printf("err: %02x %02x %02x %02x cnt=%u rem=%d last_trans=%i\n",
-						tmp[0], tmp[1], tmp[2], tmp[3], cnt, rem, last_trans);
-			}
+
+		if (last_trans > rem)
 			last_trans = rem;
-			if (last_trans > 64)
-				last_trans = 64;
-		}
+
 		memcpy(dest, tmp, last_trans);
 		dest += last_trans;
 		rem -= last_trans;
-- 
2.30.2

[PATCH 11/12] imx-usb-loader: verify correct image length

[Sascha Hauer]

When verifying the uploaded file we may only check the already uploaded
part of the image, not the whole image.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 44ad7e3f0f..f3dcc61b9e 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -1409,7 +1409,7 @@ static int do_irom_download(struct usb_work *curr, int verify)
 	if (verify) {
 		printf("verifying file...\n");
 
-		ret = verify_memory(image, fsize, header_addr);
+		ret = verify_memory(image, firststage_len, header_addr);
 		if (ret < 0) {
 			printf("verifying failed\n");
 			goto cleanup;
-- 
2.30.2

[PATCH 12/12] imx-usb-loader: drop some unnecessary casting

[Sascha Hauer]

Change the buffer pointer in transfer() to type void * to drop some
unnecessary type casting from the callers.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 scripts/imx/imx-usb-loader.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index f3dcc61b9e..450ee14354 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -456,7 +456,7 @@ static void dump_bytes(const void *src, unsigned cnt, unsigned addr)
  * EP2IN - bulk in
  * (max packet size of 512 bytes)
  */
-static int transfer(int report, unsigned char *p, unsigned cnt, int *last_trans)
+static int transfer(int report, void *p, unsigned cnt, int *last_trans)
 {
 	int err;
 	if (cnt > mach_id->max_transfer)
@@ -527,7 +527,7 @@ static int do_status(void)
 	unsigned char tmp[64];
 	int retry = 0;
 	int err;
-	static const struct sdp_command status_command = {
+	static struct sdp_command status_command = {
 		.cmd = SDP_ERROR_STATUS,
 		.addr = 0,
 		.format = 0,
@@ -537,8 +537,7 @@ static int do_status(void)
 	};
 
 	for (;;) {
-		err = transfer(1, (unsigned char *) &status_command, 16,
-			       &last_trans);
+		err = transfer(1, &status_command, 16, &last_trans);
 
 		if (verbose > 2)
 			printf("report 1, wrote %i bytes, err=%i\n", last_trans, err);
@@ -591,8 +590,7 @@ static int read_memory(unsigned addr, void *dest, unsigned cnt)
 	read_reg_command.cnt = htonl(cnt);
 
 	for (;;) {
-		err = transfer(1, (unsigned char *) &read_reg_command, 16,
-			       &last_trans);
+		err = transfer(1, &read_reg_command, 16, &last_trans);
 		if (!err)
 			break;
 		printf("read_reg_command err=%i, last_trans=%i\n", err, last_trans);
@@ -668,8 +666,7 @@ static int write_memory(unsigned addr, unsigned val, int width)
 	write_reg_command.data = htonl(val);
 
 	for (;;) {
-		err = transfer(1, (unsigned char *) &write_reg_command, 16,
-			       &last_trans);
+		err = transfer(1, &write_reg_command, 16, &last_trans);
 		if (!err)
 			break;
 		printf("write_reg_command err=%i, last_trans=%i\n", err, last_trans);
@@ -771,8 +768,7 @@ static int load_file(void *buf, unsigned len, unsigned dladdr,
 	dl_command.rsvd = type;
 
 	for (;;) {
-		err = transfer(1, (unsigned char *) &dl_command, 16,
-			       &last_trans);
+		err = transfer(1, &dl_command, 16, &last_trans);
 		if (!err)
 			break;
 
@@ -832,8 +828,7 @@ static int sdp_jump_address(unsigned addr)
 	jump_command.addr = htonl(addr);
 
 	for (;;) {
-		err = transfer(1, (unsigned char *) &jump_command, 16,
-			       &last_trans);
+		err = transfer(1, &jump_command, 16, &last_trans);
 		if (!err)
 			break;
 
@@ -1479,7 +1474,7 @@ static int mxs_load_buf(uint8_t *data, int size)
 	dl_command.cmd = MXS_CMD_FW_DOWNLOAD;
 	dl_command.dw_size = htonl(size);
 
-	err = transfer(1, (unsigned char *) &dl_command, 20, &last_trans);
+	err = transfer(1, &dl_command, 20, &last_trans);
 	if (err) {
 		printf("transfer error at init step: err=%i, last_trans=%i\n",
 		       err, last_trans);
-- 
2.30.2

Re: [PATCH 07/12] imx-usb-loader: Add i.MX8MP support

[Marco Felsch]

Hi Sascha,

please see below.

On 22-07-14, Sascha Hauer wrote:
> For the i.MX8MP NXP dropped the SDP protocol used on other SoCs.
> Instead the image is just sent straight to the device.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  scripts/imx/imx-usb-loader.c | 37 +++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
> index 5f7dc3bc0b..974ff741bf 100644
> --- a/scripts/imx/imx-usb-loader.c
> +++ b/scripts/imx/imx-usb-loader.c
> @@ -74,6 +74,7 @@ struct mach_id {
>  #define DEV_IMX		0
>  #define DEV_MXS		1
>  	unsigned char dev_type;
> +	unsigned char hid_endpoint;
>  };
>  
>  struct usb_work {
> @@ -180,6 +181,14 @@ static const struct mach_id imx_ids[] = {
>  		.header_type = HDR_MX53,
>  		.mode = MODE_HID,
>  		.max_transfer = 1024,
> +	}, {
> +		.vid = 0x1fc9,
> +		.pid = 0x0146,
> +		.name = "i.MX8MP",
> +		.header_type = HDR_MX53,
> +		.max_transfer = 1020,
> +		.mode = MODE_HID,
> +		.hid_endpoint = 1,
>  	}, {
>  		.vid = 0x1fc9,
>  		.pid = 0x012b,
> @@ -470,15 +479,22 @@ static int transfer(int report, unsigned char *p, unsigned cnt, int *last_trans)
>  
>  		if (report < 3) {
>  			memcpy(&tmp[1], p, cnt);
> -			err = libusb_control_transfer(usb_dev_handle,
> -					CTRL_OUT,
> -					HID_SET_REPORT,
> -					(HID_REPORT_TYPE_OUTPUT << 8) | report,
> -					0,
> -					tmp, cnt + 1, 1000);
> -			*last_trans = (err > 0) ? err - 1 : 0;
> -			if (err > 0)
> -				err = 0;
> +			if (mach_id->hid_endpoint) {
> +				int trans;
> +				err = libusb_interrupt_transfer(usb_dev_handle,
> +						mach_id->hid_endpoint, tmp, cnt + 1, &trans, 1000);
> +				*last_trans = trans - 1;
> +			} else {
> +				err = libusb_control_transfer(usb_dev_handle,
> +						CTRL_OUT,
> +						HID_SET_REPORT,
> +						(HID_REPORT_TYPE_OUTPUT << 8) | report,
> +						0,
> +						tmp, cnt + 1, 1000);
> +				*last_trans = (err > 0) ? err - 1 : 0;
> +				if (err > 0)
> +					err = 0;
> +			}
>  		} else {
>  			*last_trans = 0;
>  			memset(&tmp[1], 0, cnt);
> @@ -1353,6 +1369,9 @@ static int do_irom_download(struct usb_work *curr, int verify)
>  
>  	header_offset = ret;
>  
> +	if (mach_id->hid_endpoint)
> +		return send_file(buf + header_offset, fsize - header_offset);
                         ^
This function is defined nowhere and the compiler aborts. Did you forget
to send a patch defining this function?

Regards,
  Marco

> +
>  	if (plugin && (!curr->plug)) {
>  		printf("Only plugin header found\n");
>  		ret = -1;
> -- 
> 2.30.2
> 
> 
> 

Re: [PATCH 07/12] imx-usb-loader: Add i.MX8MP support

[Sascha Hauer]

On Fri, Jul 29, 2022 at 11:33:36AM +0200, Marco Felsch wrote:
> Hi Sascha,
> 
> please see below.
> 
> On 22-07-14, Sascha Hauer wrote:
> > For the i.MX8MP NXP dropped the SDP protocol used on other SoCs.
> > Instead the image is just sent straight to the device.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  scripts/imx/imx-usb-loader.c | 37 +++++++++++++++++++++++++++---------
> >  1 file changed, 28 insertions(+), 9 deletions(-)
> > 
> > diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
> > index 5f7dc3bc0b..974ff741bf 100644
> > --- a/scripts/imx/imx-usb-loader.c
> > +++ b/scripts/imx/imx-usb-loader.c
> > @@ -74,6 +74,7 @@ struct mach_id {
> >  #define DEV_IMX		0
> >  #define DEV_MXS		1
> >  	unsigned char dev_type;
> > +	unsigned char hid_endpoint;
> >  };
> >  
> >  struct usb_work {
> > @@ -180,6 +181,14 @@ static const struct mach_id imx_ids[] = {
> >  		.header_type = HDR_MX53,
> >  		.mode = MODE_HID,
> >  		.max_transfer = 1024,
> > +	}, {
> > +		.vid = 0x1fc9,
> > +		.pid = 0x0146,
> > +		.name = "i.MX8MP",
> > +		.header_type = HDR_MX53,
> > +		.max_transfer = 1020,
> > +		.mode = MODE_HID,
> > +		.hid_endpoint = 1,
> >  	}, {
> >  		.vid = 0x1fc9,
> >  		.pid = 0x012b,
> > @@ -470,15 +479,22 @@ static int transfer(int report, unsigned char *p, unsigned cnt, int *last_trans)
> >  
> >  		if (report < 3) {
> >  			memcpy(&tmp[1], p, cnt);
> > -			err = libusb_control_transfer(usb_dev_handle,
> > -					CTRL_OUT,
> > -					HID_SET_REPORT,
> > -					(HID_REPORT_TYPE_OUTPUT << 8) | report,
> > -					0,
> > -					tmp, cnt + 1, 1000);
> > -			*last_trans = (err > 0) ? err - 1 : 0;
> > -			if (err > 0)
> > -				err = 0;
> > +			if (mach_id->hid_endpoint) {
> > +				int trans;
> > +				err = libusb_interrupt_transfer(usb_dev_handle,
> > +						mach_id->hid_endpoint, tmp, cnt + 1, &trans, 1000);
> > +				*last_trans = trans - 1;
> > +			} else {
> > +				err = libusb_control_transfer(usb_dev_handle,
> > +						CTRL_OUT,
> > +						HID_SET_REPORT,
> > +						(HID_REPORT_TYPE_OUTPUT << 8) | report,
> > +						0,
> > +						tmp, cnt + 1, 1000);
> > +				*last_trans = (err > 0) ? err - 1 : 0;
> > +				if (err > 0)
> > +					err = 0;
> > +			}
> >  		} else {
> >  			*last_trans = 0;
> >  			memset(&tmp[1], 0, cnt);
> > @@ -1353,6 +1369,9 @@ static int do_irom_download(struct usb_work *curr, int verify)
> >  
> >  	header_offset = ret;
> >  
> > +	if (mach_id->hid_endpoint)
> > +		return send_file(buf + header_offset, fsize - header_offset);
>                          ^
> This function is defined nowhere and the compiler aborts. Did you forget
> to send a patch defining this function?

Should be send_buf(). I renamed this function at some point during
rebasing.

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 |