[U-Boot] [PATCH 2/3] usb: dfu: introduce dfuMANIFEST state
Lukasz Majewski
l.majewski at samsung.com
Fri Mar 14 11:47:49 CET 2014
Hi Heiko,
> 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 at denx.de>
> Cc: Lukasz Majewski <l.majewski at samsung.com>
> Cc: Kyungmin Park <kyungmin.park at samsung.com>
> Cc: Marek Vasut <marex at denx.de>
> ---
> README | 5 ++++
> drivers/dfu/dfu.c | 1 -
> drivers/usb/gadget/f_dfu.c | 60
> +++++++++++++++++++++++++++++++++++++++-------
> include/dfu.h | 3 +++ 4 files changed, 59
> insertions(+), 10 deletions(-)
>
> diff --git a/README b/README
> index 216f0c7..5076248 100644
> --- a/README
> +++ b/README
> @@ -1525,6 +1525,11 @@ The following options need to be configured:
> this to the maximum filesize (in bytes) for the
> buffer. Default is 4 MiB if undefined.
>
> + 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.
> +
Could you also add information about the DFU_DEFAULT_POLL_TIMEOUT to
the README, please (I've forgotten to do that)?
> - 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 193e047..f61e166 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -222,7 +222,6 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) ret = tret;
> }
>
> - 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..f4bf918 100644
> --- a/drivers/usb/gadget/f_dfu.c
> +++ b/drivers/usb/gadget/f_dfu.c
> @@ -164,15 +164,22 @@ 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;
> +
> + req->length = 0;
> + 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)
> {
> struct dfu_status *dstat = (struct dfu_status *)req->buf;
> struct f_dfu *f_dfu = req->context;
> + int setpolltimeout = 1;
We can go away with this flag. Please see below.
>
> switch (f_dfu->dfu_state) {
> case DFU_STATE_dfuDNLOAD_SYNC:
> @@ -180,17 +187,24 @@ static void handle_getstatus(struct usb_request
> *req) 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);
> + setpolltimeout = 0;
> default:
> break;
> }
>
> - dfu_set_poll_timeout(dstat, 0);
> + if (setpolltimeout) {
> + dfu_set_poll_timeout(dstat, 0);
>
> - if (f_dfu->poll_timeout)
> - if (!(f_dfu->blk_seq_num %
> - (dfu_get_buf_size() / DFU_USB_BUFSIZ)))
> - dfu_set_poll_timeout(dstat,
> f_dfu->poll_timeout);
> + if (f_dfu->poll_timeout)
> + if (!(f_dfu->blk_seq_num %
> + (dfu_get_buf_size() / DFU_USB_BUFSIZ)))
> + dfu_set_poll_timeout(dstat,
> +
> f_dfu->poll_timeout);
> + }
What about this solution:
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;
}
if (f_dfu->poll_timeout)
if (!(f_dfu->blk_seq_num %
(dfu_get_buf_size() / DFU_USB_BUFSIZ)))
dfu_set_poll_timeout(dstat,f_dfu->poll_timeout);
Then we could avoid setpolltimeout flag.
>
> /* send status response */
> dstat->bStatus = f_dfu->dfu_status;
> @@ -446,10 +460,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 +478,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 */
Please look into the comments globally - we do now support the
DFU_STATE_dfuMANIFEST_* states
> + 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 +581,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];
I did some preliminary DFU testing and it seems to not introduce any
regressions.
I'm looking forward for v2.
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
More information about the U-Boot
mailing list