public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/9] Remove obsolete header file
  2012-11-28 12:43 ` [U-Boot] [PATCH 1/9] Remove obsolete header file Pantelis Antoniou
@ 2012-11-27 16:45   ` Tom Rini
  2012-11-27 22:00     ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Tom Rini @ 2012-11-27 16:45 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 1/9] Remove obsolete header file
  2012-11-27 16:45   ` Tom Rini
@ 2012-11-27 22:00     ` Marek Vasut
  2012-11-28  3:01       ` Tom Rini
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2012-11-27 22:00 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined
  2012-11-28 12:43 ` [U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined Pantelis Antoniou
@ 2012-11-28  2:43   ` Marek Vasut
  2012-11-28  8:24     ` Pantelis Antoniou
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2012-11-28  2:43 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS
  2012-11-28 12:43 ` [U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS Pantelis Antoniou
@ 2012-11-28  2:45   ` Marek Vasut
  2012-11-28  8:26     ` Pantelis Antoniou
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2012-11-28  2:45 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 5/9] Generate appropriate responses for DFU
  2012-11-28 12:43 ` [U-Boot] [PATCH 5/9] Generate appropriate responses for DFU Pantelis Antoniou
@ 2012-11-28  2:46   ` Marek Vasut
  2012-11-28  8:27     ` Pantelis Antoniou
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2012-11-28  2:46 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 6/9] Properly zero out timeout value
  2012-11-28 12:43 ` [U-Boot] [PATCH 6/9] Properly zero out timeout value Pantelis Antoniou
@ 2012-11-28  2:46   ` Marek Vasut
  2012-11-28  8:28     ` Pantelis Antoniou
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2012-11-28  2:46 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 7/9] Add a partition type target
  2012-11-28 12:44 ` [U-Boot] [PATCH 7/9] Add a partition type target Pantelis Antoniou
@ 2012-11-28  2:48   ` Marek Vasut
  2012-11-28  8:31     ` Pantelis Antoniou
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2012-11-28  2:48 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 8/9] Issue connect/disconnect as appropriate
  2012-11-28 12:44 ` [U-Boot] [PATCH 8/9] Issue connect/disconnect as appropriate Pantelis Antoniou
@ 2012-11-28  2:50   ` Marek Vasut
  2012-11-28  3:03     ` Tom Rini
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2012-11-28  2:50 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 9/9] Add DFU config
  2012-11-28 12:44 ` [U-Boot] [PATCH 9/9] Add DFU config Pantelis Antoniou
@ 2012-11-28  2:52   ` Marek Vasut
  2012-11-28  3:04     ` Tom Rini
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Vasut @ 2012-11-28  2:52 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 1/9] Remove obsolete header file
  2012-11-27 22:00     ` Marek Vasut
@ 2012-11-28  3:01       ` Tom Rini
  0 siblings, 0 replies; 32+ messages in thread
From: Tom Rini @ 2012-11-28  3:01 UTC (permalink / raw)
  To: u-boot

-----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-----

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 8/9] Issue connect/disconnect as appropriate
  2012-11-28  2:50   ` Marek Vasut
@ 2012-11-28  3:03     ` Tom Rini
  0 siblings, 0 replies; 32+ messages in thread
From: Tom Rini @ 2012-11-28  3:03 UTC (permalink / raw)
  To: u-boot

-----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-----

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 9/9] Add DFU config
  2012-11-28  2:52   ` Marek Vasut
@ 2012-11-28  3:04     ` Tom Rini
  2012-11-28  8:32       ` Pantelis Antoniou
  0 siblings, 1 reply; 32+ messages in thread
From: Tom Rini @ 2012-11-28  3:04 UTC (permalink / raw)
  To: u-boot

-----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-----

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined
  2012-11-28  2:43   ` Marek Vasut
@ 2012-11-28  8:24     ` Pantelis Antoniou
  2012-11-29  6:30       ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28  8:24 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS
  2012-11-28  2:45   ` Marek Vasut
@ 2012-11-28  8:26     ` Pantelis Antoniou
  2012-11-29  6:28       ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28  8:26 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 5/9] Generate appropriate responses for DFU
  2012-11-28  2:46   ` Marek Vasut
@ 2012-11-28  8:27     ` Pantelis Antoniou
  0 siblings, 0 replies; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28  8:27 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 6/9] Properly zero out timeout value
  2012-11-28  2:46   ` Marek Vasut
@ 2012-11-28  8:28     ` Pantelis Antoniou
  2012-11-28 10:23       ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28  8:28 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 7/9] Add a partition type target
  2012-11-28  2:48   ` Marek Vasut
@ 2012-11-28  8:31     ` Pantelis Antoniou
  2012-11-28  9:22       ` Marek Vasut
  0 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28  8:31 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 9/9] Add DFU config
  2012-11-28  3:04     ` Tom Rini
@ 2012-11-28  8:32       ` Pantelis Antoniou
  0 siblings, 0 replies; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28  8:32 UTC (permalink / raw)
  To: u-boot


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-----

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 7/9] Add a partition type target
  2012-11-28  8:31     ` Pantelis Antoniou
@ 2012-11-28  9:22       ` Marek Vasut
  0 siblings, 0 replies; 32+ messages in thread
From: Marek Vasut @ 2012-11-28  9:22 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 6/9] Properly zero out timeout value
  2012-11-28  8:28     ` Pantelis Antoniou
@ 2012-11-28 10:23       ` Marek Vasut
  0 siblings, 0 replies; 32+ messages in thread
From: Marek Vasut @ 2012-11-28 10:23 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb)
@ 2012-11-28 12:43 Pantelis Antoniou
  2012-11-28 12:43 ` [U-Boot] [PATCH 1/9] Remove obsolete header file Pantelis Antoniou
                   ` (8 more replies)
  0 siblings, 9 replies; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28 12:43 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 1/9] Remove obsolete header file
  2012-11-28 12:43 [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb) Pantelis Antoniou
@ 2012-11-28 12:43 ` Pantelis Antoniou
  2012-11-27 16:45   ` Tom Rini
  2012-11-28 12:43 ` [U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined Pantelis Antoniou
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28 12:43 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined
  2012-11-28 12:43 [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb) Pantelis Antoniou
  2012-11-28 12:43 ` [U-Boot] [PATCH 1/9] Remove obsolete header file Pantelis Antoniou
@ 2012-11-28 12:43 ` Pantelis Antoniou
  2012-11-28  2:43   ` Marek Vasut
  2012-11-28 12:43 ` [U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS Pantelis Antoniou
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28 12:43 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS
  2012-11-28 12:43 [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb) Pantelis Antoniou
  2012-11-28 12:43 ` [U-Boot] [PATCH 1/9] Remove obsolete header file Pantelis Antoniou
  2012-11-28 12:43 ` [U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined Pantelis Antoniou
@ 2012-11-28 12:43 ` Pantelis Antoniou
  2012-11-28  2:45   ` Marek Vasut
  2012-11-28 12:43 ` [U-Boot] [PATCH 4/9] Fix crash when wrong number of arguments given Pantelis Antoniou
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28 12:43 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 4/9] Fix crash when wrong number of arguments given
  2012-11-28 12:43 [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb) Pantelis Antoniou
                   ` (2 preceding siblings ...)
  2012-11-28 12:43 ` [U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS Pantelis Antoniou
@ 2012-11-28 12:43 ` Pantelis Antoniou
  2012-11-28 12:43 ` [U-Boot] [PATCH 5/9] Generate appropriate responses for DFU Pantelis Antoniou
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28 12:43 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 5/9] Generate appropriate responses for DFU
  2012-11-28 12:43 [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb) Pantelis Antoniou
                   ` (3 preceding siblings ...)
  2012-11-28 12:43 ` [U-Boot] [PATCH 4/9] Fix crash when wrong number of arguments given Pantelis Antoniou
@ 2012-11-28 12:43 ` Pantelis Antoniou
  2012-11-28  2:46   ` Marek Vasut
  2012-11-28 12:43 ` [U-Boot] [PATCH 6/9] Properly zero out timeout value Pantelis Antoniou
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28 12:43 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 6/9] Properly zero out timeout value
  2012-11-28 12:43 [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb) Pantelis Antoniou
                   ` (4 preceding siblings ...)
  2012-11-28 12:43 ` [U-Boot] [PATCH 5/9] Generate appropriate responses for DFU Pantelis Antoniou
@ 2012-11-28 12:43 ` Pantelis Antoniou
  2012-11-28  2:46   ` Marek Vasut
  2012-11-28 12:44 ` [U-Boot] [PATCH 7/9] Add a partition type target Pantelis Antoniou
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28 12:43 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 7/9] Add a partition type target
  2012-11-28 12:43 [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb) Pantelis Antoniou
                   ` (5 preceding siblings ...)
  2012-11-28 12:43 ` [U-Boot] [PATCH 6/9] Properly zero out timeout value Pantelis Antoniou
@ 2012-11-28 12:44 ` Pantelis Antoniou
  2012-11-28  2:48   ` Marek Vasut
  2012-11-28 12:44 ` [U-Boot] [PATCH 8/9] Issue connect/disconnect as appropriate Pantelis Antoniou
  2012-11-28 12:44 ` [U-Boot] [PATCH 9/9] Add DFU config Pantelis Antoniou
  8 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28 12:44 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 8/9] Issue connect/disconnect as appropriate
  2012-11-28 12:43 [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb) Pantelis Antoniou
                   ` (6 preceding siblings ...)
  2012-11-28 12:44 ` [U-Boot] [PATCH 7/9] Add a partition type target Pantelis Antoniou
@ 2012-11-28 12:44 ` Pantelis Antoniou
  2012-11-28  2:50   ` Marek Vasut
  2012-11-28 12:44 ` [U-Boot] [PATCH 9/9] Add DFU config Pantelis Antoniou
  8 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28 12:44 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 9/9] Add DFU config
  2012-11-28 12:43 [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb) Pantelis Antoniou
                   ` (7 preceding siblings ...)
  2012-11-28 12:44 ` [U-Boot] [PATCH 8/9] Issue connect/disconnect as appropriate Pantelis Antoniou
@ 2012-11-28 12:44 ` Pantelis Antoniou
  2012-11-28  2:52   ` Marek Vasut
  8 siblings, 1 reply; 32+ messages in thread
From: Pantelis Antoniou @ 2012-11-28 12:44 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS
  2012-11-28  8:26     ` Pantelis Antoniou
@ 2012-11-29  6:28       ` Marek Vasut
  0 siblings, 0 replies; 32+ messages in thread
From: Marek Vasut @ 2012-11-29  6:28 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined
  2012-11-28  8:24     ` Pantelis Antoniou
@ 2012-11-29  6:30       ` Marek Vasut
  0 siblings, 0 replies; 32+ messages in thread
From: Marek Vasut @ 2012-11-29  6:30 UTC (permalink / raw)
  To: u-boot

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
[...]

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2012-11-29  6:30 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-28 12:43 [U-Boot] [PATCH 0/9] DFU on am35xx_evm (against u-boot-usb) Pantelis Antoniou
2012-11-28 12:43 ` [U-Boot] [PATCH 1/9] Remove obsolete header file Pantelis Antoniou
2012-11-27 16:45   ` Tom Rini
2012-11-27 22:00     ` Marek Vasut
2012-11-28  3:01       ` Tom Rini
2012-11-28 12:43 ` [U-Boot] [PATCH 2/9] Fix bug when both DFU & ETHER are defined Pantelis Antoniou
2012-11-28  2:43   ` Marek Vasut
2012-11-28  8:24     ` Pantelis Antoniou
2012-11-29  6:30       ` Marek Vasut
2012-11-28 12:43 ` [U-Boot] [PATCH 3/9] Only perform DFU board_usb_init() for TRATS Pantelis Antoniou
2012-11-28  2:45   ` Marek Vasut
2012-11-28  8:26     ` Pantelis Antoniou
2012-11-29  6:28       ` Marek Vasut
2012-11-28 12:43 ` [U-Boot] [PATCH 4/9] Fix crash when wrong number of arguments given Pantelis Antoniou
2012-11-28 12:43 ` [U-Boot] [PATCH 5/9] Generate appropriate responses for DFU Pantelis Antoniou
2012-11-28  2:46   ` Marek Vasut
2012-11-28  8:27     ` Pantelis Antoniou
2012-11-28 12:43 ` [U-Boot] [PATCH 6/9] Properly zero out timeout value Pantelis Antoniou
2012-11-28  2:46   ` Marek Vasut
2012-11-28  8:28     ` Pantelis Antoniou
2012-11-28 10:23       ` Marek Vasut
2012-11-28 12:44 ` [U-Boot] [PATCH 7/9] Add a partition type target Pantelis Antoniou
2012-11-28  2:48   ` Marek Vasut
2012-11-28  8:31     ` Pantelis Antoniou
2012-11-28  9:22       ` Marek Vasut
2012-11-28 12:44 ` [U-Boot] [PATCH 8/9] Issue connect/disconnect as appropriate Pantelis Antoniou
2012-11-28  2:50   ` Marek Vasut
2012-11-28  3:03     ` Tom Rini
2012-11-28 12:44 ` [U-Boot] [PATCH 9/9] Add DFU config Pantelis Antoniou
2012-11-28  2:52   ` Marek Vasut
2012-11-28  3:04     ` Tom Rini
2012-11-28  8:32       ` Pantelis Antoniou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox