[U-Boot,RFC] dfu: ram support

DFU spec mentions it as a method to upgrade firmware (software stored
in writable non-volatile memory). It also says other potential uses of
DFU is beyond scope of the spec.

Here such a beyond the scope use is being attempted - directly pumping
binary images from host via USB to RAM. This facility is a developer
centric one in that it gives advantage over upgrading non-volatile
memory for testing new images every time during development and/or
testing.

Directly putting image onto RAM would speed up upgrade process. This and
convenience was the initial thoughts that led to doing this, speed
improvement over MMC was only 1 second though - 6 sec on RAM as opposed
to 7 sec on MMC in beagle bone, perhaps enabling cache and/or optimizing
DFU framework to avoid multiple copy for ram (if worth) may help, and
on other platforms and other boot media like NAND maybe improvement
would be higher.

And for a platform that doesn't yet have proper DFU suppport for
non-volatile media's, DFU to RAM can be used.

Another minor advantage would be to increase life of mmc/nand as it
would be less used during development/testing.

usage: <image name> ram <start address> <size>
eg. kernel ram 0x81000000 0x1000000

Downloading images to RAM using DFU is not something new, this is
acheived in openmoko also.

DFU on RAM can be used for extracting RAM contents to host using dfu
upload. Perhaps this can be extended to io for squeezing out register
dump through usb, if it is worth.

Signed-off-by: Afzal Mohammed <afzal.mohd.ma@gmail.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Heiko Schocher <hs@denx.de>
---
 drivers/dfu/Makefile  |  1 +
 drivers/dfu/dfu.c     |  7 +++--
 drivers/dfu/dfu_ram.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/dfu.h         | 18 +++++++++++
 4 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 drivers/dfu/dfu_ram.c

Hi Afzal Mohammed,

> DFU spec mentions it as a method to upgrade firmware (software stored
> in writable non-volatile memory). It also says other potential uses of
> DFU is beyond scope of the spec.
> 
> Here such a beyond the scope use is being attempted - directly pumping
> binary images from host via USB to RAM. This facility is a developer
> centric one in that it gives advantage over upgrading non-volatile
> memory for testing new images every time during development and/or
> testing.
> 
> Directly putting image onto RAM would speed up upgrade process. This
> and convenience was the initial thoughts that led to doing this, speed
> improvement over MMC was only 1 second though - 6 sec on RAM as
> opposed to 7 sec on MMC in beagle bone, perhaps enabling cache and/or
> optimizing DFU framework to avoid multiple copy for ram (if worth)
> may help, and on other platforms and other boot media like NAND maybe
> improvement would be higher.
> 
> And for a platform that doesn't yet have proper DFU suppport for
> non-volatile media's, DFU to RAM can be used.
> 
> Another minor advantage would be to increase life of mmc/nand as it
> would be less used during development/testing.
> 
> usage: <image name> ram <start address> <size>
> eg. kernel ram 0x81000000 0x1000000
> 
> Downloading images to RAM using DFU is not something new, this is
> acheived in openmoko also.
> 
> DFU on RAM can be used for extracting RAM contents to host using dfu
> upload. Perhaps this can be extended to io for squeezing out register
> dump through usb, if it is worth.
> 

Above idea sounds very interesting. 
One minor thing: 
It also would be good to have dfu_alt_info environment properly defined
to have "ram" alt setting for beagle bone. Then we would have at least
one board which supports this new feature.

> Signed-off-by: Afzal Mohammed <afzal.mohd.ma@gmail.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Heiko Schocher <hs@denx.de>
> ---
>  drivers/dfu/Makefile  |  1 +
>  drivers/dfu/dfu.c     |  7 +++--
>  drivers/dfu/dfu_ram.c | 82
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> include/dfu.h         | 18 +++++++++++ 4 files changed, 106
> insertions(+), 2 deletions(-) create mode 100644 drivers/dfu/dfu_ram.c
> 
> diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile
> index fca370a..de9e44e 100644
> --- a/drivers/dfu/Makefile
> +++ b/drivers/dfu/Makefile
> @@ -12,6 +12,7 @@ LIB	= $(obj)libdfu.o
>  COBJS-$(CONFIG_DFU_FUNCTION) += dfu.o
>  COBJS-$(CONFIG_DFU_MMC) += dfu_mmc.o
>  COBJS-$(CONFIG_DFU_NAND) += dfu_nand.o
> +COBJS-$(CONFIG_DFU_RAM) += dfu_ram.o
>  
>  SRCS    := $(COBJS-y:.o=.c)
>  OBJS	:= $(addprefix $(obj),$(COBJS-y))
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index d73d510..7b3d05d 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -325,6 +325,9 @@ static int dfu_fill_entity(struct dfu_entity
> *dfu, char *s, int alt, } else if (strcmp(interface, "nand") == 0) {
>  		if (dfu_fill_entity_nand(dfu, s))
>  			return -1;
> +	} else if (strcmp(interface, "ram") == 0) {
> +		if (dfu_fill_entity_ram(dfu, s))
> +			return -1;
>  	} else {
>  		printf("%s: Device %s not (yet) supported!\n",
>  		       __func__,  interface);
> @@ -374,14 +377,14 @@ int dfu_config_entities(char *env, char
> *interface, int num) 
>  const char *dfu_get_dev_type(enum dfu_device_type t)
>  {
> -	const char *dev_t[] = {NULL, "eMMC", "OneNAND", "NAND" };
> +	const char *dev_t[] = {NULL, "eMMC", "OneNAND", "NAND",
> "RAM" }; return dev_t[t];
>  }
>  
>  const char *dfu_get_layout(enum dfu_layout l)
>  {
>  	const char *dfu_layout[] = {NULL, "RAW_ADDR", "FAT", "EXT2",
> -					   "EXT3", "EXT4" };
> +					   "EXT3", "EXT4",
> "RAM_ADDR" }; return dfu_layout[l];
>  }
>  
> diff --git a/drivers/dfu/dfu_ram.c b/drivers/dfu/dfu_ram.c
> new file mode 100644
> index 0000000..8562495
> --- /dev/null
> +++ b/drivers/dfu/dfu_ram.c
> @@ -0,0 +1,82 @@
> +/*
> + * (C) Copyright 2013
> + * Afzal Mohammed <afzal.mohd.ma@gmail.com>
> + *
> + * Reference: dfu_mmc.c
> + * Copyright (C) 2012 Samsung Electronics
> + * author: Lukasz Majewski <l.majewski@samsung.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <malloc.h>
> +#include <errno.h>
> +#include <dfu.h>
> +
> +enum dfu_ram_op {
> +	DFU_OP_READ = 1,
> +	DFU_OP_WRITE,
> +};

Minor:
Now I've realised that the dfu_nand_op and dfu_mmc_op have the same
defines. Maybe it is a good time to combine this and store it at
dfu.h?

> +
> +static int dfu_transfer_medium_ram(enum dfu_ram_op op, struct
> dfu_entity *dfu,
> +				   u64 offset, void *buf, long *len)
> +{
> +	if (dfu->layout != DFU_RAM_ADDR) {
> +		printf("%s: unsupported layout :%s\n", __func__,
> +			dfu_get_layout(dfu->layout));
> +		return  -EINVAL;
> +	}
> +
> +	if (offset > dfu->data.ram.size) {
> +		printf("%s: request exceeds allowed area\n",
> __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (op == DFU_OP_WRITE)
> +		memcpy(dfu->data.ram.start + offset, buf, *len);
> +	else
> +		memcpy(buf, dfu->data.ram.start + offset, *len);
> +
> +	return 0;
> +}
> +
> +static int dfu_write_medium_ram(struct dfu_entity *dfu, u64 offset,
> +				void *buf, long *len)
> +{
> +	return dfu_transfer_medium_ram(DFU_OP_WRITE, dfu, offset,
> buf, len); +}
> +
> +static int dfu_read_medium_ram(struct dfu_entity *dfu, u64 offset,
> +			       void *buf, long *len)
> +{
> +	if (!*len) {
> +		*len = dfu->data.ram.size;
> +		return 0;
> +	}
> +
> +	return dfu_transfer_medium_ram(DFU_OP_READ, dfu, offset,
> buf, len); +}
> +
> +int dfu_fill_entity_ram(struct dfu_entity *dfu, char *s)
> +{
> +	char *st;
> +
> +	dfu->dev_type = DFU_DEV_RAM;
> +	st = strsep(&s, " ");
> +	if (strcmp(st, "ram")) {
> +		printf("%s: unsupported device: %s\n", __func__, st);
> +		return -ENODEV;
> +	}
> +
> +	dfu->layout = DFU_RAM_ADDR;
> +	dfu->data.ram.start = (void *)simple_strtoul(s, &s, 16);
> +	dfu->data.ram.size = simple_strtoul(++s, &s, 16);
> +
> +	dfu->write_medium = dfu_write_medium_ram;
> +	dfu->read_medium = dfu_read_medium_ram;
> +
> +	dfu->inited = 0;
> +
> +	return 0;
> +}
> diff --git a/include/dfu.h b/include/dfu.h
> index 47b9055..18d288d 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -19,6 +19,7 @@ enum dfu_device_type {
>  	DFU_DEV_MMC = 1,
>  	DFU_DEV_ONENAND,
>  	DFU_DEV_NAND,
> +	DFU_DEV_RAM,
>  };
>  
>  enum dfu_layout {
> @@ -27,6 +28,7 @@ enum dfu_layout {
>  	DFU_FS_EXT2,
>  	DFU_FS_EXT3,
>  	DFU_FS_EXT4,
> +	DFU_RAM_ADDR,
>  };
>  
>  struct mmc_internal_data {
> @@ -51,6 +53,11 @@ struct nand_internal_data {
>  	unsigned int ubi;
>  };
>  
> +struct ram_internal_data {
> +	void		*start;
> +	unsigned int	size;
> +};
> +
>  static inline unsigned int get_mmc_blk_size(int dev)
>  {
>  	return find_mmc_device(dev)->read_bl_len;
> @@ -76,6 +83,7 @@ struct dfu_entity {
>  	union {
>  		struct mmc_internal_data mmc;
>  		struct nand_internal_data nand;
> +		struct ram_internal_data ram;
>  	} data;
>  
>  	int (*read_medium)(struct dfu_entity *dfu,
> @@ -137,4 +145,14 @@ static inline int dfu_fill_entity_nand(struct
> dfu_entity *dfu, char *s) }
>  #endif
>  
> +#ifdef CONFIG_DFU_RAM
> +extern int dfu_fill_entity_ram(struct dfu_entity *dfu, char *s);
> +#else
> +static inline int dfu_fill_entity_ram(struct dfu_entity *dfu, char
> *s) +{
> +	puts("RAM support not available!\n");
> +	return -1;
> +}
> +#endif
> +
>  #endif /* __DFU_ENTITY_H_ */

Despite one minor comment, I like the code and looking forward for a
patch :-).

Hi Lukasz Majewski,

On Mon, Sep 09, 2013 at 08:50:58AM +0200, Lukasz Majewski wrote:

> > usage: <image name> ram <start address> <size>
> > eg. kernel ram 0x81000000 0x1000000
> > 
> > Downloading images to RAM using DFU is not something new, this is
> > acheived in openmoko also.
> > 
> > DFU on RAM can be used for extracting RAM contents to host using dfu
> > upload. Perhaps this can be extended to io for squeezing out register
> > dump through usb, if it is worth.

> Above idea sounds very interesting. 

Thanks

> One minor thing: 
> It also would be good to have dfu_alt_info environment properly defined
> to have "ram" alt setting for beagle bone. Then we would have at least
> one board which supports this new feature.

Sure

> > +enum dfu_ram_op {
> > +	DFU_OP_READ = 1,
> > +	DFU_OP_WRITE,
> > +};

> Minor:
> Now I've realised that the dfu_nand_op and dfu_mmc_op have the same
> defines. Maybe it is a good time to combine this and store it at
> dfu.h?

Yes it is better

> Despite one minor comment, I like the code and looking forward for a
> patch :-).

Thanks, will wait for a working day for those on other side of the
globe and if no negative, would repost addressing your comments.

Regards
Afzal