* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.