Hello Lukasz,
Am 17.03.2014 10:46, schrieb Lukasz Majewski:
Hi Heiko,
Hello Lukasz,
Am 14.03.2014 11:47, schrieb Lukasz Majewski:
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/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
[...]
@@ -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
Hmm.. Yes, but we are MainfestationTolerant ... so the comment is
Ok, or?
My understanding of "ManifestationTolerant" is as follow:
"We didn't used the dfuMANIFEST state and immediately went to dfuIDLE
state from dfuDOWNLOAD-IDLE, so we are 'ManifestationTolerant'".
[...]
I'm looking forward for v2.
I am ready for sending it, but need response from you as I think the
comment is correct ...
Please explain why it is correct in your opinion.
Hmm..
http://www.usb.org/developers/devclass_docs/usbdfu10.pdf on page 26
"Figure A.1 Interface state transition diagram":
State 7 bitManifestationTolerant = 1 (=ManifestationTolerant?), if
PollTimeout is reached go back to state 6, and after DFU_GET_STATUS
go to state 2
if State 7 and "bitManifestationTolerant = 0" and PollTimeout reached,
go to state 8 ... and wait there forever?
So we are in the "bitManifestationTolerant = 1" case as before my
changes ... nothing changed ...
before my changes there was immediately the transition from state 6
to state 2 and this is only possible if "bitManifestationTolerant = 1"
is set ... I only added the dfuMANIFEST state, not changed the
"bitManifestationTolerant = 1"
I interpretate 'ManifestationTolerant' as:
multiple dfu downloads are possible, no need for reseting the device
after a download was successful ...
BTW:
With the DFU_MANIFEST_POLL_TIMEOUT solution, we have a static timeout,
which is suboptimal, as we have a big MTD partittion and we must have
a "big" timeout for erasing (in the worst case) the complete
partition. Also this big timeout is (on nand using ubi) only needed
for the nand ubi partiton, not for the raw partitions containing for
example u-boot ... where it could be 0 ...
So, the problem is with setting a worst case value for all kinds of
your dfu entities?
Yes. At least each dfu entity needs an own Timeout ... think of
an 1MiB Partition and a 900MiB Partition on one device ... we
currently use the Polltimeout for the 900MiB Partition also when
updating the 1 MiB Partition ...
Better would be, to have the information how long does it take to
erase one sector (define?) and how many sectors we have to erase, and
sending this value to the host... Do you see a chance to get this
information within the dfu code?
We have f_dfu->poll_timeout field in the dfu function. Value from
it sets the poll timeout to be waited by the host.
Maybe it would be better to reuse this field to set it accordingly for
MANIFEST_POLL_TIMEOUT. Now it is used for setting timeout when we store
big chunks of data (32 MiB).
Thinking about it, possibly we need a callback from the mtd layer,
which gives back the info, how long the flush would take, as this
is a media/partition size dependent timeout ...
I think that we could add this 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
Yep ... I look into this...
Other question is if we accept current patches and wait for above
improvement to follow of drop them and wait for solution addressing
those issues.
What do you think?
I prefer to accept current patches (I can post v2), as they are an
improvement. On top of that the optimzation of the PollTimeout
settings is a seperate patch.
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
