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