[PATCH 1/5] scripts: imx-usb-loader: const function args

[PATCH 1/5] scripts: imx-usb-loader: const function args

[Alexander Kurz]

Signed-off-by: Alexander Kurz <akurz@blala.de>
---
 scripts/imx/imx-usb-loader.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index cf9d610..9ead7b1 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -669,13 +669,14 @@ static int load_file(void *buf, unsigned len, unsigned dladdr, unsigned char typ
 	return transfer_size;
 }
 
-static int write_dcd_table_ivt(struct imx_flash_header_v2 *hdr, unsigned char *file_start, unsigned cnt)
+static int write_dcd_table_ivt(const struct imx_flash_header_v2 *hdr,
+			       const unsigned char *file_start, unsigned cnt)
 {
 	unsigned char *dcd_end;
 	unsigned m_length;
 #define cvt_dest_to_src		(((unsigned char *)hdr) - hdr->self)
 	unsigned char* dcd;
-	unsigned char* file_end = file_start + cnt;
+	const unsigned char *file_end = file_start + cnt;
 	int err = 0;
 
 	if (!hdr->dcd_ptr) {
@@ -747,8 +748,8 @@ static int write_dcd_table_ivt(struct imx_flash_header_v2 *hdr, unsigned char *f
 	return err;
 }
 
-static int get_dcd_range_old(struct imx_flash_header *hdr,
-		unsigned char *file_start, unsigned cnt,
+static int get_dcd_range_old(const struct imx_flash_header *hdr,
+		const unsigned char *file_start, unsigned cnt,
 		unsigned char **pstart, unsigned char **pend)
 {
 	unsigned char *dcd_end;
@@ -756,7 +757,7 @@ static int get_dcd_range_old(struct imx_flash_header *hdr,
 #define cvt_dest_to_src_old		(((unsigned char *)&hdr->dcd) - hdr->dcd_ptr_ptr)
 	unsigned char* dcd;
 	unsigned val;
-	unsigned char* file_end = file_start + cnt;
+	const unsigned char *file_end = file_start + cnt;
 
 	if (!hdr->dcd) {
 		printf("No dcd table, barker=%x\n", hdr->app_code_barker);
@@ -794,7 +795,8 @@ static int get_dcd_range_old(struct imx_flash_header *hdr,
 	return 0;
 }
 
-static int write_dcd_table_old(struct imx_flash_header *hdr, unsigned char *file_start, unsigned cnt)
+static int write_dcd_table_old(const struct imx_flash_header *hdr,
+			       const unsigned char *file_start, unsigned cnt)
 {
 	unsigned val;
 	unsigned char *dcd_end;
@@ -877,10 +879,12 @@ err:
 	return ret;
 }
 
-static int is_header(unsigned char *p)
+static int is_header(const unsigned char *p)
 {
-	struct imx_flash_header *ohdr = (struct imx_flash_header *)p;
-	struct imx_flash_header_v2 *hdr = (struct imx_flash_header_v2 *)p;
+	const struct imx_flash_header *ohdr =
+		(const struct imx_flash_header *)p;
+	const struct imx_flash_header_v2 *hdr =
+		(const struct imx_flash_header_v2 *)p;
 
 	switch (usb_id->mach_id->header_type) {
 	case HDR_MX51:
@@ -895,7 +899,8 @@ static int is_header(unsigned char *p)
 	return 0;
 }
 
-static int perform_dcd(unsigned char *p, unsigned char *file_start, unsigned cnt)
+static int perform_dcd(unsigned char *p, const unsigned char *file_start,
+		       unsigned cnt)
 {
 	struct imx_flash_header *ohdr = (struct imx_flash_header *)p;
 	struct imx_flash_header_v2 *hdr = (struct imx_flash_header_v2 *)p;
@@ -938,11 +943,11 @@ static int clear_dcd_ptr(unsigned char *p, unsigned char *file_start, unsigned c
 	return 0;
 }
 
-static int get_dl_start(unsigned char *p, unsigned char *file_start,
+static int get_dl_start(const unsigned char *p, const unsigned char *file_start,
 		unsigned cnt, unsigned *dladdr, unsigned *max_length, unsigned *plugin,
 		unsigned *header_addr)
 {
-	unsigned char* file_end = file_start + cnt;
+	const unsigned char *file_end = file_start + cnt;
 	switch (usb_id->mach_id->header_type) {
 	case HDR_MX51:
 	{
-- 
2.1.4


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

[PATCH 2/5] scripts: imx-usb-loader: remove useless code

[Alexander Kurz]

The configuration interface for struct usb_work is not implemented here
leaving the options set on fixed settings or even uninitialized.
Do some cleanup and remove those half-cooked dead code passages.

Signed-off-by: Alexander Kurz <akurz@blala.de>
---
 scripts/imx/imx-usb-loader.c | 49 +++-----------------------------------------
 1 file changed, 3 insertions(+), 46 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 9ead7b1..17551e3 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -67,13 +67,7 @@ struct mach_id {
 struct usb_work {
 	char filename[256];
 	unsigned char dcd;
-	unsigned char clear_dcd;
 	unsigned char plug;
-#define J_ADDR		1
-#define J_HEADER	2
-#define J_HEADER2	3
-	unsigned char jump_mode;
-	unsigned jump_addr;
 };
 
 struct usb_id {
@@ -925,24 +919,6 @@ static int perform_dcd(unsigned char *p, const unsigned char *file_start,
 	return ret;
 }
 
-static int clear_dcd_ptr(unsigned char *p, unsigned char *file_start, unsigned cnt)
-{
-	struct imx_flash_header *ohdr = (struct imx_flash_header *)p;
-	struct imx_flash_header_v2 *hdr = (struct imx_flash_header_v2 *)p;
-
-	switch (usb_id->mach_id->header_type) {
-	case HDR_MX51:
-		printf("clear dcd_ptr=0x%08x\n", ohdr->dcd);
-		ohdr->dcd = 0;
-		break;
-	case HDR_MX53:
-		printf("clear dcd_ptr=0x%08x\n", hdr->dcd_ptr);
-		hdr->dcd_ptr = 0;
-		break;
-	}
-	return 0;
-}
-
 static int get_dl_start(const unsigned char *p, const unsigned char *file_start,
 		unsigned cnt, unsigned *dladdr, unsigned *max_length, unsigned *plugin,
 		unsigned *header_addr)
@@ -1017,18 +993,6 @@ static int process_header(struct usb_work *curr, unsigned char *buf, int cnt,
 				return ret;
 			}
 			curr->dcd = 0;
-			if ((!curr->jump_mode) && (!curr->plug)) {
-				printf("!!dcd done, nothing else requested\n");
-				return 0;
-			}
-		}
-
-		if (curr->clear_dcd) {
-			ret = clear_dcd_ptr(p, buf, cnt);
-			if (ret < 0) {
-				printf("!!clear_dcd returned %i\n", ret);
-				return ret;
-			}
 		}
 
 		if (*p_plugin && (!curr->plug) && (!header_cnt)) {
@@ -1110,12 +1074,6 @@ static int do_irom_download(struct usb_work *curr, int verify)
 
 	header_offset = ret;
 
-	if ((!curr->jump_mode) && (!curr->plug)) {
-		/*  nothing else requested */
-		ret = 0;
-		goto cleanup;
-	}
-
 	if (plugin && (!curr->plug)) {
 		printf("Only plugin header found\n");
 		ret = -1;
@@ -1130,9 +1088,7 @@ static int do_irom_download(struct usb_work *curr, int verify)
 
 	file_base = header_addr - header_offset;
 
-	type = (curr->plug || curr->jump_mode) ? FT_APP : FT_LOAD_ONLY;
-
-	if (usb_id->mach_id->mode == MODE_BULK && type == FT_APP) {
+	if (usb_id->mach_id->mode == MODE_BULK) {
 		/* No jump command, dladdr should point to header */
 		dladdr = header_addr;
 	}
@@ -1153,6 +1109,8 @@ static int do_irom_download(struct usb_work *curr, int verify)
 	if (fsize > max_length)
 		fsize = max_length;
 
+	type = FT_APP;
+
 	if (verify) {
 		verify_buffer = malloc(64);
 
@@ -1314,7 +1272,6 @@ int main(int argc, char *argv[])
 
 	w.plug = 1;
 	w.dcd = 1;
-	w.jump_mode = J_HEADER;
 	strncpy(w.filename, argv[optind], sizeof(w.filename) - 1);
 
 	r = libusb_init(NULL);
-- 
2.1.4


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

[PATCH 3/5] scripts: imx-usb-loader: remove useless variable

[Alexander Kurz]

Remove a variable raising complexity for no reason.

Signed-off-by: Alexander Kurz <akurz@blala.de>
---
 scripts/imx/imx-usb-loader.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 17551e3..9607ce6 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -1029,7 +1029,6 @@ static int do_irom_download(struct usb_work *curr, int verify)
 	unsigned char *buf = NULL;
 	unsigned char *image;
 	unsigned char *verify_buffer = NULL;
-	unsigned char *p;
 	unsigned char tmp[64];
 	unsigned dladdr = 0;
 	unsigned max_length;
@@ -1102,7 +1101,6 @@ static int do_irom_download(struct usb_work *curr, int verify)
 
 	image = buf + skip;
 
-	p = image;
 	cnt -= skip;
 	fsize -= skip;
 
@@ -1120,7 +1118,7 @@ static int do_irom_download(struct usb_work *curr, int verify)
 			goto cleanup;
 		}
 
-		memcpy(verify_buffer, p, 64);
+		memcpy(verify_buffer, image, 64);
 
 		if ((type == FT_APP) && (usb_id->mach_id->mode != MODE_HID)) {
 			type = FT_LOAD_ONLY;
-- 
2.1.4


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

[PATCH 4/5] scripts: imx-usb-loader: structured protocol access

[Alexander Kurz]

Do some cleanup and access the elements of Serial Download Protocol
messages in endianess-portable manner using proper typed struct members.

Signed-off-by: Alexander Kurz <akurz@blala.de>
---
 scripts/imx/imx-usb-loader.c | 132 +++++++++++++++++++++----------------------
 1 file changed, 65 insertions(+), 67 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 9607ce6..70d3ec5 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -32,6 +32,7 @@
 #include <stdlib.h>
 #include <libusb.h>
 #include <getopt.h>
+#include <arpa/inet.h>
 #include <linux/kernel.h>
 
 #include "imx.h"
@@ -148,6 +149,21 @@ struct mach_id imx_ids[] = {
 	},
 };
 
+#define SDP_READ_REG		0x0101
+#define SDP_WRITE_REG		0x0202
+#define SDP_WRITE_FILE		0x0404
+#define SDP_ERROR_STATUS	0x0505
+#define SDP_JUMP_ADDRESS	0x0b0b
+
+struct sdp_command  {
+	uint16_t cmd;
+	uint32_t addr;
+	uint8_t format;
+	uint32_t cnt;
+	uint32_t data;
+	uint8_t rsvd;
+} __attribute__((packed));
+
 static struct mach_id *imx_device(unsigned short vid, unsigned short pid)
 {
 	int i;
@@ -362,16 +378,18 @@ int do_status(void)
 	unsigned char tmp[64];
 	int retry = 0;
 	int err;
-	const unsigned char status_command[] = {
-		5, 5, 0, 0, 0, 0,
-		0,
-		0, 0, 0, 0,
-		0, 0, 0, 0,
-		0
+	static const struct sdp_command status_command = {
+		.cmd = SDP_ERROR_STATUS,
+		.addr = 0,
+		.format = 0,
+		.cnt = 0,
+		.data = 0,
+		.rsvd = 0,
 	};
 
 	for (;;) {
-		err = transfer(1, (unsigned char*)status_command, 16, &last_trans);
+		err = transfer(1, (unsigned char *) &status_command, 16,
+			       &last_trans);
 
 		if (verbose > 2)
 			printf("report 1, wrote %i bytes, err=%i\n", last_trans, err);
@@ -408,14 +426,13 @@ int do_status(void)
 
 static int read_memory(unsigned addr, void *dest, unsigned cnt)
 {
-	static unsigned char read_reg_command[] = {
-		1,
-		1,
-		V(0),		/* address */
-		0x20,		/* format */
-		V(0x00000004),	/* data count */
-		V(0),		/* data */
-		0x00,		/* type */
+	static struct sdp_command read_reg_command = {
+		.cmd = SDP_READ_REG,
+		.addr = 0,
+		.format = 0x20,
+		.cnt = 0,
+		.data = 0,
+		.rsvd = 0,
 	};
 
 	int retry = 0;
@@ -423,18 +440,12 @@ static int read_memory(unsigned addr, void *dest, unsigned cnt)
 	int err;
 	int rem;
 	unsigned char tmp[64];
-	read_reg_command[2] = (unsigned char)(addr >> 24);
-	read_reg_command[3] = (unsigned char)(addr >> 16);
-	read_reg_command[4] = (unsigned char)(addr >> 8);
-	read_reg_command[5] = (unsigned char)(addr);
-
-	read_reg_command[7] = (unsigned char)(cnt >> 24);
-	read_reg_command[8] = (unsigned char)(cnt >> 16);
-	read_reg_command[9] = (unsigned char)(cnt >> 8);
-	read_reg_command[10] = (unsigned char)(cnt);
+	read_reg_command.addr = htonl(addr);
+	read_reg_command.cnt = htonl(cnt);
 
 	for (;;) {
-		err = transfer(1, read_reg_command, 16, &last_trans);
+		err = transfer(1, (unsigned char *) &read_reg_command, 16,
+			       &last_trans);
 		if (!err)
 			break;
 		printf("read_reg_command err=%i, last_trans=%i\n", err, last_trans);
@@ -485,47 +496,40 @@ static int write_memory(unsigned addr, unsigned val, int width)
 	int last_trans;
 	int err = 0;
 	unsigned char tmp[64];
-	unsigned char ds;
-	unsigned char write_reg_command[] = {
-		2,
-		2,
-		V(0),		/* address */
-		0x0,		/* format */
-		V(0x00000004),	/* data count */
-		V(0),		/* data */
-		0x00,		/* type */
+	static struct sdp_command write_reg_command = {
+		.cmd = SDP_WRITE_REG,
+		.addr = 0,
+		.format = 0,
+		.cnt = 0,
+		.data = 0,
+		.rsvd = 0,
 	};
-	write_reg_command[2] = (unsigned char)(addr >> 24);
-	write_reg_command[3] = (unsigned char)(addr >> 16);
-	write_reg_command[4] = (unsigned char)(addr >> 8);
-	write_reg_command[5] = (unsigned char)(addr);
+
+	write_reg_command.addr = htonl(addr);
+	write_reg_command.cnt = htonl(4);
 
 	if (verbose > 1)
 		printf("write memory reg: 0x%08x val: 0x%08x width: %d\n", addr, val, width);
 
 	switch (width) {
 		case 1:
-			ds = 0x8;
+			write_reg_command.format = 0x8;
 			break;
 		case 2:
-			ds = 0x10;
+			write_reg_command.format = 0x10;
 			break;
 		case 4:
-			ds = 0x20;
+			write_reg_command.format = 0x20;
 			break;
 		default:
 			return -1;
 	}
 
-	write_reg_command[6] = ds;
-
-	write_reg_command[11] = (unsigned char)(val >> 24);
-	write_reg_command[12] = (unsigned char)(val >> 16);
-	write_reg_command[13] = (unsigned char)(val >> 8);
-	write_reg_command[14] = (unsigned char)(val);
+	write_reg_command.data = htonl(val);
 
 	for (;;) {
-		err = transfer(1, write_reg_command, 16, &last_trans);
+		err = transfer(1, (unsigned char *) &write_reg_command, 16,
+			       &last_trans);
 		if (!err)
 			break;
 		printf("write_reg_command err=%i, last_trans=%i\n", err, last_trans);
@@ -580,15 +584,15 @@ static int modify_memory(unsigned addr, unsigned val, int width, int set_bits, i
 
 static int load_file(void *buf, unsigned len, unsigned dladdr, unsigned char type)
 {
-	static unsigned char dl_command[] = {
-		0x04,
-		0x04,
-		V(0),		/* address */
-		0x00,		/* format */
-		V(0x00000020),	/* data count */
-		V(0),		/* data */
-		0xaa,		/* type */
+	static struct sdp_command dl_command = {
+		.cmd = SDP_WRITE_FILE,
+		.addr = 0,
+		.format = 0,
+		.cnt = 0,
+		.data = 0,
+		.rsvd = 0,
 	};
+
 	int last_trans, err;
 	int retry = 0;
 	unsigned transfer_size = 0;
@@ -596,19 +600,13 @@ static int load_file(void *buf, unsigned len, unsigned dladdr, unsigned char typ
 	void *p;
 	int cnt;
 
-	dl_command[2] = (unsigned char)(dladdr >> 24);
-	dl_command[3] = (unsigned char)(dladdr >> 16);
-	dl_command[4] = (unsigned char)(dladdr >> 8);
-	dl_command[5] = (unsigned char)(dladdr);
-
-	dl_command[7] = (unsigned char)(len >> 24);
-	dl_command[8] = (unsigned char)(len >> 16);
-	dl_command[9] = (unsigned char)(len >> 8);
-	dl_command[10] = (unsigned char)(len);
-	dl_command[15] =  type;
+	dl_command.addr = htonl(dladdr);
+	dl_command.cnt = htonl(len);
+	dl_command.rsvd = type;
 
 	for (;;) {
-		err = transfer(1, dl_command, 16, &last_trans);
+		err = transfer(1, (unsigned char *) &dl_command, 16,
+			       &last_trans);
 		if (!err)
 			break;
 
-- 
2.1.4


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

[PATCH 5/5] scripts: imx-usb-loader: split off topic-code into functions

[Alexander Kurz]

Improve code understandability: extract the "jump application" Serial
Download Protocol access method and file-to-buffer reader functionality
out of do_irom_download().

Signed-off-by: Alexander Kurz <akurz@blala.de>
---
 scripts/imx/imx-usb-loader.c | 159 +++++++++++++++++++++++++------------------
 1 file changed, 92 insertions(+), 67 deletions(-)

diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
index 70d3ec5..688cd53 100644
--- a/scripts/imx/imx-usb-loader.c
+++ b/scripts/imx/imx-usb-loader.c
@@ -274,6 +274,51 @@ static long get_file_size(FILE *xfile)
 	return size;
 }
 
+static int read_file(const char *name, unsigned char **buffer, unsigned *size)
+{
+	FILE *xfile;
+	unsigned fsize;
+	int cnt;
+	unsigned char *buf;
+	xfile = fopen(name, "rb");
+	if (!xfile) {
+		printf("error, can not open input file: %s\n", name);
+		return -5;
+	}
+
+	fsize = get_file_size(xfile);
+	if (fsize < 0x20) {
+		printf("error, file: %s is too small\n", name);
+		fclose(xfile);
+		return -2;
+	}
+
+	buf = malloc(fsize);
+	if (!buf) {
+		printf("error, out of memory\n");
+		fclose(xfile);
+		return -2;
+	}
+
+	cnt = fread(buf, 1 , fsize, xfile);
+	if (cnt < fsize) {
+		printf("error, cannot read %s\n", name);
+		fclose(xfile);
+		free(buf);
+		return -1;
+	}
+
+	if (size)
+		*size = fsize;
+
+	if (buffer)
+		*buffer = buf;
+	else
+		free(buf);
+
+	return 0;
+}
+
 /*
  * HID Class-Specific Requests values. See section 7.2 of the HID specifications
  */
@@ -422,8 +467,6 @@ int do_status(void)
 	return err;
 }
 
-#define V(a) (((a) >> 24) & 0xff), (((a) >> 16) & 0xff), (((a) >> 8) & 0xff), ((a) & 0xff)
-
 static int read_memory(unsigned addr, void *dest, unsigned cnt)
 {
 	static struct sdp_command read_reg_command = {
@@ -661,6 +704,45 @@ static int load_file(void *buf, unsigned len, unsigned dladdr, unsigned char typ
 	return transfer_size;
 }
 
+static int sdp_jump_address(unsigned addr)
+{
+	unsigned char tmp[64];
+	static struct sdp_command jump_command = {
+		.cmd = SDP_JUMP_ADDRESS,
+		.addr = 0,
+		.format = 0,
+		.cnt = 0,
+		.data = 0,
+		.rsvd = 0,
+	};
+	int last_trans, err;
+	int retry = 0;
+
+	jump_command.addr = htonl(addr);
+
+	for (;;) {
+		err = transfer(1, (unsigned char *) &jump_command, 16,
+			       &last_trans);
+		if (!err)
+			break;
+
+		printf("jump_command err=%i, last_trans=%i\n", err, last_trans);
+
+		if (retry > 5)
+			return -4;
+
+		retry++;
+	}
+
+	memset(tmp, 0, sizeof(tmp));
+	err = transfer(3, tmp, sizeof(tmp), &last_trans);
+
+	if (err)
+		printf("j3 in err=%i, last_trans=%i  %02x %02x %02x %02x\n",
+			err, last_trans, tmp[0], tmp[1], tmp[2], tmp[3]);
+	return 0;
+}
+
 static int write_dcd_table_ivt(const struct imx_flash_header_v2 *hdr,
 			       const unsigned char *file_start, unsigned cnt)
 {
@@ -1014,57 +1096,27 @@ static int process_header(struct usb_work *curr, unsigned char *buf, int cnt,
 
 static int do_irom_download(struct usb_work *curr, int verify)
 {
-	static unsigned char jump_command[] = {0x0b,0x0b, V(0),  0x00, V(0x00000000), V(0), 0x00};
-
 	int ret;
-	FILE* xfile;
 	unsigned char type;
-	unsigned fsize;
+	unsigned fsize = 0;
 	unsigned header_offset;
-	int cnt;
 	unsigned file_base;
-	int last_trans, err;
 	unsigned char *buf = NULL;
 	unsigned char *image;
 	unsigned char *verify_buffer = NULL;
-	unsigned char tmp[64];
 	unsigned dladdr = 0;
 	unsigned max_length;
 	unsigned plugin = 0;
 	unsigned header_addr = 0;
-
 	unsigned skip = 0;
-	int retry = 0;
-
-	xfile = fopen(curr->filename, "rb" );
-	if (!xfile) {
-		printf("error, can not open input file: %s\n", curr->filename);
-		return -5;
-	}
-
-	fsize = get_file_size(xfile);
-	if (fsize < 0x20) {
-		printf("error, file: %s is too small\n", curr->filename);
-		ret = -2;
-		goto cleanup;
-	}
 
-	buf = malloc(fsize);
-	if (!buf) {
-		printf("error, out of memory\n");
-		ret = -2;
-		goto cleanup;
-	}
-
-	cnt = fread(buf, 1 , fsize, xfile);
-	if (cnt < fsize) {
-		printf("error, cannot read %s\n", curr->filename);
-		return -1;
-	}
+	ret = read_file(curr->filename, &buf, &fsize);
+	if (ret < 0)
+		return ret;
 
 	max_length = fsize;
 
-	ret = process_header(curr, buf, cnt,
+	ret = process_header(curr, buf, fsize,
 			&dladdr, &max_length, &plugin, &header_addr);
 	if (ret < 0)
 		goto cleanup;
@@ -1098,8 +1150,6 @@ static int do_irom_download(struct usb_work *curr, int verify)
 	skip = dladdr - file_base;
 
 	image = buf + skip;
-
-	cnt -= skip;
 	fsize -= skip;
 
 	if (fsize > max_length)
@@ -1160,38 +1210,13 @@ static int do_irom_download(struct usb_work *curr, int verify)
 	if (usb_id->mach_id->mode == MODE_HID && type == FT_APP) {
 		printf("jumping to 0x%08x\n", header_addr);
 
-		jump_command[2] = (unsigned char)(header_addr >> 24);
-		jump_command[3] = (unsigned char)(header_addr >> 16);
-		jump_command[4] = (unsigned char)(header_addr >> 8);
-		jump_command[5] = (unsigned char)(header_addr);
-
-		/* Any command will initiate jump for mx51, jump address is ignored by mx51 */
-		retry = 0;
-
-		for (;;) {
-			err = transfer(1, jump_command, 16, &last_trans);
-			if (!err)
-				break;
-
-			printf("jump_command err=%i, last_trans=%i\n", err, last_trans);
-
-			if (retry > 5)
-				return -4;
-
-			retry++;
-		}
-
-		memset(tmp, 0, sizeof(tmp));
-		err = transfer(3, tmp, sizeof(tmp), &last_trans);
-
-		if (err)
-			printf("j3 in err=%i, last_trans=%i  %02x %02x %02x %02x\n",
-					err, last_trans, tmp[0], tmp[1], tmp[2], tmp[3]);
+		ret = sdp_jump_address(header_addr);
+		if (ret < 0)
+			return ret;
 	}
 
 	ret = 0;
 cleanup:
-	fclose(xfile);
 	free(verify_buffer);
 	free(buf);
 
-- 
2.1.4


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

Re: [PATCH 5/5] scripts: imx-usb-loader: split off topic-code into functions

[Alexander Shiyan]

>Воскресенье, 17 июля 2016, 18:53 +03:00 от Alexander Kurz <akurz@blala.de>:
>
>Improve code understandability: extract the "jump application" Serial
>Download Protocol access method and file-to-buffer reader functionality
>out of do_irom_download().
>
>Signed-off-by: Alexander Kurz < akurz@blala.de >
>---
> scripts/imx/imx-usb-loader.c | 159 +++++++++++++++++++++++++------------------
> 1 file changed, 92 insertions(+), 67 deletions(-)
>
>diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
...
>+FILE *xfile;
>+unsigned fsize;
>+int cnt;
>+unsigned char *buf;
>+xfile = fopen(name, "rb");
>+if (!xfile) {
>+printf("error, can not open input file: %s\n", name);
>+return -5;
...
>+fsize = get_file_size(xfile);
>+if (fsize < 0x20) {
>+printf("error, file: %s is too small\n", name);
>+fclose(xfile);
>+return -2;
-2 ? Maybe -EINVAL or so will be more appropriate here?

---

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

Re: [PATCH 5/5] scripts: imx-usb-loader: split off topic-code into functions

[Alexander Kurz]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1281 bytes --]



On Sun, 17 Jul 2016, Alexander Shiyan wrote:

> >Воскресенье, 17 июля 2016, 18:53 +03:00 от Alexander Kurz <akurz@blala.de>:
> >
> >Improve code understandability: extract the "jump application" Serial
> >Download Protocol access method and file-to-buffer reader functionality
> >out of do_irom_download().
> >
> >Signed-off-by: Alexander Kurz < akurz@blala.de >
> >---
> > scripts/imx/imx-usb-loader.c | 159 +++++++++++++++++++++++++------------------
> > 1 file changed, 92 insertions(+), 67 deletions(-)
> >
> >diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
> ...
> >+FILE *xfile;
> >+unsigned fsize;
> >+int cnt;
> >+unsigned char *buf;
> >+xfile = fopen(name, "rb");
> >+if (!xfile) {
> >+printf("error, can not open input file: %s\n", name);
> >+return -5;
> ...
> >+fsize = get_file_size(xfile);
> >+if (fsize < 0x20) {
> >+printf("error, file: %s is too small\n", name);
> >+fclose(xfile);
> >+return -2;
> -2 ? Maybe -EINVAL or so will be more appropriate here?
> 
> ---
Yes, right,
that's just the behviour from the existing implementation (don't touch too 
many topics with one patch at one time)
There are much more magic-number returns in imx-usb-loader.c, I propose to 
do a specific cleanup on this.
Cheers, Alexander
 

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

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

Re: [PATCH 5/5] scripts: imx-usb-loader: split off topic-code into functions

[Sascha Hauer]

On Sun, Jul 17, 2016 at 10:31:47PM +0200, Alexander Kurz wrote:
> 
> 
> On Sun, 17 Jul 2016, Alexander Shiyan wrote:
> 
> > >Воскресенье, 17 июля 2016, 18:53 +03:00 от Alexander Kurz <akurz@blala.de>:
> > >
> > >Improve code understandability: extract the "jump application" Serial
> > >Download Protocol access method and file-to-buffer reader functionality
> > >out of do_irom_download().
> > >
> > >Signed-off-by: Alexander Kurz < akurz@blala.de >
> > >---
> > > scripts/imx/imx-usb-loader.c | 159 +++++++++++++++++++++++++------------------
> > > 1 file changed, 92 insertions(+), 67 deletions(-)
> > >
> > >diff --git a/scripts/imx/imx-usb-loader.c b/scripts/imx/imx-usb-loader.c
> > ...
> > >+FILE *xfile;
> > >+unsigned fsize;
> > >+int cnt;
> > >+unsigned char *buf;
> > >+xfile = fopen(name, "rb");
> > >+if (!xfile) {
> > >+printf("error, can not open input file: %s\n", name);
> > >+return -5;
> > ...
> > >+fsize = get_file_size(xfile);
> > >+if (fsize < 0x20) {
> > >+printf("error, file: %s is too small\n", name);
> > >+fclose(xfile);
> > >+return -2;
> > -2 ? Maybe -EINVAL or so will be more appropriate here?
> > 
> > ---
> Yes, right,
> that's just the behviour from the existing implementation (don't touch too 
> many topics with one patch at one time)
> There are much more magic-number returns in imx-usb-loader.c, I propose to 
> do a specific cleanup on this.

I agree with you both: Error codes should be used and also this is topic
for a separate cleanup.
What's worthwile doing is to create a scripts/lib/ directory and move
common functions like read_file there. This patch just brings the fourth
implementation of this function.
Anyway, this patch makes it easy later to replace the read_file
implementation with a common implementation later, so I applied it, also
the rest of this series.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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