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
