Dear Lukasz Majewski, > Hi Marek, > > > 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.majew...@samsung.com> > > > Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com> > > > Cc: Marek Vasut <ma...@denx.de> > > > --- > > > > [...] > > > > > +#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; > > > +} > > > > Shall we not make these all generic? They seem to be quite helpful > > components. > > It is a good topic for a discussion if those functions shall be moved > to ./lib/string.c. > I regarded them as a "little" helper functions for parsing DFU alt > setting env variable. Those are very short and build with methods > exported from string.c
Good point > > > + > > > +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]; > > > > Can we not stack-allocate it with ALLOC_CACHE_ALIGN_BUFFER()? > > The dfu_buf is 4 MiB (this is the size of DFU_DATA_BUF_SIZE). I don't > think, that allocating it on the stack (for stack allocation the > ALLOC_CACHE_ALIGN_BUFFER() is designed) is a good idea. Heh, agreed :-) > > [...] > > > > > diff --git a/include/dfu.h b/include/dfu.h > > > new file mode 100644 > > > index 0000000..f7d823b > > > --- /dev/null > > > +++ b/include/dfu.h > > > > [...] > > > > > +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; > > > > This union seems redundant ;-) > > Good point :-), but I predict, that DFU will be used to program other > memory types (OneNAND, or NAND). To support those, one needs to extend > the union with e.g struct onenand_internal_data onenand. > > Since we don't have so many memory types, I think that union usage is > acceptable. And will those pieces be implemented any soon ? :) > > [...] Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot