[U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb)

[U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb)

[Pantelis Antoniou]

This patchset fixes various problems in the u-boot DFU implementation
and supports it on the am35xx_evm.

This patchset is against the u-boot-usb tree.

Pantelis Antoniou (9):
  [usb-gadget] Remove obsolete header file
  [usb-gadget] Fix bug when both DFU & ETHER are defined
  [dfu] Only perform DFU board_usb_init() for TRATS
  [dfu] Fix crash when wrong number of arguments given
  [dfu] Generate appropriate responses for DFU
  [dfu] Properly zero out timeout value
  [dfu] Add a partition type target
  [dfu] Issue connect/disconnect as appropriate
  [am335x_evm] Add DFU config

 common/cmd_dfu.c               |  5 ++++-
 drivers/dfu/dfu_mmc.c          | 29 +++++++++++++++++++++++++++++
 drivers/usb/gadget/Makefile    | 13 +++++++++++--
 drivers/usb/gadget/composite.c |  9 +++++++++
 drivers/usb/gadget/ep0.c       |  1 +
 drivers/usb/gadget/f_dfu.c     |  4 +++-
 drivers/usb/gadget/g_dnl.c     |  9 ++++++++-
 include/configs/am335x_evm.h   | 27 +++++++++++++++++++++++++++
 include/g_dnl.h                |  1 -
 9 files changed, 92 insertions(+), 6 deletions(-)

-- 
1.7.12

[U-Boot] [PATCH 1/9] Remove obsolete header file

[Pantelis Antoniou]

usbdescriptors.h conflicts with linux/usb/ch9.h
Remove it.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/f_dfu.c | 1 -
 include/g_dnl.h            | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index 3ec4c65..10547e3 100644
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -25,7 +25,6 @@
 #include <malloc.h>
 
 #include <linux/usb/ch9.h>
-#include <usbdescriptors.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/composite.h>
 
diff --git a/include/g_dnl.h b/include/g_dnl.h
index 0ec7440..f47395f 100644
--- a/include/g_dnl.h
+++ b/include/g_dnl.h
@@ -22,7 +22,6 @@
 #define __G_DOWNLOAD_H_
 
 #include <linux/usb/ch9.h>
-#include <usbdescriptors.h>
 #include <linux/usb/gadget.h>
 
 int g_dnl_register(const char *s);
-- 
1.7.12

[U-Boot] [PATCH 1/9] Remove obsolete header file

[Tom Rini]

On Wed, Nov 28, 2012 at 02:43:54PM +0200, Pantelis Antoniou wrote:

> usbdescriptors.h conflicts with linux/usb/ch9.h
> Remove it.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>

Seeing as this change is required to fix building of trats in u-boot-usb
as well now, I am applying this now.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121127/b5203433/attachment.pgp>

[U-Boot] [PATCH 1/9] Remove obsolete header file

[Marek Vasut]

Dear Tom Rini,

> On Wed, Nov 28, 2012 at 02:43:54PM +0200, Pantelis Antoniou wrote:
> > usbdescriptors.h conflicts with linux/usb/ch9.h
> > Remove it.
> > 
> > Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> 
> Seeing as this change is required to fix building of trats in u-boot-usb
> as well now, I am applying this now.  Thanks!

Only this or the whole series?

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/9] Remove obsolete header file

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/27/12 17:00, Marek Vasut wrote:
> Dear Tom Rini,
> 
>> On Wed, Nov 28, 2012 at 02:43:54PM +0200, Pantelis Antoniou
>> wrote:
>>> usbdescriptors.h conflicts with linux/usb/ch9.h Remove it.
>>> 
>>> Signed-off-by: Pantelis Antoniou
>>> <panto@antoniou-consulting.com>
>> 
>> Seeing as this change is required to fix building of trats in
>> u-boot-usb as well now, I am applying this now.  Thanks!
> 
> Only this or the whole series?

Only this patch.  The rest of the series is new functionality that is
past the merge window, so should be staged for the new MW.  1/9 fixes
a bug exposed by Ilya's update.

- -- 
Tom

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQtX6ZAAoJENk4IS6UOR1WoI8P/2K5SmZl6VVNWzeusp31nk7w
mH4//X1+LfTg1oUX/v1xPTJGvpN8rEVcXZis/Q4HwymIEKHoG22OaP6SJqudo78P
HQjz0jSgvsOcMZhobZVCp2UpuHhC/a6iBLBUAvyRDUfCtfVRZBJU17nalMM1oLps
TyixMaHJYgCnU66WpNusTTK0qht9/tkjYEu1FmWJejngAKiwvbq1dZRsl+9qxB8L
C4WHeh3Zw3AQa19yIqIGVMsHxE5vuO1KwW0cJlkmwcRaxObFZYluYoTpSGQGbVXk
uVLbCerW/yNqmBmFGIHRAM5d5Uojoou5UdmGFfpHvYL4M/S4aHjHOXDjIEUa+Ef/
wpUpKN2WMMZUAaAzeQc9YNAr2bzNrpN5MruZ6bc+9Yr3w9vaFkcXWV5Mq5ZlgZEi
CXfbDFqyJ7ODYG9cYt4i7YNczkjd0QGN1Oswqmorjd3nQ2QF+b7IAzJWAvh+KKC3
N/loGkZiI68LB3le22FJVKtOD6MPZUsrXQ6ms8lqezHMcve+uxw9F5f3k0HQTFts
Q4zDdZ3JzcGjpl0bhQ6UJe4Pg/+xYzlij5eeCYD9WW8JIY1ZGxXrQuezYFUCn69J
ed8NaTaZeUjqejOF3ucwIdR9ptnxZmdG6+6n4RDZEJTn8/K/9nfKpuEFyu9LkF6I
KqfwCGTXhBwODdr0dE2Z
=kS/y
-----END PGP SIGNATURE-----

[U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined

[Pantelis Antoniou]

When both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
the makefile links objects twice.

Detect this and fix it with a not very elegant way in the
makefile. Revisit and clean it later.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/Makefile | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 040eaba..15206cd 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -26,14 +26,23 @@ include $(TOPDIR)/config.mk
 LIB	:= $(obj)libusb_gadget.o
 
 # new USB gadget layer dependencies
+
+# ugh; ugh; ugh common objects included twice
+ifdef CONFIG_USB_GADGET
+   COBJS-y += epautoconf.o config.o usbstring.o
+else
+  ifdef CONFIG_USB_ETHER
+     COBJS-y += epautoconf.o config.o usbstring.o
+  endif
+endif
+
 ifdef CONFIG_USB_GADGET
-COBJS-y += epautoconf.o config.o usbstring.o
 COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o
 COBJS-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o
 COBJS-$(CONFIG_DFU_FUNCTION) += f_dfu.o
 endif
 ifdef CONFIG_USB_ETHER
-COBJS-y += ether.o epautoconf.o config.o usbstring.o
+COBJS-y += ether.o
 COBJS-$(CONFIG_USB_ETH_RNDIS) += rndis.o
 COBJS-$(CONFIG_MV_UDC)	+= mv_udc.o
 COBJS-$(CONFIG_CPU_PXA25X) += pxa25x_udc.o
-- 
1.7.12

[U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined

[Marek Vasut]

Dear Pantelis Antoniou,

> When both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
> the makefile links objects twice.
> 
> Detect this and fix it with a not very elegant way in the
> makefile. Revisit and clean it later.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/usb/gadget/Makefile | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index 040eaba..15206cd 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -26,14 +26,23 @@ include $(TOPDIR)/config.mk
>  LIB	:= $(obj)libusb_gadget.o
> 
>  # new USB gadget layer dependencies
> +
> +# ugh; ugh; ugh common objects included twice
> +ifdef CONFIG_USB_GADGET
> +   COBJS-y += epautoconf.o config.o usbstring.o
> +else
> +  ifdef CONFIG_USB_ETHER
> +     COBJS-y += epautoconf.o config.o usbstring.o
> +  endif
> +endif

COBJS-$(CONFIG_ST) += st.o
COBJS-$(CONFIG_ST_ELSE) += st_else.o

if (both selected)
 scream and die.

> +
>  ifdef CONFIG_USB_GADGET
> -COBJS-y += epautoconf.o config.o usbstring.o
>  COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o
>  COBJS-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o
>  COBJS-$(CONFIG_DFU_FUNCTION) += f_dfu.o
>  endif
>  ifdef CONFIG_USB_ETHER
> -COBJS-y += ether.o epautoconf.o config.o usbstring.o
> +COBJS-y += ether.o
>  COBJS-$(CONFIG_USB_ETH_RNDIS) += rndis.o
>  COBJS-$(CONFIG_MV_UDC)	+= mv_udc.o
>  COBJS-$(CONFIG_CPU_PXA25X) += pxa25x_udc.o

Best regards,
Marek Vasut

[U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined

[Pantelis Antoniou]

Hi Marek,

On Nov 28, 2012, at 4:43 AM, Marek Vasut wrote:

> Dear Pantelis Antoniou,
> 
>> When both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
>> the makefile links objects twice.
>> 
>> Detect this and fix it with a not very elegant way in the
>> makefile. Revisit and clean it later.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>> drivers/usb/gadget/Makefile | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
>> index 040eaba..15206cd 100644
>> --- a/drivers/usb/gadget/Makefile
>> +++ b/drivers/usb/gadget/Makefile
>> @@ -26,14 +26,23 @@ include $(TOPDIR)/config.mk
>> LIB	:= $(obj)libusb_gadget.o
>> 
>> # new USB gadget layer dependencies
>> +
>> +# ugh; ugh; ugh common objects included twice
>> +ifdef CONFIG_USB_GADGET
>> +   COBJS-y += epautoconf.o config.o usbstring.o
>> +else
>> +  ifdef CONFIG_USB_ETHER
>> +     COBJS-y += epautoconf.o config.o usbstring.o
>> +  endif
>> +endif
> 
> COBJS-$(CONFIG_ST) += st.o
> COBJS-$(CONFIG_ST_ELSE) += st_else.o
> 
> if (both selected)
> scream and die.
> 

I fail to see how that would work for the problem at hand, namely
that if both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
epautoconf.o config.o usbstring.o are linked twice.

The proper thing to do would be to have a config symbol enabled
when either CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined.

Something like 

#if defined(CONFIG_USB_GADGET) || defined(CONFIG_USB_ETHER)
#define CONFIG_USB_GADGET_PLUMPING
#endif

in a header file someplace and do

COBJS-$(CONFIG_USB_GADGET_PLUMPING) += epautoconf.o config.o usbstring.o

etc.

This would require putting it either in every board file, or in a core
usb header.

The patch was the least intrusive method.

Regards

-- Pantelis


>> +
>> ifdef CONFIG_USB_GADGET
>> -COBJS-y += epautoconf.o config.o usbstring.o
>> COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o
>> COBJS-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o
>> COBJS-$(CONFIG_DFU_FUNCTION) += f_dfu.o
>> endif
>> ifdef CONFIG_USB_ETHER
>> -COBJS-y += ether.o epautoconf.o config.o usbstring.o
>> +COBJS-y += ether.o
>> COBJS-$(CONFIG_USB_ETH_RNDIS) += rndis.o
>> COBJS-$(CONFIG_MV_UDC)	+= mv_udc.o
>> COBJS-$(CONFIG_CPU_PXA25X) += pxa25x_udc.o
> 
> Best regards,
> Marek Vasut

[U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined

[Marek Vasut]

Dear Pantelis Antoniou,

> Hi Marek,
> 
> On Nov 28, 2012, at 4:43 AM, Marek Vasut wrote:
> > Dear Pantelis Antoniou,
> > 
> >> When both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
> >> the makefile links objects twice.
> >> 
> >> Detect this and fix it with a not very elegant way in the
> >> makefile. Revisit and clean it later.
> >> 
> >> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> >> ---
> >> drivers/usb/gadget/Makefile | 13 +++++++++++--
> >> 1 file changed, 11 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> >> index 040eaba..15206cd 100644
> >> --- a/drivers/usb/gadget/Makefile
> >> +++ b/drivers/usb/gadget/Makefile
> >> @@ -26,14 +26,23 @@ include $(TOPDIR)/config.mk
> >> LIB	:= $(obj)libusb_gadget.o
> >> 
> >> # new USB gadget layer dependencies
> >> +
> >> +# ugh; ugh; ugh common objects included twice
> >> +ifdef CONFIG_USB_GADGET
> >> +   COBJS-y += epautoconf.o config.o usbstring.o
> >> +else
> >> +  ifdef CONFIG_USB_ETHER
> >> +     COBJS-y += epautoconf.o config.o usbstring.o
> >> +  endif
> >> +endif
> > 
> > COBJS-$(CONFIG_ST) += st.o
> > COBJS-$(CONFIG_ST_ELSE) += st_else.o
> > 
> > if (both selected)
> > scream and die.
> 
> I fail to see how that would work for the problem at hand, namely
> that if both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
> epautoconf.o config.o usbstring.o are linked twice.

Ah, now I understand the problem at hand.

> The proper thing to do would be to have a config symbol enabled
> when either CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined.
> 
> Something like
> 
> #if defined(CONFIG_USB_GADGET) || defined(CONFIG_USB_ETHER)
> #define CONFIG_USB_GADGET_PLUMPING
> #endif
> 
> in a header file someplace and do

Can you not do "or" in conditional expression in Makefile?

> COBJS-$(CONFIG_USB_GADGET_PLUMPING) += epautoconf.o config.o usbstring.o
> 
> etc.
> 
> This would require putting it either in every board file, or in a core
> usb header.
> 
> The patch was the least intrusive method.
> 
> Regards
[...]

[U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS

[Pantelis Antoniou]

USB initialization shouldn't happen for all the boards.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 common/cmd_dfu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
index 01d6b3a..327c738 100644
--- a/common/cmd_dfu.c
+++ b/common/cmd_dfu.c
@@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		goto done;
 	}
 
+#ifdef CONFIG_TRATS
 	board_usb_init();
+#endif
+
 	g_dnl_register(s);
 	while (1) {
 		if (ctrlc())
-- 
1.7.12

[U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS

[Marek Vasut]

Dear Pantelis Antoniou,

> USB initialization shouldn't happen for all the boards.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  common/cmd_dfu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
> index 01d6b3a..327c738 100644
> --- a/common/cmd_dfu.c
> +++ b/common/cmd_dfu.c
> @@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc,
> char * const argv[]) goto done;
>  	}
> 
> +#ifdef CONFIG_TRATS
>  	board_usb_init();
> +#endif
> +

It's common code:

1) Why is it called "board_usb_init()" ? Does this have anything to do with usb 
host?

2) Make it __weak, then if it's undefined for your board, something default will 
be called.

>  	g_dnl_register(s);
>  	while (1) {
>  		if (ctrlc())

Best regards,
Marek Vasut

[U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS

[Pantelis Antoniou]

Hi Marek,

On Nov 28, 2012, at 4:45 AM, Marek Vasut wrote:

> Dear Pantelis Antoniou,
> 
>> USB initialization shouldn't happen for all the boards.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>> common/cmd_dfu.c | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
>> index 01d6b3a..327c738 100644
>> --- a/common/cmd_dfu.c
>> +++ b/common/cmd_dfu.c
>> @@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc,
>> char * const argv[]) goto done;
>> 	}
>> 
>> +#ifdef CONFIG_TRATS
>> 	board_usb_init();
>> +#endif
>> +
> 
> It's common code:
> 
> 1) Why is it called "board_usb_init()" ? Does this have anything to do with usb 
> host?
> 

No idea. It makes no sense to me, but it was there from the original DFU poster.
I don't have a TRATS board to test it anyway, but I didn't want to affect it.

> 2) Make it __weak, then if it's undefined for your board, something default will 
> be called.
> 

I see no reason why it should even exist. Perhaps we should ask the original poster.

>> 	g_dnl_register(s);
>> 	while (1) {
>> 		if (ctrlc())
> 
> Best regards,
> Marek Vasut

Regards

-- Pantelis

[U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS

[Marek Vasut]

Dear Pantelis Antoniou,

> Hi Marek,
> 
> On Nov 28, 2012, at 4:45 AM, Marek Vasut wrote:
> > Dear Pantelis Antoniou,
> > 
> >> USB initialization shouldn't happen for all the boards.
> >> 
> >> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> >> ---
> >> common/cmd_dfu.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >> 
> >> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
> >> index 01d6b3a..327c738 100644
> >> --- a/common/cmd_dfu.c
> >> +++ b/common/cmd_dfu.c
> >> @@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int
> >> argc, char * const argv[]) goto done;
> >> 
> >> 	}
> >> 
> >> +#ifdef CONFIG_TRATS
> >> 
> >> 	board_usb_init();
> >> 
> >> +#endif
> >> +
> > 
> > It's common code:
> > 
> > 1) Why is it called "board_usb_init()" ? Does this have anything to do
> > with usb host?
> 
> No idea. It makes no sense to me, but it was there from the original DFU
> poster. I don't have a TRATS board to test it anyway, but I didn't want to
> affect it.

Can you please rename it to a more sensible name then?

> > 2) Make it __weak, then if it's undefined for your board, something
> > default will be called.
> 
> I see no reason why it should even exist. Perhaps we should ask the
> original poster.

Please do it then

> >> 	g_dnl_register(s);
> >> 	while (1) {
> >> 	
> >> 		if (ctrlc())
> > 
> > Best regards,
> > Marek Vasut
> 
> Regards
> 
> -- Pantelis

Best regards,
Marek Vasut

[U-Boot] [PATCH 4/9] Fix crash when wrong number of arguments given

[Pantelis Antoniou]

Fix obvious crash when not enough arguments are given to the dfu
command.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 common/cmd_dfu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
index 327c738..83ef324 100644
--- a/common/cmd_dfu.c
+++ b/common/cmd_dfu.c
@@ -50,7 +50,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	if (ret)
 		return CMD_RET_FAILURE;
 
-	if (strcmp(argv[3], "list") == 0) {
+	if (argc > 3 && strcmp(argv[3], "list") == 0) {
 		dfu_show_entities();
 		goto done;
 	}
-- 
1.7.12

[U-Boot] [PATCH 5/9] Generate appropriate responses for DFU

[Pantelis Antoniou]

Make sure appropriate responses for the DFU protocal are
generated.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/composite.c | 9 +++++++++
 drivers/usb/gadget/ep0.c       | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index ebb5131..1ae318c 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -773,6 +773,15 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 			if (value >= 0)
 				value = min(w_length, (u16) value);
 			break;
+
+#ifdef CONFIG_DFU_FUNCTION
+		case DFU_DT_FUNC:	/* DFU */
+			value = config_desc(cdev, w_value);
+			if (value >= 0)
+				value = min(w_length, (u16) value);
+			break;
+#endif
+
 		default:
 			goto unknown;
 		}
diff --git a/drivers/usb/gadget/ep0.c b/drivers/usb/gadget/ep0.c
index aa8f916..971d846 100644
--- a/drivers/usb/gadget/ep0.c
+++ b/drivers/usb/gadget/ep0.c
@@ -221,6 +221,7 @@ static int ep0_get_descriptor (struct usb_device_instance *device,
 		break;
 
 	case USB_DESCRIPTOR_TYPE_CONFIGURATION:
+	case USB_DESCRIPTOR_TYPE_OTHER_SPEED_CONFIGURATION:
 		{
 			struct usb_configuration_descriptor
 				*configuration_descriptor;
-- 
1.7.12

[U-Boot] [PATCH 5/9] Generate appropriate responses for DFU

[Marek Vasut]

Dear Pantelis Antoniou,

> Make sure appropriate responses for the DFU protocal are
> generated.

I dont understand this patch, please explain it properly in the commit message.

> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/usb/gadget/composite.c | 9 +++++++++
>  drivers/usb/gadget/ep0.c       | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/gadget/composite.c
> b/drivers/usb/gadget/composite.c index ebb5131..1ae318c 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -773,6 +773,15 @@ composite_setup(struct usb_gadget *gadget, const
> struct usb_ctrlrequest *ctrl) if (value >= 0)
>  				value = min(w_length, (u16) value);
>  			break;
> +
> +#ifdef CONFIG_DFU_FUNCTION
> +		case DFU_DT_FUNC:	/* DFU */
> +			value = config_desc(cdev, w_value);
> +			if (value >= 0)
> +				value = min(w_length, (u16) value);
> +			break;
> +#endif
> +
>  		default:
>  			goto unknown;
>  		}
> diff --git a/drivers/usb/gadget/ep0.c b/drivers/usb/gadget/ep0.c
> index aa8f916..971d846 100644
> --- a/drivers/usb/gadget/ep0.c
> +++ b/drivers/usb/gadget/ep0.c
> @@ -221,6 +221,7 @@ static int ep0_get_descriptor (struct
> usb_device_instance *device, break;
> 
>  	case USB_DESCRIPTOR_TYPE_CONFIGURATION:
> +	case USB_DESCRIPTOR_TYPE_OTHER_SPEED_CONFIGURATION:
>  		{
>  			struct usb_configuration_descriptor
>  				*configuration_descriptor;

Best regards,
Marek Vasut

[U-Boot] [PATCH 5/9] Generate appropriate responses for DFU

[Pantelis Antoniou]

Hi Marek,

On Nov 28, 2012, at 4:46 AM, Marek Vasut wrote:

> Dear Pantelis Antoniou,
> 
>> Make sure appropriate responses for the DFU protocal are
>> generated.
> 
> I dont understand this patch, please explain it properly in the commit message.
> 

Turns out this is not even quite correct... Updated patchset in a little bit.

>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>> drivers/usb/gadget/composite.c | 9 +++++++++
>> drivers/usb/gadget/ep0.c       | 1 +
>> 2 files changed, 10 insertions(+)
>> 
>> diff --git a/drivers/usb/gadget/composite.c
>> b/drivers/usb/gadget/composite.c index ebb5131..1ae318c 100644
>> --- a/drivers/usb/gadget/composite.c
>> +++ b/drivers/usb/gadget/composite.c
>> @@ -773,6 +773,15 @@ composite_setup(struct usb_gadget *gadget, const
>> struct usb_ctrlrequest *ctrl) if (value >= 0)
>> 				value = min(w_length, (u16) value);
>> 			break;
>> +
>> +#ifdef CONFIG_DFU_FUNCTION
>> +		case DFU_DT_FUNC:	/* DFU */
>> +			value = config_desc(cdev, w_value);
>> +			if (value >= 0)
>> +				value = min(w_length, (u16) value);
>> +			break;
>> +#endif
>> +
>> 		default:
>> 			goto unknown;
>> 		}
>> diff --git a/drivers/usb/gadget/ep0.c b/drivers/usb/gadget/ep0.c
>> index aa8f916..971d846 100644
>> --- a/drivers/usb/gadget/ep0.c
>> +++ b/drivers/usb/gadget/ep0.c
>> @@ -221,6 +221,7 @@ static int ep0_get_descriptor (struct
>> usb_device_instance *device, break;
>> 
>> 	case USB_DESCRIPTOR_TYPE_CONFIGURATION:
>> +	case USB_DESCRIPTOR_TYPE_OTHER_SPEED_CONFIGURATION:
>> 		{
>> 			struct usb_configuration_descriptor
>> 				*configuration_descriptor;
> 
> Best regards,
> Marek Vasut

[U-Boot] [PATCH 6/9] Properly zero out timeout value

[Pantelis Antoniou]

Zero out timeout value; letting it filled with undefined values
ends up with the dfu host hanging.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/f_dfu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index 10547e3..a322ae5 100644
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -164,6 +164,9 @@ static void handle_getstatus(struct usb_request *req)
 
 	/* send status response */
 	dstat->bStatus = f_dfu->dfu_status;
+	dstat->bwPollTimeout[0] = 0;
+	dstat->bwPollTimeout[1] = 0;
+	dstat->bwPollTimeout[2] = 0;
 	dstat->bState = f_dfu->dfu_state;
 	dstat->iString = 0;
 }
-- 
1.7.12

[U-Boot] [PATCH 6/9] Properly zero out timeout value

[Marek Vasut]

Dear Pantelis Antoniou,

> Zero out timeout value; letting it filled with undefined values
> ends up with the dfu host hanging.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/usb/gadget/f_dfu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
> index 10547e3..a322ae5 100644
> --- a/drivers/usb/gadget/f_dfu.c
> +++ b/drivers/usb/gadget/f_dfu.c
> @@ -164,6 +164,9 @@ static void handle_getstatus(struct usb_request *req)
> 
>  	/* send status response */
>  	dstat->bStatus = f_dfu->dfu_status;
> +	dstat->bwPollTimeout[0] = 0;
> +	dstat->bwPollTimeout[1] = 0;
> +	dstat->bwPollTimeout[2] = 0;

What about calling memset() here ?

>  	dstat->bState = f_dfu->dfu_state;
>  	dstat->iString = 0;
>  }

Best regards,
Marek Vasut

[U-Boot] [PATCH 6/9] Properly zero out timeout value

[Pantelis Antoniou]

Hi Marek,

On Nov 28, 2012, at 4:46 AM, Marek Vasut wrote:

> Dear Pantelis Antoniou,
> 
>> Zero out timeout value; letting it filled with undefined values
>> ends up with the dfu host hanging.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>> drivers/usb/gadget/f_dfu.c | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
>> index 10547e3..a322ae5 100644
>> --- a/drivers/usb/gadget/f_dfu.c
>> +++ b/drivers/usb/gadget/f_dfu.c
>> @@ -164,6 +164,9 @@ static void handle_getstatus(struct usb_request *req)
>> 
>> 	/* send status response */
>> 	dstat->bStatus = f_dfu->dfu_status;
>> +	dstat->bwPollTimeout[0] = 0;
>> +	dstat->bwPollTimeout[1] = 0;
>> +	dstat->bwPollTimeout[2] = 0;
> 
> What about calling memset() here ?
> 

I would memset the whole structure just to be safe, but it seems that this is just
3 instructions. memset would take more. Either way works for me.

>> 	dstat->bState = f_dfu->dfu_state;
>> 	dstat->iString = 0;
>> }
> 
> Best regards,
> Marek Vasut

Regards

-- Pantelis

[U-Boot] [PATCH 6/9] Properly zero out timeout value

[Marek Vasut]

Dear Pantelis Antoniou,

> Hi Marek,
> 
> On Nov 28, 2012, at 4:46 AM, Marek Vasut wrote:
> > Dear Pantelis Antoniou,
> > 
> >> Zero out timeout value; letting it filled with undefined values
> >> ends up with the dfu host hanging.
> >> 
> >> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> >> ---
> >> drivers/usb/gadget/f_dfu.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >> 
> >> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
> >> index 10547e3..a322ae5 100644
> >> --- a/drivers/usb/gadget/f_dfu.c
> >> +++ b/drivers/usb/gadget/f_dfu.c
> >> @@ -164,6 +164,9 @@ static void handle_getstatus(struct usb_request
> >> *req)
> >> 
> >> 	/* send status response */
> >> 	dstat->bStatus = f_dfu->dfu_status;
> >> 
> >> +	dstat->bwPollTimeout[0] = 0;
> >> +	dstat->bwPollTimeout[1] = 0;
> >> +	dstat->bwPollTimeout[2] = 0;
> > 
> > What about calling memset() here ?
> 
> I would memset the whole structure just to be safe, but it seems that this
> is just 3 instructions. memset would take more. Either way works for me.

Can you verify that please?

> >> 	dstat->bState = f_dfu->dfu_state;
> >> 	dstat->iString = 0;
> >> 
> >> }
> > 
> > Best regards,
> > Marek Vasut
> 
> Regards
> 
> -- Pantelis

Best regards,
Marek Vasut

[U-Boot] [PATCH 7/9] Add a partition type target

[Pantelis Antoniou]

Dealing with raw block numbers with the dfu is very annoying.
Introduce a partition method.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/dfu/dfu_mmc.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index 5d504df..3733b21 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -153,6 +153,10 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu, void *buf, long *len)
 
 int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
 {
+	int dev, part;
+	struct mmc *mmc;
+	block_dev_desc_t *blk_dev;
+	disk_partition_t partinfo;
 	char *st;
 
 	dfu->dev_type = DFU_DEV_MMC;
@@ -166,8 +170,33 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
 		dfu->layout = DFU_FS_FAT;
 	} else if (!strcmp(st, "ext4")) {
 		dfu->layout = DFU_FS_EXT4;
+	} else if (!strcmp(st, "part")) {
+
+		dfu->layout = DFU_RAW_ADDR;
+
+		dev = simple_strtoul(s, &s, 10);
+		part = simple_strtoul(++s, &s, 10);
+
+		mmc = find_mmc_device(dev);
+		if (mmc == NULL || mmc_init(mmc)) {
+			printf("%s: could not find mmc device #%d!\n", __func__, dev);
+			return -1;
+		}
+
+		blk_dev = &mmc->block_dev;
+		if (get_partition_info(blk_dev, part, &partinfo) != 0) {
+			printf("%s: could not find partition #%d on mmc device #%d!\n",
+					__func__, part, dev);
+			return -1;
+		}
+
+		dfu->data.mmc.lba_start = partinfo.start;
+		dfu->data.mmc.lba_size = partinfo.size;
+		dfu->data.mmc.lba_blk_size = partinfo.blksz;
+
 	} else {
 		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+		return -1;
 	}
 
 	if (dfu->layout == DFU_FS_EXT4 || dfu->layout == DFU_FS_FAT) {
-- 
1.7.12

[U-Boot] [PATCH 7/9] Add a partition type target

[Marek Vasut]

Dear Pantelis Antoniou,

> Dealing with raw block numbers with the dfu is very annoying.
> Introduce a partition method.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/dfu/dfu_mmc.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> index 5d504df..3733b21 100644
> --- a/drivers/dfu/dfu_mmc.c
> +++ b/drivers/dfu/dfu_mmc.c
> @@ -153,6 +153,10 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu, void
> *buf, long *len)
> 
>  int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
>  {
> +	int dev, part;
> +	struct mmc *mmc;
> +	block_dev_desc_t *blk_dev;
> +	disk_partition_t partinfo;
>  	char *st;
> 
>  	dfu->dev_type = DFU_DEV_MMC;
> @@ -166,8 +170,33 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char
> *s) dfu->layout = DFU_FS_FAT;
>  	} else if (!strcmp(st, "ext4")) {
>  		dfu->layout = DFU_FS_EXT4;
> +	} else if (!strcmp(st, "part")) {
> +
> +		dfu->layout = DFU_RAW_ADDR;
> +
> +		dev = simple_strtoul(s, &s, 10);
> +		part = simple_strtoul(++s, &s, 10);

++s ... this is unreadable and definitelly prone to breakage.

> +
> +		mmc = find_mmc_device(dev);
> +		if (mmc == NULL || mmc_init(mmc)) {
> +			printf("%s: could not find mmc device #%d!\n", __func__, 
dev);
> +			return -1;
> +		}
> +
> +		blk_dev = &mmc->block_dev;
> +		if (get_partition_info(blk_dev, part, &partinfo) != 0) {
> +			printf("%s: could not find partition #%d on mmc device 
#%d!\n",
> +					__func__, part, dev);
> +			return -1;

Try using regular errno.h ... fix all around.

> +		}
> +
> +		dfu->data.mmc.lba_start = partinfo.start;
> +		dfu->data.mmc.lba_size = partinfo.size;
> +		dfu->data.mmc.lba_blk_size = partinfo.blksz;
> +
>  	} else {
>  		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
> +		return -1;

This is new ... does it fit into this patch at all?

>  	}
> 
>  	if (dfu->layout == DFU_FS_EXT4 || dfu->layout == DFU_FS_FAT) {

Best regards,
Marek Vasut

[U-Boot] [PATCH 7/9] Add a partition type target

[Pantelis Antoniou]

Hi Marek,

On Nov 28, 2012, at 4:48 AM, Marek Vasut wrote:

> Dear Pantelis Antoniou,
> 
>> Dealing with raw block numbers with the dfu is very annoying.
>> Introduce a partition method.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>> drivers/dfu/dfu_mmc.c | 29 +++++++++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>> 
>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
>> index 5d504df..3733b21 100644
>> --- a/drivers/dfu/dfu_mmc.c
>> +++ b/drivers/dfu/dfu_mmc.c
>> @@ -153,6 +153,10 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu, void
>> *buf, long *len)
>> 
>> int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
>> {
>> +	int dev, part;
>> +	struct mmc *mmc;
>> +	block_dev_desc_t *blk_dev;
>> +	disk_partition_t partinfo;
>> 	char *st;
>> 
>> 	dfu->dev_type = DFU_DEV_MMC;
>> @@ -166,8 +170,33 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char
>> *s) dfu->layout = DFU_FS_FAT;
>> 	} else if (!strcmp(st, "ext4")) {
>> 		dfu->layout = DFU_FS_EXT4;
>> +	} else if (!strcmp(st, "part")) {
>> +
>> +		dfu->layout = DFU_RAW_ADDR;
>> +
>> +		dev = simple_strtoul(s, &s, 10);
>> +		part = simple_strtoul(++s, &s, 10);
> 
> ++s ... this is unreadable and definitelly prone to breakage.
> 

Just following what the existing code does. Look around this line.
Perhaps we should modify that too.

>> +
>> +		mmc = find_mmc_device(dev);
>> +		if (mmc == NULL || mmc_init(mmc)) {
>> +			printf("%s: could not find mmc device #%d!\n", __func__, 
> dev);
>> +			return -1;
>> +		}
>> +
>> +		blk_dev = &mmc->block_dev;
>> +		if (get_partition_info(blk_dev, part, &partinfo) != 0) {
>> +			printf("%s: could not find partition #%d on mmc device 
> #%d!\n",
>> +					__func__, part, dev);
>> +			return -1;
> 
> Try using regular errno.h ... fix all around.
> 

The whole file doesn't use errno.h at all. Changing this single thing will stick
out like a sore thumb.

>> +		}
>> +
>> +		dfu->data.mmc.lba_start = partinfo.start;
>> +		dfu->data.mmc.lba_size = partinfo.size;
>> +		dfu->data.mmc.lba_blk_size = partinfo.blksz;
>> +
>> 	} else {
>> 		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
>> +		return -1;
> 
> This is new ... does it fit into this patch at all?
> 

It should, it's an error condition and the code just blindly continued.

>> 	}
>> 
>> 	if (dfu->layout == DFU_FS_EXT4 || dfu->layout == DFU_FS_FAT) {
> 
> Best regards,
> Marek Vasut

Regards

-- Pantelis

[U-Boot] [PATCH 7/9] Add a partition type target

[Marek Vasut]

Dear Pantelis Antoniou,

> Hi Marek,
> 
> On Nov 28, 2012, at 4:48 AM, Marek Vasut wrote:
> > Dear Pantelis Antoniou,
> > 
> >> Dealing with raw block numbers with the dfu is very annoying.
> >> Introduce a partition method.
> >> 
> >> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> >> ---
> >> drivers/dfu/dfu_mmc.c | 29 +++++++++++++++++++++++++++++
> >> 1 file changed, 29 insertions(+)
> >> 
> >> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> >> index 5d504df..3733b21 100644
> >> --- a/drivers/dfu/dfu_mmc.c
> >> +++ b/drivers/dfu/dfu_mmc.c
> >> @@ -153,6 +153,10 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu,
> >> void *buf, long *len)
> >> 
> >> int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
> >> {
> >> +	int dev, part;
> >> +	struct mmc *mmc;
> >> +	block_dev_desc_t *blk_dev;
> >> +	disk_partition_t partinfo;
> >> 
> >> 	char *st;
> >> 	
> >> 	dfu->dev_type = DFU_DEV_MMC;
> >> 
> >> @@ -166,8 +170,33 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu,
> >> char *s) dfu->layout = DFU_FS_FAT;
> >> 
> >> 	} else if (!strcmp(st, "ext4")) {
> >> 	
> >> 		dfu->layout = DFU_FS_EXT4;
> >> 
> >> +	} else if (!strcmp(st, "part")) {
> >> +
> >> +		dfu->layout = DFU_RAW_ADDR;
> >> +
> >> +		dev = simple_strtoul(s, &s, 10);
> >> +		part = simple_strtoul(++s, &s, 10);
> > 
> > ++s ... this is unreadable and definitelly prone to breakage.
> 
> Just following what the existing code does. Look around this line.
> Perhaps we should modify that too.

Yes, that'd be really nice ... but in a separate patch please.

> >> +
> >> +		mmc = find_mmc_device(dev);
> >> +		if (mmc == NULL || mmc_init(mmc)) {
> >> +			printf("%s: could not find mmc device #%d!\n", __func__,
> > 
> > dev);
> > 
> >> +			return -1;
> >> +		}
> >> +
> >> +		blk_dev = &mmc->block_dev;
> >> +		if (get_partition_info(blk_dev, part, &partinfo) != 0) {
> >> +			printf("%s: could not find partition #%d on mmc device
> > 
> > #%d!\n",
> > 
> >> +					__func__, part, dev);
> >> +			return -1;
> > 
> > Try using regular errno.h ... fix all around.
> 
> The whole file doesn't use errno.h at all. Changing this single thing will
> stick out like a sore thumb.

:-(

> >> +		}
> >> +
> >> +		dfu->data.mmc.lba_start = partinfo.start;
> >> +		dfu->data.mmc.lba_size = partinfo.size;
> >> +		dfu->data.mmc.lba_blk_size = partinfo.blksz;
> >> +
> >> 
> >> 	} else {
> >> 	
> >> 		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
> >> 
> >> +		return -1;
> > 
> > This is new ... does it fit into this patch at all?
> 
> It should, it's an error condition and the code just blindly continued.

Even more :-((

> >> 	}
> >> 	
> >> 	if (dfu->layout == DFU_FS_EXT4 || dfu->layout == DFU_FS_FAT) {
> > 
> > Best regards,
> > Marek Vasut
> 
> Regards
> 
> -- Pantelis

[U-Boot] [PATCH 8/9] Issue connect/disconnect as appropriate

[Pantelis Antoniou]

Call usb_gadget_connect/usb_gadget_disconnect in g_dnl_bind/g_dnl_unbind.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/g_dnl.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
index 7d87050..bbbfe02 100644
--- a/drivers/usb/gadget/g_dnl.c
+++ b/drivers/usb/gadget/g_dnl.c
@@ -83,7 +83,11 @@ static struct usb_gadget_strings *g_dnl_composite_strings[] = {
 
 static int g_dnl_unbind(struct usb_composite_dev *cdev)
 {
-	debug("%s\n", __func__);
+	struct usb_gadget *gadget = cdev->gadget;
+
+	debug("%s: calling usb_gadget_disconnect for controller '%s'\n", shortname, gadget->name);
+	usb_gadget_disconnect(gadget);
+
 	return 0;
 }
 
@@ -153,6 +157,9 @@ static int g_dnl_bind(struct usb_composite_dev *cdev)
 		device_desc.bcdDevice = __constant_cpu_to_le16(0x9999);
 	}
 
+	debug("%s: calling usb_gadget_connect for controller '%s'\n", shortname, gadget->name);
+	usb_gadget_connect(gadget);
+
 	return 0;
 
  error:
-- 
1.7.12

[U-Boot] [PATCH 8/9] Issue connect/disconnect as appropriate

[Marek Vasut]

Dear Pantelis Antoniou,

> Call usb_gadget_connect/usb_gadget_disconnect in g_dnl_bind/g_dnl_unbind.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
[...]

u-boot$ ./tools/checkpatch.pl \[PATCH\ 8_9\]\ Issue\ connect_disconnect\ as\ 
appropriate.mbox 
WARNING: line over 80 characters
#57: FILE: drivers/usb/gadget/g_dnl.c:88:
+       debug("%s: calling usb_gadget_disconnect for controller '%s'\n", 
shortname, gadget->name);

WARNING: line over 80 characters
#67: FILE: drivers/usb/gadget/g_dnl.c:160:
+       debug("%s: calling usb_gadget_connect for controller '%s'\n", shortname, 
gadget->name);

total: 0 errors, 2 warnings, 21 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX 
MULTISTATEMENT_MACRO_USE_DO_WHILE

[PATCH 8_9] Issue connect_disconnect as appropriate.mbox has style problems, 
please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

... fix the other patches too

Best regards,
Marek Vasut

[U-Boot] [PATCH 8/9] Issue connect/disconnect as appropriate

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/27/12 21:50, Marek Vasut wrote:
> Dear Pantelis Antoniou,
> 
>> Call usb_gadget_connect/usb_gadget_disconnect in
>> g_dnl_bind/g_dnl_unbind.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> [...]
> 
> u-boot$ ./tools/checkpatch.pl \[PATCH\ 8_9\]\ Issue\
> connect_disconnect\ as\ appropriate.mbox WARNING: line over 80
> characters #57: FILE: drivers/usb/gadget/g_dnl.c:88: +
> debug("%s: calling usb_gadget_disconnect for controller '%s'\n", 
> shortname, gadget->name);
> 
> WARNING: line over 80 characters #67: FILE:
> drivers/usb/gadget/g_dnl.c:160: +       debug("%s: calling
> usb_gadget_connect for controller '%s'\n", shortname, 
> gadget->name);
> 
> total: 0 errors, 2 warnings, 21 lines checked
> 

Going over the limit is OK for prints to increase grepability.  Unless
splitting at the arguments would fix it.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQtX8HAAoJENk4IS6UOR1WddgQAIwTWvCvLrKCAwIuPNoWQrWv
SQd0Pp9cQqrEgIdNRq+qwDMO1drD40w/zZEpo21tPXtWIwHLgfEz2JAfEXRPJh0P
71hjKAQUq7Y4kkseu1Hx0qH1bmt1ILv6qZw3PHCuHGcD534kp2XegHXRnxvLho64
ucluUjcf5sYiCmp1Wfb7mRJe+qojQm6Njbx2RxyEb/5KyNuvlzYfb/Pu+QztrdYc
ku52fFf4DLBhgoYcRbUaVk9GqloeZkf6UkWbwEJQjfkxS6F32sH7Ye1/F260RBe4
jbDmmhTKNe+FTftO17N0KULdeML2nRxwDP7hv1BrE/M/WoMQvSmPavZ6X+JUJ3wb
pLrvRTrui3uohESiicVCsBjZyIMqd4rX58El9PzrXBIFdsvrJnfG1yopSzDLZ4Gu
/QCpkkUzG5FsPwA0bc5CCmaD9vAqp5Pv9VN59K9Q949hKasOv7qmPMRKqmBFAZDg
t0W7xTbh9tkOGA2oDDU6pmNAfptwIns/4FX0Gxko/3Di3BO+vW6HKqYIJND4FbEE
i3JOrMrDkT1Kse3PNevSxZ2HMb2N7ALe/HnECp4wLnorRKlyavhYfqHs+lklCJDV
gfaocfzxyFjzXnEpxGqlbBZpNyH65+YSZP7Y/8UCkgs1Sp9N9SzdGCobFDlmzJwa
Z8ZFbT1FWw0pf9kEfYM+
=XhQi
-----END PGP SIGNATURE-----

[U-Boot] [PATCH 9/9] Add DFU config

[Pantelis Antoniou]

Add configuration for using DFU on the am335x_evm boards

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 include/configs/am335x_evm.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index ab9549b..0befa34 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -39,6 +39,8 @@
 #define CONFIG_SETUP_MEMORY_TAGS
 #define CONFIG_INITRD_TAG
 
+#define CONFIG_SYS_CACHELINE_SIZE       64
+
 /* commands to include */
 #include <config_cmd_default.h>
 
@@ -90,6 +92,7 @@
 			"setenv fdtfile am335x-evm.dtb; fi; " \
 		"if test $board_name = A335X_SK; then " \
 			"setenv fdtfile am335x-evmsk.dtb; fi\0" \
+	CONFIG_DFU_ALT
 
 #define CONFIG_BOOTCOMMAND \
 	"mmc dev ${mmcdev}; if mmc rescan; then " \
@@ -153,6 +156,28 @@
 #define CONFIG_CMD_SF
 #define CONFIG_SF_DEFAULT_SPEED		(24000000)
 
+/* USB Composite download gadget - g_dnl */
+#define CONFIG_USB_GADGET
+#define CONFIG_USBDOWNLOAD_GADGET
+#define CONFIG_DFU_FUNCTION
+#define CONFIG_DFU_MMC
+
+/* USB TI's IDs */
+#define CONFIG_USBD_HS
+#define CONFIG_G_DNL_VENDOR_NUM 0x0525
+#define CONFIG_G_DNL_PRODUCT_NUM 0xa4a7
+#define CONFIG_G_DNL_MANUFACTURER "Texas Instruments"
+
+#define CONFIG_DFU_ALT \
+	"dfu_alt_info=" \
+	"boot part 0 1;" \
+	"rootfs part 0 2;" \
+	"MLO fat 0 1;" \
+	"u-boot.img fat 0 1;" \
+	"uEnv.txt fat 0 1\0"
+
+#define CONFIG_CMD_DFU
+
  /* Physical Memory Map */
 #define CONFIG_NR_DRAM_BANKS		1		/*  1 bank of DRAM */
 #define PHYS_DRAM_1			0x80000000	/* DRAM Bank #1 */
@@ -265,6 +290,7 @@
 #define CONFIG_MUSB_GADGET
 #define CONFIG_MUSB_PIO_ONLY
 #define CONFIG_USB_GADGET_DUALSPEED
+#define CONFIG_USB_GADGET_VBUS_DRAW	2
 #define CONFIG_MUSB_HOST
 #define CONFIG_AM335X_USB0
 #define CONFIG_AM335X_USB0_MODE	MUSB_PERIPHERAL
-- 
1.7.12

[U-Boot] [PATCH 9/9] Add DFU config

[Marek Vasut]

Dear Pantelis Antoniou,

> Add configuration for using DFU on the am335x_evm boards
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  include/configs/am335x_evm.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
> index ab9549b..0befa34 100644
> --- a/include/configs/am335x_evm.h
> +++ b/include/configs/am335x_evm.h
> @@ -39,6 +39,8 @@
>  #define CONFIG_SETUP_MEMORY_TAGS
>  #define CONFIG_INITRD_TAG
> 
> +#define CONFIG_SYS_CACHELINE_SIZE       64

How come you need this, isn't it configured by default for ARM ?

>  /* commands to include */
>  #include <config_cmd_default.h>
> 
> @@ -90,6 +92,7 @@
>  			"setenv fdtfile am335x-evm.dtb; fi; " \
>  		"if test $board_name = A335X_SK; then " \
>  			"setenv fdtfile am335x-evmsk.dtb; fi\0" \
> +	CONFIG_DFU_ALT

What's this stuff here?

>  #define CONFIG_BOOTCOMMAND \
>  	"mmc dev ${mmcdev}; if mmc rescan; then " \
> @@ -153,6 +156,28 @@
>  #define CONFIG_CMD_SF
>  #define CONFIG_SF_DEFAULT_SPEED		(24000000)
> 
> +/* USB Composite download gadget - g_dnl */
> +#define CONFIG_USB_GADGET
> +#define CONFIG_USBDOWNLOAD_GADGET
> +#define CONFIG_DFU_FUNCTION
> +#define CONFIG_DFU_MMC
> +
> +/* USB TI's IDs */
> +#define CONFIG_USBD_HS
> +#define CONFIG_G_DNL_VENDOR_NUM 0x0525
> +#define CONFIG_G_DNL_PRODUCT_NUM 0xa4a7
> +#define CONFIG_G_DNL_MANUFACTURER "Texas Instruments"
> +
> +#define CONFIG_DFU_ALT \
> +	"dfu_alt_info=" \
> +	"boot part 0 1;" \
> +	"rootfs part 0 2;" \
> +	"MLO fat 0 1;" \
> +	"u-boot.img fat 0 1;" \
> +	"uEnv.txt fat 0 1\0"
> +
> +#define CONFIG_CMD_DFU

I see ... hm. Not my call to decide.

>   /* Physical Memory Map */
>  #define CONFIG_NR_DRAM_BANKS		1		/*  1 bank of DRAM */
>  #define PHYS_DRAM_1			0x80000000	/* DRAM Bank #1 */
> @@ -265,6 +290,7 @@
>  #define CONFIG_MUSB_GADGET
>  #define CONFIG_MUSB_PIO_ONLY
>  #define CONFIG_USB_GADGET_DUALSPEED
> +#define CONFIG_USB_GADGET_VBUS_DRAW	2
>  #define CONFIG_MUSB_HOST
>  #define CONFIG_AM335X_USB0
>  #define CONFIG_AM335X_USB0_MODE	MUSB_PERIPHERAL

Best regards,
Marek Vasut

[U-Boot] [PATCH 9/9] Add DFU config

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/27/12 21:52, Marek Vasut wrote:
> Dear Pantelis Antoniou,
> 
>> Add configuration for using DFU on the am335x_evm boards
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com> 
>> --- include/configs/am335x_evm.h | 26 ++++++++++++++++++++++++++ 
>> 1 file changed, 26 insertions(+)
>> 
>> diff --git a/include/configs/am335x_evm.h
>> b/include/configs/am335x_evm.h index ab9549b..0befa34 100644 ---
>> a/include/configs/am335x_evm.h +++
>> b/include/configs/am335x_evm.h @@ -39,6 +39,8 @@ #define
>> CONFIG_SETUP_MEMORY_TAGS #define CONFIG_INITRD_TAG
>> 
>> +#define CONFIG_SYS_CACHELINE_SIZE       64
> 
> How come you need this, isn't it configured by default for ARM ?

iirc if we don't spell it out, it's 32, which isn't correct for the
platform.  It should be a separate patch however.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQtX9UAAoJENk4IS6UOR1WVdUP/1RbqCpgmttzpUqYDwKkVA76
b+m6Wbo5fqzle4mKy8sKBjVZCYARdtp3VPLxcqfyY5ZjZQIezjQre7PWdPWFJiDf
Yg/LoajSBxDJ2yRCgJQ4suP5lGE5lG7T0xMXE6ulIB/A4WshLJPW9IxVZT2p4iHu
93NfAY6DbKFxIze8jCJnXis8MYiAe8WCKkWmAhKt07N9Fw7L4iJcryisuG//53sq
GLdxYuIsa76h+oi3D1nPOLLp1Jldp5BbtwHW9IVPHcYC6bPFebh8juRlzPJFxqPT
FpUA4d1+dF95/MBAUSN7eewxs6Ic2Y6rpK7pYAUs3IXBDq9X9nCm1dyslo7nTKpv
K5ARuQ2OzcqD7ZfeeSyVmWHQ9RzXLbepVcWWpjv0bgumgtujrQoPB+tgnIpeObf8
R9E2XPwCO9gOjUVVudGTZ/FcNS61vwacJhVRsQvjv4Ba6ZOC5Z/bjvKqkwKA9JP4
wY+jLvjeWJOo6ZZ4/CPeO48+i6ZYbipPEj+0YQ2yL2madKYlopNG//esLZ3g7koE
ptb5nbzc0dObuAwtoqGYXIBS+i1HhsQ5c4fLOOKM69FRExC1QAoQf2KF7nbvr7RU
oL28no8EIPF5QfrneK+4HapgYfdLslqlfsrqiSqnHATWAxroddTSNii/j8n2UfA0
YXlUFd7+msYT6qVdZ9dR
=QzXg
-----END PGP SIGNATURE-----

[U-Boot] [PATCH 9/9] Add DFU config

[Pantelis Antoniou]

On Nov 28, 2012, at 5:04 AM, Tom Rini wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 11/27/12 21:52, Marek Vasut wrote:
>> Dear Pantelis Antoniou,
>> 
>>> Add configuration for using DFU on the am335x_evm boards
>>> 
>>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com> 
>>> --- include/configs/am335x_evm.h | 26 ++++++++++++++++++++++++++ 
>>> 1 file changed, 26 insertions(+)
>>> 
>>> diff --git a/include/configs/am335x_evm.h
>>> b/include/configs/am335x_evm.h index ab9549b..0befa34 100644 ---
>>> a/include/configs/am335x_evm.h +++
>>> b/include/configs/am335x_evm.h @@ -39,6 +39,8 @@ #define
>>> CONFIG_SETUP_MEMORY_TAGS #define CONFIG_INITRD_TAG
>>> 
>>> +#define CONFIG_SYS_CACHELINE_SIZE       64
>> 
>> How come you need this, isn't it configured by default for ARM ?
> 
> iirc if we don't spell it out, it's 32, which isn't correct for the
> platform.  It should be a separate patch however.
> 

OK.

> - -- 
> Tom
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://www.enigmail.net/
> 
> iQIcBAEBAgAGBQJQtX9UAAoJENk4IS6UOR1WVdUP/1RbqCpgmttzpUqYDwKkVA76
> b+m6Wbo5fqzle4mKy8sKBjVZCYARdtp3VPLxcqfyY5ZjZQIezjQre7PWdPWFJiDf
> Yg/LoajSBxDJ2yRCgJQ4suP5lGE5lG7T0xMXE6ulIB/A4WshLJPW9IxVZT2p4iHu
> 93NfAY6DbKFxIze8jCJnXis8MYiAe8WCKkWmAhKt07N9Fw7L4iJcryisuG//53sq
> GLdxYuIsa76h+oi3D1nPOLLp1Jldp5BbtwHW9IVPHcYC6bPFebh8juRlzPJFxqPT
> FpUA4d1+dF95/MBAUSN7eewxs6Ic2Y6rpK7pYAUs3IXBDq9X9nCm1dyslo7nTKpv
> K5ARuQ2OzcqD7ZfeeSyVmWHQ9RzXLbepVcWWpjv0bgumgtujrQoPB+tgnIpeObf8
> R9E2XPwCO9gOjUVVudGTZ/FcNS61vwacJhVRsQvjv4Ba6ZOC5Z/bjvKqkwKA9JP4
> wY+jLvjeWJOo6ZZ4/CPeO48+i6ZYbipPEj+0YQ2yL2madKYlopNG//esLZ3g7koE
> ptb5nbzc0dObuAwtoqGYXIBS+i1HhsQ5c4fLOOKM69FRExC1QAoQf2KF7nbvr7RU
> oL28no8EIPF5QfrneK+4HapgYfdLslqlfsrqiSqnHATWAxroddTSNii/j8n2UfA0
> YXlUFd7+msYT6qVdZ9dR
> =QzXg
> -----END PGP SIGNATURE-----