[U-Boot] [PATCH RFC] dfu: ram support

[Lukasz Majewski]

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 at gmail.com>
> Cc: Lukasz Majewski <l.majewski at samsung.com>
> Cc: Pantelis Antoniou <panto at antoniou-consulting.com>
> Cc: Heiko Schocher <hs at 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 at gmail.com>
> + *
> + * Reference: dfu_mmc.c
> + * Copyright (C) 2012 Samsung Electronics
> + * author: Lukasz Majewski <l.majewski at 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 :-).

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group