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 <h...@denx.de> > Cc: Lukasz Majewski <l.majew...@samsung.com> > Cc: Kyungmin Park <kyungmin.p...@samsung.com> > Cc: Marek Vasut <ma...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot