mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/2] Y-Modem implementation change
@ 2012-11-01 17:37 Robert Jarzmik
  2012-11-01 17:37 ` [PATCH 1/2] commands: change Y-Modem implementation Robert Jarzmik
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Robert Jarzmik @ 2012-11-01 17:37 UTC (permalink / raw)
  To: plagnioj, antonynpavlov, s.hauer; +Cc: barebox

Hi everyone,

This patchset aims at changing the Y-Modem protocol implementation.
You have already seen the RFC version, this is the next version, which :
 - provides a much more tested version
 - is ready for review

The following comments have been taken into account :
 - Antony: testing on a serial line
 - Sascha: split between protocol and commands
 - Jean-Christophe: kermit protocol change

So before doing the real review, could I ask of you :
 - Antony: could you redo your test over a serial line by applying only
           the first patch so that you can compare loady and loady2 ?
           Don't use "loady2 -g", as Y-Modem/G protocol requires a 
           lossless line (USB), and a serial line cannot guarantee it.
 - Sascha: does the split command/protocol suit you ?
 - Jean-Christophe: I left the loadb implementation as it is. The goal
                    of the patch is to change X-Modem and Y-Modem(G)
                    implementation, not kermit. Could you test that I
                    have not created a regression of loadb ?

Cheers.

--
Robert

Robert Jarzmik (2):
  commands: change Y-Modem implementation
  commands: remove old Y-Modem implementation

 commands/Makefile   |    4 +-
 commands/loadb.c    |  102 +------
 commands/loads.c    |    1 -
 commands/loadxy.c   |  238 ++++++++++++++++
 commands/xymodem.c  |  552 ++++++++++++++++++++++++++++++++++++
 commands/xyzModem.c |  785 ---------------------------------------------------
 include/xymodem.h   |   25 ++
 include/xyzModem.h  |  109 -------
 8 files changed, 826 insertions(+), 990 deletions(-)
 create mode 100644 commands/loadxy.c
 create mode 100644 commands/xymodem.c
 delete mode 100644 commands/xyzModem.c
 create mode 100644 include/xymodem.h
 delete mode 100644 include/xyzModem.h

-- 
1.7.10


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/2] commands: change Y-Modem implementation
  2012-11-01 17:37 [PATCH 0/2] Y-Modem implementation change Robert Jarzmik
@ 2012-11-01 17:37 ` Robert Jarzmik
  2012-11-01 17:37 ` [PATCH 2/2] commands: remove old " Robert Jarzmik
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Robert Jarzmik @ 2012-11-01 17:37 UTC (permalink / raw)
  To: plagnioj, antonynpavlov, s.hauer; +Cc: barebox

The current Y-Modem implementation has some limitations:
 - Y-Modem/G protocol is not supported
 - Multiple files (aka. batch) transfers are not supported
 - Transfer speed over fast lines (USB console) is slow
 - Code is not trivial to maintain (personnal opinion)

This implementation tries to address all these points by
introducing loady2 command.

The effects are :
 - transfer speed for Y-Modem over USB jumps from 2kBytes/s
   to 180kBytes/s
 - transfer speed for Y-Modem/G jumps to 200kBytes/s
 - multiple file transfers are possible

This command was tested on a USB console and UART 9600bps
serial line :
 - NAKs (and retransmissions) were tested for faulty
   serial lines
 - multiple file transfers were tested
 - Y-Modem, Y-Modem/G and X-Modem transfers were tested

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 commands/Makefile  |    2 +-
 commands/loadxy.c  |  238 ++++++++++++++++++++++
 commands/xymodem.c |  552 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/xymodem.h  |   25 +++
 4 files changed, 816 insertions(+), 1 deletion(-)
 create mode 100644 commands/loadxy.c
 create mode 100644 commands/xymodem.c
 create mode 100644 include/xymodem.h

diff --git a/commands/Makefile b/commands/Makefile
index ff98051..342c1ee 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -3,7 +3,7 @@ obj-$(CONFIG_CMD_BOOTM)		+= bootm.o
 obj-$(CONFIG_CMD_UIMAGE)	+= uimage.o
 obj-$(CONFIG_CMD_LINUX16)	+= linux16.o
 obj-$(CONFIG_CMD_LOADB)		+= loadb.o xyzModem.o
-obj-$(CONFIG_CMD_LOADY)		+= loadb.o xyzModem.o
+obj-$(CONFIG_CMD_LOADY)		+= loadb.o xyzModem.o loadxy.o xymodem.o
 obj-$(CONFIG_CMD_LOADS)		+= loads.o
 obj-$(CONFIG_CMD_ECHO)		+= echo.o
 obj-$(CONFIG_CMD_MEMORY)	+= mem.o
diff --git a/commands/loadxy.c b/commands/loadxy.c
new file mode 100644
index 0000000..141bd7b
--- /dev/null
+++ b/commands/loadxy.c
@@ -0,0 +1,238 @@
+/**
+ * @file
+ * @brief loady and loadx  support.
+ *
+ * Provides loadx (over X-Modem) and loady(over Y-Modem) support to download
+ * images.
+ *
+ * FileName: commands/loadxy.c
+ */
+/*
+ * (C) Copyright 2012 Robert Jarzmik <robert.jarzmik@free.fr>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+/*
+ * Serial up- and download support
+ */
+#include <common.h>
+#include <command.h>
+#include <console.h>
+#include <xymodem.h>
+#include <errno.h>
+#include <getopt.h>
+#include <fcntl.h>
+#include <fs.h>
+#include <malloc.h>
+
+#define DEF_FILE	"image.bin"
+
+/**
+ * @brief returns current used console device
+ *
+ * @return console device which is registered with CONSOLE_STDIN and
+ * CONSOLE_STDOUT
+ */
+static struct console_device *get_current_console(void)
+{
+	struct console_device *cdev;
+	/*
+	 * Assumption to have BOTH CONSOLE_STDIN AND STDOUT in the
+	 * same output console
+	 */
+	for_each_console(cdev) {
+		if ((cdev->f_active & (CONSOLE_STDIN | CONSOLE_STDOUT)))
+			return cdev;
+	}
+	return NULL;
+}
+
+static int console_change_speed(struct console_device *cdev, int baudrate)
+{
+	int current_baudrate;
+
+	current_baudrate =
+		(int)simple_strtoul(dev_get_param(&cdev->class_dev,
+						  "baudrate"), NULL, 10);
+	if (baudrate && baudrate != current_baudrate) {
+		printf("## Switch baudrate from %d to %d bps and press ENTER ...\n",
+		       current_baudrate, baudrate);
+		mdelay(50);
+		cdev->setbrg(cdev, baudrate);
+		mdelay(50);
+	}
+	return current_baudrate;
+}
+
+/**
+ * @brief provide the loady(Y-Modem or Y-Modem/G) support
+ *
+ * @param argc number of arguments
+ * @param argv arguments of loady command
+ *
+ * @return success or failure
+ */
+static int do_loady(int argc, char *argv[])
+{
+	int is_ymodemg = 0, rc = 0, opt, rcode = 0;
+	int load_baudrate = 0, current_baudrate;
+	struct console_device *cdev = NULL;
+
+	while ((opt = getopt(argc, argv, "b:g")) > 0) {
+		switch (opt) {
+		case 'b':
+			load_baudrate = (int)simple_strtoul(optarg, NULL, 10);
+			break;
+		case 'g':
+			is_ymodemg = 1;
+			break;
+		default:
+			perror(argv[0]);
+			return 1;
+		}
+	}
+
+	cdev = get_current_console();
+	if (NULL == cdev) {
+		printf("%s:No console device with STDIN and STDOUT\n", argv[0]);
+		return -ENODEV;
+	}
+
+	current_baudrate = console_change_speed(cdev, load_baudrate);
+	printf("## Ready for binary (ymodem) download at %d bps...\n",
+	       load_baudrate ? load_baudrate : current_baudrate);
+
+	if (is_ymodemg)
+		rc = do_load_serial_ymodemg(cdev);
+	else
+		rc = do_load_serial_ymodem(cdev);
+
+	if (rc < 0) {
+		printf("## Binary (ymodem) download aborted (%d)\n", rc);
+		rcode = 1;
+	}
+
+	console_change_speed(cdev, current_baudrate);
+
+	return rcode;
+}
+
+/**
+ * @brief provide the loadx(X-Modem) support
+ *
+ * @param argc number of arguments
+ * @param argv arguments of loadx command
+ *
+ * @return success or failure
+ */
+static int do_loadx(int argc, char *argv[])
+{
+	ulong offset = 0;
+	int load_baudrate = 0, current_baudrate, ofd, opt, rcode = 0;
+	int open_mode = O_WRONLY;
+	char *output_file = NULL;
+	struct console_device *cdev = NULL;
+
+	while ((opt = getopt(argc, argv, "f:b:o:c")) > 0) {
+		switch (opt) {
+		case 'f':
+			output_file = optarg;
+			break;
+		case 'b':
+			load_baudrate = (int)simple_strtoul(optarg, NULL, 10);
+			break;
+		case 'o':
+			offset = (int)simple_strtoul(optarg, NULL, 10);
+			break;
+		case 'c':
+			open_mode |= O_CREAT;
+			break;
+		default:
+			perror(argv[0]);
+			return 1;
+		}
+	}
+
+	cdev = get_current_console();
+	if (NULL == cdev) {
+		printf("%s:No console device with STDIN and STDOUT\n", argv[0]);
+		return -ENODEV;
+	}
+
+	/* Load Defaults */
+	if (NULL == output_file)
+		output_file = DEF_FILE;
+
+	/* File should exist */
+	ofd = open(output_file, open_mode);
+	if (ofd < 0) {
+		perror(argv[0]);
+		return 3;
+	}
+	/* Seek to the right offset */
+	if (offset) {
+		int seek = lseek(ofd, offset, SEEK_SET);
+		if (seek != offset) {
+			close(ofd);
+			ofd = 0;
+			perror(argv[0]);
+			return 4;
+		}
+	}
+
+	current_baudrate = console_change_speed(cdev, load_baudrate);
+	printf("## Ready for binary (X-Modem) download "
+	       "to 0x%08lX offset on %s device at %d bps...\n", offset,
+	       output_file, load_baudrate);
+	rcode = do_load_serial_ymodem(cdev);
+	if (rcode < 0) {
+		printf("## Binary (kermit) download aborted (%d)\n", rcode);
+		rcode = 1;
+	}
+	console_change_speed(cdev, current_baudrate);
+
+	return rcode;
+}
+
+static const __maybe_unused char cmd_loadx_help[] =
+	"[OPTIONS]\n"
+	"  -f file   - where to download to - defaults to " DEF_FILE "\n"
+	"  -o offset - what offset to download - defaults to 0\n"
+	"  -b baud   - baudrate at which to download - defaults to "
+	"console baudrate\n"
+	"  -c        - Create file if it is not present - default disabled";
+
+#ifdef CONFIG_CMD_LOADB
+BAREBOX_CMD_START(loadx)
+	.cmd = do_loadx,
+	.usage = "Load binary file over serial line (X-Modem)",
+BAREBOX_CMD_HELP(cmd_loadx_help)
+BAREBOX_CMD_END
+#endif
+
+static const __maybe_unused char cmd_loady_help[] =
+	"[OPTIONS]\n"
+	"  -y        - use Y-Modem/G (only for lossless tty as USB)\n"
+	"  -b baud   - baudrate at which to download - defaults to "
+	"console baudrate\n";
+
+#ifdef CONFIG_CMD_LOADY
+BAREBOX_CMD_START(loady2)
+	.cmd = do_loady,
+	.usage = "Load binary file over serial line (Y-Modem or Y-Modem/G)",
+BAREBOX_CMD_HELP(cmd_loady_help)
+BAREBOX_CMD_END
+#endif
diff --git a/commands/xymodem.c b/commands/xymodem.c
new file mode 100644
index 0000000..90bbf1f
--- /dev/null
+++ b/commands/xymodem.c
@@ -0,0 +1,552 @@
+/*
+ * Handles the X-Modem, Y-Modem and Y-Modem/G protocols
+ *
+ * Copyright (C) 2008 Robert Jarzmik
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <common.h>
+#include <xfuncs.h>
+#include <errno.h>
+#include <crc.h>
+#include <clock.h>
+#include <console.h>
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <fs.h>
+#include <poller.h>
+#include <linux/byteorder/generic.h>
+
+#include <kfifo.h>
+
+#define proto_dbg(fmt, args...)
+
+/* Values magic to the protocol */
+#define SOH 0x01
+#define STX 0x02
+#define EOT 0x04
+#define ACK 0x06
+#define BSP 0x08
+#define NAK 0x15
+#define CAN 0x18
+
+#define PROTO_XMODEM	0
+#define PROTO_YMODEM	1
+#define PROTO_YMODEM_G	2
+#define MAX_PROTOS	3
+
+#define CRC_NONE	0	/* No CRC checking */
+#define CRC_ADD8	1	/* Add of all data bytes */
+#define CRC_CRC16	2	/* CCCIT CRC16 */
+#define MAX_CRCS	3
+
+#define MAX_RETRIES		10
+#define MAX_RETRIES_WITH_CRC	5
+#define TIMEOUT_READ		(1 * SECOND)
+#define MAX_CAN_BEFORE_ABORT	5
+
+enum proto_state {
+	PROTO_STATE_GET_FILENAME = 0,
+	PROTO_STATE_NEGOCIATE_CRC,
+	PROTO_STATE_RECEIVE_BODY,
+	PROTO_STATE_FINISHED_FILE,
+	PROTO_STATE_FINISHED_XFER,
+};
+
+/**
+ * struct xyz_ctxt - context of a x/y modem (g) transfer
+ *
+ * @cdev: console device to support *MODEM transfer
+ * @mode: protocol (XMODEM, YMODEM or YMODEM/G)
+ * @crc_mode: CRC_NONE, CRC_ADD8 or CRC_CRC16
+ * @state: protocol state (as in "state machine")
+ * @buf: buffer to store the last tranfered buffer chunk
+ * @filename : filename transmitted by sender (YMODEM* only)
+ * @fd : file descriptor of the current stored file
+ * @file_len: length declared by sender (YMODEM* only)
+ * @nb_received: number of data bytes received since session open
+ *               (this doesn't count resends)
+ * @total_SOH: number of SOH frames received (128 bytes chunks)
+ * @total_STX: number of STX frames received (1024 bytes chunks)
+ * @total_CAN: nubmer of CAN frames received (cancel frames)
+ */
+struct xyz_ctxt {
+	struct console_device *cdev;
+	int mode;
+	int crc_mode;
+	enum proto_state state;
+	char filename[1024];
+	int fd;
+	int file_len;
+	int nb_received;
+	int next_blk;
+	int total_SOH, total_STX, total_CAN, total_retries;
+};
+
+/**
+ * struct proto_block - one unitary block of x/y modem (g) transfer
+ *
+ * @buf: data buffer
+ * @len: length of data buffer (can only be 128 or 1024)
+ * @seq: block sequence number (as in X/Y/YG MODEM protocol)
+ */
+struct proto_block {
+	unsigned char buf[1024];
+	int len;
+	int seq;
+};
+
+/*
+ * For XMODEM/YMODEM, always try to use the CRC16 versions, called also
+ * XMODEM/CRC and YMODEM.
+ * Only fallback to additive CRC (8 bits) if sender doesn't cope with CRC16.
+ */
+static const char invite_filename_hdr[MAX_PROTOS][MAX_CRCS] = {
+	{ 0, NAK, 'C' },	/* XMODEM */
+	{ 0, NAK, 'C' },	/* YMODEM */
+	{ 0, 'G', 'G' },	/* YMODEM-G */
+};
+
+static const char invite_file_body[MAX_PROTOS][MAX_CRCS] = {
+	{ 0, NAK, 'C' },	/* XMODEM */
+	{ 0, NAK, 'C' },	/* YMODEM */
+	{ 0, 'G', 'G' },	/* YMODEM-G */
+};
+
+static const char block_ack[MAX_PROTOS][MAX_CRCS] = {
+	{ 0, ACK, ACK },	/* XMODEM */
+	{ 0, ACK, ACK },	/* YMODEM */
+	{ 0, 0, 0 },		/* YMODEM-G */
+};
+
+static const char block_nack[MAX_PROTOS][MAX_CRCS] = {
+	{ 0, NAK, NAK },	/* XMODEM */
+	{ 0, NAK, NAK },	/* YMODEM */
+	{ 0, 0, 0 },		/* YMODEM-G */
+};
+
+static int proto_gets(struct console_device *cdev, unsigned char *buf, int len,
+		      uint64_t timeout)
+{
+	int i, rc;
+	uint64_t start = get_time_ns();
+
+	for (i = 0, rc = 0; rc >= 0 && i < len; ) {
+		if (is_timeout(start, timeout)) {
+			rc = -ETIMEDOUT;
+			continue;
+		}
+		if (cdev->tstc(cdev))
+			buf[i++] = (unsigned char)(cdev->getc(cdev));
+	}
+
+	return rc < 0 ? rc : i;
+}
+
+static void proto_putc(struct console_device *cdev, unsigned char c)
+{
+	cdev->putc(cdev, c);
+}
+
+static void proto_flush(struct console_device *cdev)
+{
+	while (cdev->tstc(cdev))
+		cdev->getc(cdev);
+	mdelay(250);
+	while (cdev->tstc(cdev))
+		cdev->getc(cdev);
+}
+
+static int is_xmodem(struct xyz_ctxt *proto)
+{
+	return proto->mode == PROTO_XMODEM;
+}
+
+static void proto_block_ack(struct xyz_ctxt *proto)
+{
+	unsigned char c = block_ack[proto->mode][proto->crc_mode];
+
+	if (c)
+		proto_putc(proto->cdev, c);
+}
+
+static void proto_block_nack(struct xyz_ctxt *proto)
+{
+	unsigned char c = block_nack[proto->mode][proto->crc_mode];
+
+	if (c)
+		proto_putc(proto->cdev, c);
+	proto->total_retries++;
+}
+
+static int check_crc(unsigned char *buf, int len, int crc, int crc_mode)
+{
+	unsigned char crc8 = 0;
+	uint16_t crc16;
+	int i;
+
+	switch (crc_mode) {
+	case CRC_ADD8:
+		for (i = 0; i < len; i++)
+			crc8 += buf[i];
+		return crc8 == crc ? 0 : -EBADMSG;
+	case CRC_CRC16:
+		crc16 = cyg_crc16(buf, len);
+		proto_dbg("crc16: received = %x, calculated=%x\n", crc, crc16);
+		return crc16 == crc ? 0 : -EBADMSG;
+	case CRC_NONE:
+		return 0;
+	default:
+		return -EBADMSG;
+	}
+}
+
+static ssize_t proto_read_block(struct xyz_ctxt *proto, struct proto_block *blk,
+	uint64_t timeout)
+{
+	ssize_t rc, data_len = 0;
+	unsigned char hdr, seqs[2];
+	int crc = 0, hdr_found = 0;
+	uint64_t start = get_time_ns();
+
+	while (!hdr_found) {
+		rc = proto_gets(proto->cdev, &hdr, 1, timeout);
+		proto_dbg("read 0x%x(%c) -> %d\n", hdr, hdr, rc);
+		if (rc < 0)
+			goto out;
+		if (is_timeout(start, timeout))
+			goto timeout;
+		switch (hdr) {
+		case SOH:
+			data_len = 128;
+			hdr_found = 1;
+			proto->total_SOH++;
+			break;
+		case STX:
+			data_len = 1024;
+			hdr_found = 1;
+			proto->total_STX++;
+			break;
+		case CAN:
+			rc = -ECONNABORTED;
+			if (proto->total_CAN++ > MAX_CAN_BEFORE_ABORT)
+				goto out;
+			break;
+		case EOT:
+			rc = 0;
+			blk->len = 0;
+			goto out;
+		default:
+			break;
+		}
+	}
+
+	blk->seq = 0;
+	rc = proto_gets(proto->cdev, seqs, 2, timeout);
+	if (rc < 0)
+		goto out;
+	blk->seq = seqs[0];
+	if (255 - seqs[0] != seqs[1])
+		return -EBADMSG;
+
+	rc = proto_gets(proto->cdev, blk->buf, data_len, timeout);
+	if (rc < 0)
+		goto out;
+	blk->len = rc;
+
+	switch (proto->crc_mode) {
+	case CRC_ADD8:
+		rc = proto_gets(proto->cdev,
+				(unsigned char *)&crc, 1, timeout);
+		break;
+	case CRC_CRC16:
+		rc = proto_gets(proto->cdev,
+				(unsigned char *)&crc, 2, timeout);
+		crc = be16_to_cpu(crc);
+		break;
+	case CRC_NONE:
+		rc = 0;
+		break;
+	}
+	if (rc < 0)
+		goto out;
+
+	rc = check_crc(blk->buf, data_len, crc, proto->crc_mode);
+	if (rc < 0)
+		goto out;
+	return data_len;
+timeout:
+	return -ETIMEDOUT;
+out:
+	return rc;
+}
+
+static int check_blk_seq(struct xyz_ctxt *proto, struct proto_block *blk,
+	int read_rc)
+{
+	if (blk->seq == ((proto->next_blk - 1) % 256))
+		return -EALREADY;
+	if (blk->seq != proto->next_blk)
+		return -EILSEQ;
+	return read_rc;
+}
+
+static int parse_first_block(struct xyz_ctxt *proto, struct proto_block *blk)
+{
+	int filename_len;
+	char *str_num;
+
+	filename_len = strlen(blk->buf);
+	if (filename_len > blk->len)
+		return -EINVAL;
+	memset(proto->filename, 0, sizeof(proto->filename));
+	strncpy(proto->filename, blk->buf, filename_len);
+	str_num = blk->buf + filename_len + 1;
+	strsep(&str_num, " ");
+	proto->file_len = simple_strtoul(blk->buf + filename_len + 1, NULL, 10);
+	return 1;
+}
+
+static int proto_get_file_header(struct xyz_ctxt *proto)
+{
+	struct proto_block blk;
+	int tries, rc = 0;
+
+	memset(&blk, 0, sizeof(blk));
+	proto->state = PROTO_STATE_GET_FILENAME;
+	proto->crc_mode = CRC_CRC16;
+	for (tries = 0; tries < MAX_RETRIES; tries++) {
+		proto_putc(proto->cdev,
+			   invite_filename_hdr[proto->mode][proto->crc_mode]);
+		rc = proto_read_block(proto, &blk, 3 * SECOND);
+		proto_dbg("read block returned %d\n", rc);
+		switch (rc) {
+		case -ECONNABORTED:
+			goto fail;
+		case -ETIMEDOUT:
+			if (proto->mode != PROTO_YMODEM_G)
+				mdelay(1000);
+			break;
+		case -EBADMSG:
+			/* The NACK/C/G char will be sent by invite_file_body */
+			break;
+		case -EALREADY:
+		default:
+			proto->next_blk = 1;
+			proto_block_ack(proto);
+			proto->crc_mode = CRC_CRC16;
+			proto->state = PROTO_STATE_NEGOCIATE_CRC;
+			rc = parse_first_block(proto, &blk);
+			return rc;
+		}
+
+		if (rc < 0 && tries++ >= MAX_RETRIES_WITH_CRC)
+			proto->crc_mode = CRC_ADD8;
+	}
+	rc = -ETIMEDOUT;
+fail:
+	return rc;
+}
+
+static int proto_await_header(struct xyz_ctxt *proto)
+{
+	int rc;
+
+	rc = proto_get_file_header(proto);
+	if (rc < 0)
+		return rc;
+	proto->state = PROTO_STATE_NEGOCIATE_CRC;
+	proto_dbg("header received, filename=%s, file length=%d\n",
+	       proto->filename, proto->file_len);
+	if (proto->filename[0])
+		proto->fd = open(proto->filename, O_WRONLY | O_CREAT);
+	else
+		proto->state = PROTO_STATE_FINISHED_XFER;
+	proto->nb_received = 0;
+	return rc;
+}
+
+static void proto_finish_file(struct xyz_ctxt *proto)
+{
+	close(proto->fd);
+	proto->fd = 0;
+	proto->state = PROTO_STATE_FINISHED_FILE;
+}
+
+static struct xyz_ctxt *xymodem_open(struct console_device *cdev,
+				     int proto_mode, int xmodem_fd)
+{
+	struct xyz_ctxt *proto;
+
+	proto = xzalloc(sizeof(struct xyz_ctxt));
+	proto->mode = proto_mode;
+	proto->cdev = cdev;
+	proto->crc_mode = CRC_CRC16;
+
+	if (is_xmodem(proto)) {
+		proto->fd = xmodem_fd;
+		proto->state = PROTO_STATE_NEGOCIATE_CRC;
+	} else {
+		proto->state = PROTO_STATE_GET_FILENAME;
+	}
+	proto_flush(proto->cdev);
+	return proto;
+}
+
+static int xymodem_handle(struct xyz_ctxt *proto)
+{
+	int rc = 0, xfer_max, len = 0, again = 1, remain;
+	int crc_tries = 0, same_blk_retries = 0;
+	unsigned char invite;
+	struct proto_block blk;
+
+	while (again) {
+		switch (proto->state) {
+		case PROTO_STATE_GET_FILENAME:
+			crc_tries = 0;
+			rc = proto_await_header(proto);
+			if (rc < 0)
+				goto fail;
+			continue;
+		case PROTO_STATE_FINISHED_FILE:
+			if (is_xmodem(proto))
+				proto->state = PROTO_STATE_FINISHED_XFER;
+			else
+				proto->state = PROTO_STATE_GET_FILENAME;
+			proto_putc(proto->cdev, ACK);
+			continue;
+		case PROTO_STATE_FINISHED_XFER:
+			again = 0;
+			rc = 0;
+			goto out;
+		case PROTO_STATE_NEGOCIATE_CRC:
+			invite = invite_file_body[proto->mode][proto->crc_mode];
+			proto->next_blk = 1;
+			if (crc_tries++ > MAX_RETRIES_WITH_CRC)
+				proto->crc_mode = CRC_ADD8;
+			proto_putc(proto->cdev, invite);
+			/* Fall through */
+		case PROTO_STATE_RECEIVE_BODY:
+			rc = proto_read_block(proto, &blk, 3 * SECOND);
+			if (rc > 0) {
+				rc = check_blk_seq(proto, &blk, rc);
+				proto->state = PROTO_STATE_RECEIVE_BODY;
+			}
+			break;
+		}
+
+		if (proto->state != PROTO_STATE_RECEIVE_BODY)
+			continue;
+
+		switch (rc) {
+		case -ECONNABORTED:
+			goto fail;
+		case -ETIMEDOUT:
+			if (proto->mode == PROTO_YMODEM_G)
+				goto fail;
+			proto_flush(proto->cdev);
+			proto_block_nack(proto);
+			break;
+		case -EBADMSG:
+		case -EILSEQ:
+			if (proto->mode == PROTO_YMODEM_G)
+				goto fail;
+			proto_flush(proto->cdev);
+			proto_block_nack(proto);
+			break;
+		case -EALREADY:
+			proto_block_ack(proto);
+			break;
+		case 0:
+			proto_finish_file(proto);
+			break;
+		default:
+			remain = proto->file_len - proto->nb_received;
+			if (is_xmodem(proto))
+				xfer_max = blk.len;
+			else
+				xfer_max = min(blk.len, remain);
+			rc = write(proto->fd, blk.buf, xfer_max);
+			proto->next_blk = ((blk.seq + 1) % 256);
+			proto->nb_received += rc;
+			len += rc;
+			proto_block_ack(proto);
+			break;
+		}
+		if (rc < 0)
+			same_blk_retries++;
+		else
+			same_blk_retries = 0;
+		if (same_blk_retries > MAX_RETRIES)
+			goto fail;
+	}
+out:
+	return rc;
+fail:
+	if (proto->fd)
+		close(proto->fd);
+	return rc;
+}
+
+static void xymodem_close(struct xyz_ctxt *proto)
+{
+	printf("xyModem - %d(SOH)/%d(STX)/%d(CAN) packets,"
+	       " %d retries\n",
+	       proto->total_SOH, proto->total_STX,
+	       proto->total_CAN, proto->total_retries);
+}
+
+int do_load_serial_xmodem(struct console_device *cdev, int fd)
+{
+	struct xyz_ctxt *proto;
+	int rc;
+
+	proto_flush(cdev);
+	proto = xymodem_open(cdev, PROTO_XMODEM, fd);
+	do {
+		rc = xymodem_handle(proto);
+	} while (rc > 0);
+	xymodem_close(proto);
+	return rc < 0 ? rc : 0;
+}
+EXPORT_SYMBOL(do_load_serial_xmodem);
+
+int do_load_serial_ymodem(struct console_device *cdev)
+{
+	struct xyz_ctxt *proto;
+	int rc;
+
+	proto_flush(cdev);
+	proto = xymodem_open(cdev, PROTO_YMODEM, 0);
+	do {
+		rc = xymodem_handle(proto);
+	} while (rc > 0);
+	xymodem_close(proto);
+	return rc < 0 ? rc : 0;
+}
+EXPORT_SYMBOL(do_load_serial_ymodem);
+
+int do_load_serial_ymodemg(struct console_device *cdev)
+{
+	struct xyz_ctxt *proto;
+	int rc;
+
+	proto_flush(cdev);
+	proto = xymodem_open(cdev, PROTO_YMODEM_G, 0);
+	do {
+		rc = xymodem_handle(proto);
+	} while (rc > 0);
+	xymodem_close(proto);
+	return rc < 0 ? rc : 0;
+}
+EXPORT_SYMBOL(do_load_serial_ymodemg);
diff --git a/include/xymodem.h b/include/xymodem.h
new file mode 100644
index 0000000..917cecc
--- /dev/null
+++ b/include/xymodem.h
@@ -0,0 +1,25 @@
+/*
+ * Handles the X-Modem, Y-Modem and Y-Modem/G protocols
+ *
+ * Copyright (C) 2008 Robert Jarzmik
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _XYMODEM_
+#define _XYMODEM_
+struct xyz_ctxt;
+struct console_device;
+
+int do_load_serial_xmodem(struct console_device *cdev, int fd);
+int do_load_serial_ymodem(struct console_device *cdev);
+int do_load_serial_ymodemg(struct console_device *cdev);
+#endif
-- 
1.7.10


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/2] commands: remove old Y-Modem implementation
  2012-11-01 17:37 [PATCH 0/2] Y-Modem implementation change Robert Jarzmik
  2012-11-01 17:37 ` [PATCH 1/2] commands: change Y-Modem implementation Robert Jarzmik
@ 2012-11-01 17:37 ` Robert Jarzmik
  2012-11-01 17:50 ` [PATCH 0/2] Y-Modem implementation change Jean-Christophe PLAGNIOL-VILLARD
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Robert Jarzmik @ 2012-11-01 17:37 UTC (permalink / raw)
  To: plagnioj, antonynpavlov, s.hauer; +Cc: barebox, Robert Jarzmik

From: Robert Jarzmik <robert.jarzmik@intel.com>

As a new implementation of Y-Modem protocol is available,
switch from old implementation to the new one :
 - remove old xyzModem* files
 - remove old command loady2
 - rename command loady2 to loady

Signed-off-by: Robert Jarzmik <robert.jarzmik@intel.com>
---
 commands/Makefile   |    4 +-
 commands/loadb.c    |  102 +------
 commands/loads.c    |    1 -
 commands/loadxy.c   |    2 +-
 commands/xyzModem.c |  785 ---------------------------------------------------
 include/xyzModem.h  |  109 -------
 6 files changed, 12 insertions(+), 991 deletions(-)
 delete mode 100644 commands/xyzModem.c
 delete mode 100644 include/xyzModem.h

diff --git a/commands/Makefile b/commands/Makefile
index 342c1ee..833b41f 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -2,8 +2,8 @@ obj-$(CONFIG_STDDEV)		+= stddev.o
 obj-$(CONFIG_CMD_BOOTM)		+= bootm.o
 obj-$(CONFIG_CMD_UIMAGE)	+= uimage.o
 obj-$(CONFIG_CMD_LINUX16)	+= linux16.o
-obj-$(CONFIG_CMD_LOADB)		+= loadb.o xyzModem.o
-obj-$(CONFIG_CMD_LOADY)		+= loadb.o xyzModem.o loadxy.o xymodem.o
+obj-$(CONFIG_CMD_LOADB)		+= loadb.o
+obj-$(CONFIG_CMD_LOADY)		+= loadxy.o xymodem.o
 obj-$(CONFIG_CMD_LOADS)		+= loads.o
 obj-$(CONFIG_CMD_ECHO)		+= echo.o
 obj-$(CONFIG_CMD_MEMORY)	+= mem.o
diff --git a/commands/loadb.c b/commands/loadb.c
index 898b9e3..a2f3315 100644
--- a/commands/loadb.c
+++ b/commands/loadb.c
@@ -31,7 +31,6 @@
  */
 #include <common.h>
 #include <command.h>
-#include <xyzModem.h>
 #include <console.h>
 #include <errno.h>
 #include <environment.h>
@@ -59,8 +58,6 @@
 
 static int ofd;			/* output file descriptor */
 
-#ifdef CONFIG_CMD_LOADB
-
 /* Size of my buffer to write to o/p file */
 #define MAX_WRITE_BUFFER 4096	/* Write size to o/p file */
 static char *write_buffer;	/* buffer for finalized data to write */
@@ -593,66 +590,6 @@ err_quit:
 	return size;
 }
 
-#endif				/* CONFIG_CMD_LOADB */
-
-#ifdef CONFIG_CMD_LOADY
-/**
- * @brief getcxmodem
- *
- * @return if character avaiable, return the same, else return -1
- */
-static int getcxmodem(void)
-{
-	if (tstc())
-		return (getc());
-	return -1;
-}
-
-/**
- * @brief  LoadY over ymodem protocol
- *
- * @return download size
- */
-static ulong load_serial_ymodem(void)
-{
-	int size;
-	char buf[32];
-	int err;
-	int res, wr;
-	connection_info_t info;
-	char ymodemBuf[1024];
-	ulong addr = 0;
-
-	size = 0;
-	info.mode = xyzModem_ymodem;
-	res = xyzModem_stream_open(&info, &err);
-	if (!res) {
-		while ((res = xyzModem_stream_read(ymodemBuf, 1024, &err)) >
-				0) {
-			size += res;
-			addr += res;
-			wr = write(ofd, ymodemBuf, res);
-			if (res != wr) {
-				perror("ymodem");
-				break;
-			}
-
-		}
-	} else {
-		printf("%s\n", xyzModem_error(err));
-	}
-
-	xyzModem_stream_close(&err);
-	xyzModem_stream_terminate(false, &getcxmodem);
-
-	printf("## Total Size      = 0x%08x = %d Bytes\n", size, size);
-	sprintf(buf, "%X", size);
-	setenv("filesize", buf);
-
-	return res;
-}
-#endif
-
 /**
  * @brief returns current used console device
  *
@@ -754,27 +691,16 @@ static int do_load_serial_bin(int argc, char *argv[])
 				break;
 		}
 	}
-#ifdef CONFIG_CMD_LOADY
-	if (strcmp(argv[0], "loady") == 0) {
-		printf("## Ready for binary (ymodem) download "
-		       "to 0x%08lX offset on %s device at %d bps...\n", offset,
-		       output_file, load_baudrate);
-		addr = load_serial_ymodem();
-	}
-#endif
-#ifdef CONFIG_CMD_LOADB
-	if (strcmp(argv[0], "loadb") == 0) {
-
-		printf("## Ready for binary (kermit) download "
-		       "to 0x%08lX offset on %s device at %d bps...\n", offset,
-		       output_file, load_baudrate);
-		addr = load_serial_bin();
-		if (addr == 0) {
-			printf("## Binary (kermit) download aborted\n");
-			rcode = 1;
-		}
+
+	printf("## Ready for binary (kermit) download "
+	       "to 0x%08lX offset on %s device at %d bps...\n", offset,
+	       output_file, load_baudrate);
+	addr = load_serial_bin();
+	if (addr == 0) {
+		printf("## Binary (kermit) download aborted\n");
+		rcode = 1;
 	}
-#endif
+
 	if (load_baudrate != current_baudrate) {
 		printf("## Switch baudrate to %d bps and press ESC ...\n",
 		       current_baudrate);
@@ -799,18 +725,8 @@ static const __maybe_unused char cmd_loadb_help[] =
     "  -b baud   - baudrate at which to download - defaults to "
     "console baudrate\n"
     "  -c        - Create file if it is not present - default disabled";
-#ifdef CONFIG_CMD_LOADB
 BAREBOX_CMD_START(loadb)
 	.cmd = do_load_serial_bin,
 	.usage = "Load binary file over serial line (kermit mode)",
 	BAREBOX_CMD_HELP(cmd_loadb_help)
 BAREBOX_CMD_END
-#endif
-#ifdef CONFIG_CMD_LOADY
-BAREBOX_CMD_START(loady)
-	.cmd = do_load_serial_bin,
-	.usage = "Load binary file over serial line (ymodem mode)",
-	BAREBOX_CMD_HELP(cmd_loadb_help)
-BAREBOX_CMD_END
-#endif
-
diff --git a/commands/loads.c b/commands/loads.c
index 7f4c647..bfc465b 100644
--- a/commands/loads.c
+++ b/commands/loads.c
@@ -25,7 +25,6 @@
 #include <environment.h>
 #include <s_record.h>
 #include <net.h>
-#include <xyzModem.h>
 
 static ulong load_serial(ulong offset);
 static int read_record(char *buf, ulong len);
diff --git a/commands/loadxy.c b/commands/loadxy.c
index 141bd7b..8914655 100644
--- a/commands/loadxy.c
+++ b/commands/loadxy.c
@@ -230,7 +230,7 @@ static const __maybe_unused char cmd_loady_help[] =
 	"console baudrate\n";
 
 #ifdef CONFIG_CMD_LOADY
-BAREBOX_CMD_START(loady2)
+BAREBOX_CMD_START(loady)
 	.cmd = do_loady,
 	.usage = "Load binary file over serial line (Y-Modem or Y-Modem/G)",
 BAREBOX_CMD_HELP(cmd_loady_help)
diff --git a/commands/xyzModem.c b/commands/xyzModem.c
deleted file mode 100644
index 71d7d68..0000000
--- a/commands/xyzModem.c
+++ /dev/null
@@ -1,785 +0,0 @@
-/**
- * @file
- * @brief RedBoot stream handler for xyzModem protocol
- *
- * FileName: commands/xyzModem.c
- * Originally from u-boot xyzModem.c
- */
-/*
- * 2008 - Nishanth Menon <x0nishan@ti.com>
- * Modified for sparse and checkpatch.pl compliance
- */
-/*
- *==========================================================================
- *
- *      xyzModem.c
- *
- *      RedBoot stream handler for xyzModem protocol
- *
- *==========================================================================
- *####ECOSGPLCOPYRIGHTBEGIN####
- * -------------------------------------------
- * This file is part of eCos, the Embedded Configurable Operating System.
- * Copyright (C) 1998, 1999, 2000, 2001, 2002 Red Hat, Inc.
- * Copyright (C) 2002 Gary Thomas
- *
- * eCos is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 or (at your option) any later version.
- *
- * eCos is distributed in the hope that it will be useful, but WITHOUT ANY
- * WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with eCos; if not, write to the Free Software Foundation, Inc.,
- * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
- *
- * As a special exception, if other files instantiate templates or use macros
- * or inline functions from this file, or you compile this file and link it
- * with other works to produce a work based on this file, this file does not
- * by itself cause the resulting work to be covered by the GNU General Public
- * License. However the source code for this file must still be made available
- * in accordance with section (3) of the GNU General Public License.
- *
- * This exception does not invalidate any other reasons why a work based on
- * this file might be covered by the GNU General Public License.
- *
- * Alternative licenses for eCos may be arranged by contacting Red Hat, Inc.
- * at http: *sources.redhat.com/ecos/ecos-license/
- * -------------------------------------------
- *####ECOSGPLCOPYRIGHTEND####
- *==========================================================================
- *#####DESCRIPTIONBEGIN####
- *
- * Author(s):    gthomas
- * Contributors: gthomas, tsmith, Yoshinori Sato
- * Date:         2000-07-14
- * Purpose:
- * Description:
- *
- * This code is part of RedBoot (tm).
- *
- *####DESCRIPTIONEND####
- *
- *==========================================================================
- */
-#include <common.h>
-#include <xyzModem.h>
-#include <stdarg.h>
-#include <stdio.h>
-#include <console.h>
-#include <crc.h>
-#include <linux/ctype.h>
-
-/* Assumption - run xyzModem protocol over the console port */
-
-/* Values magic to the protocol */
-#define SOH 0x01
-#define STX 0x02
-#define EOT 0x04
-#define ACK 0x06
-#define BSP 0x08
-#define NAK 0x15
-#define CAN 0x18
-#define EOF 0x1A		/* ^Z for DOS officionados */
-
-#define USE_YMODEM_LENGTH
-
-/* Data & state local to the protocol */
-static struct {
-#ifdef REDBOOT
-	hal_virtual_comm_table_t *__chan;
-#else
-	int *__chan;
-#endif
-	unsigned char pkt[1024], *bufp;
-	unsigned char blk, cblk, crc1, crc2;
-	unsigned char next_blk;	/* Expected block */
-	int len, mode, total_retries;
-	int total_SOH, total_STX, total_CAN;
-	bool crc_mode, at_eof, tx_ack;
-#ifdef USE_YMODEM_LENGTH
-	unsigned long file_length, read_length;
-#endif
-} xyz;
-
-#define xyzModem_CHAR_TIMEOUT            2000	/* 2 seconds */
-#define xyzModem_MAX_RETRIES             20
-#define xyzModem_MAX_RETRIES_WITH_CRC    10
-/* Wait for 3 CAN before quitting */
-#define xyzModem_CAN_COUNT                3
-
-#ifndef REDBOOT			/*SB */
-typedef int cyg_int32;
-static int CYGACC_COMM_IF_GETC_TIMEOUT(char chan, char *c)
-{
-#define DELAY 20
-	unsigned long counter = 0;
-	while (!tstc() && (counter < xyzModem_CHAR_TIMEOUT * 1000 / DELAY)) {
-		udelay(DELAY);
-		counter++;
-	}
-	if (tstc()) {
-		*c = getc();
-		return 1;
-	}
-	return 0;
-}
-
-static void CYGACC_COMM_IF_PUTC(char x, char y)
-{
-	console_putc(CONSOLE_STDOUT, y);
-}
-
-/* Validate a hex character */
-static inline bool _is_hex(char c)
-{
-	return (((c >= '0') && (c <= '9')) ||
-		((c >= 'A') && (c <= 'F')) || ((c >= 'a') && (c <= 'f')));
-}
-
-/* Convert a single hex nibble */
-static inline int _from_hex(char c)
-{
-	int ret = 0;
-
-	if ((c >= '0') && (c <= '9'))
-		ret = (c - '0');
-	else if ((c >= 'a') && (c <= 'f'))
-		ret = (c - 'a' + 0x0a);
-	else if ((c >= 'A') && (c <= 'F'))
-		ret = (c - 'A' + 0x0A);
-
-	return ret;
-}
-
-/* Parse (scan) a number */
-static bool parse_num(char *s, unsigned long *val, char **es, char *delim)
-{
-	bool first = true;
-	int radix = 10;
-	char c;
-	unsigned long result = 0;
-	int digit = 0;
-
-	while (*s == ' ')
-		s++;
-	while (*s) {
-		if (first && (s[0] == '0') && (tolower(s[1]) == 'x')) {
-			radix = 16;
-			s += 2;
-		}
-		first = false;
-		c = *s++;
-		if (_is_hex(c))
-			digit = _from_hex(c);
-		if (_is_hex(c) && (digit < radix)) {
-			/* Valid digit */
-#ifdef CYGPKG_HAL_MIPS
-			/* FIXME: tx49 compiler generates 0x2539018 for
-			 * MUL which isn't any good. */
-			if (16 == radix)
-				result = result << 4;
-			else
-				result = 10 * result;
-			result += digit;
-#else
-			result = (result * radix) + digit;
-#endif
-		} else {
-			if (delim != (char *)0) {
-				/* See if this character is one of the
-				 * delimiters */
-				char *dp = delim;
-				while (*dp && (c != *dp))
-					dp++;
-				if (*dp)
-					break;	/* Found a good delimiter */
-			}
-			return false;	/* Malformatted number */
-		}
-	}
-	*val = result;
-	if (es != (char **)0)
-		*es = s;
-	return true;
-}
-
-#endif
-
-#define USE_SPRINTF
-#ifdef DEBUG
-#ifndef USE_SPRINTF
-/*
- * Note: this debug setup only works if the target platform has two serial ports
- * available so that the other one (currently only port 1) can be used for debug
- * messages.
- */
-static int zm_dprintf(char *fmt, ...)
-{
-	int cur_console;
-	va_list args;
-
-	va_start(args, fmt);
-#ifdef REDBOOT
-	cur_console =
-	    CYGACC_CALL_IF_SET_CONSOLE_COMM
-	    (CYGNUM_CALL_IF_SET_COMM_ID_QUERY_CURRENT);
-	CYGACC_CALL_IF_SET_CONSOLE_COMM(1);
-#endif
-	diag_vprintf(fmt, args);
-#ifdef REDBOOT
-	CYGACC_CALL_IF_SET_CONSOLE_COMM(cur_console);
-#endif
-}
-
-static void zm_flush(void)
-{
-}
-
-#else
-/*
- * Note: this debug setup works by storing the strings in a fixed buffer
- */
-#define FINAL
-#ifdef FINAL
-static char *zm_out = (char *)0x00380000;
-static char *zm_out_start = (char *)0x00380000;
-#else
-static char zm_buf[8192];
-static char *zm_out = zm_buf;
-static char *zm_out_start = zm_buf;
-
-#endif
-static int zm_dprintf(char *fmt, ...)
-{
-	int len;
-	va_list args;
-
-	va_start(args, fmt);
-	len = diag_vsprintf(zm_out, fmt, args);
-	zm_out += len;
-	return len;
-}
-
-static void zm_flush(void)
-{
-#ifdef REDBOOT
-	char *p = zm_out_start;
-	while (*p)
-		mon_write_char(*p++);
-#endif
-	zm_out = zm_out_start;
-}
-#endif
-
-static void zm_dump_buf(void *buf, int len)
-{
-#ifdef REDBOOT
-	diag_vdump_buf_with_offset(zm_dprintf, buf, len, 0);
-#else
-
-#endif
-}
-
-static unsigned char zm_buf[2048];
-static unsigned char *zm_bp;
-
-static void zm_new(void)
-{
-	zm_bp = zm_buf;
-}
-
-static void zm_save(unsigned char c)
-{
-	*zm_bp++ = c;
-}
-
-static void zm_dump(int line)
-{
-	zm_dprintf("Packet at line: %d\n", line);
-	zm_dump_buf(zm_buf, zm_bp - zm_buf);
-}
-
-#define ZM_DEBUG(x) x
-#else
-#define ZM_DEBUG(x)
-#endif
-
-/* Wait for the line to go idle */
-static void xyzModem_flush(void)
-{
-	int res;
-	char c;
-	while (true) {
-		res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, &c);
-		if (!res)
-			return;
-	}
-}
-
-static int xyzModem_get_hdr(void)
-{
-	char c;
-	int res;
-	bool hdr_found = false;
-	int i, can_total, hdr_chars;
-	unsigned short cksum;
-
-	ZM_DEBUG(zm_new());
-	/* Find the start of a header */
-	can_total = 0;
-	hdr_chars = 0;
-
-	if (xyz.tx_ack) {
-		CYGACC_COMM_IF_PUTC(*xyz.__chan, ACK);
-		xyz.tx_ack = false;
-	}
-	while (!hdr_found) {
-		res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, &c);
-		ZM_DEBUG(zm_save(c));
-		if (res) {
-			hdr_chars++;
-			switch (c) {
-			case SOH:
-				xyz.total_SOH++;
-			case STX:
-				if (c == STX)
-					xyz.total_STX++;
-				hdr_found = true;
-				break;
-			case CAN:
-				xyz.total_CAN++;
-				ZM_DEBUG(zm_dump(__LINE__));
-				if (++can_total == xyzModem_CAN_COUNT) {
-					return xyzModem_cancel;
-				} else {
-					/* Wait for multiple CAN to avoid
-					 * early quits */
-					break;
-				}
-			case EOT:
-				/* EOT only supported if no noise */
-				if (hdr_chars == 1) {
-					CYGACC_COMM_IF_PUTC(*xyz.__chan, ACK);
-					ZM_DEBUG(zm_dprintf
-						 ("ACK on EOT #%d\n",
-						  __LINE__));
-					ZM_DEBUG(zm_dump(__LINE__));
-					return xyzModem_eof;
-				}
-			default:
-				/* Ignore, waiting for start of header */
-				;
-			}
-		} else {
-			/* Data stream timed out */
-			xyzModem_flush();	/* Toss any current input */
-			ZM_DEBUG(zm_dump(__LINE__));
-			CYGACC_CALL_IF_DELAY_US((cyg_int32) 250000);
-			return xyzModem_timeout;
-		}
-	}
-
-	/* Header found, now read the data */
-	res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, (char *)&xyz.blk);
-	ZM_DEBUG(zm_save(xyz.blk));
-	if (!res) {
-		ZM_DEBUG(zm_dump(__LINE__));
-		return xyzModem_timeout;
-	}
-	res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, (char *)&xyz.cblk);
-	ZM_DEBUG(zm_save(xyz.cblk));
-	if (!res) {
-		ZM_DEBUG(zm_dump(__LINE__));
-		return xyzModem_timeout;
-	}
-	xyz.len = (c == SOH) ? 128 : 1024;
-	xyz.bufp = xyz.pkt;
-	for (i = 0; i < xyz.len; i++) {
-		res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, &c);
-		ZM_DEBUG(zm_save(c));
-		if (res) {
-			xyz.pkt[i] = c;
-		} else {
-			ZM_DEBUG(zm_dump(__LINE__));
-			return xyzModem_timeout;
-		}
-	}
-	res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, (char *)&xyz.crc1);
-	ZM_DEBUG(zm_save(xyz.crc1));
-	if (!res) {
-		ZM_DEBUG(zm_dump(__LINE__));
-		return xyzModem_timeout;
-	}
-	if (xyz.crc_mode) {
-		res =
-		    CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, (char *)&xyz.crc2);
-		ZM_DEBUG(zm_save(xyz.crc2));
-		if (!res) {
-			ZM_DEBUG(zm_dump(__LINE__));
-			return xyzModem_timeout;
-		}
-	}
-	ZM_DEBUG(zm_dump(__LINE__));
-	/* Validate the message */
-	if ((xyz.blk ^ xyz.cblk) != (unsigned char)0xFF) {
-		ZM_DEBUG(zm_dprintf
-			 ("Framing error - blk: %x/%x/%x\n", xyz.blk, xyz.cblk,
-			  (xyz.blk ^ xyz.cblk)));
-		ZM_DEBUG(zm_dump_buf(xyz.pkt, xyz.len));
-		xyzModem_flush();
-		return xyzModem_frame;
-	}
-	/* Verify checksum/CRC */
-	if (xyz.crc_mode) {
-		cksum = cyg_crc16(xyz.pkt, xyz.len);
-		if (cksum != ((xyz.crc1 << 8) | xyz.crc2)) {
-			ZM_DEBUG(zm_dprintf
-				 ("CRC error - recvd: %02x%02x, computed: %x\n",
-				  xyz.crc1, xyz.crc2, cksum & 0xFFFF));
-			return xyzModem_cksum;
-		}
-	} else {
-		cksum = 0;
-		for (i = 0; i < xyz.len; i++)
-			cksum += xyz.pkt[i];
-		if (xyz.crc1 != (cksum & 0xFF)) {
-			ZM_DEBUG(zm_dprintf
-				 ("Checksum error - recvd: %x, computed: %x\n",
-				  xyz.crc1, cksum & 0xFF));
-			return xyzModem_cksum;
-		}
-	}
-	/* If we get here, the message passes [structural] muster */
-	return 0;
-}
-
-int xyzModem_stream_open(connection_info_t *info, int *err)
-{
-#ifdef REDBOOT
-	int console_chan;
-#endif
-	int stat = 0;
-	int retries = xyzModem_MAX_RETRIES;
-	int crc_retries = xyzModem_MAX_RETRIES_WITH_CRC;
-
-/*    ZM_DEBUG(zm_out = zm_out_start); */
-#ifdef xyzModem_zmodem
-	if (info->mode == xyzModem_zmodem) {
-		*err = xyzModem_noZmodem;
-		return -1;
-	}
-#endif
-
-#ifdef REDBOOT
-	/* Set up the I/O channel.  Note: this allows for using a different
-	 * port in the future */
-	console_chan =
-	    CYGACC_CALL_IF_SET_CONSOLE_COMM
-	    (CYGNUM_CALL_IF_SET_COMM_ID_QUERY_CURRENT);
-	if (info->chan >= 0)
-		CYGACC_CALL_IF_SET_CONSOLE_COMM(info->chan);
-	else
-		CYGACC_CALL_IF_SET_CONSOLE_COMM(console_chan);
-
-	xyz.__chan = CYGACC_CALL_IF_CONSOLE_PROCS();
-
-	CYGACC_CALL_IF_SET_CONSOLE_COMM(console_chan);
-	CYGACC_COMM_IF_CONTROL(*xyz.__chan, __COMMCTL_SET_TIMEOUT,
-			       xyzModem_CHAR_TIMEOUT);
-#else
-/* TODO: CHECK ! */
-	int dummy = 0;
-	xyz.__chan = &dummy;
-#endif
-	xyz.len = 0;
-	xyz.crc_mode = true;
-	xyz.at_eof = false;
-	xyz.tx_ack = false;
-	xyz.mode = info->mode;
-	xyz.total_retries = 0;
-	xyz.total_SOH = 0;
-	xyz.total_STX = 0;
-	xyz.total_CAN = 0;
-#ifdef USE_YMODEM_LENGTH
-	xyz.read_length = 0;
-	xyz.file_length = 0;
-#endif
-
-	CYGACC_COMM_IF_PUTC(*xyz.__chan, (xyz.crc_mode ? 'C' : NAK));
-
-	if (xyz.mode == xyzModem_xmodem) {
-		/* X-modem doesn't have an information header - exit here */
-		xyz.next_blk = 1;
-		return 0;
-	}
-
-	while (retries-- > 0) {
-		stat = xyzModem_get_hdr();
-		if (stat == 0) {
-			/* Y-modem file information header */
-			if (xyz.blk == 0) {
-#ifdef USE_YMODEM_LENGTH
-				/* skip filename */
-				while (*xyz.bufp++) ;
-				/* get the length */
-				parse_num((char *)xyz.bufp, &xyz.file_length,
-					  NULL, " ");
-#endif
-				/* The rest of the file name data block
-				 * quietly discarded
-				 */
-				xyz.tx_ack = true;
-			}
-			xyz.next_blk = 1;
-			xyz.len = 0;
-			return 0;
-		} else if (stat == xyzModem_timeout) {
-			if (--crc_retries <= 0)
-				xyz.crc_mode = false;
-			/* Extra delay for startup */
-			CYGACC_CALL_IF_DELAY_US(5 * 100000);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan,
-					    (xyz.crc_mode ? 'C' : NAK));
-			xyz.total_retries++;
-			ZM_DEBUG(zm_dprintf("NAK (%d)\n", __LINE__));
-		}
-		if (stat == xyzModem_cancel)
-			break;
-	}
-	*err = stat;
-	ZM_DEBUG(zm_flush());
-	return -1;
-}
-
-int xyzModem_stream_read(char *buf, int size, int *err)
-{
-	int stat, total, len;
-	int retries;
-
-	total = 0;
-	stat = xyzModem_cancel;
-	/* Try and get 'size' bytes into the buffer */
-	while (!xyz.at_eof && (size > 0)) {
-		if (xyz.len == 0) {
-			retries = xyzModem_MAX_RETRIES;
-			while (retries-- > 0) {
-				stat = xyzModem_get_hdr();
-				if (stat == 0) {
-					if (xyz.blk == xyz.next_blk) {
-						xyz.tx_ack = true;
-						ZM_DEBUG(zm_dprintf
-							 ("ACK block %d (%d)\n",
-							  xyz.blk, __LINE__));
-						xyz.next_blk =
-						    (xyz.next_blk + 1) & 0xFF;
-
-#if defined(xyzModem_zmodem) || defined(USE_YMODEM_LENGTH)
-						if (xyz.mode == xyzModem_xmodem
-						    || xyz.file_length == 0) {
-#else
-						if (1) {
-#endif
-			/* WARNING - Leaving formatting aside for code
-			 * clarity */
-			/* Data blocks can be padded with ^Z (EOF) characters */
-			/* This code tries to detect and remove them */
-			if ((xyz.bufp[xyz.len - 1] == EOF)
-			    && (xyz.bufp[xyz.len - 2] == EOF)
-			    && (xyz.bufp[xyz.len - 3] == EOF)) {
-				while (xyz.len &&
-					(xyz.bufp[xyz.len - 1] == EOF))
-						xyz.len--;
-				}
-			}
-#ifdef USE_YMODEM_LENGTH
-			/* WARNING - Leaving formatting aside for code
-			 * clarity */
-			/*
-			 * See if accumulated length exceeds that of the file.
-			 * If so, reduce size (i.e., cut out pad bytes)
-			 * Only do this for Y-modem (and Z-modem should it ever
-			 * be supported since it can fall back to Y-modem mode).
-			 */
-			if (xyz.mode != xyzModem_xmodem
-			    && 0 != xyz.file_length) {
-				xyz.read_length += xyz.len;
-				if (xyz.read_length > xyz.file_length)
-					xyz.len -= (xyz.read_length -
-					     xyz.file_length);
-			}
-#endif
-						break;
-					} else if (xyz.blk ==
-						   ((xyz.next_blk - 1) &
-						    0xFF)) {
-						/* Just re-ACK this so sender
-						 * will get on with it */
-						CYGACC_COMM_IF_PUTC(*xyz.__chan,
-								    ACK);
-						/* Need new header */
-						continue;
-					} else
-						stat = xyzModem_sequence;
-				}
-				if (stat == xyzModem_cancel)
-					break;
-				if (stat == xyzModem_eof) {
-					CYGACC_COMM_IF_PUTC(*xyz.__chan, ACK);
-					ZM_DEBUG(zm_dprintf
-						 ("ACK (%d)\n", __LINE__));
-					if (xyz.mode == xyzModem_ymodem) {
-						CYGACC_COMM_IF_PUTC(*xyz.__chan,
-								    (xyz.
-								     crc_mode ?
-								     'C' :
-								     NAK));
-						xyz.total_retries++;
-						ZM_DEBUG(zm_dprintf
-						 ("Reading Final Header\n"));
-						stat = xyzModem_get_hdr();
-						CYGACC_COMM_IF_PUTC(*xyz.__chan,
-								    ACK);
-						ZM_DEBUG(zm_dprintf
-							 ("FINAL ACK (%d)\n",
-							  __LINE__));
-					}
-					xyz.at_eof = true;
-					break;
-				}
-				CYGACC_COMM_IF_PUTC(*xyz.__chan,
-						    (xyz.crc_mode ? 'C' : NAK));
-				xyz.total_retries++;
-				ZM_DEBUG(zm_dprintf("NAK (%d)\n", __LINE__));
-			}
-			if (stat < 0) {
-				*err = stat;
-				xyz.len = -1;
-				return total;
-			}
-		}
-		/* Don't "read" data from the EOF protocol package */
-		if (!xyz.at_eof) {
-			len = xyz.len;
-			if (size < len)
-				len = size;
-			memcpy(buf, xyz.bufp, len);
-			size -= len;
-			buf += len;
-			total += len;
-			xyz.len -= len;
-			xyz.bufp += len;
-		}
-	}
-	return total;
-}
-
-void xyzModem_stream_close(int *err)
-{
-	diag_printf
-	    ("xyzModem - %s mode, %d(SOH)/%d(STX)/%d(CAN) packets,"
-	     " %d retries\n",
-	     xyz.crc_mode ? "CRC" : "Cksum", xyz.total_SOH, xyz.total_STX,
-	     xyz.total_CAN, xyz.total_retries);
-	ZM_DEBUG(zm_flush());
-}
-
-/* Need to be able to clean out the input buffer, so have to take the */
-/* getc */
-void xyzModem_stream_terminate(bool abort, int (*getc) (void))
-{
-	int c;
-
-	if (abort) {
-		ZM_DEBUG(zm_dprintf("!!!! TRANSFER ABORT !!!!\n"));
-		switch (xyz.mode) {
-		case xyzModem_xmodem:
-		case xyzModem_ymodem:
-			/* The X/YMODEM Spec seems to suggest that multiple CAN
-			 * followed by an equal number of Backspaces is a
-			 * friendly way to get the other end to abort. */
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, CAN);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, CAN);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, CAN);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, CAN);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, BSP);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, BSP);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, BSP);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, BSP);
-			/* Now consume the rest of what's waiting on the line.*/
-			ZM_DEBUG(zm_dprintf("Flushing serial line.\n"));
-			xyzModem_flush();
-			xyz.at_eof = true;
-			break;
-#ifdef xyzModem_zmodem
-		case xyzModem_zmodem:
-			/* Might support it some day I suppose. */
-#endif
-			break;
-		}
-	} else {
-		ZM_DEBUG(zm_dprintf("Engaging cleanup mode...\n"));
-		/*
-		 * Consume any trailing crap left in the inbuffer from
-		 * previous recieved blocks. Since very few files are an
-		 * exact multiple of the transfer block size, there will
-		 * almost always be some gunk here.
-		 * If we don't eat it now, RedBoot will think the user typed it.
-		 */
-		ZM_DEBUG(zm_dprintf("Trailing gunk:\n"));
-		while ((c = (*getc) ()) > -1) ;
-		ZM_DEBUG(zm_dprintf("\n"));
-		/*
-		 * Make a small delay to give terminal programs like minicom
-		 * time to get control again after their file transfer program
-		 * exits.
-		 */
-		CYGACC_CALL_IF_DELAY_US((cyg_int32) 250000);
-	}
-}
-
-char *xyzModem_error(int err)
-{
-	switch (err) {
-	case xyzModem_access:
-		return "Can't access file";
-		break;
-	case xyzModem_noZmodem:
-		return "Sorry, zModem not available yet";
-		break;
-	case xyzModem_timeout:
-		return "Timed out";
-		break;
-	case xyzModem_eof:
-		return "End of file";
-		break;
-	case xyzModem_cancel:
-		return "Cancelled";
-		break;
-	case xyzModem_frame:
-		return "Invalid framing";
-		break;
-	case xyzModem_cksum:
-		return "CRC/checksum error";
-		break;
-	case xyzModem_sequence:
-		return "Block sequence error";
-		break;
-	default:
-		return "Unknown error";
-		break;
-	}
-}
-
-/*
- * RedBoot interface
- */
-#if 0				/* SB */
-GETC_IO_FUNCS(xyzModem_io, xyzModem_stream_open, xyzModem_stream_close,
-	      xyzModem_stream_terminate, xyzModem_stream_read, xyzModem_error);
-RedBoot_load(xmodem, xyzModem_io, false, false, xyzModem_xmodem);
-RedBoot_load(ymodem, xyzModem_io, false, false, xyzModem_ymodem);
-#endif
diff --git a/include/xyzModem.h b/include/xyzModem.h
deleted file mode 100644
index a3ea768..0000000
--- a/include/xyzModem.h
+++ /dev/null
@@ -1,109 +0,0 @@
-/*
- *==========================================================================
- *
- *      xyzModem.h
- *
- *      RedBoot stream handler for xyzModem protocol
- *
- *==========================================================================
- *####ECOSGPLCOPYRIGHTBEGIN####
- * -------------------------------------------
- * This file is part of eCos, the Embedded Configurable Operating System.
- * Copyright (C) 1998, 1999, 2000, 2001, 2002 Red Hat, Inc.
- * Copyright (C) 2002 Gary Thomas
- *
- * eCos is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 or (at your option) any later version.
- *
- * eCos is distributed in the hope that it will be useful, but WITHOUT ANY
- * WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with eCos; if not, write to the Free Software Foundation, Inc.,
- * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
- *
- * As a special exception, if other files instantiate templates or use macros
- * or inline functions from this file, or you compile this file and link it
- * with other works to produce a work based on this file, this file does not
- * by itself cause the resulting work to be covered by the GNU General Public
- * License. However the source code for this file must still be made available
- * in accordance with section (3) of the GNU General Public License.
- *
- * This exception does not invalidate any other reasons why a work based on
- * this file might be covered by the GNU General Public License.
- *
- * Alternative licenses for eCos may be arranged by contacting Red Hat, Inc.
- * at http: *sources.redhat.com/ecos/ecos-license/
- * -------------------------------------------
- *####ECOSGPLCOPYRIGHTEND####
- *==========================================================================
- *#####DESCRIPTIONBEGIN####
- *
- * Author(s):    gthomas
- * Contributors: gthomas
- * Date:         2000-07-14
- * Purpose:
- * Description:
- *
- * This code is part of RedBoot (tm).
- *
- *####DESCRIPTIONEND####
- *
- *==========================================================================
- */
-
-#ifndef _XYZMODEM_H_
-#define _XYZMODEM_H_
-
-#define xyzModem_xmodem 1
-#define xyzModem_ymodem 2
-/* Don't define this until the protocol support is in place */
-/*#define xyzModem_zmodem 3 */
-
-#define xyzModem_access   -1
-#define xyzModem_noZmodem -2
-#define xyzModem_timeout  -3
-#define xyzModem_eof      -4
-#define xyzModem_cancel   -5
-#define xyzModem_frame    -6
-#define xyzModem_cksum    -7
-#define xyzModem_sequence -8
-
-#define xyzModem_close 1
-#define xyzModem_abort 2
-
-
-#ifdef REDBOOT
-extern getc_io_funcs_t xyzModem_io;
-#else
-#define CYGNUM_CALL_IF_SET_COMM_ID_QUERY_CURRENT
-#define CYGACC_CALL_IF_SET_CONSOLE_COMM(x)
-
-#define diag_vprintf vprintf
-#define diag_printf printf
-#define diag_vsprintf vsprintf
-
-#define CYGACC_CALL_IF_DELAY_US(x) udelay(x)
-
-typedef struct {
-    char *filename;
-    int   mode;
-    int   chan;
-#ifdef CYGPKG_REDBOOT_NETWORKING
-    struct sockaddr_in *server;
-#endif
-} connection_info_t;
-
-#endif
-
-
-int   xyzModem_stream_open(connection_info_t *info, int *err);
-void  xyzModem_stream_close(int *err);
-void  xyzModem_stream_terminate(bool method, int (*getc)(void));
-int   xyzModem_stream_read(char *buf, int size, int *err);
-char *xyzModem_error(int err);
-
-#endif /* _XYZMODEM_H_ */
-- 
1.7.10


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Y-Modem implementation change
  2012-11-01 17:37 [PATCH 0/2] Y-Modem implementation change Robert Jarzmik
  2012-11-01 17:37 ` [PATCH 1/2] commands: change Y-Modem implementation Robert Jarzmik
  2012-11-01 17:37 ` [PATCH 2/2] commands: remove old " Robert Jarzmik
@ 2012-11-01 17:50 ` Jean-Christophe PLAGNIOL-VILLARD
  2012-11-01 18:47   ` Antony Pavlov
  2012-11-01 19:19 ` Antony Pavlov
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-11-01 17:50 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: barebox

On 18:37 Thu 01 Nov     , Robert Jarzmik wrote:
> Hi everyone,
> 
> This patchset aims at changing the Y-Modem protocol implementation.
> You have already seen the RFC version, this is the next version, which :
>  - provides a much more tested version
>  - is ready for review
> 
> The following comments have been taken into account :
>  - Antony: testing on a serial line
>  - Sascha: split between protocol and commands
>  - Jean-Christophe: kermit protocol change
> 
> So before doing the real review, could I ask of you :
>  - Antony: could you redo your test over a serial line by applying only
>            the first patch so that you can compare loady and loady2 ?
>            Don't use "loady2 -g", as Y-Modem/G protocol requires a 
>            lossless line (USB), and a serial line cannot guarantee it.
>  - Sascha: does the split command/protocol suit you ?
>  - Jean-Christophe: I left the loadb implementation as it is. The goal
>                     of the patch is to change X-Modem and Y-Modem(G)
>                     implementation, not kermit. Could you test that I
>                     have not created a regression of loadb ?

I'll prefer to have a new command named xyz so we have no confusion between
loadb and the new one

Best Regards,
J.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Y-Modem implementation change
  2012-11-01 17:50 ` [PATCH 0/2] Y-Modem implementation change Jean-Christophe PLAGNIOL-VILLARD
@ 2012-11-01 18:47   ` Antony Pavlov
  0 siblings, 0 replies; 19+ messages in thread
From: Antony Pavlov @ 2012-11-01 18:47 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

On 1 November 2012 21:50, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 18:37 Thu 01 Nov     , Robert Jarzmik wrote:
>> Hi everyone,
>>
>> This patchset aims at changing the Y-Modem protocol implementation.
>> You have already seen the RFC version, this is the next version, which :
>>  - provides a much more tested version
>>  - is ready for review
>>
>> The following comments have been taken into account :
>>  - Antony: testing on a serial line
>>  - Sascha: split between protocol and commands
>>  - Jean-Christophe: kermit protocol change
>>
>> So before doing the real review, could I ask of you :
>>  - Antony: could you redo your test over a serial line by applying only
>>            the first patch so that you can compare loady and loady2 ?
>>            Don't use "loady2 -g", as Y-Modem/G protocol requires a
>>            lossless line (USB), and a serial line cannot guarantee it.
>>  - Sascha: does the split command/protocol suit you ?
>>  - Jean-Christophe: I left the loadb implementation as it is. The goal
>>                     of the patch is to change X-Modem and Y-Modem(G)
>>                     implementation, not kermit. Could you test that I
>>                     have not created a regression of loadb ?
>
> I'll prefer to have a new command named xyz so we have no confusion between
> loadb and the new one

AFAIK, the lrzsz's command for Y-Modem file receive is named 'rb'.

-- 
Best regards,
  Antony Pavlov

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Y-Modem implementation change
  2012-11-01 17:37 [PATCH 0/2] Y-Modem implementation change Robert Jarzmik
                   ` (2 preceding siblings ...)
  2012-11-01 17:50 ` [PATCH 0/2] Y-Modem implementation change Jean-Christophe PLAGNIOL-VILLARD
@ 2012-11-01 19:19 ` Antony Pavlov
  2012-11-01 19:57   ` Robert Jarzmik
  2012-11-01 19:33 ` Sascha Hauer
  2012-11-04 17:55 ` [PATCH v2 1/2] commands: change Y-Modem implementation Robert Jarzmik
  5 siblings, 1 reply; 19+ messages in thread
From: Antony Pavlov @ 2012-11-01 19:19 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: barebox

On 1 November 2012 21:37, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Hi everyone,
>
> This patchset aims at changing the Y-Modem protocol implementation.
> You have already seen the RFC version, this is the next version, which :
>  - provides a much more tested version
>  - is ready for review
>
> The following comments have been taken into account :
>  - Antony: testing on a serial line
>  - Sascha: split between protocol and commands
>  - Jean-Christophe: kermit protocol change
>
> So before doing the real review, could I ask of you :
>  - Antony: could you redo your test over a serial line by applying only
>            the first patch so that you can compare loady and loady2 ?
>            Don't use "loady2 -g", as Y-Modem/G protocol requires a
>            lossless line (USB), and a serial line cannot guarantee it.

I have redone the test.

Sorry, but the new 'loady2' does not work. I tested it over 9600 and
over 115200 serial line.

>  - Sascha: does the split command/protocol suit you ?
>  - Jean-Christophe: I left the loadb implementation as it is. The goal
>                     of the patch is to change X-Modem and Y-Modem(G)
>                     implementation, not kermit. Could you test that I
>                     have not created a regression of loadb ?
>
> Cheers.
>
> --
> Robert
>
> Robert Jarzmik (2):
>   commands: change Y-Modem implementation
>   commands: remove old Y-Modem implementation
>
>  commands/Makefile   |    4 +-
>  commands/loadb.c    |  102 +------
>  commands/loads.c    |    1 -
>  commands/loadxy.c   |  238 ++++++++++++++++
>  commands/xymodem.c  |  552 ++++++++++++++++++++++++++++++++++++
>  commands/xyzModem.c |  785 ---------------------------------------------------
>  include/xymodem.h   |   25 ++
>  include/xyzModem.h  |  109 -------
>  8 files changed, 826 insertions(+), 990 deletions(-)
>  create mode 100644 commands/loadxy.c
>  create mode 100644 commands/xymodem.c
>  delete mode 100644 commands/xyzModem.c
>  create mode 100644 include/xymodem.h
>  delete mode 100644 include/xyzModem.h
>
> --
> 1.7.10
>



-- 
Best regards,
  Antony Pavlov

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Y-Modem implementation change
  2012-11-01 17:37 [PATCH 0/2] Y-Modem implementation change Robert Jarzmik
                   ` (3 preceding siblings ...)
  2012-11-01 19:19 ` Antony Pavlov
@ 2012-11-01 19:33 ` Sascha Hauer
  2012-11-04 17:55 ` [PATCH v2 1/2] commands: change Y-Modem implementation Robert Jarzmik
  5 siblings, 0 replies; 19+ messages in thread
From: Sascha Hauer @ 2012-11-01 19:33 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: barebox

On Thu, Nov 01, 2012 at 06:37:16PM +0100, Robert Jarzmik wrote:
> Hi everyone,
> 
> This patchset aims at changing the Y-Modem protocol implementation.
> You have already seen the RFC version, this is the next version, which :
>  - provides a much more tested version
>  - is ready for review
> 
> The following comments have been taken into account :
>  - Antony: testing on a serial line
>  - Sascha: split between protocol and commands
>  - Jean-Christophe: kermit protocol change
> 
> So before doing the real review, could I ask of you :
>  - Antony: could you redo your test over a serial line by applying only
>            the first patch so that you can compare loady and loady2 ?
>            Don't use "loady2 -g", as Y-Modem/G protocol requires a 
>            lossless line (USB), and a serial line cannot guarantee it.
>  - Sascha: does the split command/protocol suit you ?

Almost, yes. The backend support should go to common/ or lib/ and get
its own (invisible) Kconfig symbol which is then selected by the command.
So:

lib/ or common/:

config XYZMODEM
	bool

command/:

config CMD_XYZMODEM
	bool "bla"
	select XYZMODEM
	help
	  ...

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Y-Modem implementation change
  2012-11-01 19:19 ` Antony Pavlov
@ 2012-11-01 19:57   ` Robert Jarzmik
  0 siblings, 0 replies; 19+ messages in thread
From: Robert Jarzmik @ 2012-11-01 19:57 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox

Antony Pavlov <antonynpavlov@gmail.com> writes:

>> So before doing the real review, could I ask of you :
>>  - Antony: could you redo your test over a serial line by applying only
>>            the first patch so that you can compare loady and loady2 ?
>>            Don't use "loady2 -g", as Y-Modem/G protocol requires a
>>            lossless line (USB), and a serial line cannot guarantee it.
>
> I have redone the test.
>
> Sorry, but the new 'loady2' does not work. I tested it over 9600 and
> over 115200 serial line.
Oh that's very disappointing.
I have tested that on a serial line, and it does work in my case ...

Could you provide me the "sb" command line you use (or rather your minicom or
other tool) uses please ?

And by private mail, I'd like the output of :
while true; do strace -T -tt -f -xx -s 4096 -o /tmp/forRobert.txt -p $(pidof sb); done

Cheers.

-- 
Robert

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/2] commands: change Y-Modem implementation
  2012-11-01 17:37 [PATCH 0/2] Y-Modem implementation change Robert Jarzmik
                   ` (4 preceding siblings ...)
  2012-11-01 19:33 ` Sascha Hauer
@ 2012-11-04 17:55 ` Robert Jarzmik
  2012-11-04 17:55   ` [PATCH v2 2/2] commands: remove old " Robert Jarzmik
  2012-11-04 18:36   ` [PATCH v2 1/2] commands: change " Jean-Christophe PLAGNIOL-VILLARD
  5 siblings, 2 replies; 19+ messages in thread
From: Robert Jarzmik @ 2012-11-04 17:55 UTC (permalink / raw)
  To: plagnioj, antonynpavlov, s.hauer; +Cc: barebox

The current Y-Modem implementation has some limitations:
 - Y-Modem/G protocol is not supported
 - Multiple files (aka. batch) transfers are not supported
 - Transfer speed over fast lines (USB console) is slow
 - Code is not trivial to maintain (personnal opinion)

This implementation tries to address all these points by
introducing loady2 command.

The effects are :
 - transfer speed for Y-Modem over USB jumps from 2kBytes/s
   to 180kBytes/s
 - transfer speed for Y-Modem/G jumps to 200kBytes/s
 - multiple file transfers are possible

This command was tested on a USB console and UART 9600bps
serial line :
 - NAKs (and retransmissions) were tested for faulty
   serial lines
 - multiple file transfers were tested
 - Y-Modem, Y-Modem/G and X-Modem transfers were tested

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

---
Since V1:
 - add input fifo for small fifo hardwares
Add a FIFO so that each getc will empty the hardware
FIFO. This is very similar to the generic console code,
except that the getc won't block each time for 100us,
enabling faster lines (USB) to benefit their full speed.
 - fix CRC calculation for big endian architectures
   Thanks a lot Antony for the many patches testing !
 - added some documentation
 - amended the split as Sascha recommended
---
 commands/Kconfig  |    1 +
 commands/Makefile |    2 +-
 commands/loadxy.c |  238 +++++++++++++++++++++
 include/xymodem.h |   25 +++
 lib/Kconfig       |    3 +
 lib/Makefile      |    1 +
 lib/xymodem.c     |  591 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 860 insertions(+), 1 deletion(-)
 create mode 100644 commands/loadxy.c
 create mode 100644 include/xymodem.h
 create mode 100644 lib/xymodem.c

diff --git a/commands/Kconfig b/commands/Kconfig
index a52a01a..a7e9974 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -261,6 +261,7 @@ config CMD_LOADB
 
 config CMD_LOADY
 	select CRC16
+	select XYMODEM
 	tristate
 	prompt "loady"
 
diff --git a/commands/Makefile b/commands/Makefile
index ff98051..44ad904 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -3,7 +3,7 @@ obj-$(CONFIG_CMD_BOOTM)		+= bootm.o
 obj-$(CONFIG_CMD_UIMAGE)	+= uimage.o
 obj-$(CONFIG_CMD_LINUX16)	+= linux16.o
 obj-$(CONFIG_CMD_LOADB)		+= loadb.o xyzModem.o
-obj-$(CONFIG_CMD_LOADY)		+= loadb.o xyzModem.o
+obj-$(CONFIG_CMD_LOADY)		+= loadb.o xyzModem.o loadxy.o
 obj-$(CONFIG_CMD_LOADS)		+= loads.o
 obj-$(CONFIG_CMD_ECHO)		+= echo.o
 obj-$(CONFIG_CMD_MEMORY)	+= mem.o
diff --git a/commands/loadxy.c b/commands/loadxy.c
new file mode 100644
index 0000000..141bd7b
--- /dev/null
+++ b/commands/loadxy.c
@@ -0,0 +1,238 @@
+/**
+ * @file
+ * @brief loady and loadx  support.
+ *
+ * Provides loadx (over X-Modem) and loady(over Y-Modem) support to download
+ * images.
+ *
+ * FileName: commands/loadxy.c
+ */
+/*
+ * (C) Copyright 2012 Robert Jarzmik <robert.jarzmik@free.fr>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+/*
+ * Serial up- and download support
+ */
+#include <common.h>
+#include <command.h>
+#include <console.h>
+#include <xymodem.h>
+#include <errno.h>
+#include <getopt.h>
+#include <fcntl.h>
+#include <fs.h>
+#include <malloc.h>
+
+#define DEF_FILE	"image.bin"
+
+/**
+ * @brief returns current used console device
+ *
+ * @return console device which is registered with CONSOLE_STDIN and
+ * CONSOLE_STDOUT
+ */
+static struct console_device *get_current_console(void)
+{
+	struct console_device *cdev;
+	/*
+	 * Assumption to have BOTH CONSOLE_STDIN AND STDOUT in the
+	 * same output console
+	 */
+	for_each_console(cdev) {
+		if ((cdev->f_active & (CONSOLE_STDIN | CONSOLE_STDOUT)))
+			return cdev;
+	}
+	return NULL;
+}
+
+static int console_change_speed(struct console_device *cdev, int baudrate)
+{
+	int current_baudrate;
+
+	current_baudrate =
+		(int)simple_strtoul(dev_get_param(&cdev->class_dev,
+						  "baudrate"), NULL, 10);
+	if (baudrate && baudrate != current_baudrate) {
+		printf("## Switch baudrate from %d to %d bps and press ENTER ...\n",
+		       current_baudrate, baudrate);
+		mdelay(50);
+		cdev->setbrg(cdev, baudrate);
+		mdelay(50);
+	}
+	return current_baudrate;
+}
+
+/**
+ * @brief provide the loady(Y-Modem or Y-Modem/G) support
+ *
+ * @param argc number of arguments
+ * @param argv arguments of loady command
+ *
+ * @return success or failure
+ */
+static int do_loady(int argc, char *argv[])
+{
+	int is_ymodemg = 0, rc = 0, opt, rcode = 0;
+	int load_baudrate = 0, current_baudrate;
+	struct console_device *cdev = NULL;
+
+	while ((opt = getopt(argc, argv, "b:g")) > 0) {
+		switch (opt) {
+		case 'b':
+			load_baudrate = (int)simple_strtoul(optarg, NULL, 10);
+			break;
+		case 'g':
+			is_ymodemg = 1;
+			break;
+		default:
+			perror(argv[0]);
+			return 1;
+		}
+	}
+
+	cdev = get_current_console();
+	if (NULL == cdev) {
+		printf("%s:No console device with STDIN and STDOUT\n", argv[0]);
+		return -ENODEV;
+	}
+
+	current_baudrate = console_change_speed(cdev, load_baudrate);
+	printf("## Ready for binary (ymodem) download at %d bps...\n",
+	       load_baudrate ? load_baudrate : current_baudrate);
+
+	if (is_ymodemg)
+		rc = do_load_serial_ymodemg(cdev);
+	else
+		rc = do_load_serial_ymodem(cdev);
+
+	if (rc < 0) {
+		printf("## Binary (ymodem) download aborted (%d)\n", rc);
+		rcode = 1;
+	}
+
+	console_change_speed(cdev, current_baudrate);
+
+	return rcode;
+}
+
+/**
+ * @brief provide the loadx(X-Modem) support
+ *
+ * @param argc number of arguments
+ * @param argv arguments of loadx command
+ *
+ * @return success or failure
+ */
+static int do_loadx(int argc, char *argv[])
+{
+	ulong offset = 0;
+	int load_baudrate = 0, current_baudrate, ofd, opt, rcode = 0;
+	int open_mode = O_WRONLY;
+	char *output_file = NULL;
+	struct console_device *cdev = NULL;
+
+	while ((opt = getopt(argc, argv, "f:b:o:c")) > 0) {
+		switch (opt) {
+		case 'f':
+			output_file = optarg;
+			break;
+		case 'b':
+			load_baudrate = (int)simple_strtoul(optarg, NULL, 10);
+			break;
+		case 'o':
+			offset = (int)simple_strtoul(optarg, NULL, 10);
+			break;
+		case 'c':
+			open_mode |= O_CREAT;
+			break;
+		default:
+			perror(argv[0]);
+			return 1;
+		}
+	}
+
+	cdev = get_current_console();
+	if (NULL == cdev) {
+		printf("%s:No console device with STDIN and STDOUT\n", argv[0]);
+		return -ENODEV;
+	}
+
+	/* Load Defaults */
+	if (NULL == output_file)
+		output_file = DEF_FILE;
+
+	/* File should exist */
+	ofd = open(output_file, open_mode);
+	if (ofd < 0) {
+		perror(argv[0]);
+		return 3;
+	}
+	/* Seek to the right offset */
+	if (offset) {
+		int seek = lseek(ofd, offset, SEEK_SET);
+		if (seek != offset) {
+			close(ofd);
+			ofd = 0;
+			perror(argv[0]);
+			return 4;
+		}
+	}
+
+	current_baudrate = console_change_speed(cdev, load_baudrate);
+	printf("## Ready for binary (X-Modem) download "
+	       "to 0x%08lX offset on %s device at %d bps...\n", offset,
+	       output_file, load_baudrate);
+	rcode = do_load_serial_ymodem(cdev);
+	if (rcode < 0) {
+		printf("## Binary (kermit) download aborted (%d)\n", rcode);
+		rcode = 1;
+	}
+	console_change_speed(cdev, current_baudrate);
+
+	return rcode;
+}
+
+static const __maybe_unused char cmd_loadx_help[] =
+	"[OPTIONS]\n"
+	"  -f file   - where to download to - defaults to " DEF_FILE "\n"
+	"  -o offset - what offset to download - defaults to 0\n"
+	"  -b baud   - baudrate at which to download - defaults to "
+	"console baudrate\n"
+	"  -c        - Create file if it is not present - default disabled";
+
+#ifdef CONFIG_CMD_LOADB
+BAREBOX_CMD_START(loadx)
+	.cmd = do_loadx,
+	.usage = "Load binary file over serial line (X-Modem)",
+BAREBOX_CMD_HELP(cmd_loadx_help)
+BAREBOX_CMD_END
+#endif
+
+static const __maybe_unused char cmd_loady_help[] =
+	"[OPTIONS]\n"
+	"  -y        - use Y-Modem/G (only for lossless tty as USB)\n"
+	"  -b baud   - baudrate at which to download - defaults to "
+	"console baudrate\n";
+
+#ifdef CONFIG_CMD_LOADY
+BAREBOX_CMD_START(loady2)
+	.cmd = do_loady,
+	.usage = "Load binary file over serial line (Y-Modem or Y-Modem/G)",
+BAREBOX_CMD_HELP(cmd_loady_help)
+BAREBOX_CMD_END
+#endif
diff --git a/include/xymodem.h b/include/xymodem.h
new file mode 100644
index 0000000..917cecc
--- /dev/null
+++ b/include/xymodem.h
@@ -0,0 +1,25 @@
+/*
+ * Handles the X-Modem, Y-Modem and Y-Modem/G protocols
+ *
+ * Copyright (C) 2008 Robert Jarzmik
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _XYMODEM_
+#define _XYMODEM_
+struct xyz_ctxt;
+struct console_device;
+
+int do_load_serial_xmodem(struct console_device *cdev, int fd);
+int do_load_serial_ymodem(struct console_device *cdev);
+int do_load_serial_ymodemg(struct console_device *cdev);
+#endif
diff --git a/lib/Kconfig b/lib/Kconfig
index 9882d2d..13ecab0 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -38,6 +38,9 @@ config BITREV
 config QSORT
 	bool
 
+config XYMODEM
+	bool
+
 source lib/gui/Kconfig
 
 endmenu
diff --git a/lib/Makefile b/lib/Makefile
index 41e6a0f..eb0af92 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -35,3 +35,4 @@ obj-$(CONFIG_BCH)	+= bch.o
 obj-$(CONFIG_BITREV)	+= bitrev.o
 obj-$(CONFIG_QSORT)	+= qsort.o
 obj-y			+= gui/
+obj-$(CONFIG_XYMODEM)	+= xymodem.o
diff --git a/lib/xymodem.c b/lib/xymodem.c
new file mode 100644
index 0000000..1469a9a
--- /dev/null
+++ b/lib/xymodem.c
@@ -0,0 +1,591 @@
+/*
+ * Handles the X-Modem, Y-Modem and Y-Modem/G protocols
+ *
+ * Copyright (C) 2008 Robert Jarzmik
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * This file provides functions to receive X-Modem or Y-Modem(/G) protocols.
+ *
+ * References:
+ *   *-Modem: http://www.techfest.com/hardware/modem/xymodem.htm
+ *            XMODEM/YMODEM PROTOCOL REFERENCE, Chuck Forsberg
+ */
+#include <common.h>
+#include <xfuncs.h>
+#include <errno.h>
+#include <crc.h>
+#include <clock.h>
+#include <console.h>
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+#include <fs.h>
+#include <kfifo.h>
+#include <linux/byteorder/generic.h>
+
+
+#define xy_dbg(fmt, args...)
+
+/* Values magic to the protocol */
+#define SOH 0x01
+#define STX 0x02
+#define EOT 0x04
+#define ACK 0x06
+#define BSP 0x08
+#define NAK 0x15
+#define CAN 0x18
+
+#define PROTO_XMODEM	0
+#define PROTO_YMODEM	1
+#define PROTO_YMODEM_G	2
+#define MAX_PROTOS	3
+
+#define CRC_NONE	0	/* No CRC checking */
+#define CRC_ADD8	1	/* Add of all data bytes */
+#define CRC_CRC16	2	/* CCCIT CRC16 */
+#define MAX_CRCS	3
+
+#define MAX_RETRIES		10
+#define MAX_RETRIES_WITH_CRC	5
+#define TIMEOUT_READ		(1 * SECOND)
+#define MAX_CAN_BEFORE_ABORT	5
+#define INPUT_FIFO_SIZE		(4 * 1024)	/* Should always be > 1029 */
+
+enum proto_state {
+	PROTO_STATE_GET_FILENAME = 0,
+	PROTO_STATE_NEGOCIATE_CRC,
+	PROTO_STATE_RECEIVE_BODY,
+	PROTO_STATE_FINISHED_FILE,
+	PROTO_STATE_FINISHED_XFER,
+};
+
+/**
+ * struct xyz_ctxt - context of a x/y modem (g) transfer
+ *
+ * @cdev: console device to support *MODEM transfer
+ * @fifo: fifo to buffer input from serial line
+ *        This is necessary for low hardware FIFOs buffers as UARTs.
+ * @mode: protocol (XMODEM, YMODEM or YMODEM/G)
+ * @crc_mode: CRC_NONE, CRC_ADD8 or CRC_CRC16
+ * @state: protocol state (as in "state machine")
+ * @buf: buffer to store the last tranfered buffer chunk
+ * @filename : filename transmitted by sender (YMODEM* only)
+ * @fd : file descriptor of the current stored file
+ * @file_len: length declared by sender (YMODEM* only)
+ * @nb_received: number of data bytes received since session open
+ *               (this doesn't count resends)
+ * @total_SOH: number of SOH frames received (128 bytes chunks)
+ * @total_STX: number of STX frames received (1024 bytes chunks)
+ * @total_CAN: nubmer of CAN frames received (cancel frames)
+ */
+struct xyz_ctxt {
+	struct console_device *cdev;
+	struct kfifo *fifo;
+	int mode;
+	int crc_mode;
+	enum proto_state state;
+	char filename[1024];
+	int fd;
+	int file_len;
+	int nb_received;
+	int next_blk;
+	int total_SOH, total_STX, total_CAN, total_retries;
+};
+
+/**
+ * struct xy_block - one unitary block of x/y modem (g) transfer
+ *
+ * @buf: data buffer
+ * @len: length of data buffer (can only be 128 or 1024)
+ * @seq: block sequence number (as in X/Y/YG MODEM protocol)
+ */
+struct xy_block {
+	unsigned char buf[1024];
+	int len;
+	int seq;
+};
+
+/*
+ * For XMODEM/YMODEM, always try to use the CRC16 versions, called also
+ * XMODEM/CRC and YMODEM.
+ * Only fallback to additive CRC (8 bits) if sender doesn't cope with CRC16.
+ */
+static const char invite_filename_hdr[MAX_PROTOS][MAX_CRCS] = {
+	{ 0, NAK, 'C' },	/* XMODEM */
+	{ 0, NAK, 'C' },	/* YMODEM */
+	{ 0, 'G', 'G' },	/* YMODEM-G */
+};
+
+static const char invite_file_body[MAX_PROTOS][MAX_CRCS] = {
+	{ 0, NAK, 'C' },	/* XMODEM */
+	{ 0, NAK, 'C' },	/* YMODEM */
+	{ 0, 'G', 'G' },	/* YMODEM-G */
+};
+
+static const char block_ack[MAX_PROTOS][MAX_CRCS] = {
+	{ 0, ACK, ACK },	/* XMODEM */
+	{ 0, ACK, ACK },	/* YMODEM */
+	{ 0, 0, 0 },		/* YMODEM-G */
+};
+
+static const char block_nack[MAX_PROTOS][MAX_CRCS] = {
+	{ 0, NAK, NAK },	/* XMODEM */
+	{ 0, NAK, NAK },	/* YMODEM */
+	{ 0, 0, 0 },		/* YMODEM-G */
+};
+
+static int input_fifo_fill(struct console_device *cdev, struct kfifo *fifo)
+{
+	while (cdev->tstc(cdev) && kfifo_len(fifo) < INPUT_FIFO_SIZE)
+		kfifo_putc(fifo, (unsigned char)(cdev->getc(cdev)));
+	return kfifo_len(fifo);
+}
+
+/*
+ * This function is optimized to :
+ * - maximize throughput (ie. read as much as is available in lower layer fifo)
+ * - minimize latencies (no delay or wait timeout if data available)
+ * - have a timeout
+ * This is why standard getc() is not used, and input_fifo_fill() exists.
+ */
+static int xy_gets(struct console_device *cdev, struct kfifo *fifo,
+		      unsigned char *buf, int len, uint64_t timeout)
+{
+	int i, rc;
+	uint64_t start = get_time_ns();
+
+	for (i = 0, rc = 0; rc >= 0 && i < len; ) {
+		if (is_timeout(start, timeout)) {
+			rc = -ETIMEDOUT;
+			continue;
+		}
+		if (input_fifo_fill(cdev, fifo))
+			kfifo_getc(fifo, &buf[i++]);
+	}
+
+	return rc < 0 ? rc : i;
+}
+
+static void xy_putc(struct console_device *cdev, unsigned char c)
+{
+	cdev->putc(cdev, c);
+}
+
+static void xy_flush(struct console_device *cdev, struct kfifo *fifo)
+{
+	while (cdev->tstc(cdev))
+		cdev->getc(cdev);
+	mdelay(250);
+	while (cdev->tstc(cdev))
+		cdev->getc(cdev);
+	kfifo_reset(fifo);
+}
+
+static int is_xmodem(struct xyz_ctxt *proto)
+{
+	return proto->mode == PROTO_XMODEM;
+}
+
+static void xy_block_ack(struct xyz_ctxt *proto)
+{
+	unsigned char c = block_ack[proto->mode][proto->crc_mode];
+
+	if (c)
+		xy_putc(proto->cdev, c);
+}
+
+static void xy_block_nack(struct xyz_ctxt *proto)
+{
+	unsigned char c = block_nack[proto->mode][proto->crc_mode];
+
+	if (c)
+		xy_putc(proto->cdev, c);
+	proto->total_retries++;
+}
+
+static int check_crc(unsigned char *buf, int len, int crc, int crc_mode)
+{
+	unsigned char crc8 = 0;
+	uint16_t crc16;
+	int i;
+
+	switch (crc_mode) {
+	case CRC_ADD8:
+		for (i = 0; i < len; i++)
+			crc8 += buf[i];
+		return crc8 == crc ? 0 : -EBADMSG;
+	case CRC_CRC16:
+		crc16 = cyg_crc16(buf, len);
+		xy_dbg("crc16: received = %x, calculated=%x\n", crc, crc16);
+		return crc16 == crc ? 0 : -EBADMSG;
+	case CRC_NONE:
+		return 0;
+	default:
+		return -EBADMSG;
+	}
+}
+
+/**
+ * xy_read_block - read a X-Modem or Y-Modem(G) block
+ * @proto: protocol control structure
+ * @blk: block read
+ * @timeout: maximal time to get data
+ *
+ * This is the pivotal function for block receptions. It attempts to receive one
+ * block, ie. one 128 bytes or one 1024 bytes block.  The received data can also
+ * be an end of transmission, or a cancel.
+ *
+ * Returns :
+ *  >0           : size of the received block
+ *  0            : last block, ie. end of transmission, ie. EOT
+ * -EBADMSG      : malformed message (ie. sequence bi-bytes are not
+ *                 complementary), or CRC check error
+ * -EILSEQ       : block sequence number error wrt previously received block
+ * -ETIMEDOUT    : block not received before timeout passed
+ * -ECONNABORTED : transfer aborted by sender, ie. CAN
+ */
+static ssize_t xy_read_block(struct xyz_ctxt *proto, struct xy_block *blk,
+	uint64_t timeout)
+{
+	ssize_t rc, data_len = 0;
+	unsigned char hdr, seqs[2], crcs[2];
+	int crc = 0, hdr_found = 0;
+	uint64_t start = get_time_ns();
+
+	while (!hdr_found) {
+		rc = xy_gets(proto->cdev, proto->fifo, &hdr, 1, timeout);
+		xy_dbg("read 0x%x(%c) -> %d\n", hdr, hdr, rc);
+		if (rc < 0)
+			goto out;
+		if (is_timeout(start, timeout))
+			goto timeout;
+		switch (hdr) {
+		case SOH:
+			data_len = 128;
+			hdr_found = 1;
+			proto->total_SOH++;
+			break;
+		case STX:
+			data_len = 1024;
+			hdr_found = 1;
+			proto->total_STX++;
+			break;
+		case CAN:
+			rc = -ECONNABORTED;
+			if (proto->total_CAN++ > MAX_CAN_BEFORE_ABORT)
+				goto out;
+			break;
+		case EOT:
+			rc = 0;
+			blk->len = 0;
+			goto out;
+		default:
+			break;
+		}
+	}
+
+	blk->seq = 0;
+	rc = xy_gets(proto->cdev, proto->fifo, seqs, 2, timeout);
+	if (rc < 0)
+		goto out;
+	blk->seq = seqs[0];
+	if (255 - seqs[0] != seqs[1])
+		return -EBADMSG;
+
+	rc = xy_gets(proto->cdev, proto->fifo, blk->buf, data_len, timeout);
+	if (rc < 0)
+		goto out;
+	blk->len = rc;
+
+	switch (proto->crc_mode) {
+	case CRC_ADD8:
+		rc = xy_gets(proto->cdev, proto->fifo, crcs, 1, timeout);
+		crc = crcs[0];
+		break;
+	case CRC_CRC16:
+		rc = xy_gets(proto->cdev, proto->fifo, crcs, 2, timeout);
+		crc = be16_to_cpu(*(uint16_t *)crcs);
+		break;
+	case CRC_NONE:
+		rc = 0;
+		break;
+	}
+	if (rc < 0)
+		goto out;
+
+	rc = check_crc(blk->buf, data_len, crc, proto->crc_mode);
+	if (rc < 0)
+		goto out;
+	return data_len;
+timeout:
+	return -ETIMEDOUT;
+out:
+	return rc;
+}
+
+static int check_blk_seq(struct xyz_ctxt *proto, struct xy_block *blk,
+	int read_rc)
+{
+	if (blk->seq == ((proto->next_blk - 1) % 256))
+		return -EALREADY;
+	if (blk->seq != proto->next_blk)
+		return -EILSEQ;
+	return read_rc;
+}
+
+static int parse_first_block(struct xyz_ctxt *proto, struct xy_block *blk)
+{
+	int filename_len;
+	char *str_num;
+
+	filename_len = strlen(blk->buf);
+	if (filename_len > blk->len)
+		return -EINVAL;
+	memset(proto->filename, 0, sizeof(proto->filename));
+	strncpy(proto->filename, blk->buf, filename_len);
+	str_num = blk->buf + filename_len + 1;
+	strsep(&str_num, " ");
+	proto->file_len = simple_strtoul(blk->buf + filename_len + 1, NULL, 10);
+	return 1;
+}
+
+static int xy_get_file_header(struct xyz_ctxt *proto)
+{
+	struct xy_block blk;
+	int tries, rc = 0;
+
+	memset(&blk, 0, sizeof(blk));
+	proto->state = PROTO_STATE_GET_FILENAME;
+	proto->crc_mode = CRC_CRC16;
+	for (tries = 0; tries < MAX_RETRIES; tries++) {
+		xy_putc(proto->cdev,
+			   invite_filename_hdr[proto->mode][proto->crc_mode]);
+		rc = xy_read_block(proto, &blk, 3 * SECOND);
+		xy_dbg("read block returned %d\n", rc);
+		switch (rc) {
+		case -ECONNABORTED:
+			goto fail;
+		case -ETIMEDOUT:
+		case -EBADMSG:
+			if (proto->mode != PROTO_YMODEM_G)
+				xy_flush(proto->cdev, proto->fifo);
+			break;
+		case -EALREADY:
+		default:
+			proto->next_blk = 1;
+			xy_block_ack(proto);
+			proto->state = PROTO_STATE_NEGOCIATE_CRC;
+			rc = parse_first_block(proto, &blk);
+			return rc;
+		}
+
+		if (rc < 0 && tries++ >= MAX_RETRIES_WITH_CRC)
+			proto->crc_mode = CRC_ADD8;
+	}
+	rc = -ETIMEDOUT;
+fail:
+	proto->total_retries += tries;
+	return rc;
+}
+
+static int xy_await_header(struct xyz_ctxt *proto)
+{
+	int rc;
+
+	rc = xy_get_file_header(proto);
+	if (rc < 0)
+		return rc;
+	proto->state = PROTO_STATE_NEGOCIATE_CRC;
+	xy_dbg("header received, filename=%s, file length=%d\n",
+	       proto->filename, proto->file_len);
+	if (proto->filename[0])
+		proto->fd = open(proto->filename, O_WRONLY | O_CREAT);
+	else
+		proto->state = PROTO_STATE_FINISHED_XFER;
+	proto->nb_received = 0;
+	return rc;
+}
+
+static void xy_finish_file(struct xyz_ctxt *proto)
+{
+	close(proto->fd);
+	proto->fd = 0;
+	proto->state = PROTO_STATE_FINISHED_FILE;
+}
+
+static struct xyz_ctxt *xymodem_open(struct console_device *cdev,
+				     int proto_mode, int xmodem_fd)
+{
+	struct xyz_ctxt *proto;
+
+	proto = xzalloc(sizeof(struct xyz_ctxt));
+	proto->fifo = kfifo_alloc(INPUT_FIFO_SIZE);
+	proto->mode = proto_mode;
+	proto->cdev = cdev;
+	proto->crc_mode = CRC_CRC16;
+
+	if (is_xmodem(proto)) {
+		proto->fd = xmodem_fd;
+		proto->state = PROTO_STATE_NEGOCIATE_CRC;
+	} else {
+		proto->state = PROTO_STATE_GET_FILENAME;
+	}
+	xy_flush(proto->cdev, proto->fifo);
+	return proto;
+}
+
+static int xymodem_handle(struct xyz_ctxt *proto)
+{
+	int rc = 0, xfer_max, len = 0, again = 1, remain;
+	int crc_tries = 0, same_blk_retries = 0;
+	unsigned char invite;
+	struct xy_block blk;
+
+	while (again) {
+		switch (proto->state) {
+		case PROTO_STATE_GET_FILENAME:
+			crc_tries = 0;
+			rc = xy_await_header(proto);
+			if (rc < 0)
+				goto fail;
+			continue;
+		case PROTO_STATE_FINISHED_FILE:
+			if (is_xmodem(proto))
+				proto->state = PROTO_STATE_FINISHED_XFER;
+			else
+				proto->state = PROTO_STATE_GET_FILENAME;
+			xy_putc(proto->cdev, ACK);
+			continue;
+		case PROTO_STATE_FINISHED_XFER:
+			again = 0;
+			rc = 0;
+			goto out;
+		case PROTO_STATE_NEGOCIATE_CRC:
+			invite = invite_file_body[proto->mode][proto->crc_mode];
+			proto->next_blk = 1;
+			if (crc_tries++ > MAX_RETRIES_WITH_CRC)
+				proto->crc_mode = CRC_ADD8;
+			xy_putc(proto->cdev, invite);
+			/* Fall through */
+		case PROTO_STATE_RECEIVE_BODY:
+			rc = xy_read_block(proto, &blk, 3 * SECOND);
+			if (rc > 0) {
+				rc = check_blk_seq(proto, &blk, rc);
+				proto->state = PROTO_STATE_RECEIVE_BODY;
+			}
+			break;
+		}
+
+		if (proto->state != PROTO_STATE_RECEIVE_BODY)
+			continue;
+
+		switch (rc) {
+		case -ECONNABORTED:
+			goto fail;
+		case -ETIMEDOUT:
+			if (proto->mode == PROTO_YMODEM_G)
+				goto fail;
+			xy_flush(proto->cdev, proto->fifo);
+			xy_block_nack(proto);
+			break;
+		case -EBADMSG:
+		case -EILSEQ:
+			if (proto->mode == PROTO_YMODEM_G)
+				goto fail;
+			xy_flush(proto->cdev, proto->fifo);
+			xy_block_nack(proto);
+			break;
+		case -EALREADY:
+			xy_block_ack(proto);
+			break;
+		case 0:
+			xy_finish_file(proto);
+			break;
+		default:
+			remain = proto->file_len - proto->nb_received;
+			if (is_xmodem(proto))
+				xfer_max = blk.len;
+			else
+				xfer_max = min(blk.len, remain);
+			rc = write(proto->fd, blk.buf, xfer_max);
+			proto->next_blk = ((blk.seq + 1) % 256);
+			proto->nb_received += rc;
+			len += rc;
+			xy_block_ack(proto);
+			break;
+		}
+		if (rc < 0)
+			same_blk_retries++;
+		else
+			same_blk_retries = 0;
+		if (same_blk_retries > MAX_RETRIES)
+			goto fail;
+	}
+out:
+	return rc;
+fail:
+	if (proto->fd)
+		close(proto->fd);
+	return rc;
+}
+
+static void xymodem_close(struct xyz_ctxt *proto)
+{
+	xy_flush(proto->cdev, proto->fifo);
+	printf("\nxyModem - %d(SOH)/%d(STX)/%d(CAN) packets,"
+	       " %d retries\n",
+	       proto->total_SOH, proto->total_STX,
+	       proto->total_CAN, proto->total_retries);
+	kfifo_free(proto->fifo);
+}
+
+int do_load_serial_xmodem(struct console_device *cdev, int fd)
+{
+	struct xyz_ctxt *proto;
+	int rc;
+
+	proto = xymodem_open(cdev, PROTO_XMODEM, fd);
+	do {
+		rc = xymodem_handle(proto);
+	} while (rc > 0);
+	xymodem_close(proto);
+	return rc < 0 ? rc : 0;
+}
+EXPORT_SYMBOL(do_load_serial_xmodem);
+
+int do_load_serial_ymodem(struct console_device *cdev)
+{
+	struct xyz_ctxt *proto;
+	int rc;
+
+	proto = xymodem_open(cdev, PROTO_YMODEM, 0);
+	do {
+		rc = xymodem_handle(proto);
+	} while (rc > 0);
+	xymodem_close(proto);
+	return rc < 0 ? rc : 0;
+}
+EXPORT_SYMBOL(do_load_serial_ymodem);
+
+int do_load_serial_ymodemg(struct console_device *cdev)
+{
+	struct xyz_ctxt *proto;
+	int rc;
+
+	proto = xymodem_open(cdev, PROTO_YMODEM_G, 0);
+	do {
+		rc = xymodem_handle(proto);
+	} while (rc > 0);
+	xymodem_close(proto);
+	return rc < 0 ? rc : 0;
+}
+EXPORT_SYMBOL(do_load_serial_ymodemg);
-- 
1.7.10


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 2/2] commands: remove old Y-Modem implementation
  2012-11-04 17:55 ` [PATCH v2 1/2] commands: change Y-Modem implementation Robert Jarzmik
@ 2012-11-04 17:55   ` Robert Jarzmik
  2012-11-04 18:36   ` [PATCH v2 1/2] commands: change " Jean-Christophe PLAGNIOL-VILLARD
  1 sibling, 0 replies; 19+ messages in thread
From: Robert Jarzmik @ 2012-11-04 17:55 UTC (permalink / raw)
  To: plagnioj, antonynpavlov, s.hauer; +Cc: barebox, Robert Jarzmik

From: Robert Jarzmik <robert.jarzmik@intel.com>

As a new implementation of Y-Modem protocol is available,
switch from old implementation to the new one :
 - remove old xyzModem* files
 - remove old command loady2
 - rename command loady2 to loady

Signed-off-by: Robert Jarzmik <robert.jarzmik@intel.com>
---
 commands/Makefile   |    4 +-
 commands/loadb.c    |  102 +------
 commands/loads.c    |    1 -
 commands/loadxy.c   |    2 +-
 commands/xyzModem.c |  785 ---------------------------------------------------
 include/xyzModem.h  |  109 -------
 6 files changed, 12 insertions(+), 991 deletions(-)
 delete mode 100644 commands/xyzModem.c
 delete mode 100644 include/xyzModem.h

diff --git a/commands/Makefile b/commands/Makefile
index 44ad904..14c68dd 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -2,8 +2,8 @@ obj-$(CONFIG_STDDEV)		+= stddev.o
 obj-$(CONFIG_CMD_BOOTM)		+= bootm.o
 obj-$(CONFIG_CMD_UIMAGE)	+= uimage.o
 obj-$(CONFIG_CMD_LINUX16)	+= linux16.o
-obj-$(CONFIG_CMD_LOADB)		+= loadb.o xyzModem.o
-obj-$(CONFIG_CMD_LOADY)		+= loadb.o xyzModem.o loadxy.o
+obj-$(CONFIG_CMD_LOADB)		+= loadb.o
+obj-$(CONFIG_CMD_LOADY)		+= loadxy.o
 obj-$(CONFIG_CMD_LOADS)		+= loads.o
 obj-$(CONFIG_CMD_ECHO)		+= echo.o
 obj-$(CONFIG_CMD_MEMORY)	+= mem.o
diff --git a/commands/loadb.c b/commands/loadb.c
index 898b9e3..a2f3315 100644
--- a/commands/loadb.c
+++ b/commands/loadb.c
@@ -31,7 +31,6 @@
  */
 #include <common.h>
 #include <command.h>
-#include <xyzModem.h>
 #include <console.h>
 #include <errno.h>
 #include <environment.h>
@@ -59,8 +58,6 @@
 
 static int ofd;			/* output file descriptor */
 
-#ifdef CONFIG_CMD_LOADB
-
 /* Size of my buffer to write to o/p file */
 #define MAX_WRITE_BUFFER 4096	/* Write size to o/p file */
 static char *write_buffer;	/* buffer for finalized data to write */
@@ -593,66 +590,6 @@ err_quit:
 	return size;
 }
 
-#endif				/* CONFIG_CMD_LOADB */
-
-#ifdef CONFIG_CMD_LOADY
-/**
- * @brief getcxmodem
- *
- * @return if character avaiable, return the same, else return -1
- */
-static int getcxmodem(void)
-{
-	if (tstc())
-		return (getc());
-	return -1;
-}
-
-/**
- * @brief  LoadY over ymodem protocol
- *
- * @return download size
- */
-static ulong load_serial_ymodem(void)
-{
-	int size;
-	char buf[32];
-	int err;
-	int res, wr;
-	connection_info_t info;
-	char ymodemBuf[1024];
-	ulong addr = 0;
-
-	size = 0;
-	info.mode = xyzModem_ymodem;
-	res = xyzModem_stream_open(&info, &err);
-	if (!res) {
-		while ((res = xyzModem_stream_read(ymodemBuf, 1024, &err)) >
-				0) {
-			size += res;
-			addr += res;
-			wr = write(ofd, ymodemBuf, res);
-			if (res != wr) {
-				perror("ymodem");
-				break;
-			}
-
-		}
-	} else {
-		printf("%s\n", xyzModem_error(err));
-	}
-
-	xyzModem_stream_close(&err);
-	xyzModem_stream_terminate(false, &getcxmodem);
-
-	printf("## Total Size      = 0x%08x = %d Bytes\n", size, size);
-	sprintf(buf, "%X", size);
-	setenv("filesize", buf);
-
-	return res;
-}
-#endif
-
 /**
  * @brief returns current used console device
  *
@@ -754,27 +691,16 @@ static int do_load_serial_bin(int argc, char *argv[])
 				break;
 		}
 	}
-#ifdef CONFIG_CMD_LOADY
-	if (strcmp(argv[0], "loady") == 0) {
-		printf("## Ready for binary (ymodem) download "
-		       "to 0x%08lX offset on %s device at %d bps...\n", offset,
-		       output_file, load_baudrate);
-		addr = load_serial_ymodem();
-	}
-#endif
-#ifdef CONFIG_CMD_LOADB
-	if (strcmp(argv[0], "loadb") == 0) {
-
-		printf("## Ready for binary (kermit) download "
-		       "to 0x%08lX offset on %s device at %d bps...\n", offset,
-		       output_file, load_baudrate);
-		addr = load_serial_bin();
-		if (addr == 0) {
-			printf("## Binary (kermit) download aborted\n");
-			rcode = 1;
-		}
+
+	printf("## Ready for binary (kermit) download "
+	       "to 0x%08lX offset on %s device at %d bps...\n", offset,
+	       output_file, load_baudrate);
+	addr = load_serial_bin();
+	if (addr == 0) {
+		printf("## Binary (kermit) download aborted\n");
+		rcode = 1;
 	}
-#endif
+
 	if (load_baudrate != current_baudrate) {
 		printf("## Switch baudrate to %d bps and press ESC ...\n",
 		       current_baudrate);
@@ -799,18 +725,8 @@ static const __maybe_unused char cmd_loadb_help[] =
     "  -b baud   - baudrate at which to download - defaults to "
     "console baudrate\n"
     "  -c        - Create file if it is not present - default disabled";
-#ifdef CONFIG_CMD_LOADB
 BAREBOX_CMD_START(loadb)
 	.cmd = do_load_serial_bin,
 	.usage = "Load binary file over serial line (kermit mode)",
 	BAREBOX_CMD_HELP(cmd_loadb_help)
 BAREBOX_CMD_END
-#endif
-#ifdef CONFIG_CMD_LOADY
-BAREBOX_CMD_START(loady)
-	.cmd = do_load_serial_bin,
-	.usage = "Load binary file over serial line (ymodem mode)",
-	BAREBOX_CMD_HELP(cmd_loadb_help)
-BAREBOX_CMD_END
-#endif
-
diff --git a/commands/loads.c b/commands/loads.c
index 7f4c647..bfc465b 100644
--- a/commands/loads.c
+++ b/commands/loads.c
@@ -25,7 +25,6 @@
 #include <environment.h>
 #include <s_record.h>
 #include <net.h>
-#include <xyzModem.h>
 
 static ulong load_serial(ulong offset);
 static int read_record(char *buf, ulong len);
diff --git a/commands/loadxy.c b/commands/loadxy.c
index 141bd7b..8914655 100644
--- a/commands/loadxy.c
+++ b/commands/loadxy.c
@@ -230,7 +230,7 @@ static const __maybe_unused char cmd_loady_help[] =
 	"console baudrate\n";
 
 #ifdef CONFIG_CMD_LOADY
-BAREBOX_CMD_START(loady2)
+BAREBOX_CMD_START(loady)
 	.cmd = do_loady,
 	.usage = "Load binary file over serial line (Y-Modem or Y-Modem/G)",
 BAREBOX_CMD_HELP(cmd_loady_help)
diff --git a/commands/xyzModem.c b/commands/xyzModem.c
deleted file mode 100644
index 71d7d68..0000000
--- a/commands/xyzModem.c
+++ /dev/null
@@ -1,785 +0,0 @@
-/**
- * @file
- * @brief RedBoot stream handler for xyzModem protocol
- *
- * FileName: commands/xyzModem.c
- * Originally from u-boot xyzModem.c
- */
-/*
- * 2008 - Nishanth Menon <x0nishan@ti.com>
- * Modified for sparse and checkpatch.pl compliance
- */
-/*
- *==========================================================================
- *
- *      xyzModem.c
- *
- *      RedBoot stream handler for xyzModem protocol
- *
- *==========================================================================
- *####ECOSGPLCOPYRIGHTBEGIN####
- * -------------------------------------------
- * This file is part of eCos, the Embedded Configurable Operating System.
- * Copyright (C) 1998, 1999, 2000, 2001, 2002 Red Hat, Inc.
- * Copyright (C) 2002 Gary Thomas
- *
- * eCos is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 or (at your option) any later version.
- *
- * eCos is distributed in the hope that it will be useful, but WITHOUT ANY
- * WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with eCos; if not, write to the Free Software Foundation, Inc.,
- * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
- *
- * As a special exception, if other files instantiate templates or use macros
- * or inline functions from this file, or you compile this file and link it
- * with other works to produce a work based on this file, this file does not
- * by itself cause the resulting work to be covered by the GNU General Public
- * License. However the source code for this file must still be made available
- * in accordance with section (3) of the GNU General Public License.
- *
- * This exception does not invalidate any other reasons why a work based on
- * this file might be covered by the GNU General Public License.
- *
- * Alternative licenses for eCos may be arranged by contacting Red Hat, Inc.
- * at http: *sources.redhat.com/ecos/ecos-license/
- * -------------------------------------------
- *####ECOSGPLCOPYRIGHTEND####
- *==========================================================================
- *#####DESCRIPTIONBEGIN####
- *
- * Author(s):    gthomas
- * Contributors: gthomas, tsmith, Yoshinori Sato
- * Date:         2000-07-14
- * Purpose:
- * Description:
- *
- * This code is part of RedBoot (tm).
- *
- *####DESCRIPTIONEND####
- *
- *==========================================================================
- */
-#include <common.h>
-#include <xyzModem.h>
-#include <stdarg.h>
-#include <stdio.h>
-#include <console.h>
-#include <crc.h>
-#include <linux/ctype.h>
-
-/* Assumption - run xyzModem protocol over the console port */
-
-/* Values magic to the protocol */
-#define SOH 0x01
-#define STX 0x02
-#define EOT 0x04
-#define ACK 0x06
-#define BSP 0x08
-#define NAK 0x15
-#define CAN 0x18
-#define EOF 0x1A		/* ^Z for DOS officionados */
-
-#define USE_YMODEM_LENGTH
-
-/* Data & state local to the protocol */
-static struct {
-#ifdef REDBOOT
-	hal_virtual_comm_table_t *__chan;
-#else
-	int *__chan;
-#endif
-	unsigned char pkt[1024], *bufp;
-	unsigned char blk, cblk, crc1, crc2;
-	unsigned char next_blk;	/* Expected block */
-	int len, mode, total_retries;
-	int total_SOH, total_STX, total_CAN;
-	bool crc_mode, at_eof, tx_ack;
-#ifdef USE_YMODEM_LENGTH
-	unsigned long file_length, read_length;
-#endif
-} xyz;
-
-#define xyzModem_CHAR_TIMEOUT            2000	/* 2 seconds */
-#define xyzModem_MAX_RETRIES             20
-#define xyzModem_MAX_RETRIES_WITH_CRC    10
-/* Wait for 3 CAN before quitting */
-#define xyzModem_CAN_COUNT                3
-
-#ifndef REDBOOT			/*SB */
-typedef int cyg_int32;
-static int CYGACC_COMM_IF_GETC_TIMEOUT(char chan, char *c)
-{
-#define DELAY 20
-	unsigned long counter = 0;
-	while (!tstc() && (counter < xyzModem_CHAR_TIMEOUT * 1000 / DELAY)) {
-		udelay(DELAY);
-		counter++;
-	}
-	if (tstc()) {
-		*c = getc();
-		return 1;
-	}
-	return 0;
-}
-
-static void CYGACC_COMM_IF_PUTC(char x, char y)
-{
-	console_putc(CONSOLE_STDOUT, y);
-}
-
-/* Validate a hex character */
-static inline bool _is_hex(char c)
-{
-	return (((c >= '0') && (c <= '9')) ||
-		((c >= 'A') && (c <= 'F')) || ((c >= 'a') && (c <= 'f')));
-}
-
-/* Convert a single hex nibble */
-static inline int _from_hex(char c)
-{
-	int ret = 0;
-
-	if ((c >= '0') && (c <= '9'))
-		ret = (c - '0');
-	else if ((c >= 'a') && (c <= 'f'))
-		ret = (c - 'a' + 0x0a);
-	else if ((c >= 'A') && (c <= 'F'))
-		ret = (c - 'A' + 0x0A);
-
-	return ret;
-}
-
-/* Parse (scan) a number */
-static bool parse_num(char *s, unsigned long *val, char **es, char *delim)
-{
-	bool first = true;
-	int radix = 10;
-	char c;
-	unsigned long result = 0;
-	int digit = 0;
-
-	while (*s == ' ')
-		s++;
-	while (*s) {
-		if (first && (s[0] == '0') && (tolower(s[1]) == 'x')) {
-			radix = 16;
-			s += 2;
-		}
-		first = false;
-		c = *s++;
-		if (_is_hex(c))
-			digit = _from_hex(c);
-		if (_is_hex(c) && (digit < radix)) {
-			/* Valid digit */
-#ifdef CYGPKG_HAL_MIPS
-			/* FIXME: tx49 compiler generates 0x2539018 for
-			 * MUL which isn't any good. */
-			if (16 == radix)
-				result = result << 4;
-			else
-				result = 10 * result;
-			result += digit;
-#else
-			result = (result * radix) + digit;
-#endif
-		} else {
-			if (delim != (char *)0) {
-				/* See if this character is one of the
-				 * delimiters */
-				char *dp = delim;
-				while (*dp && (c != *dp))
-					dp++;
-				if (*dp)
-					break;	/* Found a good delimiter */
-			}
-			return false;	/* Malformatted number */
-		}
-	}
-	*val = result;
-	if (es != (char **)0)
-		*es = s;
-	return true;
-}
-
-#endif
-
-#define USE_SPRINTF
-#ifdef DEBUG
-#ifndef USE_SPRINTF
-/*
- * Note: this debug setup only works if the target platform has two serial ports
- * available so that the other one (currently only port 1) can be used for debug
- * messages.
- */
-static int zm_dprintf(char *fmt, ...)
-{
-	int cur_console;
-	va_list args;
-
-	va_start(args, fmt);
-#ifdef REDBOOT
-	cur_console =
-	    CYGACC_CALL_IF_SET_CONSOLE_COMM
-	    (CYGNUM_CALL_IF_SET_COMM_ID_QUERY_CURRENT);
-	CYGACC_CALL_IF_SET_CONSOLE_COMM(1);
-#endif
-	diag_vprintf(fmt, args);
-#ifdef REDBOOT
-	CYGACC_CALL_IF_SET_CONSOLE_COMM(cur_console);
-#endif
-}
-
-static void zm_flush(void)
-{
-}
-
-#else
-/*
- * Note: this debug setup works by storing the strings in a fixed buffer
- */
-#define FINAL
-#ifdef FINAL
-static char *zm_out = (char *)0x00380000;
-static char *zm_out_start = (char *)0x00380000;
-#else
-static char zm_buf[8192];
-static char *zm_out = zm_buf;
-static char *zm_out_start = zm_buf;
-
-#endif
-static int zm_dprintf(char *fmt, ...)
-{
-	int len;
-	va_list args;
-
-	va_start(args, fmt);
-	len = diag_vsprintf(zm_out, fmt, args);
-	zm_out += len;
-	return len;
-}
-
-static void zm_flush(void)
-{
-#ifdef REDBOOT
-	char *p = zm_out_start;
-	while (*p)
-		mon_write_char(*p++);
-#endif
-	zm_out = zm_out_start;
-}
-#endif
-
-static void zm_dump_buf(void *buf, int len)
-{
-#ifdef REDBOOT
-	diag_vdump_buf_with_offset(zm_dprintf, buf, len, 0);
-#else
-
-#endif
-}
-
-static unsigned char zm_buf[2048];
-static unsigned char *zm_bp;
-
-static void zm_new(void)
-{
-	zm_bp = zm_buf;
-}
-
-static void zm_save(unsigned char c)
-{
-	*zm_bp++ = c;
-}
-
-static void zm_dump(int line)
-{
-	zm_dprintf("Packet at line: %d\n", line);
-	zm_dump_buf(zm_buf, zm_bp - zm_buf);
-}
-
-#define ZM_DEBUG(x) x
-#else
-#define ZM_DEBUG(x)
-#endif
-
-/* Wait for the line to go idle */
-static void xyzModem_flush(void)
-{
-	int res;
-	char c;
-	while (true) {
-		res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, &c);
-		if (!res)
-			return;
-	}
-}
-
-static int xyzModem_get_hdr(void)
-{
-	char c;
-	int res;
-	bool hdr_found = false;
-	int i, can_total, hdr_chars;
-	unsigned short cksum;
-
-	ZM_DEBUG(zm_new());
-	/* Find the start of a header */
-	can_total = 0;
-	hdr_chars = 0;
-
-	if (xyz.tx_ack) {
-		CYGACC_COMM_IF_PUTC(*xyz.__chan, ACK);
-		xyz.tx_ack = false;
-	}
-	while (!hdr_found) {
-		res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, &c);
-		ZM_DEBUG(zm_save(c));
-		if (res) {
-			hdr_chars++;
-			switch (c) {
-			case SOH:
-				xyz.total_SOH++;
-			case STX:
-				if (c == STX)
-					xyz.total_STX++;
-				hdr_found = true;
-				break;
-			case CAN:
-				xyz.total_CAN++;
-				ZM_DEBUG(zm_dump(__LINE__));
-				if (++can_total == xyzModem_CAN_COUNT) {
-					return xyzModem_cancel;
-				} else {
-					/* Wait for multiple CAN to avoid
-					 * early quits */
-					break;
-				}
-			case EOT:
-				/* EOT only supported if no noise */
-				if (hdr_chars == 1) {
-					CYGACC_COMM_IF_PUTC(*xyz.__chan, ACK);
-					ZM_DEBUG(zm_dprintf
-						 ("ACK on EOT #%d\n",
-						  __LINE__));
-					ZM_DEBUG(zm_dump(__LINE__));
-					return xyzModem_eof;
-				}
-			default:
-				/* Ignore, waiting for start of header */
-				;
-			}
-		} else {
-			/* Data stream timed out */
-			xyzModem_flush();	/* Toss any current input */
-			ZM_DEBUG(zm_dump(__LINE__));
-			CYGACC_CALL_IF_DELAY_US((cyg_int32) 250000);
-			return xyzModem_timeout;
-		}
-	}
-
-	/* Header found, now read the data */
-	res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, (char *)&xyz.blk);
-	ZM_DEBUG(zm_save(xyz.blk));
-	if (!res) {
-		ZM_DEBUG(zm_dump(__LINE__));
-		return xyzModem_timeout;
-	}
-	res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, (char *)&xyz.cblk);
-	ZM_DEBUG(zm_save(xyz.cblk));
-	if (!res) {
-		ZM_DEBUG(zm_dump(__LINE__));
-		return xyzModem_timeout;
-	}
-	xyz.len = (c == SOH) ? 128 : 1024;
-	xyz.bufp = xyz.pkt;
-	for (i = 0; i < xyz.len; i++) {
-		res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, &c);
-		ZM_DEBUG(zm_save(c));
-		if (res) {
-			xyz.pkt[i] = c;
-		} else {
-			ZM_DEBUG(zm_dump(__LINE__));
-			return xyzModem_timeout;
-		}
-	}
-	res = CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, (char *)&xyz.crc1);
-	ZM_DEBUG(zm_save(xyz.crc1));
-	if (!res) {
-		ZM_DEBUG(zm_dump(__LINE__));
-		return xyzModem_timeout;
-	}
-	if (xyz.crc_mode) {
-		res =
-		    CYGACC_COMM_IF_GETC_TIMEOUT(*xyz.__chan, (char *)&xyz.crc2);
-		ZM_DEBUG(zm_save(xyz.crc2));
-		if (!res) {
-			ZM_DEBUG(zm_dump(__LINE__));
-			return xyzModem_timeout;
-		}
-	}
-	ZM_DEBUG(zm_dump(__LINE__));
-	/* Validate the message */
-	if ((xyz.blk ^ xyz.cblk) != (unsigned char)0xFF) {
-		ZM_DEBUG(zm_dprintf
-			 ("Framing error - blk: %x/%x/%x\n", xyz.blk, xyz.cblk,
-			  (xyz.blk ^ xyz.cblk)));
-		ZM_DEBUG(zm_dump_buf(xyz.pkt, xyz.len));
-		xyzModem_flush();
-		return xyzModem_frame;
-	}
-	/* Verify checksum/CRC */
-	if (xyz.crc_mode) {
-		cksum = cyg_crc16(xyz.pkt, xyz.len);
-		if (cksum != ((xyz.crc1 << 8) | xyz.crc2)) {
-			ZM_DEBUG(zm_dprintf
-				 ("CRC error - recvd: %02x%02x, computed: %x\n",
-				  xyz.crc1, xyz.crc2, cksum & 0xFFFF));
-			return xyzModem_cksum;
-		}
-	} else {
-		cksum = 0;
-		for (i = 0; i < xyz.len; i++)
-			cksum += xyz.pkt[i];
-		if (xyz.crc1 != (cksum & 0xFF)) {
-			ZM_DEBUG(zm_dprintf
-				 ("Checksum error - recvd: %x, computed: %x\n",
-				  xyz.crc1, cksum & 0xFF));
-			return xyzModem_cksum;
-		}
-	}
-	/* If we get here, the message passes [structural] muster */
-	return 0;
-}
-
-int xyzModem_stream_open(connection_info_t *info, int *err)
-{
-#ifdef REDBOOT
-	int console_chan;
-#endif
-	int stat = 0;
-	int retries = xyzModem_MAX_RETRIES;
-	int crc_retries = xyzModem_MAX_RETRIES_WITH_CRC;
-
-/*    ZM_DEBUG(zm_out = zm_out_start); */
-#ifdef xyzModem_zmodem
-	if (info->mode == xyzModem_zmodem) {
-		*err = xyzModem_noZmodem;
-		return -1;
-	}
-#endif
-
-#ifdef REDBOOT
-	/* Set up the I/O channel.  Note: this allows for using a different
-	 * port in the future */
-	console_chan =
-	    CYGACC_CALL_IF_SET_CONSOLE_COMM
-	    (CYGNUM_CALL_IF_SET_COMM_ID_QUERY_CURRENT);
-	if (info->chan >= 0)
-		CYGACC_CALL_IF_SET_CONSOLE_COMM(info->chan);
-	else
-		CYGACC_CALL_IF_SET_CONSOLE_COMM(console_chan);
-
-	xyz.__chan = CYGACC_CALL_IF_CONSOLE_PROCS();
-
-	CYGACC_CALL_IF_SET_CONSOLE_COMM(console_chan);
-	CYGACC_COMM_IF_CONTROL(*xyz.__chan, __COMMCTL_SET_TIMEOUT,
-			       xyzModem_CHAR_TIMEOUT);
-#else
-/* TODO: CHECK ! */
-	int dummy = 0;
-	xyz.__chan = &dummy;
-#endif
-	xyz.len = 0;
-	xyz.crc_mode = true;
-	xyz.at_eof = false;
-	xyz.tx_ack = false;
-	xyz.mode = info->mode;
-	xyz.total_retries = 0;
-	xyz.total_SOH = 0;
-	xyz.total_STX = 0;
-	xyz.total_CAN = 0;
-#ifdef USE_YMODEM_LENGTH
-	xyz.read_length = 0;
-	xyz.file_length = 0;
-#endif
-
-	CYGACC_COMM_IF_PUTC(*xyz.__chan, (xyz.crc_mode ? 'C' : NAK));
-
-	if (xyz.mode == xyzModem_xmodem) {
-		/* X-modem doesn't have an information header - exit here */
-		xyz.next_blk = 1;
-		return 0;
-	}
-
-	while (retries-- > 0) {
-		stat = xyzModem_get_hdr();
-		if (stat == 0) {
-			/* Y-modem file information header */
-			if (xyz.blk == 0) {
-#ifdef USE_YMODEM_LENGTH
-				/* skip filename */
-				while (*xyz.bufp++) ;
-				/* get the length */
-				parse_num((char *)xyz.bufp, &xyz.file_length,
-					  NULL, " ");
-#endif
-				/* The rest of the file name data block
-				 * quietly discarded
-				 */
-				xyz.tx_ack = true;
-			}
-			xyz.next_blk = 1;
-			xyz.len = 0;
-			return 0;
-		} else if (stat == xyzModem_timeout) {
-			if (--crc_retries <= 0)
-				xyz.crc_mode = false;
-			/* Extra delay for startup */
-			CYGACC_CALL_IF_DELAY_US(5 * 100000);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan,
-					    (xyz.crc_mode ? 'C' : NAK));
-			xyz.total_retries++;
-			ZM_DEBUG(zm_dprintf("NAK (%d)\n", __LINE__));
-		}
-		if (stat == xyzModem_cancel)
-			break;
-	}
-	*err = stat;
-	ZM_DEBUG(zm_flush());
-	return -1;
-}
-
-int xyzModem_stream_read(char *buf, int size, int *err)
-{
-	int stat, total, len;
-	int retries;
-
-	total = 0;
-	stat = xyzModem_cancel;
-	/* Try and get 'size' bytes into the buffer */
-	while (!xyz.at_eof && (size > 0)) {
-		if (xyz.len == 0) {
-			retries = xyzModem_MAX_RETRIES;
-			while (retries-- > 0) {
-				stat = xyzModem_get_hdr();
-				if (stat == 0) {
-					if (xyz.blk == xyz.next_blk) {
-						xyz.tx_ack = true;
-						ZM_DEBUG(zm_dprintf
-							 ("ACK block %d (%d)\n",
-							  xyz.blk, __LINE__));
-						xyz.next_blk =
-						    (xyz.next_blk + 1) & 0xFF;
-
-#if defined(xyzModem_zmodem) || defined(USE_YMODEM_LENGTH)
-						if (xyz.mode == xyzModem_xmodem
-						    || xyz.file_length == 0) {
-#else
-						if (1) {
-#endif
-			/* WARNING - Leaving formatting aside for code
-			 * clarity */
-			/* Data blocks can be padded with ^Z (EOF) characters */
-			/* This code tries to detect and remove them */
-			if ((xyz.bufp[xyz.len - 1] == EOF)
-			    && (xyz.bufp[xyz.len - 2] == EOF)
-			    && (xyz.bufp[xyz.len - 3] == EOF)) {
-				while (xyz.len &&
-					(xyz.bufp[xyz.len - 1] == EOF))
-						xyz.len--;
-				}
-			}
-#ifdef USE_YMODEM_LENGTH
-			/* WARNING - Leaving formatting aside for code
-			 * clarity */
-			/*
-			 * See if accumulated length exceeds that of the file.
-			 * If so, reduce size (i.e., cut out pad bytes)
-			 * Only do this for Y-modem (and Z-modem should it ever
-			 * be supported since it can fall back to Y-modem mode).
-			 */
-			if (xyz.mode != xyzModem_xmodem
-			    && 0 != xyz.file_length) {
-				xyz.read_length += xyz.len;
-				if (xyz.read_length > xyz.file_length)
-					xyz.len -= (xyz.read_length -
-					     xyz.file_length);
-			}
-#endif
-						break;
-					} else if (xyz.blk ==
-						   ((xyz.next_blk - 1) &
-						    0xFF)) {
-						/* Just re-ACK this so sender
-						 * will get on with it */
-						CYGACC_COMM_IF_PUTC(*xyz.__chan,
-								    ACK);
-						/* Need new header */
-						continue;
-					} else
-						stat = xyzModem_sequence;
-				}
-				if (stat == xyzModem_cancel)
-					break;
-				if (stat == xyzModem_eof) {
-					CYGACC_COMM_IF_PUTC(*xyz.__chan, ACK);
-					ZM_DEBUG(zm_dprintf
-						 ("ACK (%d)\n", __LINE__));
-					if (xyz.mode == xyzModem_ymodem) {
-						CYGACC_COMM_IF_PUTC(*xyz.__chan,
-								    (xyz.
-								     crc_mode ?
-								     'C' :
-								     NAK));
-						xyz.total_retries++;
-						ZM_DEBUG(zm_dprintf
-						 ("Reading Final Header\n"));
-						stat = xyzModem_get_hdr();
-						CYGACC_COMM_IF_PUTC(*xyz.__chan,
-								    ACK);
-						ZM_DEBUG(zm_dprintf
-							 ("FINAL ACK (%d)\n",
-							  __LINE__));
-					}
-					xyz.at_eof = true;
-					break;
-				}
-				CYGACC_COMM_IF_PUTC(*xyz.__chan,
-						    (xyz.crc_mode ? 'C' : NAK));
-				xyz.total_retries++;
-				ZM_DEBUG(zm_dprintf("NAK (%d)\n", __LINE__));
-			}
-			if (stat < 0) {
-				*err = stat;
-				xyz.len = -1;
-				return total;
-			}
-		}
-		/* Don't "read" data from the EOF protocol package */
-		if (!xyz.at_eof) {
-			len = xyz.len;
-			if (size < len)
-				len = size;
-			memcpy(buf, xyz.bufp, len);
-			size -= len;
-			buf += len;
-			total += len;
-			xyz.len -= len;
-			xyz.bufp += len;
-		}
-	}
-	return total;
-}
-
-void xyzModem_stream_close(int *err)
-{
-	diag_printf
-	    ("xyzModem - %s mode, %d(SOH)/%d(STX)/%d(CAN) packets,"
-	     " %d retries\n",
-	     xyz.crc_mode ? "CRC" : "Cksum", xyz.total_SOH, xyz.total_STX,
-	     xyz.total_CAN, xyz.total_retries);
-	ZM_DEBUG(zm_flush());
-}
-
-/* Need to be able to clean out the input buffer, so have to take the */
-/* getc */
-void xyzModem_stream_terminate(bool abort, int (*getc) (void))
-{
-	int c;
-
-	if (abort) {
-		ZM_DEBUG(zm_dprintf("!!!! TRANSFER ABORT !!!!\n"));
-		switch (xyz.mode) {
-		case xyzModem_xmodem:
-		case xyzModem_ymodem:
-			/* The X/YMODEM Spec seems to suggest that multiple CAN
-			 * followed by an equal number of Backspaces is a
-			 * friendly way to get the other end to abort. */
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, CAN);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, CAN);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, CAN);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, CAN);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, BSP);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, BSP);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, BSP);
-			CYGACC_COMM_IF_PUTC(*xyz.__chan, BSP);
-			/* Now consume the rest of what's waiting on the line.*/
-			ZM_DEBUG(zm_dprintf("Flushing serial line.\n"));
-			xyzModem_flush();
-			xyz.at_eof = true;
-			break;
-#ifdef xyzModem_zmodem
-		case xyzModem_zmodem:
-			/* Might support it some day I suppose. */
-#endif
-			break;
-		}
-	} else {
-		ZM_DEBUG(zm_dprintf("Engaging cleanup mode...\n"));
-		/*
-		 * Consume any trailing crap left in the inbuffer from
-		 * previous recieved blocks. Since very few files are an
-		 * exact multiple of the transfer block size, there will
-		 * almost always be some gunk here.
-		 * If we don't eat it now, RedBoot will think the user typed it.
-		 */
-		ZM_DEBUG(zm_dprintf("Trailing gunk:\n"));
-		while ((c = (*getc) ()) > -1) ;
-		ZM_DEBUG(zm_dprintf("\n"));
-		/*
-		 * Make a small delay to give terminal programs like minicom
-		 * time to get control again after their file transfer program
-		 * exits.
-		 */
-		CYGACC_CALL_IF_DELAY_US((cyg_int32) 250000);
-	}
-}
-
-char *xyzModem_error(int err)
-{
-	switch (err) {
-	case xyzModem_access:
-		return "Can't access file";
-		break;
-	case xyzModem_noZmodem:
-		return "Sorry, zModem not available yet";
-		break;
-	case xyzModem_timeout:
-		return "Timed out";
-		break;
-	case xyzModem_eof:
-		return "End of file";
-		break;
-	case xyzModem_cancel:
-		return "Cancelled";
-		break;
-	case xyzModem_frame:
-		return "Invalid framing";
-		break;
-	case xyzModem_cksum:
-		return "CRC/checksum error";
-		break;
-	case xyzModem_sequence:
-		return "Block sequence error";
-		break;
-	default:
-		return "Unknown error";
-		break;
-	}
-}
-
-/*
- * RedBoot interface
- */
-#if 0				/* SB */
-GETC_IO_FUNCS(xyzModem_io, xyzModem_stream_open, xyzModem_stream_close,
-	      xyzModem_stream_terminate, xyzModem_stream_read, xyzModem_error);
-RedBoot_load(xmodem, xyzModem_io, false, false, xyzModem_xmodem);
-RedBoot_load(ymodem, xyzModem_io, false, false, xyzModem_ymodem);
-#endif
diff --git a/include/xyzModem.h b/include/xyzModem.h
deleted file mode 100644
index a3ea768..0000000
--- a/include/xyzModem.h
+++ /dev/null
@@ -1,109 +0,0 @@
-/*
- *==========================================================================
- *
- *      xyzModem.h
- *
- *      RedBoot stream handler for xyzModem protocol
- *
- *==========================================================================
- *####ECOSGPLCOPYRIGHTBEGIN####
- * -------------------------------------------
- * This file is part of eCos, the Embedded Configurable Operating System.
- * Copyright (C) 1998, 1999, 2000, 2001, 2002 Red Hat, Inc.
- * Copyright (C) 2002 Gary Thomas
- *
- * eCos is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 or (at your option) any later version.
- *
- * eCos is distributed in the hope that it will be useful, but WITHOUT ANY
- * WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with eCos; if not, write to the Free Software Foundation, Inc.,
- * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
- *
- * As a special exception, if other files instantiate templates or use macros
- * or inline functions from this file, or you compile this file and link it
- * with other works to produce a work based on this file, this file does not
- * by itself cause the resulting work to be covered by the GNU General Public
- * License. However the source code for this file must still be made available
- * in accordance with section (3) of the GNU General Public License.
- *
- * This exception does not invalidate any other reasons why a work based on
- * this file might be covered by the GNU General Public License.
- *
- * Alternative licenses for eCos may be arranged by contacting Red Hat, Inc.
- * at http: *sources.redhat.com/ecos/ecos-license/
- * -------------------------------------------
- *####ECOSGPLCOPYRIGHTEND####
- *==========================================================================
- *#####DESCRIPTIONBEGIN####
- *
- * Author(s):    gthomas
- * Contributors: gthomas
- * Date:         2000-07-14
- * Purpose:
- * Description:
- *
- * This code is part of RedBoot (tm).
- *
- *####DESCRIPTIONEND####
- *
- *==========================================================================
- */
-
-#ifndef _XYZMODEM_H_
-#define _XYZMODEM_H_
-
-#define xyzModem_xmodem 1
-#define xyzModem_ymodem 2
-/* Don't define this until the protocol support is in place */
-/*#define xyzModem_zmodem 3 */
-
-#define xyzModem_access   -1
-#define xyzModem_noZmodem -2
-#define xyzModem_timeout  -3
-#define xyzModem_eof      -4
-#define xyzModem_cancel   -5
-#define xyzModem_frame    -6
-#define xyzModem_cksum    -7
-#define xyzModem_sequence -8
-
-#define xyzModem_close 1
-#define xyzModem_abort 2
-
-
-#ifdef REDBOOT
-extern getc_io_funcs_t xyzModem_io;
-#else
-#define CYGNUM_CALL_IF_SET_COMM_ID_QUERY_CURRENT
-#define CYGACC_CALL_IF_SET_CONSOLE_COMM(x)
-
-#define diag_vprintf vprintf
-#define diag_printf printf
-#define diag_vsprintf vsprintf
-
-#define CYGACC_CALL_IF_DELAY_US(x) udelay(x)
-
-typedef struct {
-    char *filename;
-    int   mode;
-    int   chan;
-#ifdef CYGPKG_REDBOOT_NETWORKING
-    struct sockaddr_in *server;
-#endif
-} connection_info_t;
-
-#endif
-
-
-int   xyzModem_stream_open(connection_info_t *info, int *err);
-void  xyzModem_stream_close(int *err);
-void  xyzModem_stream_terminate(bool method, int (*getc)(void));
-int   xyzModem_stream_read(char *buf, int size, int *err);
-char *xyzModem_error(int err);
-
-#endif /* _XYZMODEM_H_ */
-- 
1.7.10


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] commands: change Y-Modem implementation
  2012-11-04 17:55 ` [PATCH v2 1/2] commands: change Y-Modem implementation Robert Jarzmik
  2012-11-04 17:55   ` [PATCH v2 2/2] commands: remove old " Robert Jarzmik
@ 2012-11-04 18:36   ` Jean-Christophe PLAGNIOL-VILLARD
  2012-11-05 18:07     ` Robert Jarzmik
  1 sibling, 1 reply; 19+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-11-04 18:36 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: barebox

On 18:55 Sun 04 Nov     , Robert Jarzmik wrote:
> The current Y-Modem implementation has some limitations:
>  - Y-Modem/G protocol is not supported
>  - Multiple files (aka. batch) transfers are not supported
>  - Transfer speed over fast lines (USB console) is slow
>  - Code is not trivial to maintain (personnal opinion)
> 
> This implementation tries to address all these points by
> introducing loady2 command.
> 
> The effects are :
>  - transfer speed for Y-Modem over USB jumps from 2kBytes/s
>    to 180kBytes/s
>  - transfer speed for Y-Modem/G jumps to 200kBytes/s
>  - multiple file transfers are possible
> 
> This command was tested on a USB console and UART 9600bps
> serial line :
>  - NAKs (and retransmissions) were tested for faulty
>    serial lines
>  - multiple file transfers were tested
>  - Y-Modem, Y-Modem/G and X-Modem transfers were tested
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> 
> ---
> Since V1:
>  - add input fifo for small fifo hardwares
> Add a FIFO so that each getc will empty the hardware
> FIFO. This is very similar to the generic console code,
> except that the getc won't block each time for 100us,
> enabling faster lines (USB) to benefit their full speed.
>  - fix CRC calculation for big endian architectures
>    Thanks a lot Antony for the many patches testing !
>  - added some documentation
>  - amended the split as Sascha recommended
> ---
>  commands/Kconfig  |    1 +
>  commands/Makefile |    2 +-
>  commands/loadxy.c |  238 +++++++++++++++++++++
>  include/xymodem.h |   25 +++
>  lib/Kconfig       |    3 +
>  lib/Makefile      |    1 +
>  lib/xymodem.c     |  591 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 860 insertions(+), 1 deletion(-)
>  create mode 100644 commands/loadxy.c
>  create mode 100644 include/xymodem.h
>  create mode 100644 lib/xymodem.c
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index a52a01a..a7e9974 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -261,6 +261,7 @@ config CMD_LOADB
>  
>  config CMD_LOADY
>  	select CRC16
> +	select XYMODEM
>  	tristate
>  	prompt "loady"
>  
> diff --git a/commands/Makefile b/commands/Makefile
> index ff98051..44ad904 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -3,7 +3,7 @@ obj-$(CONFIG_CMD_BOOTM)		+= bootm.o
>  obj-$(CONFIG_CMD_UIMAGE)	+= uimage.o
>  obj-$(CONFIG_CMD_LINUX16)	+= linux16.o
>  obj-$(CONFIG_CMD_LOADB)		+= loadb.o xyzModem.o
> -obj-$(CONFIG_CMD_LOADY)		+= loadb.o xyzModem.o
> +obj-$(CONFIG_CMD_LOADY)		+= loadb.o xyzModem.o loadxy.o
>  obj-$(CONFIG_CMD_LOADS)		+= loads.o
>  obj-$(CONFIG_CMD_ECHO)		+= echo.o
>  obj-$(CONFIG_CMD_MEMORY)	+= mem.o
> diff --git a/commands/loadxy.c b/commands/loadxy.c
> new file mode 100644
> index 0000000..141bd7b
> --- /dev/null
> +++ b/commands/loadxy.c
> @@ -0,0 +1,238 @@
> +/**
> + * @file
> + * @brief loady and loadx  support.
> + *
> + * Provides loadx (over X-Modem) and loady(over Y-Modem) support to download
> + * images.
> + *
> + * FileName: commands/loadxy.c
> + */
> +/*
> + * (C) Copyright 2012 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +/*
> + * Serial up- and download support
> + */
> +#include <common.h>
> +#include <command.h>
> +#include <console.h>
> +#include <xymodem.h>
> +#include <errno.h>
> +#include <getopt.h>
> +#include <fcntl.h>
> +#include <fs.h>
> +#include <malloc.h>
> +
> +#define DEF_FILE	"image.bin"
> +
> +/**
> + * @brief returns current used console device
> + *
> + * @return console device which is registered with CONSOLE_STDIN and
> + * CONSOLE_STDOUT
> + */
> +static struct console_device *get_current_console(void)
> +{
> +	struct console_device *cdev;
> +	/*
> +	 * Assumption to have BOTH CONSOLE_STDIN AND STDOUT in the
> +	 * same output console
> +	 */
> +	for_each_console(cdev) {
> +		if ((cdev->f_active & (CONSOLE_STDIN | CONSOLE_STDOUT)))
> +			return cdev;
> +	}
> +	return NULL;
> +}
> +
> +static int console_change_speed(struct console_device *cdev, int baudrate)
> +{
> +	int current_baudrate;
> +
> +	current_baudrate =
> +		(int)simple_strtoul(dev_get_param(&cdev->class_dev,
> +						  "baudrate"), NULL, 10);
> +	if (baudrate && baudrate != current_baudrate) {
> +		printf("## Switch baudrate from %d to %d bps and press ENTER ...\n",
> +		       current_baudrate, baudrate);
> +		mdelay(50);
> +		cdev->setbrg(cdev, baudrate);
> +		mdelay(50);
> +	}
> +	return current_baudrate;
> +}
> +
> +/**
> + * @brief provide the loady(Y-Modem or Y-Modem/G) support
> + *
> + * @param argc number of arguments
> + * @param argv arguments of loady command
> + *
> + * @return success or failure
> + */
> +static int do_loady(int argc, char *argv[])
> +{
> +	int is_ymodemg = 0, rc = 0, opt, rcode = 0;
> +	int load_baudrate = 0, current_baudrate;
> +	struct console_device *cdev = NULL;
> +
> +	while ((opt = getopt(argc, argv, "b:g")) > 0) {
> +		switch (opt) {
> +		case 'b':
> +			load_baudrate = (int)simple_strtoul(optarg, NULL, 10);
> +			break;
> +		case 'g':
> +			is_ymodemg = 1;
> +			break;
> +		default:
> +			perror(argv[0]);
> +			return 1;
> +		}
> +	}
> +
> +	cdev = get_current_console();
> +	if (NULL == cdev) {
this really look wired
	if (!cdev)
> +		printf("%s:No console device with STDIN and STDOUT\n", argv[0]);
> +		return -ENODEV;
> +	}
> +
> +	current_baudrate = console_change_speed(cdev, load_baudrate);
> +	printf("## Ready for binary (ymodem) download at %d bps...\n",
> +	       load_baudrate ? load_baudrate : current_baudrate);
> +
> +	if (is_ymodemg)
> +		rc = do_load_serial_ymodemg(cdev);
> +	else
> +		rc = do_load_serial_ymodem(cdev);
> +
> +	if (rc < 0) {
> +		printf("## Binary (ymodem) download aborted (%d)\n", rc);
> +		rcode = 1;
> +	}
> +
> +	console_change_speed(cdev, current_baudrate);
> +
> +	return rcode;
> +}
> +
> +/**
> + * @brief provide the loadx(X-Modem) support
> + *
> + * @param argc number of arguments
> + * @param argv arguments of loadx command
> + *
> + * @return success or failure
> + */
> +static int do_loadx(int argc, char *argv[])
> +{
> +	ulong offset = 0;
> +	int load_baudrate = 0, current_baudrate, ofd, opt, rcode = 0;
> +	int open_mode = O_WRONLY;
> +	char *output_file = NULL;
> +	struct console_device *cdev = NULL;
> +
> +	while ((opt = getopt(argc, argv, "f:b:o:c")) > 0) {
> +		switch (opt) {
> +		case 'f':
> +			output_file = optarg;
> +			break;
> +		case 'b':
> +			load_baudrate = (int)simple_strtoul(optarg, NULL, 10);
> +			break;
> +		case 'o':
> +			offset = (int)simple_strtoul(optarg, NULL, 10);
> +			break;
> +		case 'c':
> +			open_mode |= O_CREAT;
> +			break;
> +		default:
> +			perror(argv[0]);
> +			return 1;
> +		}
> +	}
> +
> +	cdev = get_current_console();
> +	if (NULL == cdev) {
	ditto
> +		printf("%s:No console device with STDIN and STDOUT\n", argv[0]);
> +		return -ENODEV;
> +	}
> +
> +	/* Load Defaults */
> +	if (NULL == output_file)
	ditto
> +		output_file = DEF_FILE;
> +
> +	/* File should exist */
> +	ofd = open(output_file, open_mode);
> +	if (ofd < 0) {
> +		perror(argv[0]);
> +		return 3;
> +	}
> +	/* Seek to the right offset */
> +	if (offset) {
> +		int seek = lseek(ofd, offset, SEEK_SET);
> +		if (seek != offset) {
> +			close(ofd);
> +			ofd = 0;
> +			perror(argv[0]);
> +			return 4;
> +		}
> +	}
> +
> +	current_baudrate = console_change_speed(cdev, load_baudrate);
> +	printf("## Ready for binary (X-Modem) download "
> +	       "to 0x%08lX offset on %s device at %d bps...\n", offset,
> +	       output_file, load_baudrate);
> +	rcode = do_load_serial_ymodem(cdev);
> +	if (rcode < 0) {
> +		printf("## Binary (kermit) download aborted (%d)\n", rcode);
> +		rcode = 1;
> +	}
> +	console_change_speed(cdev, current_baudrate);
> +
> +	return rcode;
> +}

> +static void xy_flush(struct console_device *cdev, struct kfifo *fifo)
> +{
> +	while (cdev->tstc(cdev))
no timeout?
> +		cdev->getc(cdev);
> +	mdelay(250);
> +	while (cdev->tstc(cdev))
ditto
> +		cdev->getc(cdev);
> +	kfifo_reset(fifo);
> +}
> +

> + */
> +static ssize_t xy_read_block(struct xyz_ctxt *proto, struct xy_block *blk,
> +	uint64_t timeout)
> +{
> +	ssize_t rc, data_len = 0;
> +	unsigned char hdr, seqs[2], crcs[2];
> +	int crc = 0, hdr_found = 0;
> +	uint64_t start = get_time_ns();
> +
> +	while (!hdr_found) {
> +		rc = xy_gets(proto->cdev, proto->fifo, &hdr, 1, timeout);
> +		xy_dbg("read 0x%x(%c) -> %d\n", hdr, hdr, rc);
> +		if (rc < 0)
> +			goto out;
> +		if (is_timeout(start, timeout))
> +			goto timeout;
> +		switch (hdr) {
> +		case SOH:
> +			data_len = 128;
> +			hdr_found = 1;
> +			proto->total_SOH++;
no capital please
> +			break;
> +		case STX:
> +			data_len = 1024;
> +			hdr_found = 1;
boolean
> +			proto->total_STX++;
> +			break;
> +		case CAN:
> +			rc = -ECONNABORTED;
> +			if (proto->total_CAN++ > MAX_CAN_BEFORE_ABORT)
> +				goto out;
> +			break;
> +		case EOT:
> +			rc = 0;
> +			blk->len = 0;
> +			goto out;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	blk->seq = 0;
> +	rc = xy_gets(proto->cdev, proto->fifo, seqs, 2, timeout);
> +	if (rc < 0)
> +		goto out;
> +	blk->seq = seqs[0];
> +	if (255 - seqs[0] != seqs[1])
> +		return -EBADMSG;
> +
> +	rc = xy_gets(proto->cdev, proto->fifo, blk->buf, data_len, timeout);
> +	if (rc < 0)
> +		goto out;
> +	blk->len = rc;
> +
> +	switch (proto->crc_mode) {
> +	case CRC_ADD8:
> +		rc = xy_gets(proto->cdev, proto->fifo, crcs, 1, timeout);
> +		crc = crcs[0];
> +		break;
> +	case CRC_CRC16:
> +		rc = xy_gets(proto->cdev, proto->fifo, crcs, 2, timeout);
> +		crc = be16_to_cpu(*(uint16_t *)crcs);
> +		break;
> +	case CRC_NONE:
> +		rc = 0;
> +		break;
> +	}
> +	if (rc < 0)
> +		goto out;
> +
> +	rc = check_crc(blk->buf, data_len, crc, proto->crc_mode);
> +	if (rc < 0)
> +		goto out;
> +	return data_len;
> +timeout:
> +	return -ETIMEDOUT;
> +out:
> +	return rc;
> +}
> +
> +static int check_blk_seq(struct xyz_ctxt *proto, struct xy_block *blk,
> +	int read_rc)
> +{
> +	if (blk->seq == ((proto->next_blk - 1) % 256))
> +		return -EALREADY;
> +	if (blk->seq != proto->next_blk)
> +		return -EILSEQ;
> +	return read_rc;
> +}
> +
> +static int parse_first_block(struct xyz_ctxt *proto, struct xy_block *blk)
> +{
> +	int filename_len;
> +	char *str_num;
> +
> +	filename_len = strlen(blk->buf);
> +	if (filename_len > blk->len)
> +		return -EINVAL;
> +	memset(proto->filename, 0, sizeof(proto->filename));
no need just add 0 at the end
> +	strncpy(proto->filename, blk->buf, filename_len);
> +	str_num = blk->buf + filename_len + 1;
> +	strsep(&str_num, " ");
> +	proto->file_len = simple_strtoul(blk->buf + filename_len + 1, NULL, 10);
> +	return 1;
> +}
Best Regards,
J.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] commands: change Y-Modem implementation
  2012-11-04 18:36   ` [PATCH v2 1/2] commands: change " Jean-Christophe PLAGNIOL-VILLARD
@ 2012-11-05 18:07     ` Robert Jarzmik
  2012-11-05 18:25       ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 19+ messages in thread
From: Robert Jarzmik @ 2012-11-05 18:07 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:

>> +	cdev = get_current_console();
>> +	if (NULL == cdev) {
> this really look wired
> 	if (!cdev)
Yes, true. A copy-paste from loadb.c, I'll amend for v3.

>> +	if (NULL == cdev) {
> 	ditto
Yes again.

>> +		printf("%s:No console device with STDIN and STDOUT\n", argv[0]);
>> +		return -ENODEV;
>> +	}
>> +
>> +	/* Load Defaults */
>> +	if (NULL == output_file)
> 	ditto
Yes again.

>> +static void xy_flush(struct console_device *cdev, struct kfifo *fifo)
>> +{
>> +	while (cdev->tstc(cdev))
> no timeout?
No timeout indeed, on purpose.

The short explanation is that in a purge situation, all incoming data must be
flushed, regardless of the time it takes.

The long explantion is :
 - suppose the while(cdev->tstc(cdev)) takes a lot of time
 - this implies that :
   => the sender is sending data as fast as the reader can consume (not quite
   probable, but why not ...)
   => the sender has gone crazy, as in *-Modem protocol when this function is
   called, no more than 1029 bytes can be sent by a *sane* sender

So let's take as granted that the sender has gone crazy (or hardware is brain
dead). What will happen if I place a timeout here ?
 - first, I'll obviously get out of xy_flush()
 - then, as I receive garbage, xy_handle() will exit with either -EBADMSG or
 -EILSEQ. So far so good.
 - then the loady command will finish on error. Still good.
 - then the input console will take over, and execute all the garbage sent to
 it.
   This is definitely something we don't want. Imagine in the garbage you have
   something like "erase /dev/mtd0" ... Therefore, no timeout.

>> +	while (cdev->tstc(cdev))
> ditto
Ditto.

>> +			proto->total_SOH++;
> no capital please
Yep, for v3.

>> +			break;
>> +		case STX:
>> +			data_len = 1024;
>> +			hdr_found = 1;
> boolean
I don't catch that one.
Did you mean that hdr_found should be declared as "bool" ?

>> +static int parse_first_block(struct xyz_ctxt *proto, struct xy_block *blk)
>> +{
>> +	int filename_len;
>> +	char *str_num;
>> +
>> +	filename_len = strlen(blk->buf);
>> +	if (filename_len > blk->len)
>> +		return -EINVAL;
>> +	memset(proto->filename, 0, sizeof(proto->filename));
> no need just add 0 at the end
>> +	strncpy(proto->filename, blk->buf, filename_len);
Why not use strlcpy instead of memset+strncpy now I think of it ?

Cheers, and thanks for the review.

-- 
Robert

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] commands: change Y-Modem implementation
  2012-11-05 18:07     ` Robert Jarzmik
@ 2012-11-05 18:25       ` Jean-Christophe PLAGNIOL-VILLARD
  2012-11-05 18:49         ` Robert Jarzmik
  0 siblings, 1 reply; 19+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-11-05 18:25 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: barebox

On 19:07 Mon 05 Nov     , Robert Jarzmik wrote:
> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:
> 
> >> +	cdev = get_current_console();
> >> +	if (NULL == cdev) {
> > this really look wired
> > 	if (!cdev)
> Yes, true. A copy-paste from loadb.c, I'll amend for v3.
> 
> >> +	if (NULL == cdev) {
> > 	ditto
> Yes again.
> 
> >> +		printf("%s:No console device with STDIN and STDOUT\n", argv[0]);
> >> +		return -ENODEV;
> >> +	}
> >> +
> >> +	/* Load Defaults */
> >> +	if (NULL == output_file)
> > 	ditto
> Yes again.
> 
> >> +static void xy_flush(struct console_device *cdev, struct kfifo *fifo)
> >> +{
> >> +	while (cdev->tstc(cdev))
> > no timeout?
> No timeout indeed, on purpose.
> 
> The short explanation is that in a purge situation, all incoming data must be
> flushed, regardless of the time it takes.
> 
> The long explantion is :
>  - suppose the while(cdev->tstc(cdev)) takes a lot of time
>  - this implies that :
>    => the sender is sending data as fast as the reader can consume (not quite
>    probable, but why not ...)
>    => the sender has gone crazy, as in *-Modem protocol when this function is
>    called, no more than 1029 bytes can be sent by a *sane* sender
> 
> So let's take as granted that the sender has gone crazy (or hardware is brain
> dead). What will happen if I place a timeout here ?
>  - first, I'll obviously get out of xy_flush()
>  - then, as I receive garbage, xy_handle() will exit with either -EBADMSG or
>  -EILSEQ. So far so good.
>  - then the loady command will finish on error. Still good.
>  - then the input console will take over, and execute all the garbage sent to
>  it.
>    This is definitely something we don't want. Imagine in the garbage you have
>    something like "erase /dev/mtd0" ... Therefore, no timeout.
my issue is how can I interrupt it from barebox
> 
> >> +	while (cdev->tstc(cdev))
> > ditto
> Ditto.
> 
> >> +			proto->total_SOH++;
> > no capital please
> Yep, for v3.
> 
> >> +			break;
> >> +		case STX:
> >> +			data_len = 1024;
> >> +			hdr_found = 1;
> > boolean
> I don't catch that one.
> Did you mean that hdr_found should be declared as "bool" ?
yes
> 
> >> +static int parse_first_block(struct xyz_ctxt *proto, struct xy_block *blk)
> >> +{
> >> +	int filename_len;
> >> +	char *str_num;
> >> +
> >> +	filename_len = strlen(blk->buf);
> >> +	if (filename_len > blk->len)
> >> +		return -EINVAL;
> >> +	memset(proto->filename, 0, sizeof(proto->filename));
> > no need just add 0 at the end
> >> +	strncpy(proto->filename, blk->buf, filename_len);
> Why not use strlcpy instead of memset+strncpy now I think of it ?
> 
yeap

can you put the v3 on a tree somewhere so I can pull it

Best Regards,
J.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] commands: change Y-Modem implementation
  2012-11-05 18:25       ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-11-05 18:49         ` Robert Jarzmik
  2012-11-05 19:49           ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 19+ messages in thread
From: Robert Jarzmik @ 2012-11-05 18:49 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:
> On 19:07 Mon 05 Nov     , Robert Jarzmik wrote:
>> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:
>> No timeout indeed, on purpose.
>> 
>> The short explanation is that in a purge situation, all incoming data must be
>> flushed, regardless of the time it takes.
>> 
>> The long explantion is :
>>  - suppose the while(cdev->tstc(cdev)) takes a lot of time
>>  - this implies that :
>>    => the sender is sending data as fast as the reader can consume (not quite
>>    probable, but why not ...)
>>    => the sender has gone crazy, as in *-Modem protocol when this function is
>>    called, no more than 1029 bytes can be sent by a *sane* sender
>> 
>> So let's take as granted that the sender has gone crazy (or hardware is brain
>> dead). What will happen if I place a timeout here ?
>>  - first, I'll obviously get out of xy_flush()
>>  - then, as I receive garbage, xy_handle() will exit with either -EBADMSG or
>>  -EILSEQ. So far so good.
>>  - then the loady command will finish on error. Still good.
>>  - then the input console will take over, and execute all the garbage sent to
>>  it.
>>    This is definitely something we don't want. Imagine in the garbage you have
>>    something like "erase /dev/mtd0" ... Therefore, no timeout.
> my issue is how can I interrupt it from barebox
You can't, how could you ? Your console is overwhelmed by garbage input, what
could you possibly do to interrupt it apart from cutting down the sender ?

> can you put the v3 on a tree somewhere so I can pull it
I'll try. I have no external git I can push to. If you have a hint ...

-- 
Robert

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] commands: change Y-Modem implementation
  2012-11-05 18:49         ` Robert Jarzmik
@ 2012-11-05 19:49           ` Jean-Christophe PLAGNIOL-VILLARD
  2012-11-05 21:31             ` Robert Jarzmik
  0 siblings, 1 reply; 19+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-11-05 19:49 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: barebox

On 19:49 Mon 05 Nov     , Robert Jarzmik wrote:
> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:
> > On 19:07 Mon 05 Nov     , Robert Jarzmik wrote:
> >> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:
> >> No timeout indeed, on purpose.
> >> 
> >> The short explanation is that in a purge situation, all incoming data must be
> >> flushed, regardless of the time it takes.
> >> 
> >> The long explantion is :
> >>  - suppose the while(cdev->tstc(cdev)) takes a lot of time
> >>  - this implies that :
> >>    => the sender is sending data as fast as the reader can consume (not quite
> >>    probable, but why not ...)
> >>    => the sender has gone crazy, as in *-Modem protocol when this function is
> >>    called, no more than 1029 bytes can be sent by a *sane* sender
> >> 
> >> So let's take as granted that the sender has gone crazy (or hardware is brain
> >> dead). What will happen if I place a timeout here ?
> >>  - first, I'll obviously get out of xy_flush()
> >>  - then, as I receive garbage, xy_handle() will exit with either -EBADMSG or
> >>  -EILSEQ. So far so good.
> >>  - then the loady command will finish on error. Still good.
> >>  - then the input console will take over, and execute all the garbage sent to
> >>  it.
> >>    This is definitely something we don't want. Imagine in the garbage you have
> >>    something like "erase /dev/mtd0" ... Therefore, no timeout.
> > my issue is how can I interrupt it from barebox
> You can't, how could you ? Your console is overwhelmed by garbage input, what
> could you possibly do to interrupt it apart from cutting down the sender ?
this is an issue if we have more than one we need be able to do so and I may
want to use it on an other console_device not necessarely the current one

I have often hw that have more than one uart
> 
> > can you put the v3 on a tree somewhere so I can pull it
> I'll try. I have no external git I can push to. If you have a hint ...
https://git.wiki.kernel.org/index.php/GitHosting

Best Regards,
J.
> 
> -- 
> Robert

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] commands: change Y-Modem implementation
  2012-11-05 19:49           ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-11-05 21:31             ` Robert Jarzmik
  2012-11-06  7:34               ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 19+ messages in thread
From: Robert Jarzmik @ 2012-11-05 21:31 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:

>> You can't, how could you ? Your console is overwhelmed by garbage input, what
>> could you possibly do to interrupt it apart from cutting down the sender ?
> this is an issue if we have more than one we need be able to do so and I may
> want to use it on an other console_device not necessarely the current one
>
> I have often hw that have more than one uart
And how do you choose the alternative UART ? loady is not provided such a
parameter in its new form.
Now if you want, I can add it. And if I add it, I'll add a timeout also to the
xy_flush() function ...

>> > can you put the v3 on a tree somewhere so I can pull it
>> I'll try. I have no external git I can push to. If you have a hint ...
> https://git.wiki.kernel.org/index.php/GitHosting
Ah yes, ok. I'll send you the URL once I've amended the patches, along the
patch mails.

Cheers.

-- 
Robert

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] commands: change Y-Modem implementation
  2012-11-05 21:31             ` Robert Jarzmik
@ 2012-11-06  7:34               ` Jean-Christophe PLAGNIOL-VILLARD
  2012-11-06 20:50                 ` Robert Jarzmik
  0 siblings, 1 reply; 19+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-11-06  7:34 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: barebox

On 22:31 Mon 05 Nov     , Robert Jarzmik wrote:
> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:
> 
> >> You can't, how could you ? Your console is overwhelmed by garbage input, what
> >> could you possibly do to interrupt it apart from cutting down the sender ?
> > this is an issue if we have more than one we need be able to do so and I may
> > want to use it on an other console_device not necessarely the current one
> >
> > I have often hw that have more than one uart
> And how do you choose the alternative UART ? loady is not provided such a
> parameter in its new form.
by it's name cs1 as example

> Now if you want, I can add it. And if I add it, I'll add a timeout also to the
> xy_flush() function ...
yes please
> 
> >> > can you put the v3 on a tree somewhere so I can pull it
> >> I'll try. I have no external git I can push to. If you have a hint ...
> > https://git.wiki.kernel.org/index.php/GitHosting
> Ah yes, ok. I'll send you the URL once I've amended the patches, along the
> patch mails.
personnaly for at91 we use github tks

Best Regards,
J.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] commands: change Y-Modem implementation
  2012-11-06  7:34               ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-11-06 20:50                 ` Robert Jarzmik
  2012-11-07  8:22                   ` Antony Pavlov
  0 siblings, 1 reply; 19+ messages in thread
From: Robert Jarzmik @ 2012-11-06 20:50 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:

> On 22:31 Mon 05 Nov     , Robert Jarzmik wrote:
>> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:
>> And how do you choose the alternative UART ? loady is not provided such a
>> parameter in its new form.
> by it's name cs1 as example

>> Now if you want, I can add it. And if I add it, I'll add a timeout also to the
>> xy_flush() function ...
> yes please

>> >> > can you put the v3 on a tree somewhere so I can pull it
>> >> I'll try. I have no external git I can push to. If you have a hint ...
>> > https://git.wiki.kernel.org/index.php/GitHosting
>> Ah yes, ok. I'll send you the URL once I've amended the patches, along the
>> patch mails.
> personnaly for at91 we use github tks
Here is my candidate for v3 :
     git fetch git@github.com:rjarzmik/barebox.git xymodem

If you could test the "loady -t cs1", as I don't happen to have multiple
consoles ...
And Antony, if you have a little bit of time, would you test it also, as that
endianess bug drove me mad.

If it does work, I'll send the v3 of the patchset.

-- 
Robert

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/2] commands: change Y-Modem implementation
  2012-11-06 20:50                 ` Robert Jarzmik
@ 2012-11-07  8:22                   ` Antony Pavlov
  0 siblings, 0 replies; 19+ messages in thread
From: Antony Pavlov @ 2012-11-07  8:22 UTC (permalink / raw)
  To: Robert Jarzmik; +Cc: barebox

On 7 November 2012 00:50, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:
>
>> On 22:31 Mon 05 Nov     , Robert Jarzmik wrote:
>>> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:
>>> And how do you choose the alternative UART ? loady is not provided such a
>>> parameter in its new form.
>> by it's name cs1 as example
>
>>> Now if you want, I can add it. And if I add it, I'll add a timeout also to the
>>> xy_flush() function ...
>> yes please
>
>>> >> > can you put the v3 on a tree somewhere so I can pull it
>>> >> I'll try. I have no external git I can push to. If you have a hint ...
>>> > https://git.wiki.kernel.org/index.php/GitHosting
>>> Ah yes, ok. I'll send you the URL once I've amended the patches, along the
>>> patch mails.
>> personnaly for at91 we use github tks
> Here is my candidate for v3 :
>      git fetch git@github.com:rjarzmik/barebox.git xymodem
>
> If you could test the "loady -t cs1", as I don't happen to have multiple
> consoles ...
> And Antony, if you have a little bit of time, would you test it also, as that
> endianess bug drove me mad.

I just have tested your v3 patchset. It works fine on my little-endian
MIPS CPU, but it needs a small patch for correct work on the
big-endian one. You can find the patch in my next message.

I have fetched your v3 branch from your github repo. I'm sorry, but I
have no time to test it just now on my hardware. But I think where
will be no problems with squashing my patch into it.

-- 
Best regards,
  Antony Pavlov

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2012-11-07  8:22 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-01 17:37 [PATCH 0/2] Y-Modem implementation change Robert Jarzmik
2012-11-01 17:37 ` [PATCH 1/2] commands: change Y-Modem implementation Robert Jarzmik
2012-11-01 17:37 ` [PATCH 2/2] commands: remove old " Robert Jarzmik
2012-11-01 17:50 ` [PATCH 0/2] Y-Modem implementation change Jean-Christophe PLAGNIOL-VILLARD
2012-11-01 18:47   ` Antony Pavlov
2012-11-01 19:19 ` Antony Pavlov
2012-11-01 19:57   ` Robert Jarzmik
2012-11-01 19:33 ` Sascha Hauer
2012-11-04 17:55 ` [PATCH v2 1/2] commands: change Y-Modem implementation Robert Jarzmik
2012-11-04 17:55   ` [PATCH v2 2/2] commands: remove old " Robert Jarzmik
2012-11-04 18:36   ` [PATCH v2 1/2] commands: change " Jean-Christophe PLAGNIOL-VILLARD
2012-11-05 18:07     ` Robert Jarzmik
2012-11-05 18:25       ` Jean-Christophe PLAGNIOL-VILLARD
2012-11-05 18:49         ` Robert Jarzmik
2012-11-05 19:49           ` Jean-Christophe PLAGNIOL-VILLARD
2012-11-05 21:31             ` Robert Jarzmik
2012-11-06  7:34               ` Jean-Christophe PLAGNIOL-VILLARD
2012-11-06 20:50                 ` Robert Jarzmik
2012-11-07  8:22                   ` Antony Pavlov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox