diff mbox

[U-Boot,v2,3/7] dfu: DFU backend implementation

Message ID 1341416922-13792-4-git-send-email-l.majewski@samsung.com
State Superseded
Delegated to: Marek Vasut
Headers show

Commit Message

Łukasz Majewski July 4, 2012, 3:48 p.m. UTC
New, separate driver at ./drivers/dfu has been added. It allows platform
and storage independent operation of DFU.

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Vasut <marex@denx.de>

---
Changes for v2:
- None
---
 Makefile             |    1 +
 drivers/dfu/Makefile |   43 ++++++++
 drivers/dfu/dfu.c    |  259 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/dfu.h        |   99 +++++++++++++++++++
 4 files changed, 402 insertions(+), 0 deletions(-)
 create mode 100644 drivers/dfu/Makefile
 create mode 100644 drivers/dfu/dfu.c
 create mode 100644 include/dfu.h

Comments

Marek Vasut July 9, 2012, 4:35 p.m. UTC | #1
Dear Lukasz Majewski,

> New, separate driver at ./drivers/dfu has been added. It allows platform
> and storage independent operation of DFU.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> 

[...]

> +char *dfu_extract_token(char** e, int *n)
> +{
> +	char *st = *e;
> +
> +	debug("%s: %s\n", __func__, st);
> +
> +	strsep(e, " ");
> +	*n = *e - st;
> +
> +	return st;
> +}

Sigh, what about these? btw don't we have strtok?
[...]


Best regards,
Marek Vasut
Wolfgang Denk July 27, 2012, 11:58 a.m. UTC | #2
Dear Lukasz Majewski,

In message <1341416922-13792-4-git-send-email-l.majewski@samsung.com> you wrote:
> New, separate driver at ./drivers/dfu has been added. It allows platform
> and storage independent operation of DFU.

Sorry for a probably stupid question, but I know basicly zero about
DFU.   We are talking about "storage independent" here.  Does this
also mean file system independent?

> +static char *dfu_get_dev_type(enum dfu_device_type t)
> +{
> +	static char *dev_t[] = {NULL, "MMC", "ONENAND", "NAND" };
> +	return dev_t[t];
> +}

So this currently supports MMC, OneNAND and NAND as storage devices?

> +static char *dfu_get_layout(enum dfu_device_type l)
> +{
> +	static char *dfu_layout[] = {NULL, "RAW_ADDR", "FAT", "EXT" };
> +	return dfu_layout[l];
> +}

And FAT (or VFAT?) and EXT (as in EXT2? or EXT3? or ... ?) as file
systems?

> +enum dfu_device_type {
> +	MMC = 1,
> +	ONENAND,
> +	NAND
> +};
> +
> +enum dfu_layout {
> +	RAW_ADDR = 1,
> +	FAT,
> +	EXT,
> +};

MMC, NAND, FAT and EXT are very generic names that heavily pollute on
the global name space.  Please chose more specific names, probaly also
indicating the meaning (EXT could be some "extension" or "external" or
whatever - the name does not indicate that this is a file system type
here.


Best regards,

Wolfgang Denk
Łukasz Majewski July 27, 2012, 1:15 p.m. UTC | #3
Dear Wolfgang Denk,

> Dear Lukasz Majewski,
> 
> In message <1341416922-13792-4-git-send-email-l.majewski@samsung.com>
> you wrote:
> > New, separate driver at ./drivers/dfu has been added. It allows
> > platform and storage independent operation of DFU.
> 
> Sorry for a probably stupid question, but I know basicly zero about
> DFU.   We are talking about "storage independent" here.  Does this
> also mean file system independent?

Some clarification is needed. I've divided DFU support (PATCH v2) to
three separate parts:
1. DFU transmission handling (via USB)
with ./drivers/usb/gadget/g_dnl.c|f_dfu.c

2. Generic DFU functions ./drivers/dfu/dfu.c - which try to abstract
DFU operation to be platform independent.
	Generic dfu_{write|read} functions have been defined and are
	accessible from USB code. On the other hand dfu_{write|read}
	calls function pointers dfu->{read|write}_medium(), which points
	to medium specific functions.

3. Code for MMC write/read - dfu_mmc.c. 
It is possible to read/write raw data to MMC (with passing LBA address)
or to file systems (like FAT). For now MMC is only supported. It uses
(in my opinion) "generic" sprintf+run_command() calls, which can be
easily extended. 
To support OneNAND one needs to define dfu_onenand.c file with OneNAND
specific functions.


Considering above, there are already defined "generic" access functions
- dfu_{write|read}.



> 
> > +static char *dfu_get_dev_type(enum dfu_device_type t)
> > +{
> > +	static char *dev_t[] = {NULL, "MMC", "ONENAND", "NAND" };
> > +	return dev_t[t];
> > +}
> 
> So this currently supports MMC, OneNAND and NAND as storage devices?

It currently only supports MMC devices. Others (ONENAND/NAND) have been
added as place holders for future usage.

> 
> > +static char *dfu_get_layout(enum dfu_device_type l)
> > +{
> > +	static char *dfu_layout[] = {NULL, "RAW_ADDR", "FAT",
> > "EXT" };
> > +	return dfu_layout[l];
> > +}
> 
> And FAT (or VFAT?) and EXT (as in EXT2? or EXT3? or ... ?) as file
> systems?
> 
> > +enum dfu_device_type {
> > +	MMC = 1,
> > +	ONENAND,
> > +	NAND
> > +};
> > +
> > +enum dfu_layout {
> > +	RAW_ADDR = 1,
> > +	FAT,
> > +	EXT,
> > +};
> 
> MMC, NAND, FAT and EXT are very generic names that heavily pollute on
> the global name space.  Please chose more specific names, probaly also
> indicating the meaning (EXT could be some "extension" or "external" or
> whatever - the name does not indicate that this is a file system type
> here.

Ok, no problem with this.

> 
> 
> Best regards,
> 
> Wolfgang Denk
>
Wolfgang Denk July 27, 2012, 1:35 p.m. UTC | #4
Dear Lukasz,

In message <20120727151523.41406989@amdc308.digital.local> you wrote:
> 
> Some clarification is needed. I've divided DFU support (PATCH v2) to
> three separate parts:
> 1. DFU transmission handling (via USB)
...
> 2. Generic DFU functions ./drivers/dfu/dfu.c - which try to abstract
> DFU operation to be platform independent.
...
> 3. Code for MMC write/read - dfu_mmc.c. 

OK, than my understanding was mostly correct.

> It is possible to read/write raw data to MMC (with passing LBA address)
> or to file systems (like FAT). For now MMC is only supported. It uses
> (in my opinion) "generic" sprintf+run_command() calls, which can be
> easily extended. 
> To support OneNAND one needs to define dfu_onenand.c file with OneNAND
> specific functions.

Correct.  And adaption for other devices (say, NAND or USB mass
storage) should be trivial as well.

> Considering above, there are already defined "generic" access functions
> - dfu_{write|read}.

OK - but the device specific stuff is only used in the sprintf()
command then.  That's why I recommend to move just this very small
function into a separate file, which can be replaced or removed later.

> > So this currently supports MMC, OneNAND and NAND as storage devices?
> 
> It currently only supports MMC devices. Others (ONENAND/NAND) have been
> added as place holders for future usage.

Yes, I understand.  But then, adding such support looks pretty
straightforward, and even trivial to me.  You provided a pretty clear
infrastructure for this, thanks.

Best regards,

Wolfgang Denk
Łukasz Majewski July 27, 2012, 1:47 p.m. UTC | #5
Dear Wolfgang Denk,

> Correct.  And adaption for other devices (say, NAND or USB mass
> storage) should be trivial as well.


USB Mass Storage is also under development. It is supposed to be a
function (f_ums.c) for g_dnl.c composite gadget.

I will postpone posting it to ML until we agree for the DFU shape (and
thereof the g_dnl composite gadget driver).
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 0197239..c37dcf3 100644
--- a/Makefile
+++ b/Makefile
@@ -271,6 +271,7 @@  LIBS += drivers/pci/libpci.o
 LIBS += drivers/pcmcia/libpcmcia.o
 LIBS += drivers/power/libpower.o
 LIBS += drivers/spi/libspi.o
+LIBS += drivers/dfu/libdfu.o
 ifeq ($(CPU),mpc83xx)
 LIBS += drivers/qe/libqe.o
 LIBS += arch/powerpc/cpu/mpc8xxx/ddr/libddr.o
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile
new file mode 100644
index 0000000..7736485
--- /dev/null
+++ b/drivers/dfu/Makefile
@@ -0,0 +1,43 @@ 
+#
+# Copyright (C) 2012 Samsung Electronics
+# Lukasz Majewski <l.majewski@samsung.com>
+#
+# 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.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)libdfu.o
+
+COBJS-$(CONFIG_DFU_FUNCTION) += dfu.o
+
+SRCS    := $(COBJS-y:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS-y))
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
new file mode 100644
index 0000000..63733fb
--- /dev/null
+++ b/drivers/dfu/dfu.c
@@ -0,0 +1,259 @@ 
+/*
+ * dfu.c -- DFU back-end routines
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * authors: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
+ *	    Lukasz Majewski <l.majewski@samsung.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <mmc.h>
+#include <fat.h>
+#include <dfu.h>
+#include <linux/list.h>
+#include <linux/compiler.h>
+
+static LIST_HEAD(dfu_list);
+static int dfu_alt_num;
+
+static int dfu_find_alt_num(char *s)
+{
+	int i = 0;
+
+	for (; *s; s++)
+		if (*s == ';')
+			i++;
+
+	return ++i;
+}
+
+static char *dfu_extract_entity(char** env)
+{
+	char *s = *env;
+
+	strsep(env, ";");
+	return s;
+}
+
+char *dfu_extract_token(char** e, int *n)
+{
+	char *st = *e;
+
+	debug("%s: %s\n", __func__, st);
+
+	strsep(e, " ");
+	*n = *e - st;
+
+	return st;
+}
+
+static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
+				     dfu_buf[DFU_DATA_BUF_SIZE];
+
+int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
+{
+	static unsigned char *i_buf;
+	static int i_blk_seq_num;
+	long w_size = 0;
+	int ret = 0;
+
+	debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: 0x%p\n",
+	       __func__, dfu->name, buf, size, blk_seq_num, i_buf);
+
+	if (blk_seq_num == 0) {
+		memset(dfu_buf, '\0', sizeof(dfu_buf));
+		i_buf = dfu_buf;
+		i_blk_seq_num = 0;
+	}
+
+	if (i_blk_seq_num++ != blk_seq_num) {
+		printf("%s: Wrong sequence number! [%d] [%d]\n",
+		       __func__, i_blk_seq_num, blk_seq_num);
+		return -1;
+	}
+
+	memcpy(i_buf, buf, size);
+	i_buf += size;
+
+	if (size == 0) {
+		/* Integrity check (if needed) */
+		debug("%s: %s %d [B] CRC32: 0x%x\n", __func__, dfu->name,
+		       i_buf - dfu_buf, crc32(0, dfu_buf, i_buf - dfu_buf));
+
+		w_size = i_buf - dfu_buf;
+		ret = dfu->write_medium(dfu, dfu_buf, &w_size);
+		if (ret)
+			debug("%s: Write error!\n", __func__);
+
+		i_blk_seq_num = 0;
+		i_buf = NULL;
+		return ret;
+	}
+
+	return ret;
+}
+
+int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
+{
+	static unsigned char *i_buf;
+	static int i_blk_seq_num;
+	static long r_size;
+	static u32 crc;
+	int ret = 0;
+
+	debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: 0x%p\n",
+	       __func__, dfu->name, buf, size, blk_seq_num, i_buf);
+
+	if (blk_seq_num == 0) {
+		i_buf = dfu_buf;
+		memset(dfu_buf, '\0', sizeof(dfu_buf));
+		ret = dfu->read_medium(dfu, i_buf, &r_size);
+		debug("%s: %s %ld [B]\n", __func__, dfu->name, r_size);
+		i_blk_seq_num = 0;
+		/* Integrity check (if needed) */
+		crc = crc32(0, dfu_buf, r_size);
+	}
+
+	if (i_blk_seq_num++ != blk_seq_num) {
+		printf("%s: Wrong sequence number! [%d] [%d]\n",
+		       __func__, i_blk_seq_num, blk_seq_num);
+		return -1;
+	}
+
+	if (r_size >= size) {
+		memcpy(buf, i_buf, size);
+		i_buf += size;
+		r_size -= size;
+		return size;
+	} else {
+		memcpy(buf, i_buf, r_size);
+		i_buf += r_size;
+		debug("%s: %s CRC32: 0x%x\n", __func__, dfu->name, crc);
+		puts("UPLOAD ... done\n");
+		puts("Ctrl+C to exit ...\n");
+
+		i_buf = NULL;
+		i_blk_seq_num = 0;
+		crc = 0;
+		return r_size;
+	}
+	return ret;
+}
+
+static int dfu_fill_entity(struct dfu_entity *dfu, char* s, int alt,
+			    char *interface, int num)
+{
+	char *st = NULL;
+	int n = 0;
+
+	debug("%s: %s interface: %s num: %d\n", __func__, s, interface, num);
+
+	st = dfu_extract_token(&s, &n);
+	strncpy((char *) &dfu->name, st, n);
+
+	dfu->dev_num = num;
+	dfu->alt = alt;
+
+	/* Specific for mmc device */
+	if (strcmp(interface, "mmc") == 0) {
+		if (dfu_fill_entity_mmc(dfu, s))
+			return -1;
+	} else {
+		printf("dfu: Device %s not supported\n", interface);
+		return -1;
+	}
+
+	return 0;
+}
+
+int dfu_config_entities(char *env, char *interface, int num)
+{
+	struct dfu_entity *dfu;
+	int i, ret;
+	char *s;
+
+	dfu_alt_num = dfu_find_alt_num(env);
+	debug("%s: dfu_alt_num=%d\n", __func__, dfu_alt_num);
+
+	for (i = 0; i < dfu_alt_num; i++) {
+		dfu = calloc(sizeof(struct dfu_entity), 1);
+
+		s = dfu_extract_entity(&env);
+		ret = dfu_fill_entity(dfu, s, i, interface, num);
+		if (ret)
+			return -1;
+
+		list_add_tail(&dfu->list, &dfu_list);
+	}
+
+	return 0;
+}
+
+void dfu_free_entities(void)
+{
+	struct dfu_entity *dfu = NULL, *p = NULL;
+
+	list_for_each_entry_safe_reverse(dfu, p, &dfu_list, list) {
+		list_del(&dfu->list);
+		free(dfu);
+	}
+
+	INIT_LIST_HEAD(&dfu_list);
+}
+
+static char *dfu_get_dev_type(enum dfu_device_type t)
+{
+	static char *dev_t[] = {NULL, "MMC", "ONENAND", "NAND" };
+	return dev_t[t];
+}
+
+static char *dfu_get_layout(enum dfu_device_type l)
+{
+	static char *dfu_layout[] = {NULL, "RAW_ADDR", "FAT", "EXT" };
+	return dfu_layout[l];
+}
+
+void dfu_show_entities(void)
+{
+	struct dfu_entity *dfu = NULL;
+
+	puts("DFU alt settings list:\n");
+
+	list_for_each_entry(dfu, &dfu_list, list) {
+		printf("dev: %s alt: %d name: %s layout: %s\n",
+		       dfu_get_dev_type(dfu->dev_type), dfu->alt,
+		       dfu->name, dfu_get_layout(dfu->layout));
+	}
+}
+
+int dfu_get_alt_number(void)
+{
+	return dfu_alt_num;
+}
+
+struct dfu_entity *dfu_get_entity(int alt)
+{
+	struct dfu_entity *dfu = NULL;
+
+	list_for_each_entry(dfu, &dfu_list, list) {
+		if (dfu->alt == alt)
+			return dfu;
+	}
+
+	return NULL;
+}
diff --git a/include/dfu.h b/include/dfu.h
new file mode 100644
index 0000000..f7d823b
--- /dev/null
+++ b/include/dfu.h
@@ -0,0 +1,99 @@ 
+/*
+ * dfu.h - DFU flashable area description
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * authors: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
+ *	    Lukasz Majewski <l.majewski@samsung.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __DFU_ENTITY_H_
+#define __DFU_ENTITY_H_
+
+#include <common.h>
+#include <linux/list.h>
+#include <mmc.h>
+
+enum dfu_device_type {
+	MMC = 1,
+	ONENAND,
+	NAND
+};
+
+enum dfu_layout {
+	RAW_ADDR = 1,
+	FAT,
+	EXT,
+};
+
+struct mmc_internal_data {
+	/* RAW programming */
+	unsigned int lba_start;
+	unsigned int lba_size;
+	unsigned int lba_blk_size;
+
+	/* FAT/EXT */
+	unsigned int dev;
+	unsigned int part;
+};
+
+static inline unsigned int get_mmc_blk_size(int dev)
+{
+	return find_mmc_device(dev)->read_bl_len;
+}
+
+#define DFU_NAME_SIZE 32
+#define DFU_CMD_BUF_SIZE 128
+#define DFU_DATA_BUF_SIZE (1024*1024*4) /* 4 MiB */
+
+struct dfu_entity {
+	char			name[DFU_NAME_SIZE];
+	int                     alt;
+	void                    *dev_private;
+	int                     dev_num;
+	enum dfu_device_type    dev_type;
+	enum dfu_layout         layout;
+
+	union {
+		struct mmc_internal_data mmc;
+	} data;
+
+	int (*read_medium)(struct dfu_entity *dfu, void *buf, long *len);
+	int (*write_medium)(struct dfu_entity *dfu, void *buf, long *len);
+
+	struct list_head list;
+};
+
+int dfu_config_entities(char *s, char *interface, int num);
+void dfu_free_entities(void);
+void dfu_show_entities(void);
+int dfu_get_alt_number(void);
+struct dfu_entity *dfu_get_entity(int alt);
+char *dfu_extract_token(char** e, int *n);
+
+int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
+int dfu_write(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
+/* Device specific */
+#ifdef CONFIG_DFU_MMC
+extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char* s);
+#else
+static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char* s)
+{
+	puts("MMC support not available!\n");
+	return -1;
+}
+#endif
+#endif /* __DFU_ENTITY_H_ */