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
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
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
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
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
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
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
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
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
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
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
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
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
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 > > >
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 |