public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 0/3] usb: dfu: introduce dfuMANIFEST state
@ 2014-03-17 10:56 Heiko Schocher
  2014-03-17 10:56 ` [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function Heiko Schocher
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Heiko Schocher @ 2014-03-17 10:56 UTC (permalink / raw)
  To: u-boot

on nand flash using ubi, after the download of the new image into
the flash, the "rest" of the nand sectors get erased while flushing
the medium. With current u-boot version dfu-util may show:

Starting download: [##################################################] finished!
state(7) = dfuMANIFEST, status(0) = No error condition is present
unable to read DFU status

as dfu_get_status is not answered while erasing sectors, if erasing
needs some time.

So do the following changes to prevent this:

- introduce dfuManifest state
  According to dfu specification [1] section 7:
  "the device enters the dfuMANIFEST-SYNC state and awaits the solicitation
   of the status report by the host. Upon receipt of the anticipated
   DFU_GETSTATUS, the device enters the dfuMANIFEST state, where it
   completes its reprogramming operations."

- when stepping into dfuManifest state, sending a PollTimeout
  DFU_MANIFEST_POLL_TIMEOUT in ms, to the host, so the host
  (dfu-util) waits the PollTimeout before sending a get_status again.


Patch 0002-usb-dfu-introduce-dfuMANIFEST-state.patch shows
following checkpatch errors, but I think they are OK, as the
hole file uses CamelCase for the statenames as this in sync
with [1]

CHECK: Avoid CamelCase: <DFU_STATE_dfuMANIFEST>
#103: FILE: drivers/usb/gadget/f_dfu.c:190:
+		f_dfu->dfu_state = DFU_STATE_dfuMANIFEST;

CHECK: Avoid CamelCase: <DFU_STATE_dfuIDLE>
#156: FILE: drivers/usb/gadget/f_dfu.c:491:
+		f_dfu->dfu_state = DFU_STATE_dfuIDLE;

CHECK: Avoid CamelCase: <DFU_STATE_dfuERROR>
#166: FILE: drivers/usb/gadget/f_dfu.c:501:
+		f_dfu->dfu_state = DFU_STATE_dfuERROR;

total: 0 errors, 0 warnings, 3 checks, 133 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE USLEEP_RANGE

[1]:
http://www.usb.org/developers/devclass_docs/usbdfu10.pdf

- changes for v2:
  - add Pantelis Antoniou to Cc
  - add comment from Marek Vasut
    - move comment and "if" back to dfu_write()
  - add comment from Lukasz Majewski:
    - remove unneccessary comment "end?"
    - add Readme entry for DFU_DEFAULT_POLL_TIMEOUT
    - remove unneeded flag setpolltimeout

- ToDo:
   - look if it is possible to get rid of the constant DFU_MANIFEST_POLL_TIMEOUT
     for all dfu entities. Maybe as Lukasz suggested:
     Add a callback - called e.g. (*poll_timeout) to the struct [mmc|nand]_internal_data.
     Then some helper functions would be defined at dfu_[mmc|nand].c and used at f_dfu.c
     so we can decide on a per medium/partition base, how long a PollTimeout
     would be for a dfu entity ... This can be done in a seperate
     step.

Heiko Schocher (3):
  usb, dfu: extract flush code into seperate function
  usb: dfu: introduce dfuMANIFEST state
  am335x, dfu: add DFU_MANIFEST_POLL_TIMEOUT to the siemens boards

 README                                 | 10 +++++++
 drivers/dfu/dfu.c                      | 42 +++++++++++++++--------------
 drivers/usb/gadget/f_dfu.c             | 48 +++++++++++++++++++++++++++++-----
 include/configs/siemens-am33x-common.h |  1 +
 include/dfu.h                          |  4 +++
 5 files changed, 79 insertions(+), 26 deletions(-)

Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>

-- 
1.8.3.1

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-17 10:56 [U-Boot] [PATCH v2 0/3] usb: dfu: introduce dfuMANIFEST state Heiko Schocher
@ 2014-03-17 10:56 ` Heiko Schocher
  2014-03-18  0:21   ` Marek Vasut
  2014-03-17 10:56 ` [U-Boot] [PATCH v2 2/3] usb: dfu: introduce dfuMANIFEST state Heiko Schocher
  2014-03-17 10:56 ` [U-Boot] [PATCH v2 3/3] am335x, dfu: add DFU_MANIFEST_POLL_TIMEOUT to the siemens boards Heiko Schocher
  2 siblings, 1 reply; 15+ messages in thread
From: Heiko Schocher @ 2014-03-17 10:56 UTC (permalink / raw)
  To: u-boot

move the flushing code into an extra function dfu_flush(),
so it can be used from other code.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>

---
- changes for v2
  - add comment from Marek Vasut
    - move comment and "if" back to dfu_write()
  - add comment from Lukasz Majewski:
    - remove unneccessary comment
    - add Pantelis to Cc

Signed-off-by: Heiko Schocher <hs@denx.de>
---
 drivers/dfu/dfu.c | 40 +++++++++++++++++++++++-----------------
 include/dfu.h     |  1 +
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 56e69fd..11401d1 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -127,6 +127,28 @@ static int dfu_write_buffer_drain(struct dfu_entity *dfu)
 	return ret;
 }
 
+int dfu_flush(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
+{
+	int ret = 0;
+
+	if (dfu->flush_medium)
+		ret = dfu->flush_medium(dfu);
+
+	printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
+
+	/* clear everything */
+	dfu_free_buf();
+	dfu->crc = 0;
+	dfu->offset = 0;
+	dfu->i_blk_seq_num = 0;
+	dfu->i_buf_start = dfu_buf;
+	dfu->i_buf_end = dfu_buf;
+	dfu->i_buf = dfu->i_buf_start;
+	dfu->inited = 0;
+
+	return ret;
+}
+
 int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 {
 	int ret = 0;
@@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 
 	/* end? */
 	if (size == 0) {
-		/* Now try and flush to the medium if needed. */
-		if (dfu->flush_medium)
-			ret = dfu->flush_medium(dfu);
-		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
-
-		/* clear everything */
-		dfu_free_buf();
-		dfu->crc = 0;
-		dfu->offset = 0;
-		dfu->i_blk_seq_num = 0;
-		dfu->i_buf_start = dfu_buf;
-		dfu->i_buf_end = dfu_buf;
-		dfu->i_buf = dfu->i_buf_start;
-
-		dfu->inited = 0;
-
-	}
+		ret = dfu_flush(dfu, buf, size, blk_seq_num);
 
 	return ret = 0 ? size : ret;
 }
diff --git a/include/dfu.h b/include/dfu.h
index f973426..272a245 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -138,6 +138,7 @@ unsigned long dfu_get_buf_size(void);
 
 int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
 int dfu_write(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
+int dfu_flush(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
 /* Device specific */
 #ifdef CONFIG_DFU_MMC
 extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s);
-- 
1.8.3.1

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

* [U-Boot] [PATCH v2 2/3] usb: dfu: introduce dfuMANIFEST state
  2014-03-17 10:56 [U-Boot] [PATCH v2 0/3] usb: dfu: introduce dfuMANIFEST state Heiko Schocher
  2014-03-17 10:56 ` [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function Heiko Schocher
@ 2014-03-17 10:56 ` Heiko Schocher
  2014-03-17 10:56 ` [U-Boot] [PATCH v2 3/3] am335x, dfu: add DFU_MANIFEST_POLL_TIMEOUT to the siemens boards Heiko Schocher
  2 siblings, 0 replies; 15+ messages in thread
From: Heiko Schocher @ 2014-03-17 10:56 UTC (permalink / raw)
  To: u-boot

on nand flash using ubi, after the download of the new image into
the flash, the "rest" of the nand sectors get erased while flushing
the medium. With current u-boot version dfu-util may show:

Starting download: [##################################################] finished!
state(7) = dfuMANIFEST, status(0) = No error condition is present
unable to read DFU status

as get_status is not answered while erasing sectors, if erasing
needs some time.

So do the following changes to prevent this:

- introduce dfuManifest state
  According to dfu specification
  ( http://www.usb.org/developers/devclass_docs/usbdfu10.pdf ) section 7:
  "the device enters the dfuMANIFEST-SYNC state and awaits the solicitation
   of the status report by the host. Upon receipt of the anticipated
   DFU_GETSTATUS, the device enters the dfuMANIFEST state, where it
   completes its reprogramming operations."

- when stepping into dfuManifest state, sending a PollTimeout
  DFU_MANIFEST_POLL_TIMEOUT in ms, to the host, so the host
  (dfu-util) waits the PollTimeout before sending a get_status again.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>

---
- changes for v2:
  - reworked as patch 01 of this series changed as Marek Vasut commented
    -> setting req->length = 0 before calling dfu_flush() not necessary
  - add comment from Lukasz Majewski:
    - add Pantelis to Cc
    - add Readme entry for DFU_DEFAULT_POLL_TIMEOUT
    - remove unneeded flag setpolltimeout
---
 README                     | 10 ++++++++++
 drivers/dfu/dfu.c          |  4 ----
 drivers/usb/gadget/f_dfu.c | 48 ++++++++++++++++++++++++++++++++++++++++------
 include/dfu.h              |  3 +++
 4 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/README b/README
index 216f0c7..7cb7c4f 100644
--- a/README
+++ b/README
@@ -1525,6 +1525,16 @@ The following options need to be configured:
 		this to the maximum filesize (in bytes) for the buffer.
 		Default is 4 MiB if undefined.
 
+		DFU_DEFAULT_POLL_TIMEOUT
+		Poll timeout [ms], is the timeout a device can send to the
+		host. The host must wait for this timeout before sending
+		a subsequent DFU_GET_STATUS request to the device.
+
+		DFU_MANIFEST_POLL_TIMEOUT
+		Poll timeout [ms], which the device sends to the host when
+		entering dfuMANIFEST state. Host waits this timeout, before
+		sending again an USB request to the device.
+
 - Journaling Flash filesystem support:
 		CONFIG_JFFS2_NAND, CONFIG_JFFS2_NAND_OFF, CONFIG_JFFS2_NAND_SIZE,
 		CONFIG_JFFS2_NAND_DEV
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 11401d1..8a09aaf 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -219,10 +219,6 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 			ret = tret;
 	}
 
-	/* end? */
-	if (size == 0) {
-		ret = dfu_flush(dfu, buf, size, blk_seq_num);
-
 	return ret = 0 ? size : ret;
 }
 
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index a045864..de75ff1 100644
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -164,9 +164,14 @@ static void dnload_request_complete(struct usb_ep *ep, struct usb_request *req)
 
 	dfu_write(dfu_get_entity(f_dfu->altsetting), req->buf,
 		  req->length, f_dfu->blk_seq_num);
+}
 
-	if (req->length == 0)
-		puts("DOWNLOAD ... OK\nCtrl+C to exit ...\n");
+static void dnload_request_flush(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_dfu *f_dfu = req->context;
+
+	dfu_flush(dfu_get_entity(f_dfu->altsetting), req->buf,
+		  req->length, f_dfu->blk_seq_num);
 }
 
 static void handle_getstatus(struct usb_request *req)
@@ -174,19 +179,22 @@ static void handle_getstatus(struct usb_request *req)
 	struct dfu_status *dstat = (struct dfu_status *)req->buf;
 	struct f_dfu *f_dfu = req->context;
 
+	dfu_set_poll_timeout(dstat, 0);
+
 	switch (f_dfu->dfu_state) {
 	case DFU_STATE_dfuDNLOAD_SYNC:
 	case DFU_STATE_dfuDNBUSY:
 		f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_IDLE;
 		break;
 	case DFU_STATE_dfuMANIFEST_SYNC:
+		f_dfu->dfu_state = DFU_STATE_dfuMANIFEST;
 		break;
+	case DFU_STATE_dfuMANIFEST:
+		dfu_set_poll_timeout(dstat, DFU_MANIFEST_POLL_TIMEOUT);
 	default:
 		break;
 	}
 
-	dfu_set_poll_timeout(dstat, 0);
-
 	if (f_dfu->poll_timeout)
 		if (!(f_dfu->blk_seq_num %
 		      (dfu_get_buf_size() / DFU_USB_BUFSIZ)))
@@ -446,10 +454,11 @@ static int state_dfu_manifest_sync(struct f_dfu *f_dfu,
 	switch (ctrl->bRequest) {
 	case USB_REQ_DFU_GETSTATUS:
 		/* We're MainfestationTolerant */
-		f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+		f_dfu->dfu_state = DFU_STATE_dfuMANIFEST;
 		handle_getstatus(req);
 		f_dfu->blk_seq_num = 0;
 		value = RET_STAT_LEN;
+		req->complete = dnload_request_flush;
 		break;
 	case USB_REQ_DFU_GETSTATE:
 		handle_getstate(req);
@@ -463,6 +472,33 @@ static int state_dfu_manifest_sync(struct f_dfu *f_dfu,
 	return value;
 }
 
+static int state_dfu_manifest(struct f_dfu *f_dfu,
+			      const struct usb_ctrlrequest *ctrl,
+			      struct usb_gadget *gadget,
+			      struct usb_request *req)
+{
+	int value = 0;
+
+	switch (ctrl->bRequest) {
+	case USB_REQ_DFU_GETSTATUS:
+		/* We're MainfestationTolerant */
+		f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+		handle_getstatus(req);
+		f_dfu->blk_seq_num = 0;
+		value = RET_STAT_LEN;
+		puts("DOWNLOAD ... OK\nCtrl+C to exit ...\n");
+		break;
+	case USB_REQ_DFU_GETSTATE:
+		handle_getstate(req);
+		break;
+	default:
+		f_dfu->dfu_state = DFU_STATE_dfuERROR;
+		value = RET_STALL;
+		break;
+	}
+	return value;
+}
+
 static int state_dfu_upload_idle(struct f_dfu *f_dfu,
 				 const struct usb_ctrlrequest *ctrl,
 				 struct usb_gadget *gadget,
@@ -539,7 +575,7 @@ static dfu_state_fn dfu_state[] = {
 	state_dfu_dnbusy,        /* DFU_STATE_dfuDNBUSY */
 	state_dfu_dnload_idle,   /* DFU_STATE_dfuDNLOAD_IDLE */
 	state_dfu_manifest_sync, /* DFU_STATE_dfuMANIFEST_SYNC */
-	NULL,                    /* DFU_STATE_dfuMANIFEST */
+	state_dfu_manifest,	 /* DFU_STATE_dfuMANIFEST */
 	NULL,                    /* DFU_STATE_dfuMANIFEST_WAIT_RST */
 	state_dfu_upload_idle,   /* DFU_STATE_dfuUPLOAD_IDLE */
 	state_dfu_error          /* DFU_STATE_dfuERROR */
diff --git a/include/dfu.h b/include/dfu.h
index 272a245..6c71ecb 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -80,6 +80,9 @@ static inline unsigned int get_mmc_blk_size(int dev)
 #ifndef DFU_DEFAULT_POLL_TIMEOUT
 #define DFU_DEFAULT_POLL_TIMEOUT 0
 #endif
+#ifndef DFU_MANIFEST_POLL_TIMEOUT
+#define DFU_MANIFEST_POLL_TIMEOUT	DFU_DEFAULT_POLL_TIMEOUT
+#endif
 
 struct dfu_entity {
 	char			name[DFU_NAME_SIZE];
-- 
1.8.3.1

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

* [U-Boot] [PATCH v2 3/3] am335x, dfu: add DFU_MANIFEST_POLL_TIMEOUT to the siemens boards
  2014-03-17 10:56 [U-Boot] [PATCH v2 0/3] usb: dfu: introduce dfuMANIFEST state Heiko Schocher
  2014-03-17 10:56 ` [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function Heiko Schocher
  2014-03-17 10:56 ` [U-Boot] [PATCH v2 2/3] usb: dfu: introduce dfuMANIFEST state Heiko Schocher
@ 2014-03-17 10:56 ` Heiko Schocher
  2 siblings, 0 replies; 15+ messages in thread
From: Heiko Schocher @ 2014-03-17 10:56 UTC (permalink / raw)
  To: u-boot

as the siemens boards use dfu for updating a nand ubi partition
add DFU_MANIFEST_POLL_TIMEOUT to them, so dfu host waits after
complete transfer of the new image for DFU_MANIFEST_POLL_TIMEOUT
ms before sending again an usb request. So the board have enough
time to erase rest of the nand sectors.

Signed-off-by: Heiko Schocher <hs@denx.de>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>

---
- changes for v2:
  - add Reviewed-by from Lukasz Majewski
  - add Pantelis Antoniou to Cc
---
 include/configs/siemens-am33x-common.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/configs/siemens-am33x-common.h b/include/configs/siemens-am33x-common.h
index 98b6e72..721c4e6 100644
--- a/include/configs/siemens-am33x-common.h
+++ b/include/configs/siemens-am33x-common.h
@@ -265,6 +265,7 @@
 #define CONFIG_DFU_NAND
 #define CONFIG_CMD_DFU
 #define CONFIG_SYS_DFU_DATA_BUF_SIZE	(1 << 20)
+#define DFU_MANIFEST_POLL_TIMEOUT	25000
 
 #endif /* CONFIG_SPL_BUILD */
 
-- 
1.8.3.1

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-17 10:56 ` [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function Heiko Schocher
@ 2014-03-18  0:21   ` Marek Vasut
  2014-03-18  6:02     ` Heiko Schocher
  2014-03-18  6:51     ` Lukasz Majewski
  0 siblings, 2 replies; 15+ messages in thread
From: Marek Vasut @ 2014-03-18  0:21 UTC (permalink / raw)
  To: u-boot

On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher wrote:
> move the flushing code into an extra function dfu_flush(),
> so it can be used from other code.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>

[...]

> @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int
> size, int blk_seq_num)
> 
>  	/* end? */
>  	if (size == 0) {
> -		/* Now try and flush to the medium if needed. */
> -		if (dfu->flush_medium)
> -			ret = dfu->flush_medium(dfu);
> -		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
> -
> -		/* clear everything */
> -		dfu_free_buf();
> -		dfu->crc = 0;
> -		dfu->offset = 0;
> -		dfu->i_blk_seq_num = 0;
> -		dfu->i_buf_start = dfu_buf;
> -		dfu->i_buf_end = dfu_buf;
> -		dfu->i_buf = dfu->i_buf_start;
> -
> -		dfu->inited = 0;
> -
> -	}
> +		ret = dfu_flush(dfu, buf, size, blk_seq_num);

This seems broken, at least because you didn't close the opened brace (see 'if 
(size == 0) {' .... I can fix this up when applying.

Do you want this in 2014.04 or in -next ?

[...]

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-18  0:21   ` Marek Vasut
@ 2014-03-18  6:02     ` Heiko Schocher
  2014-03-18  6:55       ` Lukasz Majewski
  2014-03-18 11:31       ` Marek Vasut
  2014-03-18  6:51     ` Lukasz Majewski
  1 sibling, 2 replies; 15+ messages in thread
From: Heiko Schocher @ 2014-03-18  6:02 UTC (permalink / raw)
  To: u-boot

Hello Marek,

Am 18.03.2014 01:21, schrieb Marek Vasut:
> On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher wrote:
>> move the flushing code into an extra function dfu_flush(),
>> so it can be used from other code.
>>
>> Signed-off-by: Heiko Schocher<hs@denx.de>
>> Cc: Lukasz Majewski<l.majewski@samsung.com>
>> Cc: Kyungmin Park<kyungmin.park@samsung.com>
>> Cc: Marek Vasut<marex@denx.de>
>> Cc: Pantelis Antoniou<panto@antoniou-consulting.com>
>
> [...]
>
>> @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int
>> size, int blk_seq_num)
>>
>>   	/* end? */
>>   	if (size == 0) {
>> -		/* Now try and flush to the medium if needed. */
>> -		if (dfu->flush_medium)
>> -			ret = dfu->flush_medium(dfu);
>> -		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
>> -
>> -		/* clear everything */
>> -		dfu_free_buf();
>> -		dfu->crc = 0;
>> -		dfu->offset = 0;
>> -		dfu->i_blk_seq_num = 0;
>> -		dfu->i_buf_start = dfu_buf;
>> -		dfu->i_buf_end = dfu_buf;
>> -		dfu->i_buf = dfu->i_buf_start;
>> -
>> -		dfu->inited = 0;
>> -
>> -	}
>> +		ret = dfu_flush(dfu, buf, size, blk_seq_num);
>
> This seems broken, at least because you didn't close the opened brace (see 'if
> (size == 0) {' .... I can fix this up when applying.

Argh... you are right .... patch 2/3 of this series deletes the
complete if ... I can send a new version, but if you want to fix
it that would be great!

> Do you want this in 2014.04 or in -next ?

Hmm.. 2014.04 would be nice, but I think this is Lukasz decision,
because I changed the dfu state machine, and maybe he want to see
more tests?

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

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-18  0:21   ` Marek Vasut
  2014-03-18  6:02     ` Heiko Schocher
@ 2014-03-18  6:51     ` Lukasz Majewski
  2014-03-18 11:33       ` Marek Vasut
  1 sibling, 1 reply; 15+ messages in thread
From: Lukasz Majewski @ 2014-03-18  6:51 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher wrote:
> > move the flushing code into an extra function dfu_flush(),
> > so it can be used from other code.
> > 
> > Signed-off-by: Heiko Schocher <hs@denx.de>
> > Cc: Lukasz Majewski <l.majewski@samsung.com>
> > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > Cc: Marek Vasut <marex@denx.de>
> > Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> 
> [...]
> 
> > @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu, void
> > *buf, int size, int blk_seq_num)
> > 
> >  	/* end? */
> >  	if (size == 0) {
> > -		/* Now try and flush to the medium if needed. */
> > -		if (dfu->flush_medium)
> > -			ret = dfu->flush_medium(dfu);
> > -		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
> > -
> > -		/* clear everything */
> > -		dfu_free_buf();
> > -		dfu->crc = 0;
> > -		dfu->offset = 0;
> > -		dfu->i_blk_seq_num = 0;
> > -		dfu->i_buf_start = dfu_buf;
> > -		dfu->i_buf_end = dfu_buf;
> > -		dfu->i_buf = dfu->i_buf_start;
> > -
> > -		dfu->inited = 0;
> > -
> > -	}
> > +		ret = dfu_flush(dfu, buf, size, blk_seq_num);
> 
> This seems broken, at least because you didn't close the opened brace
> (see 'if (size == 0) {' .... I can fix this up when applying.
> 

I was prepared to pull those patches to u-boot-dfu repo today and test
them :-).

Afterwards, I would send PR to Tom.

Are you OK with this?

> Do you want this in 2014.04 or in -next ?

After testing I would go for 2014.04, if Tom doesn't mind. All in all
it "fixes" the DFU state machine - by adding missing states. Up till
now we had short cut in the DFU state machine.

> 
> [...]
> 
> Best regards,
> Marek Vasut



-- 
Best regards,

Lukasz Majewski

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

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-18  6:02     ` Heiko Schocher
@ 2014-03-18  6:55       ` Lukasz Majewski
  2014-03-18 11:32         ` Marek Vasut
  2014-03-18 11:31       ` Marek Vasut
  1 sibling, 1 reply; 15+ messages in thread
From: Lukasz Majewski @ 2014-03-18  6:55 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

> Hello Marek,
> 
> Am 18.03.2014 01:21, schrieb Marek Vasut:
> > On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher wrote:
> >> move the flushing code into an extra function dfu_flush(),
> >> so it can be used from other code.
> >>
> >> Signed-off-by: Heiko Schocher<hs@denx.de>
> >> Cc: Lukasz Majewski<l.majewski@samsung.com>
> >> Cc: Kyungmin Park<kyungmin.park@samsung.com>
> >> Cc: Marek Vasut<marex@denx.de>
> >> Cc: Pantelis Antoniou<panto@antoniou-consulting.com>
> >
> > [...]
> >
> >> @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu, void
> >> *buf, int size, int blk_seq_num)
> >>
> >>   	/* end? */
> >>   	if (size == 0) {
> >> -		/* Now try and flush to the medium if needed. */
> >> -		if (dfu->flush_medium)
> >> -			ret = dfu->flush_medium(dfu);
> >> -		printf("\nDFU complete CRC32: 0x%08x\n",
> >> dfu->crc); -
> >> -		/* clear everything */
> >> -		dfu_free_buf();
> >> -		dfu->crc = 0;
> >> -		dfu->offset = 0;
> >> -		dfu->i_blk_seq_num = 0;
> >> -		dfu->i_buf_start = dfu_buf;
> >> -		dfu->i_buf_end = dfu_buf;
> >> -		dfu->i_buf = dfu->i_buf_start;
> >> -
> >> -		dfu->inited = 0;
> >> -
> >> -	}
> >> +		ret = dfu_flush(dfu, buf, size, blk_seq_num);
> >
> > This seems broken, at least because you didn't close the opened
> > brace (see 'if (size == 0) {' .... I can fix this up when applying.
> 
> Argh... you are right .... patch 2/3 of this series deletes the
> complete if ... I can send a new version, but if you want to fix
> it that would be great!

In this case, please prepare v3.

> 
> > Do you want this in 2014.04 or in -next ?
> 
> Hmm.. 2014.04 would be nice, but I think this is Lukasz decision,
> because I changed the dfu state machine, and maybe he want to see
> more tests?

When you send the v3, I will test it for regression and then I will
prepare pull request to Tom for v2014.04.

Those patches will be added to u-boot-dfu repository.

> 
> bye,
> Heiko


-- 
Best regards,

Lukasz Majewski

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

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-18  6:02     ` Heiko Schocher
  2014-03-18  6:55       ` Lukasz Majewski
@ 2014-03-18 11:31       ` Marek Vasut
  1 sibling, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2014-03-18 11:31 UTC (permalink / raw)
  To: u-boot

On Tuesday, March 18, 2014 at 07:02:59 AM, Heiko Schocher wrote:
> Hello Marek,
> 
> Am 18.03.2014 01:21, schrieb Marek Vasut:
> > On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher wrote:
> >> move the flushing code into an extra function dfu_flush(),
> >> so it can be used from other code.
> >> 
> >> Signed-off-by: Heiko Schocher<hs@denx.de>
> >> Cc: Lukasz Majewski<l.majewski@samsung.com>
> >> Cc: Kyungmin Park<kyungmin.park@samsung.com>
> >> Cc: Marek Vasut<marex@denx.de>
> >> Cc: Pantelis Antoniou<panto@antoniou-consulting.com>
> > 
> > [...]
> > 
> >> @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
> >> int size, int blk_seq_num)
> >> 
> >>   	/* end? */
> >>   	if (size == 0) {
> >> 
> >> -		/* Now try and flush to the medium if needed. */
> >> -		if (dfu->flush_medium)
> >> -			ret = dfu->flush_medium(dfu);
> >> -		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
> >> -
> >> -		/* clear everything */
> >> -		dfu_free_buf();
> >> -		dfu->crc = 0;
> >> -		dfu->offset = 0;
> >> -		dfu->i_blk_seq_num = 0;
> >> -		dfu->i_buf_start = dfu_buf;
> >> -		dfu->i_buf_end = dfu_buf;
> >> -		dfu->i_buf = dfu->i_buf_start;
> >> -
> >> -		dfu->inited = 0;
> >> -
> >> -	}
> >> +		ret = dfu_flush(dfu, buf, size, blk_seq_num);
> > 
> > This seems broken, at least because you didn't close the opened brace
> > (see 'if (size == 0) {' .... I can fix this up when applying.
> 
> Argh... you are right .... patch 2/3 of this series deletes the
> complete if ... I can send a new version, but if you want to fix
> it that would be great!

OK, fixed up.

> > Do you want this in 2014.04 or in -next ?
> 
> Hmm.. 2014.04 would be nice, but I think this is Lukasz decision,
> because I changed the dfu state machine, and maybe he want to see
> more tests?

OK, let's see.

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-18  6:55       ` Lukasz Majewski
@ 2014-03-18 11:32         ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2014-03-18 11:32 UTC (permalink / raw)
  To: u-boot

On Tuesday, March 18, 2014 at 07:55:04 AM, Lukasz Majewski wrote:
> Hi Heiko,
> 
> > Hello Marek,
> > 
> > Am 18.03.2014 01:21, schrieb Marek Vasut:
> > > On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher wrote:
> > >> move the flushing code into an extra function dfu_flush(),
> > >> so it can be used from other code.
> > >> 
> > >> Signed-off-by: Heiko Schocher<hs@denx.de>
> > >> Cc: Lukasz Majewski<l.majewski@samsung.com>
> > >> Cc: Kyungmin Park<kyungmin.park@samsung.com>
> > >> Cc: Marek Vasut<marex@denx.de>
> > >> Cc: Pantelis Antoniou<panto@antoniou-consulting.com>
> > > 
> > > [...]
> > > 
> > >> @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu, void
> > >> *buf, int size, int blk_seq_num)
> > >> 
> > >>   	/* end? */
> > >>   	if (size == 0) {
> > >> 
> > >> -		/* Now try and flush to the medium if needed. */
> > >> -		if (dfu->flush_medium)
> > >> -			ret = dfu->flush_medium(dfu);
> > >> -		printf("\nDFU complete CRC32: 0x%08x\n",
> > >> dfu->crc); -
> > >> -		/* clear everything */
> > >> -		dfu_free_buf();
> > >> -		dfu->crc = 0;
> > >> -		dfu->offset = 0;
> > >> -		dfu->i_blk_seq_num = 0;
> > >> -		dfu->i_buf_start = dfu_buf;
> > >> -		dfu->i_buf_end = dfu_buf;
> > >> -		dfu->i_buf = dfu->i_buf_start;
> > >> -
> > >> -		dfu->inited = 0;
> > >> -
> > >> -	}
> > >> +		ret = dfu_flush(dfu, buf, size, blk_seq_num);
> > > 
> > > This seems broken, at least because you didn't close the opened
> > > brace (see 'if (size == 0) {' .... I can fix this up when applying.
> > 
> > Argh... you are right .... patch 2/3 of this series deletes the
> > complete if ... I can send a new version, but if you want to fix
> > it that would be great!
> 
> In this case, please prepare v3.
> 
> > > Do you want this in 2014.04 or in -next ?
> > 
> > Hmm.. 2014.04 would be nice, but I think this is Lukasz decision,
> > because I changed the dfu state machine, and maybe he want to see
> > more tests?
> 
> When you send the v3, I will test it for regression and then I will
> prepare pull request to Tom for v2014.04.
> 
> Those patches will be added to u-boot-dfu repository.

Feel free to test u-boot-usb/master please.

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-18  6:51     ` Lukasz Majewski
@ 2014-03-18 11:33       ` Marek Vasut
  2014-03-18 12:35         ` Lukasz Majewski
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2014-03-18 11:33 UTC (permalink / raw)
  To: u-boot

On Tuesday, March 18, 2014 at 07:51:05 AM, Lukasz Majewski wrote:
> Hi Marek,
> 
> > On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher wrote:
> > > move the flushing code into an extra function dfu_flush(),
> > > so it can be used from other code.
> > > 
> > > Signed-off-by: Heiko Schocher <hs@denx.de>
> > > Cc: Lukasz Majewski <l.majewski@samsung.com>
> > > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > > Cc: Marek Vasut <marex@denx.de>
> > > Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> > 
> > [...]
> > 
> > > @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu, void
> > > *buf, int size, int blk_seq_num)
> > > 
> > >  	/* end? */
> > >  	if (size == 0) {
> > > 
> > > -		/* Now try and flush to the medium if needed. */
> > > -		if (dfu->flush_medium)
> > > -			ret = dfu->flush_medium(dfu);
> > > -		printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc);
> > > -
> > > -		/* clear everything */
> > > -		dfu_free_buf();
> > > -		dfu->crc = 0;
> > > -		dfu->offset = 0;
> > > -		dfu->i_blk_seq_num = 0;
> > > -		dfu->i_buf_start = dfu_buf;
> > > -		dfu->i_buf_end = dfu_buf;
> > > -		dfu->i_buf = dfu->i_buf_start;
> > > -
> > > -		dfu->inited = 0;
> > > -
> > > -	}
> > > +		ret = dfu_flush(dfu, buf, size, blk_seq_num);
> > 
> > This seems broken, at least because you didn't close the opened brace
> > (see 'if (size == 0) {' .... I can fix this up when applying.
> 
> I was prepared to pull those patches to u-boot-dfu repo today and test
> them :-).
> 
> Afterwards, I would send PR to Tom.
> 
> Are you OK with this?

Doesn't -dfu go through -usb ? ;-) Or do you want to send to Tom directly ? 
Either way WFM. Nonetheless, I picked the patches via -usb, they're in master 
now.

> > Do you want this in 2014.04 or in -next ?
> 
> After testing I would go for 2014.04, if Tom doesn't mind. All in all
> it "fixes" the DFU state machine - by adding missing states. Up till
> now we had short cut in the DFU state machine.

OK

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-18 11:33       ` Marek Vasut
@ 2014-03-18 12:35         ` Lukasz Majewski
  2014-03-18 15:23           ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Lukasz Majewski @ 2014-03-18 12:35 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On Tuesday, March 18, 2014 at 07:51:05 AM, Lukasz Majewski wrote:
> > Hi Marek,
> > 
> > > On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher wrote:
> > > > move the flushing code into an extra function dfu_flush(),
> > > > so it can be used from other code.
> > > > 
> > > > Signed-off-by: Heiko Schocher <hs@denx.de>
> > > > Cc: Lukasz Majewski <l.majewski@samsung.com>
> > > > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > > > Cc: Marek Vasut <marex@denx.de>
> > > > Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> > > 
> > > [...]
> > > 
> > > > @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu, void
> > > > *buf, int size, int blk_seq_num)
> > > > 
> > > >  	/* end? */
> > > >  	if (size == 0) {
> > > > 
> > > > -		/* Now try and flush to the medium if needed.
> > > > */
> > > > -		if (dfu->flush_medium)
> > > > -			ret = dfu->flush_medium(dfu);
> > > > -		printf("\nDFU complete CRC32: 0x%08x\n",
> > > > dfu->crc); -
> > > > -		/* clear everything */
> > > > -		dfu_free_buf();
> > > > -		dfu->crc = 0;
> > > > -		dfu->offset = 0;
> > > > -		dfu->i_blk_seq_num = 0;
> > > > -		dfu->i_buf_start = dfu_buf;
> > > > -		dfu->i_buf_end = dfu_buf;
> > > > -		dfu->i_buf = dfu->i_buf_start;
> > > > -
> > > > -		dfu->inited = 0;
> > > > -
> > > > -	}
> > > > +		ret = dfu_flush(dfu, buf, size, blk_seq_num);
> > > 
> > > This seems broken, at least because you didn't close the opened
> > > brace (see 'if (size == 0) {' .... I can fix this up when
> > > applying.
> > 
> > I was prepared to pull those patches to u-boot-dfu repo today and
> > test them :-).
> > 
> > Afterwards, I would send PR to Tom.
> > 
> > Are you OK with this?
> 
> Doesn't -dfu go through -usb ? ;-) 

As fair as I remember the dfu code from ./drivers/dfu/* shall go via
u-boot-dfu tree directly to Tom.

> Or do you want to send to Tom
> directly ? Either way WFM.

After writing the other message you waited for a response less than one
minute :-).

> Nonetheless, I picked the patches via
> -usb, they're in master now.

I've just tested them and they doesn't cause any regression.

Would you be so kind and apply one more patch, which was supposed to
go with u-boot-dfu PR (this patch waits for submission from 21.02.2014)?

http://patchwork.ozlabs.org/patch/322450/

It is solely drivers/dfu/dfu_mmc.c related.

And I don't want to send PR to Tom with only one patch included.

> 
> > > Do you want this in 2014.04 or in -next ?
> > 
> > After testing I would go for 2014.04, if Tom doesn't mind. All in
> > all it "fixes" the DFU state machine - by adding missing states. Up
> > till now we had short cut in the DFU state machine.
> 
> OK
> 
> Best regards,
> Marek Vasut



-- 
Best regards,

Lukasz Majewski

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

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-18 12:35         ` Lukasz Majewski
@ 2014-03-18 15:23           ` Marek Vasut
  2014-03-18 15:53             ` Lukasz Majewski
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2014-03-18 15:23 UTC (permalink / raw)
  To: u-boot

On Tuesday, March 18, 2014 at 01:35:47 PM, Lukasz Majewski wrote:
> Hi Marek,
> 
> > On Tuesday, March 18, 2014 at 07:51:05 AM, Lukasz Majewski wrote:
> > > Hi Marek,
> > > 
> > > > On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher wrote:
> > > > > move the flushing code into an extra function dfu_flush(),
> > > > > so it can be used from other code.
> > > > > 
> > > > > Signed-off-by: Heiko Schocher <hs@denx.de>
> > > > > Cc: Lukasz Majewski <l.majewski@samsung.com>
> > > > > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > > > > Cc: Marek Vasut <marex@denx.de>
> > > > > Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> > > > 
> > > > [...]
> > > > 
> > > > > @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu, void
> > > > > *buf, int size, int blk_seq_num)
> > > > > 
> > > > >  	/* end? */
> > > > >  	if (size == 0) {
> > > > > 
> > > > > -		/* Now try and flush to the medium if needed.
> > > > > */
> > > > > -		if (dfu->flush_medium)
> > > > > -			ret = dfu->flush_medium(dfu);
> > > > > -		printf("\nDFU complete CRC32: 0x%08x\n",
> > > > > dfu->crc); -
> > > > > -		/* clear everything */
> > > > > -		dfu_free_buf();
> > > > > -		dfu->crc = 0;
> > > > > -		dfu->offset = 0;
> > > > > -		dfu->i_blk_seq_num = 0;
> > > > > -		dfu->i_buf_start = dfu_buf;
> > > > > -		dfu->i_buf_end = dfu_buf;
> > > > > -		dfu->i_buf = dfu->i_buf_start;
> > > > > -
> > > > > -		dfu->inited = 0;
> > > > > -
> > > > > -	}
> > > > > +		ret = dfu_flush(dfu, buf, size, blk_seq_num);
> > > > 
> > > > This seems broken, at least because you didn't close the opened
> > > > brace (see 'if (size == 0) {' .... I can fix this up when
> > > > applying.
> > > 
> > > I was prepared to pull those patches to u-boot-dfu repo today and
> > > test them :-).
> > > 
> > > Afterwards, I would send PR to Tom.
> > > 
> > > Are you OK with this?
> > 
> > Doesn't -dfu go through -usb ? ;-)
> 
> As fair as I remember the dfu code from ./drivers/dfu/* shall go via
> u-boot-dfu tree directly to Tom.
> 
> > Or do you want to send to Tom
> > directly ? Either way WFM.
> 
> After writing the other message you waited for a response less than one
> minute :-).
> 
> > Nonetheless, I picked the patches via
> > -usb, they're in master now.
> 
> I've just tested them and they doesn't cause any regression.
> 
> Would you be so kind and apply one more patch, which was supposed to
> go with u-boot-dfu PR (this patch waits for submission from 21.02.2014)?
> 
> http://patchwork.ozlabs.org/patch/322450/

OK, picked and pushed. I think panto picked it up as well tho, but maybe I 
unleashed too much mental terror unto him with my OvisLink WL-1100SD that he is 
paralyzed by it ;-) Panto, I will pick this patch, OK ?

As for the DFU, what do we do about the PRs ? DFU is part of USB, so I shall 
logically pick the PRs from you and move them to trini, but if you're unhappy 
about this, we need to talk about that. Tom, Lukasz ... thoughts ?

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-18 15:23           ` Marek Vasut
@ 2014-03-18 15:53             ` Lukasz Majewski
  2014-03-18 18:21               ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Lukasz Majewski @ 2014-03-18 15:53 UTC (permalink / raw)
  To: u-boot

Hi Marek,

> On Tuesday, March 18, 2014 at 01:35:47 PM, Lukasz Majewski wrote:
> > Hi Marek,
> > 
> > > On Tuesday, March 18, 2014 at 07:51:05 AM, Lukasz Majewski wrote:
> > > > Hi Marek,
> > > > 
> > > > > On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher
> > > > > wrote:
> > > > > > move the flushing code into an extra function dfu_flush(),
> > > > > > so it can be used from other code.
> > > > > > 
> > > > > > Signed-off-by: Heiko Schocher <hs@denx.de>
> > > > > > Cc: Lukasz Majewski <l.majewski@samsung.com>
> > > > > > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > > > > > Cc: Marek Vasut <marex@denx.de>
> > > > > > Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> > > > > 
> > > > > [...]
> > > > > 
> > > > > > @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu,
> > > > > > void *buf, int size, int blk_seq_num)
> > > > > > 
> > > > > >  	/* end? */
> > > > > >  	if (size == 0) {
> > > > > > 
> > > > > > -		/* Now try and flush to the medium if
> > > > > > needed. */
> > > > > > -		if (dfu->flush_medium)
> > > > > > -			ret = dfu->flush_medium(dfu);
> > > > > > -		printf("\nDFU complete CRC32: 0x%08x\n",
> > > > > > dfu->crc); -
> > > > > > -		/* clear everything */
> > > > > > -		dfu_free_buf();
> > > > > > -		dfu->crc = 0;
> > > > > > -		dfu->offset = 0;
> > > > > > -		dfu->i_blk_seq_num = 0;
> > > > > > -		dfu->i_buf_start = dfu_buf;
> > > > > > -		dfu->i_buf_end = dfu_buf;
> > > > > > -		dfu->i_buf = dfu->i_buf_start;
> > > > > > -
> > > > > > -		dfu->inited = 0;
> > > > > > -
> > > > > > -	}
> > > > > > +		ret = dfu_flush(dfu, buf, size,
> > > > > > blk_seq_num);
> > > > > 
> > > > > This seems broken, at least because you didn't close the
> > > > > opened brace (see 'if (size == 0) {' .... I can fix this up
> > > > > when applying.
> > > > 
> > > > I was prepared to pull those patches to u-boot-dfu repo today
> > > > and test them :-).
> > > > 
> > > > Afterwards, I would send PR to Tom.
> > > > 
> > > > Are you OK with this?
> > > 
> > > Doesn't -dfu go through -usb ? ;-)
> > 
> > As fair as I remember the dfu code from ./drivers/dfu/* shall go via
> > u-boot-dfu tree directly to Tom.
> > 
> > > Or do you want to send to Tom
> > > directly ? Either way WFM.
> > 
> > After writing the other message you waited for a response less than
> > one minute :-).
> > 
> > > Nonetheless, I picked the patches via
> > > -usb, they're in master now.
> > 
> > I've just tested them and they doesn't cause any regression.
> > 
> > Would you be so kind and apply one more patch, which was supposed to
> > go with u-boot-dfu PR (this patch waits for submission from
> > 21.02.2014)?
> > 
> > http://patchwork.ozlabs.org/patch/322450/
> 
> OK, picked and pushed. I think panto picked it up as well tho, but
> maybe I unleashed too much mental terror unto him with my OvisLink
> WL-1100SD that he is paralyzed by it ;-) Panto, I will pick this
> patch, OK ?

This patch looks like a little orphan :-). I'm glad that someone pull
it finally.

> 
> As for the DFU, what do we do about the PRs ? DFU is part of USB, so
> I shall logically pick the PRs from you and move them to trini, but
> if you're unhappy about this, we need to talk about that. Tom,
> Lukasz ... thoughts ?

I'm fine with sending PRs to you.


-- 
Best regards,

Lukasz Majewski

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

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

* [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function
  2014-03-18 15:53             ` Lukasz Majewski
@ 2014-03-18 18:21               ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2014-03-18 18:21 UTC (permalink / raw)
  To: u-boot

On Tuesday, March 18, 2014 at 04:53:30 PM, Lukasz Majewski wrote:
> Hi Marek,
> 
> > On Tuesday, March 18, 2014 at 01:35:47 PM, Lukasz Majewski wrote:
> > > Hi Marek,
> > > 
> > > > On Tuesday, March 18, 2014 at 07:51:05 AM, Lukasz Majewski wrote:
> > > > > Hi Marek,
> > > > > 
> > > > > > On Monday, March 17, 2014 at 11:56:16 AM, Heiko Schocher
> > > > > > 
> > > > > > wrote:
> > > > > > > move the flushing code into an extra function dfu_flush(),
> > > > > > > so it can be used from other code.
> > > > > > > 
> > > > > > > Signed-off-by: Heiko Schocher <hs@denx.de>
> > > > > > > Cc: Lukasz Majewski <l.majewski@samsung.com>
> > > > > > > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > > > > > > Cc: Marek Vasut <marex@denx.de>
> > > > > > > Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> > > > > > 
> > > > > > [...]
> > > > > > 
> > > > > > > @@ -199,23 +221,7 @@ int dfu_write(struct dfu_entity *dfu,
> > > > > > > void *buf, int size, int blk_seq_num)
> > > > > > > 
> > > > > > >  	/* end? */
> > > > > > >  	if (size == 0) {
> > > > > > > 
> > > > > > > -		/* Now try and flush to the medium if
> > > > > > > needed. */
> > > > > > > -		if (dfu->flush_medium)
> > > > > > > -			ret = dfu->flush_medium(dfu);
> > > > > > > -		printf("\nDFU complete CRC32: 0x%08x\n",
> > > > > > > dfu->crc); -
> > > > > > > -		/* clear everything */
> > > > > > > -		dfu_free_buf();
> > > > > > > -		dfu->crc = 0;
> > > > > > > -		dfu->offset = 0;
> > > > > > > -		dfu->i_blk_seq_num = 0;
> > > > > > > -		dfu->i_buf_start = dfu_buf;
> > > > > > > -		dfu->i_buf_end = dfu_buf;
> > > > > > > -		dfu->i_buf = dfu->i_buf_start;
> > > > > > > -
> > > > > > > -		dfu->inited = 0;
> > > > > > > -
> > > > > > > -	}
> > > > > > > +		ret = dfu_flush(dfu, buf, size,
> > > > > > > blk_seq_num);
> > > > > > 
> > > > > > This seems broken, at least because you didn't close the
> > > > > > opened brace (see 'if (size == 0) {' .... I can fix this up
> > > > > > when applying.
> > > > > 
> > > > > I was prepared to pull those patches to u-boot-dfu repo today
> > > > > and test them :-).
> > > > > 
> > > > > Afterwards, I would send PR to Tom.
> > > > > 
> > > > > Are you OK with this?
> > > > 
> > > > Doesn't -dfu go through -usb ? ;-)
> > > 
> > > As fair as I remember the dfu code from ./drivers/dfu/* shall go via
> > > u-boot-dfu tree directly to Tom.
> > > 
> > > > Or do you want to send to Tom
> > > > directly ? Either way WFM.
> > > 
> > > After writing the other message you waited for a response less than
> > > one minute :-).
> > > 
> > > > Nonetheless, I picked the patches via
> > > > -usb, they're in master now.
> > > 
> > > I've just tested them and they doesn't cause any regression.
> > > 
> > > Would you be so kind and apply one more patch, which was supposed to
> > > go with u-boot-dfu PR (this patch waits for submission from
> > > 21.02.2014)?
> > > 
> > > http://patchwork.ozlabs.org/patch/322450/
> > 
> > OK, picked and pushed. I think panto picked it up as well tho, but
> > maybe I unleashed too much mental terror unto him with my OvisLink
> > WL-1100SD that he is paralyzed by it ;-) Panto, I will pick this
> > patch, OK ?
> 
> This patch looks like a little orphan :-). I'm glad that someone pull
> it finally.

OK.

> > As for the DFU, what do we do about the PRs ? DFU is part of USB, so
> > I shall logically pick the PRs from you and move them to trini, but
> > if you're unhappy about this, we need to talk about that. Tom,
> > Lukasz ... thoughts ?
> 
> I'm fine with sending PRs to you.

PR will go out once I do builds.

Marek Vasut

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

end of thread, other threads:[~2014-03-18 18:21 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-17 10:56 [U-Boot] [PATCH v2 0/3] usb: dfu: introduce dfuMANIFEST state Heiko Schocher
2014-03-17 10:56 ` [U-Boot] [PATCH v2 1/3] usb, dfu: extract flush code into seperate function Heiko Schocher
2014-03-18  0:21   ` Marek Vasut
2014-03-18  6:02     ` Heiko Schocher
2014-03-18  6:55       ` Lukasz Majewski
2014-03-18 11:32         ` Marek Vasut
2014-03-18 11:31       ` Marek Vasut
2014-03-18  6:51     ` Lukasz Majewski
2014-03-18 11:33       ` Marek Vasut
2014-03-18 12:35         ` Lukasz Majewski
2014-03-18 15:23           ` Marek Vasut
2014-03-18 15:53             ` Lukasz Majewski
2014-03-18 18:21               ` Marek Vasut
2014-03-17 10:56 ` [U-Boot] [PATCH v2 2/3] usb: dfu: introduce dfuMANIFEST state Heiko Schocher
2014-03-17 10:56 ` [U-Boot] [PATCH v2 3/3] am335x, dfu: add DFU_MANIFEST_POLL_TIMEOUT to the siemens boards Heiko Schocher

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