[U-Boot] [PATCH 0/4] dfu: Various fixes and code optimizations

[U-Boot] [PATCH 0/4] dfu: Various fixes and code optimizations

[Lukasz Majewski]

Various DFU code fixes and optimizations were enclosed by this
patch series.

Most notable change is to move "dfu_alt_info" handling code to
dfu core.

Lukasz Majewski (4):
  dfu:cosmetic: Fix printf text for buffer overflow condition
  dfu: Make maximum DFU file size equal to default DFU data buffer
  dfu: Find DFU alt setting number by passing its name
  dfu: Extract common DFU code to handle "dfu_alt_info" environment
    variable

 common/cmd_dfu.c  |   16 ++--------------
 drivers/dfu/dfu.c |   39 +++++++++++++++++++++++++++++++++++++--
 include/dfu.h     |    4 +++-
 3 files changed, 42 insertions(+), 17 deletions(-)

-- 
1.7.10.4

[U-Boot] [PATCH 1/4] dfu:cosmetic: Fix printf text for buffer overflow condition

[Lukasz Majewski]

Correct error message if overflow is detected.

Change-Id: I8a915c7353d49822c046fbc36241237b370e6c98
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
---
 drivers/dfu/dfu.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index d73d510..2f1e2af 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -153,8 +153,8 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 
 	/* we should be in buffer now (if not then size too large) */
 	if ((dfu->i_buf + size) > dfu->i_buf_end) {
-		printf("%s: Wrong size! [%d] [%d] - %d\n",
-		       __func__, dfu->i_blk_seq_num, blk_seq_num, size);
+		error("Buffer overflow! (0x%p + 0x%x > 0x%p)\n", dfu->i_buf,
+		      size, dfu->i_buf_end);
 		return -1;
 	}
 
-- 
1.7.10.4

[U-Boot] [PATCH 2/4] dfu: Make maximum DFU file size equal to default DFU data buffer

[Lukasz Majewski]

Up till now the DFU maximum file size (to be written to e.g. eMMC)
was different from the DFU data buffer size. It caused errors when
one buffer was smaller than data to be written.

Now, the maximum DFU file size is equal to default DFU buffer size.
In spite of this, user is still able to manually adjust those default
values.

Change-Id: Ied75d0f7b59588ebd79dae9a22af801d36622216
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
---
 include/dfu.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/dfu.h b/include/dfu.h
index 47b9055..7779710 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -62,7 +62,7 @@ static inline unsigned int get_mmc_blk_size(int dev)
 #define CONFIG_SYS_DFU_DATA_BUF_SIZE		(1024*1024*8)	/* 8 MiB */
 #endif
 #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE
-#define CONFIG_SYS_DFU_MAX_FILE_SIZE	(4 << 20)	/* 4 MiB */
+#define CONFIG_SYS_DFU_MAX_FILE_SIZE CONFIG_SYS_DFU_DATA_BUF_SIZE
 #endif
 
 struct dfu_entity {
-- 
1.7.10.4

[U-Boot] [PATCH 3/4] dfu: Find DFU alt setting number by passing its name

[Lukasz Majewski]

New function - dfu_get_alt() has been added to dfu core. If proper
alt setting is present, this function returns its number
corresponding to passed name.

Change-Id: Icd75f3aa3a6f6e306c77b28cabe620e4e6a253ea
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
---
 drivers/dfu/dfu.c |   12 ++++++++++++
 include/dfu.h     |    1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 2f1e2af..180d083 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -414,3 +414,15 @@ struct dfu_entity *dfu_get_entity(int alt)
 
 	return NULL;
 }
+
+int dfu_get_alt(const char *name)
+{
+	struct dfu_entity *dfu;
+
+	list_for_each_entry(dfu, &dfu_list, list) {
+		if (!strncmp(dfu->name, name, strlen(dfu->name)))
+			return dfu->alt;
+	}
+
+	return -ENODEV;
+}
diff --git a/include/dfu.h b/include/dfu.h
index 7779710..8838f9c 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -112,6 +112,7 @@ const char *dfu_get_layout(enum dfu_layout l);
 struct dfu_entity *dfu_get_entity(int alt);
 char *dfu_extract_token(char** e, int *n);
 void dfu_trigger_reset(void);
+int dfu_get_alt(const char *name);
 bool dfu_reset(void);
 
 int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
-- 
1.7.10.4

[U-Boot] [PATCH 4/4] dfu: Extract common DFU code to handle "dfu_alt_info" environment variable

[Lukasz Majewski]

New dfu_init_env_entities() function has been extracted from cmd_dfu.c and
stored at dfu core.

This is a dfu centric code, so it shall be processed in the core.

Change-Id: I756c5de922fa31399d8804eaadc004ee98844ec2
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
---
 common/cmd_dfu.c  |   16 ++--------------
 drivers/dfu/dfu.c |   23 +++++++++++++++++++++++
 include/dfu.h     |    1 +
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
index 793c422..d3658cf 100644
--- a/common/cmd_dfu.c
+++ b/common/cmd_dfu.c
@@ -17,26 +17,15 @@
 
 static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	const char *str_env;
 	char *s = "dfu";
 	int ret, i = 0;
-	char *env_bkp;
 
 	if (argc < 3)
 		return CMD_RET_USAGE;
 
-	str_env = getenv("dfu_alt_info");
-	if (str_env == NULL) {
-		printf("%s: \"dfu_alt_info\" env variable not defined!\n",
-		       __func__);
-		return CMD_RET_FAILURE;
-	}
-
-	env_bkp = strdup(str_env);
-	ret = dfu_config_entities(env_bkp, argv[1],
-			    (int)simple_strtoul(argv[2], NULL, 10));
+	ret = dfu_init_env_entities(argv[1], simple_strtoul(argv[2], NULL, 10));
 	if (ret)
-		return CMD_RET_FAILURE;
+		return ret;
 
 	if (argc > 3 && strcmp(argv[3], "list") == 0) {
 		dfu_show_entities();
@@ -67,7 +56,6 @@ exit:
 	g_dnl_unregister();
 done:
 	dfu_free_entities();
-	free(env_bkp);
 
 	if (dfu_reset())
 		run_command("reset", 0);
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 180d083..8eca5db 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -41,6 +41,29 @@ static int dfu_find_alt_num(const char *s)
 	return ++i;
 }
 
+int dfu_init_env_entities(char *interface, int dev)
+{
+	const char *str_env;
+	char *env_bkp;
+	int ret;
+
+	str_env = getenv("dfu_alt_info");
+	if (!str_env) {
+		error("\"dfu_alt_info\" env variable not defined!\n");
+		return -EINVAL;
+	}
+
+	env_bkp = strdup(str_env);
+	ret = dfu_config_entities(env_bkp, interface, dev);
+	if (ret) {
+		error("DFU entities configuration failed!\n");
+		return ret;
+	}
+
+	free(env_bkp);
+	return 0;
+}
+
 static unsigned char *dfu_buf;
 static unsigned long dfu_buf_size = CONFIG_SYS_DFU_DATA_BUF_SIZE;
 
diff --git a/include/dfu.h b/include/dfu.h
index 8838f9c..58db1ef 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -114,6 +114,7 @@ char *dfu_extract_token(char** e, int *n);
 void dfu_trigger_reset(void);
 int dfu_get_alt(const char *name);
 bool dfu_reset(void);
+int dfu_init_env_entities(char *interface, int dev);
 
 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);
-- 
1.7.10.4

[U-Boot] [PATCH 3/4] dfu: Find DFU alt setting number by passing its name

[Marek Vasut]

Dear Lukasz Majewski,

> New function - dfu_get_alt() has been added to dfu core. If proper
> alt setting is present, this function returns its number
> corresponding to passed name.
> 
> Change-Id: Icd75f3aa3a6f6e306c77b28cabe620e4e6a253ea
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> ---
>  drivers/dfu/dfu.c |   12 ++++++++++++
>  include/dfu.h     |    1 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 2f1e2af..180d083 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -414,3 +414,15 @@ struct dfu_entity *dfu_get_entity(int alt)
> 
>  	return NULL;
>  }
> +
> +int dfu_get_alt(const char *name)
> +{
> +	struct dfu_entity *dfu;
> +
> +	list_for_each_entry(dfu, &dfu_list, list) {
> +		if (!strncmp(dfu->name, name, strlen(dfu->name)))
> +			return dfu->alt;
> +	}
> +
> +	return -ENODEV;
> +}
> diff --git a/include/dfu.h b/include/dfu.h
> index 7779710..8838f9c 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -112,6 +112,7 @@ const char *dfu_get_layout(enum dfu_layout l);
>  struct dfu_entity *dfu_get_entity(int alt);
>  char *dfu_extract_token(char** e, int *n);
>  void dfu_trigger_reset(void);
> +int dfu_get_alt(const char *name);
>  bool dfu_reset(void);
> 
>  int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num);

Is this code used anywhere ?

Best regards,
Marek Vasut

[U-Boot] [PATCH 3/4] dfu: Find DFU alt setting number by passing its name

[Lukasz Majewski]

Hi Marek,

> Dear Lukasz Majewski,
> 
> > New function - dfu_get_alt() has been added to dfu core. If proper
> > alt setting is present, this function returns its number
> > corresponding to passed name.
> > 
> > Change-Id: Icd75f3aa3a6f6e306c77b28cabe620e4e6a253ea
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > ---
> >  drivers/dfu/dfu.c |   12 ++++++++++++
> >  include/dfu.h     |    1 +
> >  2 files changed, 13 insertions(+)
> > 
> > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> > index 2f1e2af..180d083 100644
> > --- a/drivers/dfu/dfu.c
> > +++ b/drivers/dfu/dfu.c
> > @@ -414,3 +414,15 @@ struct dfu_entity *dfu_get_entity(int alt)
> > 
> >  	return NULL;
> >  }
> > +
> > +int dfu_get_alt(const char *name)
> > +{
> > +	struct dfu_entity *dfu;
> > +
> > +	list_for_each_entry(dfu, &dfu_list, list) {
> > +		if (!strncmp(dfu->name, name, strlen(dfu->name)))
> > +			return dfu->alt;
> > +	}
> > +
> > +	return -ENODEV;
> > +}
> > diff --git a/include/dfu.h b/include/dfu.h
> > index 7779710..8838f9c 100644
> > --- a/include/dfu.h
> > +++ b/include/dfu.h
> > @@ -112,6 +112,7 @@ const char *dfu_get_layout(enum dfu_layout l);
> >  struct dfu_entity *dfu_get_entity(int alt);
> >  char *dfu_extract_token(char** e, int *n);
> >  void dfu_trigger_reset(void);
> > +int dfu_get_alt(const char *name);
> >  bool dfu_reset(void);
> > 
> >  int dfu_read(struct dfu_entity *de, void *buf, int size, int
> > blk_seq_num);
> 
> Is this code used anywhere ?

Hmm.... By mistake I've added part of my ongoing USB related work to
DFU fixes (as you see it is DFU related).

Since it is (for now) a dead code, please don't consider this patch.

Shall I prepare v2 without this patch or will you be so kind and
review other patches in the current patch set?



> 
> Best regards,
> Marek Vasut


-- 
Best regards,

Lukasz Majewski

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

[U-Boot] [PATCH 3/4] dfu: Find DFU alt setting number by passing its name

[Marek Vasut]

Dear Lukasz Majewski,

> Hi Marek,
> 
> > Dear Lukasz Majewski,
> > 
> > > New function - dfu_get_alt() has been added to dfu core. If proper
> > > alt setting is present, this function returns its number
> > > corresponding to passed name.
> > > 
> > > Change-Id: Icd75f3aa3a6f6e306c77b28cabe620e4e6a253ea
> > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > > ---
> > > 
> > >  drivers/dfu/dfu.c |   12 ++++++++++++
> > >  include/dfu.h     |    1 +
> > >  2 files changed, 13 insertions(+)
> > > 
> > > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> > > index 2f1e2af..180d083 100644
> > > --- a/drivers/dfu/dfu.c
> > > +++ b/drivers/dfu/dfu.c
> > > @@ -414,3 +414,15 @@ struct dfu_entity *dfu_get_entity(int alt)
> > > 
> > >  	return NULL;
> > >  
> > >  }
> > > 
> > > +
> > > +int dfu_get_alt(const char *name)
> > > +{
> > > +	struct dfu_entity *dfu;
> > > +
> > > +	list_for_each_entry(dfu, &dfu_list, list) {
> > > +		if (!strncmp(dfu->name, name, strlen(dfu->name)))
> > > +			return dfu->alt;
> > > +	}
> > > +
> > > +	return -ENODEV;
> > > +}
> > > diff --git a/include/dfu.h b/include/dfu.h
> > > index 7779710..8838f9c 100644
> > > --- a/include/dfu.h
> > > +++ b/include/dfu.h
> > > @@ -112,6 +112,7 @@ const char *dfu_get_layout(enum dfu_layout l);
> > > 
> > >  struct dfu_entity *dfu_get_entity(int alt);
> > >  char *dfu_extract_token(char** e, int *n);
> > >  void dfu_trigger_reset(void);
> > > 
> > > +int dfu_get_alt(const char *name);
> > > 
> > >  bool dfu_reset(void);
> > >  
> > >  int dfu_read(struct dfu_entity *de, void *buf, int size, int
> > > 
> > > blk_seq_num);
> > 
> > Is this code used anywhere ?
> 
> Hmm.... By mistake I've added part of my ongoing USB related work to
> DFU fixes (as you see it is DFU related).
> 
> Since it is (for now) a dead code, please don't consider this patch.
> 
> Shall I prepare v2 without this patch or will you be so kind and
> review other patches in the current patch set?

Rest is OK, I'd like to know Heiko's opinion too before applying though.

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/4] dfu:cosmetic: Fix printf text for buffer overflow condition

[Heiko Schocher]

Hello Lukasz,

Am 10.09.2013 15:29, schrieb Lukasz Majewski:
> Correct error message if overflow is detected.
>
> Change-Id: I8a915c7353d49822c046fbc36241237b370e6c98
> Signed-off-by: Lukasz Majewski<l.majewski@samsung.com>
> ---
>   drivers/dfu/dfu.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Tested on the dxr2 and rut board, so:

Tested-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH 2/4] dfu: Make maximum DFU file size equal to default DFU data buffer

[Heiko Schocher]

Hello Lukasz,

Am 10.09.2013 15:29, schrieb Lukasz Majewski:
> Up till now the DFU maximum file size (to be written to e.g. eMMC)
> was different from the DFU data buffer size. It caused errors when
> one buffer was smaller than data to be written.
>
> Now, the maximum DFU file size is equal to default DFU buffer size.
> In spite of this, user is still able to manually adjust those default
> values.
>
> Change-Id: Ied75d0f7b59588ebd79dae9a22af801d36622216
> Signed-off-by: Lukasz Majewski<l.majewski@samsung.com>
> ---
>   include/dfu.h |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Tested on the dxr2 and rut board, so:

Tested-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH 4/4] dfu: Extract common DFU code to handle "dfu_alt_info" environment variable

[Heiko Schocher]

Hello Lukasz,

Am 10.09.2013 15:29, schrieb Lukasz Majewski:
> New dfu_init_env_entities() function has been extracted from cmd_dfu.c and
> stored at dfu core.
>
> This is a dfu centric code, so it shall be processed in the core.
>
> Change-Id: I756c5de922fa31399d8804eaadc004ee98844ec2
> Signed-off-by: Lukasz Majewski<l.majewski@samsung.com>
> ---
>   common/cmd_dfu.c  |   16 ++--------------
>   drivers/dfu/dfu.c |   23 +++++++++++++++++++++++
>   include/dfu.h     |    1 +
>   3 files changed, 26 insertions(+), 14 deletions(-)

Tested on the dxr2 and rut board, so:

Tested-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH 3/4] dfu: Find DFU alt setting number by passing its name

[Heiko Schocher]

Hello Marek,

Am 10.09.2013 23:20, schrieb Marek Vasut:
> Dear Lukasz Majewski,
>
>> Hi Marek,
>>
>>> Dear Lukasz Majewski,
>>>
>>>> New function - dfu_get_alt() has been added to dfu core. If proper
>>>> alt setting is present, this function returns its number
>>>> corresponding to passed name.
>>>>
>>>> Change-Id: Icd75f3aa3a6f6e306c77b28cabe620e4e6a253ea
>>>> Signed-off-by: Lukasz Majewski<l.majewski@samsung.com>
>>>> ---
>>>>
>>>>   drivers/dfu/dfu.c |   12 ++++++++++++
>>>>   include/dfu.h     |    1 +
>>>>   2 files changed, 13 insertions(+)
>>>>
>>>> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
>>>> index 2f1e2af..180d083 100644
>>>> --- a/drivers/dfu/dfu.c
>>>> +++ b/drivers/dfu/dfu.c
>>>> @@ -414,3 +414,15 @@ struct dfu_entity *dfu_get_entity(int alt)
>>>>
>>>>   	return NULL;
>>>>
>>>>   }
>>>>
>>>> +
>>>> +int dfu_get_alt(const char *name)
>>>> +{
>>>> +	struct dfu_entity *dfu;
>>>> +
>>>> +	list_for_each_entry(dfu,&dfu_list, list) {
>>>> +		if (!strncmp(dfu->name, name, strlen(dfu->name)))
>>>> +			return dfu->alt;
>>>> +	}
>>>> +
>>>> +	return -ENODEV;
>>>> +}
>>>> diff --git a/include/dfu.h b/include/dfu.h
>>>> index 7779710..8838f9c 100644
>>>> --- a/include/dfu.h
>>>> +++ b/include/dfu.h
>>>> @@ -112,6 +112,7 @@ const char *dfu_get_layout(enum dfu_layout l);
>>>>
>>>>   struct dfu_entity *dfu_get_entity(int alt);
>>>>   char *dfu_extract_token(char** e, int *n);
>>>>   void dfu_trigger_reset(void);
>>>>
>>>> +int dfu_get_alt(const char *name);
>>>>
>>>>   bool dfu_reset(void);
>>>>
>>>>   int dfu_read(struct dfu_entity *de, void *buf, int size, int
>>>>
>>>> blk_seq_num);
>>>
>>> Is this code used anywhere ?
>>
>> Hmm.... By mistake I've added part of my ongoing USB related work to
>> DFU fixes (as you see it is DFU related).
>>
>> Since it is (for now) a dead code, please don't consider this patch.
>>
>> Shall I prepare v2 without this patch or will you be so kind and
>> review other patches in the current patch set?
>
> Rest is OK, I'd like to know Heiko's opinion too before applying though.

Yep, rest of the patches are OK, I tested them on the dxr2 and rut
boards. So from my side they can go in ... without this patch, as
it adds dead code. @Lukasz: Please post this patch again, if you
use this code, thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH 3/4] dfu: Find DFU alt setting number by passing its name

[Lukasz Majewski]

Hi Heiko, Marek,

> Hello Marek,
> 
> Am 10.09.2013 23:20, schrieb Marek Vasut:
> > Dear Lukasz Majewski,
> >
> >> Hi Marek,
> >>
> >>> Dear Lukasz Majewski,
> >>>
> >>>> New function - dfu_get_alt() has been added to dfu core. If
> >>>> proper alt setting is present, this function returns its number
> >>>> corresponding to passed name.
> >>>>
> >>>> Change-Id: Icd75f3aa3a6f6e306c77b28cabe620e4e6a253ea
> >>>> Signed-off-by: Lukasz Majewski<l.majewski@samsung.com>
> >>>> ---
> >>>>
> >>>>   drivers/dfu/dfu.c |   12 ++++++++++++
> >>>>   include/dfu.h     |    1 +
> >>>>   2 files changed, 13 insertions(+)
> >>>>
> >>>> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> >>>> index 2f1e2af..180d083 100644
> >>>> --- a/drivers/dfu/dfu.c
> >>>> +++ b/drivers/dfu/dfu.c
> >>>> @@ -414,3 +414,15 @@ struct dfu_entity *dfu_get_entity(int alt)
> >>>>
> >>>>   	return NULL;
> >>>>
> >>>>   }
> >>>>
> >>>> +
> >>>> +int dfu_get_alt(const char *name)
> >>>> +{
> >>>> +	struct dfu_entity *dfu;
> >>>> +
> >>>> +	list_for_each_entry(dfu,&dfu_list, list) {
> >>>> +		if (!strncmp(dfu->name, name,
> >>>> strlen(dfu->name)))
> >>>> +			return dfu->alt;
> >>>> +	}
> >>>> +
> >>>> +	return -ENODEV;
> >>>> +}
> >>>> diff --git a/include/dfu.h b/include/dfu.h
> >>>> index 7779710..8838f9c 100644
> >>>> --- a/include/dfu.h
> >>>> +++ b/include/dfu.h
> >>>> @@ -112,6 +112,7 @@ const char *dfu_get_layout(enum dfu_layout
> >>>> l);
> >>>>
> >>>>   struct dfu_entity *dfu_get_entity(int alt);
> >>>>   char *dfu_extract_token(char** e, int *n);
> >>>>   void dfu_trigger_reset(void);
> >>>>
> >>>> +int dfu_get_alt(const char *name);
> >>>>
> >>>>   bool dfu_reset(void);
> >>>>
> >>>>   int dfu_read(struct dfu_entity *de, void *buf, int size, int
> >>>>
> >>>> blk_seq_num);
> >>>
> >>> Is this code used anywhere ?
> >>
> >> Hmm.... By mistake I've added part of my ongoing USB related work
> >> to DFU fixes (as you see it is DFU related).
> >>
> >> Since it is (for now) a dead code, please don't consider this
> >> patch.
> >>
> >> Shall I prepare v2 without this patch or will you be so kind and
> >> review other patches in the current patch set?
> >
> > Rest is OK, I'd like to know Heiko's opinion too before applying
> > though.
> 
> Yep, rest of the patches are OK, I tested them on the dxr2 and rut
> boards. So from my side they can go in ... without this patch, as
> it adds dead code. @Lukasz: Please post this patch again, if you
> use this code, thanks!

I will, since I'm developing new USB gadget, which reuses DFU write
backend. 

> 
> bye,
> Heiko



-- 
Best regards,

Lukasz Majewski

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

[U-Boot] [PATCH 4/4] dfu: Extract common DFU code to handle "dfu_alt_info" environment variable

[Marek Vasut]

Dear Lukasz Majewski,

> New dfu_init_env_entities() function has been extracted from cmd_dfu.c and
> stored at dfu core.
> 
> This is a dfu centric code, so it shall be processed in the core.
> 
> Change-Id: I756c5de922fa31399d8804eaadc004ee98844ec2
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>

This does not apply for some reason, I applied 1/2 and 2/2 and pushed u-boot-
usb/master

Best regards,
Marek Vasut

[U-Boot] [PATCH 4/4] dfu: Extract common DFU code to handle "dfu_alt_info" environment variable

[Lukasz Majewski]

Hi Marek,

> Dear Lukasz Majewski,
> 
> > New dfu_init_env_entities() function has been extracted from
> > cmd_dfu.c and stored at dfu core.
> > 
> > This is a dfu centric code, so it shall be processed in the core.
> > 
> > Change-Id: I756c5de922fa31399d8804eaadc004ee98844ec2
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> 
> This does not apply for some reason, I applied 1/2 and 2/2 and pushed
> u-boot- usb/master

Marek, please test if it works for you:

1. Fetch u-boot-denx-usb/master -> Patch 1 and 2 applied on top
2. Use download mbox from patchwork:
http://patchwork.ozlabs.org/patch/273876/
3. Apply the patch:
git am -3
U-Boot-4-4-dfu-Extract-common-DFU-code-to-handle-dfu_alt_info-environment-variable.patch

I'm using git - version 1.7.10.4

> 
> Best regards,
> Marek Vasut



-- 
Best regards,

Lukasz Majewski

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

[U-Boot] [PATCH 4/4] dfu: Extract common DFU code to handle "dfu_alt_info" environment variable

[Marek Vasut]

Dear Lukasz Majewski,

> Hi Marek,
> 
> > Dear Lukasz Majewski,
> > 
> > > New dfu_init_env_entities() function has been extracted from
> > > cmd_dfu.c and stored at dfu core.
> > > 
> > > This is a dfu centric code, so it shall be processed in the core.
> > > 
> > > Change-Id: I756c5de922fa31399d8804eaadc004ee98844ec2
> > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > 
> > This does not apply for some reason, I applied 1/2 and 2/2 and pushed
> > u-boot- usb/master
> 
> Marek, please test if it works for you:
> 
> 1. Fetch u-boot-denx-usb/master -> Patch 1 and 2 applied on top
> 2. Use download mbox from patchwork:
> http://patchwork.ozlabs.org/patch/273876/
> 3. Apply the patch:
> git am -3
> U-Boot-4-4-dfu-Extract-common-DFU-code-to-handle-dfu_alt_info-environment-v
> ariable.patch
> 
> I'm using git - version 1.7.10.4

No and it cannot:

$ git am /tmp/U-Boot-4-4-dfu-Extract-common-DFU-code-to-handle-dfu_alt_info-
environment-variable.patch
Applying: dfu: Extract common DFU code to handle "dfu_alt_info" environment 
variable                                                                                        
error: patch failed: include/dfu.h:114                                                                                                                                      
error: include/dfu.h: patch does not apply                                                                                                                                  
Patch failed at 0001 dfu: Extract common DFU code to handle "dfu_alt_info" 
environment variable                                                                             
When you have resolved this problem run "git am --resolved".                                                                                                                
If you would prefer to skip this patch, instead run "git am --skip".                                                                                                        
To restore the original branch and stop patching run "git am --abort".

Best regards,
Marek Vasut

[U-Boot] [PATCH 4/4] dfu: Extract common DFU code to handle "dfu_alt_info" environment variable

[Lukasz Majewski]

On Wed, 11 Sep 2013 12:04:26 +0200 Marek Vasut marex at denx.de wrote,
> Dear Lukasz Majewski,
> 
> > Hi Marek,
> > 
> > > Dear Lukasz Majewski,
> > > 
> > > > New dfu_init_env_entities() function has been extracted from
> > > > cmd_dfu.c and stored at dfu core.
> > > > 
> > > > This is a dfu centric code, so it shall be processed in the
> > > > core.
> > > > 
> > > > Change-Id: I756c5de922fa31399d8804eaadc004ee98844ec2
> > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > > 
> > > This does not apply for some reason, I applied 1/2 and 2/2 and
> > > pushed u-boot- usb/master
> > 
> > Marek, please test if it works for you:
> > 
> > 1. Fetch u-boot-denx-usb/master -> Patch 1 and 2 applied on top
> > 2. Use download mbox from patchwork:
> > http://patchwork.ozlabs.org/patch/273876/
> > 3. Apply the patch:
> > git am -3
> > U-Boot-4-4-dfu-Extract-common-DFU-code-to-handle-dfu_alt_info-environment-v
> > ariable.patch
> > 
> > I'm using git - version 1.7.10.4
> 
> No and it cannot:
> 
> $ git am 

I'm using git am -3 (three way merge).

Can you test it with it?

> /tmp/U-Boot-4-4-dfu-Extract-common-DFU-code-to-handle-dfu_alt_info-
> environment-variable.patch Applying: dfu: Extract common DFU code to
> handle "dfu_alt_info" environment
> variable error: patch failed:
> include/dfu.h:114 error: include/dfu.h: patch does not
> apply Patch failed at 0001 dfu: Extract common DFU code to handle
> "dfu_alt_info" environment
> variable When you have resolved this problem run "git am
> --resolved". If you would prefer to skip this patch, instead run "git
> am --skip". To restore the original branch and stop patching run "git
> am --abort".
> 
> Best regards,
> Marek Vasut


-- 
Best regards,

Lukasz Majewski

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

[U-Boot] [PATCH 4/4] dfu: Extract common DFU code to handle "dfu_alt_info" environment variable

[Marek Vasut]

Dear Lukasz Majewski,

> On Wed, 11 Sep 2013 12:04:26 +0200 Marek Vasut marex at denx.de wrote,
> 
> > Dear Lukasz Majewski,
> > 
> > > Hi Marek,
> > > 
> > > > Dear Lukasz Majewski,
> > > > 
> > > > > New dfu_init_env_entities() function has been extracted from
> > > > > cmd_dfu.c and stored at dfu core.
> > > > > 
> > > > > This is a dfu centric code, so it shall be processed in the
> > > > > core.
> > > > > 
> > > > > Change-Id: I756c5de922fa31399d8804eaadc004ee98844ec2
> > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > > > 
> > > > This does not apply for some reason, I applied 1/2 and 2/2 and
> > > > pushed u-boot- usb/master
> > > 
> > > Marek, please test if it works for you:
> > > 
> > > 1. Fetch u-boot-denx-usb/master -> Patch 1 and 2 applied on top
> > > 2. Use download mbox from patchwork:
> > > http://patchwork.ozlabs.org/patch/273876/
> > > 3. Apply the patch:
> > > git am -3
> > > U-Boot-4-4-dfu-Extract-common-DFU-code-to-handle-dfu_alt_info-environme
> > > nt-v ariable.patch
> > > 
> > > I'm using git - version 1.7.10.4
> > 
> > No and it cannot:
> > 
> > $ git am
> 
> I'm using git am -3 (three way merge).
> 
> Can you test it with it?

As you can clearly see in the patch and compared to dfu.h , the patch cannot be 
applied. The hunk in dfu.h just doesn't match up.

Best regards,
Marek Vasut

[U-Boot] [PATCH 4/4 v2] dfu: Extract common DFU code to handle "dfu_alt_info" environment variable

[Lukasz Majewski]

New dfu_init_env_entities() function has been extracted from cmd_dfu.c and
stored at dfu core.

This is a dfu centric code, so it shall be processed in the core.

Change-Id: I756c5de922fa31399d8804eaadc004ee98844ec2
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Tested-by: Heiko Schocher <hs@denx.de>
---
Changes for v2:
- Rebased on u-boot-denx-usb/master branch

---
 common/cmd_dfu.c  |   16 ++--------------
 drivers/dfu/dfu.c |   23 +++++++++++++++++++++++
 include/dfu.h     |    1 +
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
index 793c422..d3658cf 100644
--- a/common/cmd_dfu.c
+++ b/common/cmd_dfu.c
@@ -17,26 +17,15 @@
 
 static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	const char *str_env;
 	char *s = "dfu";
 	int ret, i = 0;
-	char *env_bkp;
 
 	if (argc < 3)
 		return CMD_RET_USAGE;
 
-	str_env = getenv("dfu_alt_info");
-	if (str_env == NULL) {
-		printf("%s: \"dfu_alt_info\" env variable not defined!\n",
-		       __func__);
-		return CMD_RET_FAILURE;
-	}
-
-	env_bkp = strdup(str_env);
-	ret = dfu_config_entities(env_bkp, argv[1],
-			    (int)simple_strtoul(argv[2], NULL, 10));
+	ret = dfu_init_env_entities(argv[1], simple_strtoul(argv[2], NULL, 10));
 	if (ret)
-		return CMD_RET_FAILURE;
+		return ret;
 
 	if (argc > 3 && strcmp(argv[3], "list") == 0) {
 		dfu_show_entities();
@@ -67,7 +56,6 @@ exit:
 	g_dnl_unregister();
 done:
 	dfu_free_entities();
-	free(env_bkp);
 
 	if (dfu_reset())
 		run_command("reset", 0);
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 2f1e2af..689f5db 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -41,6 +41,29 @@ static int dfu_find_alt_num(const char *s)
 	return ++i;
 }
 
+int dfu_init_env_entities(char *interface, int dev)
+{
+	const char *str_env;
+	char *env_bkp;
+	int ret;
+
+	str_env = getenv("dfu_alt_info");
+	if (!str_env) {
+		error("\"dfu_alt_info\" env variable not defined!\n");
+		return -EINVAL;
+	}
+
+	env_bkp = strdup(str_env);
+	ret = dfu_config_entities(env_bkp, interface, dev);
+	if (ret) {
+		error("DFU entities configuration failed!\n");
+		return ret;
+	}
+
+	free(env_bkp);
+	return 0;
+}
+
 static unsigned char *dfu_buf;
 static unsigned long dfu_buf_size = CONFIG_SYS_DFU_DATA_BUF_SIZE;
 
diff --git a/include/dfu.h b/include/dfu.h
index 7779710..392cef1 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -113,6 +113,7 @@ struct dfu_entity *dfu_get_entity(int alt);
 char *dfu_extract_token(char** e, int *n);
 void dfu_trigger_reset(void);
 bool dfu_reset(void);
+int dfu_init_env_entities(char *interface, int dev);
 
 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);
-- 
1.7.10.4

[U-Boot] [PATCH 4/4 v2] dfu: Extract common DFU code to handle "dfu_alt_info" environment variable

[Marek Vasut]

Dear Lukasz Majewski,

> New dfu_init_env_entities() function has been extracted from cmd_dfu.c and
> stored at dfu core.
> 
> This is a dfu centric code, so it shall be processed in the core.
> 
> Change-Id: I756c5de922fa31399d8804eaadc004ee98844ec2
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Tested-by: Heiko Schocher <hs@denx.de>

Applied, thanks! And sorry for the confusion!

Best regards,
Marek Vasut