[U-Boot] [PATCH 1/2] musb-new, dfu: first send request answer then call completions
comment in ep0_txstate() states:
"report completions as soon as the fifo's loaded; there's no win in waiting till this last packet gets acked".
This is wrong for using dfu. In the dfu usecase we must send a PollTimeout to the host, so the host can wait until the U-Boot Code is ready for answering new usb requests. So the answer which contains the PollTimeout must send *before* U-Boot calls req->complete.
The req->complete is used in the dfu case for flushing the medium, when entering DFU_STATE_dfuMANIFEST_SYNC state.
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
---
Tested on the dxr2 and pxm2 boards. If dfu_flush() needs longer then 5 sec, dfu-util breaks with current mainline code:
[...] finished! unable to read DFU status $
With this patch, it shows again:
[...] finished! state(7) = dfuMANIFEST, status(0) = No error condition is present state(2) = dfuIDLE, status(0) = No error condition is present Done! $ --- drivers/usb/musb-new/musb_gadget_ep0.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/musb-new/musb_gadget_ep0.c b/drivers/usb/musb-new/musb_gadget_ep0.c index 6599d38..8c3b0a1 100644 --- a/drivers/usb/musb-new/musb_gadget_ep0.c +++ b/drivers/usb/musb-new/musb_gadget_ep0.c @@ -576,6 +576,10 @@ static void ep0_txstate(struct musb *musb) } else request = NULL;
+ /* send it out, triggering a "txpktrdy cleared" irq */ + musb_ep_select(musb->mregs, 0); + musb_writew(regs, MUSB_CSR0, csr); + /* report completions as soon as the fifo's loaded; there's no * win in waiting till this last packet gets acked. (other than * very precise fault reporting, needed by USB TMC; possible with @@ -588,10 +592,6 @@ static void ep0_txstate(struct musb *musb) return; musb->ackpend = 0; } - - /* send it out, triggering a "txpktrdy cleared" irq */ - musb_ep_select(musb->mregs, 0); - musb_writew(regs, MUSB_CSR0, csr); }
/*
add a possibility to add a medium specific polltimeout function. So it is possible to define different poll timeouts.
Used on nand medium, for setting the DFU_MANIFEST_POLL_TIMEOUT only on nand ubi partitions, which is currently the only usecase.
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 --- drivers/dfu/dfu_nand.c | 13 +++++++++++++ drivers/usb/gadget/f_dfu.c | 14 +++++++++++++- include/dfu.h | 1 + 3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c index 2d07097..ccdbef6 100644 --- a/drivers/dfu/dfu_nand.c +++ b/drivers/dfu/dfu_nand.c @@ -163,6 +163,18 @@ static int dfu_flush_medium_nand(struct dfu_entity *dfu) return ret; }
+unsigned int dfu_polltimeout_nand(struct dfu_entity *dfu) +{ + /* + * Currently, Poll Timeout != 0 is only needed on nand + * ubi partition, as the not used sectors need an erase + */ + if (dfu->data.nand.ubi) + return DFU_MANIFEST_POLL_TIMEOUT; + + return DFU_DEFAULT_POLL_TIMEOUT; +} + int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s) { char *st; @@ -211,6 +223,7 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s) dfu->read_medium = dfu_read_medium_nand; dfu->write_medium = dfu_write_medium_nand; dfu->flush_medium = dfu_flush_medium_nand; + dfu->poll_timeout = dfu_polltimeout_nand;
/* initial state */ dfu->inited = 0; diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index de75ff1..9128add 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -174,6 +174,17 @@ static void dnload_request_flush(struct usb_ep *ep, struct usb_request *req) req->length, f_dfu->blk_seq_num); }
+static void dfu_set_poll_timeout_manifest(struct dfu_status *dstat, + struct f_dfu *f_dfu) +{ + struct dfu_entity *dfu = dfu_get_entity(f_dfu->altsetting); + + if (dfu->poll_timeout) + dfu_set_poll_timeout(dstat, dfu->poll_timeout(dfu)); + else + dfu_set_poll_timeout(dstat, DFU_MANIFEST_POLL_TIMEOUT); +} + static void handle_getstatus(struct usb_request *req) { struct dfu_status *dstat = (struct dfu_status *)req->buf; @@ -190,7 +201,8 @@ static void handle_getstatus(struct usb_request *req) f_dfu->dfu_state = DFU_STATE_dfuMANIFEST; break; case DFU_STATE_dfuMANIFEST: - dfu_set_poll_timeout(dstat, DFU_MANIFEST_POLL_TIMEOUT); + dfu_set_poll_timeout_manifest(dstat, f_dfu); + break; default: break; } diff --git a/include/dfu.h b/include/dfu.h index 6c71ecb..c7fd270 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -105,6 +105,7 @@ struct dfu_entity { u64 offset, void *buf, long *len);
int (*flush_medium)(struct dfu_entity *dfu); + unsigned int (*poll_timeout)(struct dfu_entity *dfu);
struct list_head list;
On Thursday, April 10, 2014 at 07:08:06 AM, Heiko Schocher wrote:
add a possibility to add a medium specific polltimeout function. So it is possible to define different poll timeouts.
Used on nand medium, for setting the DFU_MANIFEST_POLL_TIMEOUT only on nand ubi partitions, which is currently the only usecase.
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
[...]
@@ -174,6 +174,17 @@ static void dnload_request_flush(struct usb_ep *ep, struct usb_request *req) req->length, f_dfu->blk_seq_num); }
+static void dfu_set_poll_timeout_manifest(struct dfu_status *dstat,
struct f_dfu *f_dfu)+{
- struct dfu_entity *dfu = dfu_get_entity(f_dfu->altsetting);
- if (dfu->poll_timeout)
dfu_set_poll_timeout(dstat, dfu->poll_timeout(dfu));- else
dfu_set_poll_timeout(dstat, DFU_MANIFEST_POLL_TIMEOUT);+}
Don't you think it'd be better (yet more intrusive) to have all the DFU users have default implementation of dfu->poll_timeout() ? Then you'd be able to avoid this if and even get rid of this dfu_set_poll_timeout_manifest() function.
[...]
Best regards, Marek Vasut
Hi Marek,
On Apr 10, 2014, at 10:54 AM, Marek Vasut wrote:
On Thursday, April 10, 2014 at 07:08:06 AM, Heiko Schocher wrote:
add a possibility to add a medium specific polltimeout function. So it is possible to define different poll timeouts.
Used on nand medium, for setting the DFU_MANIFEST_POLL_TIMEOUT only on nand ubi partitions, which is currently the only usecase.
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
[...]
@@ -174,6 +174,17 @@ static void dnload_request_flush(struct usb_ep *ep, struct usb_request *req) req->length, f_dfu->blk_seq_num); }
+static void dfu_set_poll_timeout_manifest(struct dfu_status *dstat,
struct f_dfu *f_dfu)+{
- struct dfu_entity *dfu = dfu_get_entity(f_dfu->altsetting);
- if (dfu->poll_timeout)
dfu_set_poll_timeout(dstat, dfu->poll_timeout(dfu));- else
dfu_set_poll_timeout(dstat, DFU_MANIFEST_POLL_TIMEOUT);+}
Don't you think it'd be better (yet more intrusive) to have all the DFU users have default implementation of dfu->poll_timeout() ? Then you'd be able to avoid this if and even get rid of this dfu_set_poll_timeout_manifest() function.
Could work, but why not a simple accessor like this:
static inline unsigned int dfu_get_poll_timeout(struct dfu_entity *dfu) {
return dfu->poll_timeout ? dfu->poll_timeout(dfu); DFU_MANIFEST_POLL_TIMEOUT); }
and dfu_set_poll_timeout(dstat, dfu_get_poll_timeout(dfu));
You even get the benefit of have a method to read the timeout value if we ever needed sometime in the future.
[...]
Best regards, Marek Vasut
Regards
-- Pantelis
Hi Pantelis,
Hi Marek,
On Apr 10, 2014, at 10:54 AM, Marek Vasut wrote:
On Thursday, April 10, 2014 at 07:08:06 AM, Heiko Schocher wrote:
add a possibility to add a medium specific polltimeout function. So it is possible to define different poll timeouts.
Used on nand medium, for setting the DFU_MANIFEST_POLL_TIMEOUT only on nand ubi partitions, which is currently the only usecase.
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
[...]
@@ -174,6 +174,17 @@ static void dnload_request_flush(struct usb_ep *ep, struct usb_request *req) req->length, f_dfu->blk_seq_num); }
+static void dfu_set_poll_timeout_manifest(struct dfu_status *dstat,
struct f_dfu *f_dfu)+{
- struct dfu_entity *dfu =
dfu_get_entity(f_dfu->altsetting); +
- if (dfu->poll_timeout)
dfu_set_poll_timeout(dstat,dfu->poll_timeout(dfu));
- else
dfu_set_poll_timeout(dstat,DFU_MANIFEST_POLL_TIMEOUT); +}
Don't you think it'd be better (yet more intrusive) to have all the DFU users have default implementation of dfu->poll_timeout() ? Then you'd be able to avoid this if and even get rid of this dfu_set_poll_timeout_manifest() function.
Could work, but why not a simple accessor like this:
static inline unsigned int dfu_get_poll_timeout(struct dfu_entity *dfu) {
return dfu->poll_timeout ? dfu->poll_timeout(dfu); DFU_MANIFEST_POLL_TIMEOUT); }
and dfu_set_poll_timeout(dstat, dfu_get_poll_timeout(dfu));
You even get the benefit of have a method to read the timeout value if we ever needed sometime in the future.
Seems reasonable for me: +1
Some comment:
Guys, please be consistent with CCing people. I didn't receive this thread. Also this original reply from Pantelis was not CCed to Heiko.
[...]
Best regards, Marek Vasut
Regards
-- Pantelis
Hello Lukasz,
Am 10.04.2014 12:08, schrieb Lukasz Majewski:
Hi Pantelis,
Hi Marek,
On Apr 10, 2014, at 10:54 AM, Marek Vasut wrote:
On Thursday, April 10, 2014 at 07:08:06 AM, Heiko Schocher wrote:
add a possibility to add a medium specific polltimeout function. So it is possible to define different poll timeouts.
Used on nand medium, for setting the DFU_MANIFEST_POLL_TIMEOUT only on nand ubi partitions, which is currently the only usecase.
Signed-off-by: Heiko Schocherhs@denx.de Cc: Lukasz Majewskil.majewski@samsung.com Cc: Kyungmin Parkkyungmin.park@samsung.com Cc: Marek Vasutmarex@denx.de Cc: Pantelis Antonioupanto@antoniou-consulting.com
[...]
@@ -174,6 +174,17 @@ static void dnload_request_flush(struct usb_ep *ep, struct usb_request *req) req->length, f_dfu->blk_seq_num); }
+static void dfu_set_poll_timeout_manifest(struct dfu_status *dstat,
struct f_dfu *f_dfu)+{
- struct dfu_entity *dfu =
dfu_get_entity(f_dfu->altsetting); +
- if (dfu->poll_timeout)
dfu_set_poll_timeout(dstat,dfu->poll_timeout(dfu));
- else
dfu_set_poll_timeout(dstat,DFU_MANIFEST_POLL_TIMEOUT); +}
Don't you think it'd be better (yet more intrusive) to have all the DFU users have default implementation of dfu->poll_timeout() ? Then you'd be able to avoid this if and even get rid of this dfu_set_poll_timeout_manifest() function.
Could work, but why not a simple accessor like this:
static inline unsigned int dfu_get_poll_timeout(struct dfu_entity *dfu) {
return dfu->poll_timeout ? dfu->poll_timeout(dfu); DFU_MANIFEST_POLL_TIMEOUT); }
and dfu_set_poll_timeout(dstat, dfu_get_poll_timeout(dfu));
You even get the benefit of have a method to read the timeout value if we ever needed sometime in the future.
Seems reasonable for me: +1
Yep, good idea, I change this.
Some comment:
Guys, please be consistent with CCing people. I didn't receive this thread. Also this original reply from Pantelis was not CCed to Heiko.
Hmm.. I lloked in my received EMails, and I see you always on cc ... ?
bye, Heiko
Hi Heiko,
Hello Lukasz,
Am 10.04.2014 12:08, schrieb Lukasz Majewski:
Hi Pantelis,
Hi Marek,
On Apr 10, 2014, at 10:54 AM, Marek Vasut wrote:
On Thursday, April 10, 2014 at 07:08:06 AM, Heiko Schocher wrote:
add a possibility to add a medium specific polltimeout function. So it is possible to define different poll timeouts.
Used on nand medium, for setting the DFU_MANIFEST_POLL_TIMEOUT only on nand ubi partitions, which is currently the only usecase.
Signed-off-by: Heiko Schocherhs@denx.de Cc: Lukasz Majewskil.majewski@samsung.com Cc: Kyungmin Parkkyungmin.park@samsung.com Cc: Marek Vasutmarex@denx.de Cc: Pantelis Antonioupanto@antoniou-consulting.com
[...]
@@ -174,6 +174,17 @@ static void dnload_request_flush(struct usb_ep *ep, struct usb_request *req) req->length, f_dfu->blk_seq_num); }
+static void dfu_set_poll_timeout_manifest(struct dfu_status *dstat,
struct f_dfu *f_dfu)+{
- struct dfu_entity *dfu =
dfu_get_entity(f_dfu->altsetting); +
- if (dfu->poll_timeout)
dfu_set_poll_timeout(dstat,dfu->poll_timeout(dfu));
- else
dfu_set_poll_timeout(dstat,DFU_MANIFEST_POLL_TIMEOUT); +}
Don't you think it'd be better (yet more intrusive) to have all the DFU users have default implementation of dfu->poll_timeout() ? Then you'd be able to avoid this if and even get rid of this dfu_set_poll_timeout_manifest() function.
Could work, but why not a simple accessor like this:
static inline unsigned int dfu_get_poll_timeout(struct dfu_entity *dfu) {
return dfu->poll_timeout ? dfu->poll_timeout(dfu); DFU_MANIFEST_POLL_TIMEOUT); }
and dfu_set_poll_timeout(dstat, dfu_get_poll_timeout(dfu));
You even get the benefit of have a method to read the timeout value if we ever needed sometime in the future.
Seems reasonable for me: +1
Yep, good idea, I change this.
Some comment:
Guys, please be consistent with CCing people. I didn't receive this thread. Also this original reply from Pantelis was not CCed to Heiko.
Hmm.. I lloked in my received EMails, and I see you always on cc ... ?
I wasn't added to CC in the original patch 2/2.
I was only added to Cc below the Signed-of-by, but then I was missing in the CC of the message itself.
bye, Heiko
Hello Lukasz,
Am 10.04.2014 16:31, schrieb Lukasz Majewski:
Hi Heiko,
Hello Lukasz,
Am 10.04.2014 12:08, schrieb Lukasz Majewski:
Hi Pantelis,
Hi Marek,
On Apr 10, 2014, at 10:54 AM, Marek Vasut wrote:
On Thursday, April 10, 2014 at 07:08:06 AM, Heiko Schocher wrote:
add a possibility to add a medium specific polltimeout function. So it is possible to define different poll timeouts.
Used on nand medium, for setting the DFU_MANIFEST_POLL_TIMEOUT only on nand ubi partitions, which is currently the only usecase.
Signed-off-by: Heiko Schocherhs@denx.de Cc: Lukasz Majewskil.majewski@samsung.com Cc: Kyungmin Parkkyungmin.park@samsung.com Cc: Marek Vasutmarex@denx.de Cc: Pantelis Antonioupanto@antoniou-consulting.com
[...]
@@ -174,6 +174,17 @@ static void dnload_request_flush(struct usb_ep *ep, struct usb_request *req) req->length, f_dfu->blk_seq_num); }
+static void dfu_set_poll_timeout_manifest(struct dfu_status *dstat,
struct f_dfu *f_dfu)+{
- struct dfu_entity *dfu =
dfu_get_entity(f_dfu->altsetting); +
- if (dfu->poll_timeout)
dfu_set_poll_timeout(dstat,dfu->poll_timeout(dfu));
- else
dfu_set_poll_timeout(dstat,DFU_MANIFEST_POLL_TIMEOUT); +}
Don't you think it'd be better (yet more intrusive) to have all the DFU users have default implementation of dfu->poll_timeout() ? Then you'd be able to avoid this if and even get rid of this dfu_set_poll_timeout_manifest() function.
Could work, but why not a simple accessor like this:
static inline unsigned int dfu_get_poll_timeout(struct dfu_entity *dfu) {
return dfu->poll_timeout ? dfu->poll_timeout(dfu); DFU_MANIFEST_POLL_TIMEOUT); }
and dfu_set_poll_timeout(dstat, dfu_get_poll_timeout(dfu));
You even get the benefit of have a method to read the timeout value if we ever needed sometime in the future.
Seems reasonable for me: +1
Yep, good idea, I change this.
Some comment:
Guys, please be consistent with CCing people. I didn't receive this thread. Also this original reply from Pantelis was not CCed to Heiko.
Hmm.. I lloked in my received EMails, and I see you always on cc ... ?
I wasn't added to CC in the original patch 2/2.
I was only added to Cc below the Signed-of-by, but then I was missing in the CC of the message itself.
Yes, I see ... Hmm.. I sent patches always with git send-mail ...
Header I got:
From - Thu Apr 10 07:12:33 2014 X-Account-Key: account2 X-UIDL: 1130185039.262989 X-Mozilla-Status: 0001 X-Mozilla-Status2: 00000000 X-Mozilla-Keys: Return-Path: hs@denx.de Received: from murder ([192.168.8.180]) by backend11 (Cyrus v2.2.12) with LMTPA; Thu, 10 Apr 2014 07:08:28 +0200 X-Sieve: CMU Sieve 2.2 Received: from mail.m-online.net (localhost [127.0.0.1]) by frontend1.mail.m-online.net (Cyrus v2.2.12) with LMTPA; Thu, 10 Apr 2014 07:08:27 +0200 [...] Received: from pollux.denx.de (pollux [192.168.1.1]) by mail.denx.de (Postfix) with ESMTP id D0851342155; Thu, 10 Apr 2014 07:08:08 +0200 (CEST) Received: by pollux.denx.de (Postfix, from userid 515) id 98C9BF6E; Thu, 10 Apr 2014 07:08:08 +0200 (CEST) From: Heiko Schocher hs@denx.de To: u-boot@lists.denx.de Cc: Heiko Schocher hs@denx.de, Lukasz Majewski l.majewski@samsung.com, Kyungmin Park kyungmin.park@samsung.com, Marek Vasut marex@denx.de, Pantelis Antoniou panto@antoniou-consulting.com Subject: [PATCH 2/2] dfu, nand: add medium specific polltimeout function Date: Thu, 10 Apr 2014 07:08:06 +0200 Message-Id: 1397106486-1233-2-git-send-email-hs@denx.de X-Mailer: git-send-email 1.8.3.1 In-Reply-To: 1397106486-1233-1-git-send-email-hs@denx.de References: 1397106486-1233-1-git-send-email-hs@denx.de
There you are in the cc list ... but I see, in patchwork:
http://patchwork.ozlabs.org/patch/337981/
(click on "Headers show") [...] Message-Id: 1397106486-1233-2-git-send-email-hs@denx.de X-Mailer: git-send-email 1.8.3.1 In-Reply-To: 1397106486-1233-1-git-send-email-hs@denx.de References: 1397106486-1233-1-git-send-email-hs@denx.de Cc: Marek Vasut marex@denx.de, Pantelis Antoniou panto@antoniou-consulting.com, Kyungmin Park kyungmin.park@samsung.com Subject: [U-Boot] [PATCH 2/2] dfu, nand: add medium specific polltimeout function
There you are missing ... ?
bye, Heiko
Hi Heiko,
Hello Lukasz,
Am 10.04.2014 16:31, schrieb Lukasz Majewski:
Hi Heiko,
Hello Lukasz,
Am 10.04.2014 12:08, schrieb Lukasz Majewski:
Hi Pantelis,
Hi Marek,
On Apr 10, 2014, at 10:54 AM, Marek Vasut wrote:
On Thursday, April 10, 2014 at 07:08:06 AM, Heiko Schocher wrote: > add a possibility to add a medium specific polltimeout > function. So it is possible to define different > poll timeouts. > > Used on nand medium, for setting the DFU_MANIFEST_POLL_TIMEOUT > only on nand ubi partitions, which is currently the only > usecase. > > Signed-off-by: Heiko Schocherhs@denx.de > Cc: Lukasz Majewskil.majewski@samsung.com > Cc: Kyungmin Parkkyungmin.park@samsung.com > Cc: Marek Vasutmarex@denx.de > Cc: Pantelis Antonioupanto@antoniou-consulting.com
[...]
> @@ -174,6 +174,17 @@ static void dnload_request_flush(struct > usb_ep *ep, struct usb_request *req) req->length, > f_dfu->blk_seq_num); } > > +static void dfu_set_poll_timeout_manifest(struct dfu_status > *dstat, > + struct f_dfu *f_dfu) > +{ > + struct dfu_entity *dfu = > dfu_get_entity(f_dfu->altsetting); + > + if (dfu->poll_timeout) > + dfu_set_poll_timeout(dstat, > dfu->poll_timeout(dfu)); > + else > + dfu_set_poll_timeout(dstat, > DFU_MANIFEST_POLL_TIMEOUT); +}
Don't you think it'd be better (yet more intrusive) to have all the DFU users have default implementation of dfu->poll_timeout() ? Then you'd be able to avoid this if and even get rid of this dfu_set_poll_timeout_manifest() function.
Could work, but why not a simple accessor like this:
static inline unsigned int dfu_get_poll_timeout(struct dfu_entity *dfu) {
return dfu->poll_timeout ? dfu->poll_timeout(dfu); DFU_MANIFEST_POLL_TIMEOUT); }
and dfu_set_poll_timeout(dstat, dfu_get_poll_timeout(dfu));
You even get the benefit of have a method to read the timeout value if we ever needed sometime in the future.
Seems reasonable for me: +1
Yep, good idea, I change this.
Some comment:
Guys, please be consistent with CCing people. I didn't receive this thread. Also this original reply from Pantelis was not CCed to Heiko.
Hmm.. I lloked in my received EMails, and I see you always on cc ... ?
I wasn't added to CC in the original patch 2/2.
I was only added to Cc below the Signed-of-by, but then I was missing in the CC of the message itself.
Yes, I see ... Hmm.. I sent patches always with git send-mail ...
Header I got:
From - Thu Apr 10 07:12:33 2014 X-Account-Key: account2 X-UIDL: 1130185039.262989 X-Mozilla-Status: 0001 X-Mozilla-Status2: 00000000 X-Mozilla-Keys: Return-Path: hs@denx.de Received: from murder ([192.168.8.180]) by backend11 (Cyrus v2.2.12) with LMTPA; Thu, 10 Apr 2014 07:08:28 +0200 X-Sieve: CMU Sieve 2.2 Received: from mail.m-online.net (localhost [127.0.0.1]) by frontend1.mail.m-online.net (Cyrus v2.2.12) with LMTPA; Thu, 10 Apr 2014 07:08:27 +0200 [...] Received: from pollux.denx.de (pollux [192.168.1.1]) by mail.denx.de (Postfix) with ESMTP id D0851342155; Thu, 10 Apr 2014 07:08:08 +0200 (CEST) Received: by pollux.denx.de (Postfix, from userid 515) id 98C9BF6E; Thu, 10 Apr 2014 07:08:08 +0200 (CEST) From: Heiko Schocher hs@denx.de To: u-boot@lists.denx.de Cc: Heiko Schocher hs@denx.de, Lukasz Majewski l.majewski@samsung.com, Kyungmin Park kyungmin.park@samsung.com, Marek Vasut marex@denx.de, Pantelis Antoniou panto@antoniou-consulting.com Subject: [PATCH 2/2] dfu, nand: add medium specific polltimeout function Date: Thu, 10 Apr 2014 07:08:06 +0200 Message-Id: 1397106486-1233-2-git-send-email-hs@denx.de X-Mailer: git-send-email 1.8.3.1 In-Reply-To: 1397106486-1233-1-git-send-email-hs@denx.de References: 1397106486-1233-1-git-send-email-hs@denx.de
There you are in the cc list ... but I see, in patchwork:
http://patchwork.ozlabs.org/patch/337981/
(click on "Headers show") [...] Message-Id: 1397106486-1233-2-git-send-email-hs@denx.de X-Mailer: git-send-email 1.8.3.1 In-Reply-To: 1397106486-1233-1-git-send-email-hs@denx.de References: 1397106486-1233-1-git-send-email-hs@denx.de Cc: Marek Vasut marex@denx.de, Pantelis Antoniou panto@antoniou-consulting.com, Kyungmin Park kyungmin.park@samsung.com Subject: [U-Boot] [PATCH 2/2] dfu, nand: add medium specific polltimeout function
There you are missing ... ?
Yes. Here is the problem. When you reply to patch, then Cc from the commit message is not taken into account.
This is why I've asked for being consistent with CC list.
For this reason (also the cover letter apply to this), when I send patches with git-send-email, I add all concerned people with explicit --cc/--to option.
bye, Heiko
On Thursday, April 10, 2014 at 12:08:44 PM, Lukasz Majewski wrote:
[...]
Seems reasonable for me: +1
Some comment:
Guys, please be consistent with CCing people. I didn't receive this thread. Also this original reply from Pantelis was not CCed to Heiko.
For mutt, I have the key 'a' bound to:
bind index,pager a group-reply
Best regards, Marek Vasut
On Thursday, April 10, 2014 at 07:08:05 AM, Heiko Schocher wrote:
comment in ep0_txstate() states:
"report completions as soon as the fifo's loaded; there's no win in waiting till this last packet gets acked".
This is wrong for using dfu. In the dfu usecase we must send a PollTimeout to the host, so the host can wait until the U-Boot Code is ready for answering new usb requests. So the answer which contains the PollTimeout must send *before* U-Boot calls req->complete.
The req->complete is used in the dfu case for flushing the medium, when entering DFU_STATE_dfuMANIFEST_SYNC state.
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
To me, this looks OK, yes. We need to "commit" the packet into the hardware before calling ->complete.
Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
Hi Heiko,
comment in ep0_txstate() states:
"report completions as soon as the fifo's loaded; there's no win in waiting till this last packet gets acked".
This is wrong for using dfu. In the dfu usecase we must send a PollTimeout to the host, so the host can wait until the U-Boot Code is ready for answering new usb requests. So the answer which contains the PollTimeout must send *before* U-Boot calls req->complete.
The req->complete is used in the dfu case for flushing the medium, when entering DFU_STATE_dfuMANIFEST_SYNC state.
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
Applied to u-boot-dfu. Thanks.
participants (4)
-
Heiko Schocher -
Lukasz Majewski -
Marek Vasut -
Pantelis Antoniou