Re: [U-Boot] [PATCH] dfu: initial implementation
Hi Andrzej, [...] If my assumption is correct, then what would it take to split off protocol part and make it independent of the actual driver interface? I guess that in the situation given it would be of little use. What do you think would be of little use? As far as I understand you correctly, you would like the state machine code extracted from the initial DFU implementation posted on this mailing list. And then you would like the DFU implemented in terms of this abstract state machine. This means actually more code, probably not a desired thing. The generic state machine would also require some accompanying generic data structures, and this would also mean code to translate back and forth between USB/DFU (DFU _is_ USB) and state machine's data structures. All in all, this means adding more complexity and code. This looks like a premature optimization (which is widely known to be the root of all evil), while at the moment, there are no use cases to justify it. Should the use cases appear, the code can be reworked based on the needs of both USB/DFU and new state machine users. Just to add my personal opinion here - I am occassionally lobbying for a long time on this mailing list that mainline U-Boot gains DFU support, so please let's get this working with the existing tools and in the existing contexts, i.e. USB. Personally I consider it to be completely out of scope to dis-entangle DFU from its specification and abstract it into something which it _could_ be, but really _isn't_. When the implementation for USB is here, we all have much better grounds to discuss possible generalizations (although I doubt that there are many). Cheers Detlev -- Perfecting oneself is as much unlearning as it is learning. -- Edsger Dijkstra -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Dear Wolfgang Denk, Please see my comments inline. DFU is part of USB; an extension to be precise, but an extension bound so tightly to the design and philosophy of USB that it is rather inconceivable to separate the two. Could you please be so kind and explain which exact issues you see for such a separation? First of all, I think that it is more proper to speak of communications part and state machine part. They are bound together by the standard. For example, in the dfuIDLE state, the receipt of DFU_DNLOAD request with wLength == 0 triggers a device stalls the control pipe action. This kind of stuff is very USB specific - I am not sure what would the control pipe be for serial/Ethernet, nor what stall the control pipe would mean for either of them. Another example, in the dfuMANIFEST state, when the status poll timeout occurs and bitManifestationTolerant == 1 then device can still communicate via the USB. The communications part is of course the very USB itself, so the only thing which could be extracted is the state machine part. But, then, in order not to be bound to USB but reusable, it should be abstract. Being abstract, it would not be DFU anymore, but some Generic Update Protocol that you have on your mind, but we don't know of. Is there any standard for such a protocol? Seriously speaking, in view of ties between DFU and USB IMHO it is impossible, or, at least, highly impractical. Can you please support this statement with a few facts? Please see my comment above. If my assumption is correct, then what would it take to split off protocol part and make it independent of the actual driver interface? I guess that in the situation given it would be of little use. What do you think would be of little use? As far as I understand you correctly, you would like the state machine code extracted from the initial DFU implementation posted on this mailing list. And then you would like the DFU implemented in terms of this abstract state machine. This means actually more code, probably not a desired thing. The generic state machine would also require some accompanying generic data structures, and this would also mean code to translate back and forth between USB/DFU (DFU _is_ USB) and state machine's data structures. All in all, this means adding more complexity and code. This looks like a premature optimization (which is widely known to be the root of all evil), while at the moment, there are no use cases to justify it. Should the use cases appear, the code can be reworked based on the needs of both USB/DFU and new state machine users. Regards, Andrzej ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello. On Sat, 2011-11-05 at 16:31, Wolfgang Denk wrote: In message 2002200717.GP17069@excalibur.local you wrote: While I think a dfu command is usefull I don't like the need to execute it before any DFU interaction can happen. That may be an option during development but for field upgrades or receovery it is not. Yes, a command is the natural way in U-Boot to start any activities. I already wrote a bit about my approach here. At OpenMoko we used a button to enter u-boot during startup when pressed. This offered a This is just another way to start a command. So bounding a key press to the command would be the way to achieve what some people want. Fine with me. usbtty interface as usb gadget as well as the runtime descripto for DFU. With dfu-util it was possible to iniate the DFU download or upload procedure while being in the mode. Another option would be to directly jump into DFU mode when a button is pressed. NAK. Let's stick to standard interfaces. What do you mean here? You don't want the DFU descripto available with USBTTY or you don't want to start DFU directly in DFU mode? regards Stefan Schmidt ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello. On Sat, 2011-11-05 at 16:33, Wolfgang Denk wrote: In message 000601cc9abe$4f544bd0$edfce370$%p...@samsung.com you wrote: http://www.usb.org/developers/devclass_docs/DFU_1.1.pdf DFU is part of USB; an extension to be precise, but an extension bound so tightly to the design and philosophy of USB that it is rather inconceivable to separate the two. Could you please be so kind and explain which exact issues you see for such a separation? As Andrzej pointed out the DFU spec is written by the USB forum and one can see that there target are USB devices. Let me bring in some facts from the spec. One part that is not covered here is the DFU suffix mandatory for all firmware files. It gets stripped from the flashing tool before sending it to the device code and thus it was not covered here so far. This suffix holds, among others, fields with the PID of VID of the USB device. Only allowing to be flashed if the device has the correct IDs. How would assign the USB IDs to a network based approach for example? The core part, I understand, is the DFU state machine you are referring to. This sate machine relies on the USB descriptors in run-time or DFU mode. It is hardcoded wot USB descriptors using bDeviceClass, bDeviceSubClass, idVendor and idProduct to name a few. (See p. 14, 15 of the 1.1 spec). In the reconfiguration phase the spec mandates the host to issue a DFU_DETACH request on the USB EP0 and then issuing an USB reset. (p. 17) All bound to the USB spec and hardware for using the spec in a standard compliant way. Eventually it should be possible to run this protocol over Ethernet or even over a serial line? Of course there is no such a reason, provided we lay USB over Ethernet or serial line first ;) This is of course not intended. I was thinking about a plain standard UDP based link. As always in computers one could try to come up with solutions to replace the USB hardcoded parts with something else for a UDP based solution. Its just now longer compliant with the DFU spec. All DFU flashing utils out there are only working over USB. All the windows flash utils as well as dfu-programmer, a small dfu util from the bluez project and also dfu-util. The last ones are using libusb for this. The main point is that if you replace the USB related parts with something else not much of DFU stays anymore. It would be a different flashing procedure. And for systems with a network interface available there are way better ways to flash it then DFU. Even TFP would be more convenient for them. You are way more flexible with Ethernet here. DFU was designed for really small devices with minimal software originally. Best examples are Bluetooth and WiFi USB dongles which can be flashed this way if they. Its just that it becomes more popular as many consumer electronics device are coming with a USB port but no Ethernet these days. regards Stefan Schmidt ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Dear Stefan Schmidt, In message 2006163605.GA20104@excalibur.local you wrote: usbtty interface as usb gadget as well as the runtime descripto for DFU. With dfu-util it was possible to iniate the DFU download or upload procedure while being in the mode. Another option would be to directly jump into DFU mode when a button is pressed. NAK. Let's stick to standard interfaces. What do you mean here? You don't want the DFU descripto available with USBTTY or you don't want to start DFU directly in DFU mode? I don't want random transfers of control between unrelated sub-systems. If you want to run a command, then run a command. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de In the beginning, there was nothing, which exploded. - Terry Pratchett, _Lords and Ladies_ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Dear Stefan Schmidt, In message 2006170219.GB20104@excalibur.local you wrote: Could you please be so kind and explain which exact issues you see for such a separation? As Andrzej pointed out the DFU spec is written by the USB forum and one can see that there target are USB devices. Let me bring in some facts from the spec. ... All bound to the USB spec and hardware for using the spec in a standard compliant way. Yes, indeed. But I feel you stick way too close to this spec, and are not trying to abstract awway the low level details a bit. Yes, this spec was written by the USB forum, and they did not even attempt to look over the rim of their plates. But does that mean we all have to do the same? I'm not willing to accept such a narrow view. Let's - just as a gedankenexperiment - assume our task was to implement a solution that provides similar capabilities as DFU, but must work over serial line, Ethernet, Bluetooth, Infrared, whatever. Assume we thing DFU could be used as base, but we need to find a way to move the USB related things into the USB layer, while things like the state machine, the requests, device status, state transitions, etc. are generic enough. Yes, the DFU File Suffix contains fields that map nicely to USB devices - but what prevents us to provide similar data on non-USB media, too? We just need to make sure that th code which handles this allows to plug in handlers for other transfer media. As always in computers one could try to come up with solutions to replace the USB hardcoded parts with something else for a UDP based solution. Its just now longer compliant with the DFU spec. All DFU I'm not sure what exactly you want to tell me here. My idea is that of an implementation that is split into a generic and a device specific part. If implemented corectly, the combination of the generic part with the USB specific part would be strictly conformant to the DFU spec. Of course, the combination of for example the DFU generic part with an Ethernet network interface would not be conformant to the DFU spec, but it would be something that works in a very similar way. And that would most probably be better than inventing yet another download method. flashing utils out there are only working over USB. All the windows flash utils as well as dfu-programmer, a small dfu util from the bluez project and also dfu-util. The last ones are using libusb for this. That's the status quo. Agreed. Bit this can be changed, extended. Or not? The main point is that if you replace the USB related parts with something else not much of DFU stays anymore. It would be a different I disagree here. DFU is different in a number of aspects, and you know this very well. flashing procedure. And for systems with a network interface available there are way better ways to flash it then DFU. Even TFP would be more convenient for them. You are way more flexible with Ethernet here. I'm not sure what your exact argument is here. I'm not the one asking for DFU support. I would be happy to use TFTP over Ethenret over USB. We're actually doing this in some products. DFU was designed for really small devices with minimal software originally. Best examples are Bluetooth and WiFi USB dongles which can be flashed this way if they. Its just that it becomes more popular as many consumer electronics device are coming with a USB port but no Ethernet these days. Agreed. And if it's becoming more popular, it might even be a good idea to think about a less restricted implementation, which opens new oportunites. Yes, I think we should strive for such a separation of USB transport layer and protocol implementation / state machine / command interface. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de While money can't buy happiness, it certainly lets you choose your own form of misery. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Dear Andrzej Pietrasiewicz, In message 000601cc9abe$4f544bd0$edfce370$%p...@samsung.com you wrote: http://www.usb.org/developers/devclass_docs/DFU_1.1.pdf DFU is part of USB; an extension to be precise, but an extension bound so tightly to the design and philosophy of USB that it is rather inconceivable to separate the two. Could you please be so kind and explain which exact issues you see for such a separation? Eventually it should be possible to run this protocol over Ethernet or even over a serial line? Of course there is no such a reason, provided we lay USB over Ethernet or serial line first ;) This is of course not intended. I was thinking about a plain standard UDP based link. Seriously speaking, in view of ties between DFU and USB IMHO it is impossible, or, at least, highly impractical. Can you please support this statement with a few facts? If my assumption is correct, then what would it take to split off protocol part and make it independent of the actual driver interface? I guess that in the situation given it would be of little use. What do you think would be of little use? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de This restaurant was advertising breakfast any time. So I ordered french toast in the renaissance.- Steven Wright, comedian ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Dear Stefan Schmidt, In message 2002200717.GP17069@excalibur.local you wrote: While I think a dfu command is usefull I don't like the need to execute it before any DFU interaction can happen. That may be an option during development but for field upgrades or receovery it is not. Yes, a command is the natural way in U-Boot to start any activities. I already wrote a bit about my approach here. At OpenMoko we used a button to enter u-boot during startup when pressed. This offered a This is just another way to start a command. usbtty interface as usb gadget as well as the runtime descripto for DFU. With dfu-util it was possible to iniate the DFU download or upload procedure while being in the mode. Another option would be to directly jump into DFU mode when a button is pressed. NAK. Let's stick to standard interfaces. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Each honest calling, each walk of life, has its own elite, its own aristocracy based on excellence of performance. - James Bryant Conant ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Dear Wolfgang Denk, Please see my comments inline. On Thursday, November 03, 2011 7:14 PM Wolfgang Denk wrote: http://www.usb.org/developers/devclass_docs/DFU_1.1.pdf ... DFU is part of USB; an extension to be precise, but an extension bound so tightly to the design and philosophy of USB that it is rather inconceivable to separate the two. If I understand correctly, there is no fundamental reason why the DFU protocol would only be possible to implement over a USB link. Or am I missing something here? Eventually it should be possible to run this protocol over Ethernet or even over a serial line? Of course there is no such a reason, provided we lay USB over Ethernet or serial line first ;) Seriously speaking, in view of ties between DFU and USB IMHO it is impossible, or, at least, highly impractical. If my assumption is correct, then what would it take to split off protocol part and make it independent of the actual driver interface? I guess that in the situation given it would be of little use. Regards, Andrzej ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello, On Thursday, November 03, 2011 3:01 PM Stefan Schmidt wrote: o I will send out my not ready for mainline patch to give you and others an impression how it is tackled in my patch. o I like your split between dfu and flashing and also the addition of the dfu command. Could you split this part from the rest and send it as self conatining patch without other dependencies? o And then one patch with the mmc/fat flashing backend in moved out of the board file (no idea what the best place is, maybe just another file for now) and a last patch with all the board specific changes? I assume splitting into 4 parts: (1) dfu gadget, (2) board specific code, (3) flashing backend generic code, (4) dfu command. As I today discovered, after latest merges in u-boot master the mmc subsystem in Goni does not work properly - the detected capacity is 0, and the number of available partitions is 0. This took me some time to discover. I hope the responsible person will fix this soon. So, as far as the splitting of my dfu implementation into separate patches is concerned, I think I will come up with it early next week. o I will then rebase my code on yours and prepare patches against the dfu implementations as needed and also flashing backend for NAND. o After this we need to review was is missing and bring it over. Not all details covered in the plan but a start. Comments? In the beginning I intend to split the code into respective parts, to give you something to work with. Then I intend to clean the code up, taking into account your and Mike's review. Regards, Andrzej ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello Stefan, On Wednesday, November 02, 2011 3:29 PM Stefan Shmidt wrote: Have you fully implemented 1.1? With the detahc logic inside the device implementation? As you noticed in another post, the state machine is reused. Just curious. What version of dfu-util your are using for your tests? I released 0.5 earlier today. I used what was available in my desktop's Ubuntu 10.04 LTS, and it happens to be version 0.1+svn :O It works, though. Hmm, that sounds like you only implemented the DFU mode but not the run-time mode yet? This conclusion is not correct. After issuing the dfu command my implementation begins in runtime mode. Only after using dfu-util from the host is the mode changed to DFU mode. Actually, as far as usage of dfu-util is concerned, I find it inconvenient when the device is in runtime mode, because there (at least with my version of dfu-util) is no way for the user to guess what altsettings are actually available. I personally do dfu-util -U file -a 0 or a similar command to kick the device into DFU mode, then dfu-util -l to discover what altsettings there are. So, it could be tempting to omit the runtime mode at all and start in DFU mode, but, to be standard compliant, I start in runtime mode. How do you discover with dfu-util what altsettings there are in the device? While the original purpose of DFU was to make updating USB device easy enough for end-users as well. True, that's what the standard says about the very purpose of DFU. However, pressing keys to bring the device into a specific mode is probably very much device-specific. It would be nice if we are able to come up with a generic solution which only delegates the necessary details to board-specific files. In the end I would like to see different options available: 1) Entering DFU mode directly when a button is pressed on startup. An update mode if you want to call it like this. Please see my comment above. 2) Normal startup, but with DFU descripton to allow dfu-util detaching and entering the DFU mode for flashing. 3) Starting dfu mode form the command line as you did implement it. I think 2 and 3 don't differ too much - it seems that enriching a setup without DFU with additional USB descriptors will do the job. But 2 and 3 probably should not be merged, though. The implementation is split into two parts: the dfu gadget implementation and a flashing backend, the interface between the two being the struct flash_entity. The flashing backend's implementation is board-specific and is implemented on the Goni target. What is your reasoning behind this split? I have it working here with the normal nand_flashing routines taking care of bad blocks, etc. The reason is to factor out board-specific stuff from the dfu gadget implementation. However, in what I factored out probably there are parts which are generic enough not to be board-specific. Since there seems to be considerable interest in DFU implementation for u-boot, this could be reworked. To make it clear, I'm happy to see somebody else working on this Whap! --- Oh, man, we are not alone! ;) I am happy to hear from someone else who works on this, too. Regards, Andrzej ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello Mike, Thank you for your review. Please see my comments inline. On Wednesday, November 02, 2011 4:16 PM Mike Frysinger wrote: Dear All, This is Device Firmware Upgrade (DFU) implementation which supports data upload and download function to devices which are equipped with a UDC. this information belongs in the changelog (above the --- marker) I generally agree to remarks related to coding style and implementation's distribution into respective patches. are you working with the elinux.org guys ? http://elinux.org/Merge_DFU_support_into_mainline_U-Boot That's not me. this should be split up into at least the dfu core and board-specific changes. although i'd wonder how much of the board/samsung/ stuff is really board specific and couldn't be generalized ... You are right, probably the flashing backend part contains some code which can be generalized. Regards, Andrzej ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello Stefan, On Wednesday, November 02, 2011 8:30 PM Stefan Schmidt writes: Agreed. The eMMC flashing with files on FAT is nothing goni specific. Others should be able to use this as well. I see three different parts here that can be separated: I agree. Since there is interest in DFU implementation this can and should be generalized. 1) The DFU protocol implementation which lives in usb gadget with an interface for flashing backends 2) The flashing backends (no idea where to put them best). At the moment that would be the implementation here with eMMC and files on FAT, mine with raw NAND devices and in the future maybe others. Each flashing backend should be generic enough to allow different boards using it. 3) Board specific information like partitions or hints for the flashing backend. The information itself should be in the board file here and only used by the flashing backends. How does this sound to you? Andrzej? Sounds good to me. In my implementation the interface between dfu gadget and flashing backend is around the struct flash_entity. It contains a character string intended to provide a human-readable name, a void * context which is not interpreted by the gadget code, but is passed to the flashing backend and understood by it. The struct flash_entity also contains prepare-finish methods to be called before and after read/write operations, and the read_block- write_block pair. What do you think? As far as generalization is concerned, in my flashing backend implementation I see these parts as candidates for generalization: 1) mbr/ebr reading 2) reading/writing mmc 3) read/write fat 4) generic prepare/finish; not sure if fat-specific prepare can be generalized 5) read/write_block 6) some more work should be put to create an interface between the board initialization routine, the flashing backend and the DFU gadget implementation. In my implementation the board initialization routine calls board-specific register_flash_areas, which, in turn, calls DFU gadget's register_flash_entities. What's your opinion? Thanks, Andrzej ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello Stefan, Thank you for your review. Please see comments inline. On Wednesday, November 02, 2011 9:07 PM Stefan Schmidt wrote: First, and only high level, review for the DFU part. Against which u-boot tree/branch/version is this patch? I tried to apply it against HEAD and it failed for me. Anything I miss? Sorry about that. I forgot to mention the reference. It is http://patchwork.ozlabs.org/patch/122080 On Wed, 2011-11-02 at 11:00, Andrzej Pietrasiewicz wrote: The implementation is split into two parts: the dfu gadget implementation and a flashing backend, the interface between the two being the struct flash_entity. The flashing backend's implementation is board-specific and is implemented on the Goni target. I replied to Mike's mail with my ideas on this. Split between dfu and flashing backends is good and missng in my approach currently. I would not put it into the board file though but make it generic for other boards as well and only define the needed informations in the board file. Please see my other mail for details. Comments appreciated! Comments sent in other mails;) I agree that since there is interest in implementing DFU in u-boot the code which in fact is not board specific can and should be generalized. We need to think of an appropriate place in the tree to store it, though. diff --git a/common/Makefile b/common/Makefile index ae795e0..de926d9 100644 --- a/common/Makefile +++ b/common/Makefile @@ -162,6 +162,7 @@ COBJS-y += cmd_usb.o COBJS-y += usb.o COBJS-$(CONFIG_USB_STORAGE) += usb_storage.o endif +COBJS-$(CONFIG_CMD_DEVICE_FIRMWARE_UPGRADE) += cmd_dfu.o CONFIG_CMD_DFU instead? Looks a bit long to me. ACK. No problem with changing that. I already wrote a bit about my approach here. At OpenMoko we used a button to enter u-boot during startup when pressed. This offered a usbtty interface as usb gadget as well as the runtime descripto for DFU. With dfu-util it was possible to iniate the DFU download or upload procedure while being in the mode. Another option would be to directly jump into DFU mode when a button is pressed. Please see my comment in the other mail. A generic way to handle a button pressed seems necessary. Actually in my opinion just providing the dfu command is enough. If, in some hardware, initiating the runtime mode is desired by pressing some keys, then the board initialization code should check for such an event and if it happens then just programmatically run the dfu command. What do you think? +/* #include usbstring.c */ Not needed, can be removed. I agree to coding style/reduntant code comments; will not specifically comment on them unless I disagree. +static const struct dfu_function_descriptor dfu_func = { + .bLength = sizeof dfu_func, + .bDescriptorType = DFU_DT_FUNC, + .bmAttributes = DFU_BIT_WILL_DETACH | Here are you setting the WILL_DETACH bit. Is the device really detaching itself? Its not obvious to me from the code that it does. It seems it does not. The dfu-util session says Resetting USB, so I assume that resetting is initiated by the host, not by the device. To be removed. +static char manufacturer[50]; +static const char longname[] = DFU Gadget; +/* default serial number takes at least two packets */ +static const char serial[] = 0123456789.0123456789.012345678; +static const char dfu_name[] = Device Firmware Upgrade; +static const char shortname[] = dfu; This surely should be come from the board configuration? Yes and no - depends on whether we want to version the gadget itself or the flashing backend, or both. As probably the last option is best, the answer is probably yes. We need some interface then. +static bool is_runtime(struct dfu_dev *dev) +{ + return dev-dfu_state == DFU_STATE_appIDLE || + dev-dfu_state == DFU_STATE_appDETACH; +} Hmm, here you are checking if you are in run-time mode. In the introduction you wrote that it is in DFU mode when the dfu command is executed. When does it enter the run-time mode? After the first flashing? I think this is over-interpretation of what I wrote in introduction. I gave an example dfu-util session when the device is already in DFU mode. I didn't say that it only has DFU mode. It has both. + /* send status response */ + dstat-bStatus = dev-dfu_status; + /* FIXME: set dstat-bwPollTimeout */ This really needs to be set. It was a nightmare with dfu-util not having it set with the initial u-boot implementation. setting the correct values here is not that easy though. It depends on the flash layer and if can get the information about the maximal time a flash write can take. Not sure what to do at this moment, either. + case USB_REQ_DFU_DETACH: + /* Proprietary extension: 'detach' from idle mode and +* get back to runtime mode in case of USB Reset. As +
Re: [U-Boot] [PATCH] dfu: initial implementation
On Thursday, November 03, 2011 11:33 AM Andrzej Pietrasiewicz wrote: Sorry about that. I forgot to mention the reference. It is http://patchwork.ozlabs.org/patch/122080 Sorry again. I meant http://patchwork.ozlabs.org/patch/122079 Andrzej ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello. On Thu, 2011-11-03 at 13:47, Andrzej Pietrasiewicz wrote: On Thursday, November 03, 2011 11:33 AM Andrzej Pietrasiewicz wrote: Sorry about that. I forgot to mention the reference. It is http://patchwork.ozlabs.org/patch/122080 Sorry again. I meant http://patchwork.ozlabs.org/patch/122079 Hmm, applied this one but the DFU patch stills fails to apply in goni.c and goni.h. Seems there is something else applied that is not in mainline yet. I can't test the goni part anyway as I don't have the hardware so I will split it out of the patch and test the dfu command and its implementation. regards Stefan Schmidt ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello. On Thu, 2011-11-03 at 09:12, Andrzej Pietrasiewicz wrote: On Wednesday, November 02, 2011 3:29 PM Stefan Shmidt wrote: Have you fully implemented 1.1? With the detahc logic inside the device implementation? As you noticed in another post, the state machine is reused. Yes, sorry about this. I was fouled into thinking you did not as it was splitted out and no mentioning of Harald copyright. Having the same state machine is a good base for further cooperation on this. :) Just curious. What version of dfu-util your are using for your tests? I released 0.5 earlier today. I used what was available in my desktop's Ubuntu 10.04 LTS, and it happens to be version 0.1+svn :O It works, though. Ah, latest Ubuntu has 0.3 and hopefully they will update as well. Already pinged the maintainer. Hmm, that sounds like you only implemented the DFU mode but not the run-time mode yet? This conclusion is not correct. Indeed. I stand corrected. After issuing the dfu command my implementation begins in runtime mode. Only after using dfu-util from the host is the mode changed to DFU mode. Actually, as far as usage of dfu-util is concerned, I find it inconvenient when the device is in runtime mode, because there (at least with my version of dfu-util) is no way for the user to guess what altsettings are actually available. Correct. Sadly this is due to the standard only allowing the functional descriptor in runtime mode. On the other hand the exposed alt settings in run-time mode would clutter the USB device settings. I personally do dfu-util -U file -a 0 or a similar command to kick the device into DFU mode, then dfu-util -l to discover what altsettings there are. I did run exactly in the same problem. :) I added a detach command to dfu-util to switch between the different modes. From run-time to dfu and from dfu to run-time. I wanted to get the release out before bringing it in. Pushed it to the master branch of dfu-util now. Feel free to test and let me know if it works for you. So, it could be tempting to omit the runtime mode at all and start in DFU mode, but, to be standard compliant, I start in runtime mode. How do you discover with dfu-util what altsettings there are in the device? See above. Not available in run-time mode. :/ While the original purpose of DFU was to make updating USB device easy enough for end-users as well. True, that's what the standard says about the very purpose of DFU. However, pressing keys to bring the device into a specific mode is probably very much device-specific. It would be nice if we are able to come up with a generic solution which only delegates the necessary details to board-specific files. Agreed. This should not be handled in the dfu implementation at all. Your split between dfu and flashing backend is a good step in the right direction here. Invoking the dfu mode through a command is also a nice addition. The implementation is split into two parts: the dfu gadget implementation and a flashing backend, the interface between the two being the struct flash_entity. The flashing backend's implementation is board-specific and is implemented on the Goni target. What is your reasoning behind this split? I have it working here with the normal nand_flashing routines taking care of bad blocks, etc. The reason is to factor out board-specific stuff from the dfu gadget implementation. However, in what I factored out probably there are parts which are generic enough not to be board-specific. Since there seems to be considerable interest in DFU implementation for u-boot, this could be reworked. After thinking more about it I agree with you here. The split makes sense and it will allow for more flexibility when adding more flashing backend to the implementation. To make it clear, I'm happy to see somebody else working on this Whap! --- Oh, man, we are not alone! ;) I am happy to hear from someone else who works on this, too. Good to know. :) Brainstorming about a way going forward. Please comment if with your view on this. o I will send out my not ready for mainline patch to give you and others an impression how it is tackled in my patch. o I like your split between dfu and flashing and also the addition of the dfu command. Could you split this part from the rest and send it as self conatining patch without other dependencies? o And then one patch with the mmc/fat flashing backend in moved out of the board file (no idea what the best place is, maybe just another file for now) and a last patch with all the board specific changes? o I will then rebase my code on yours and prepare patches against the dfu implementations as needed and also flashing backend for NAND. o After this we need to review was is missing and bring it over. Not all details covered in the plan but a start. Comments? regards Stefan Schmidt ___ U-Boot mailing
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello. On Thu, 2011-11-03 at 09:44, Andrzej Pietrasiewicz wrote: On Wednesday, November 02, 2011 8:30 PM Stefan Schmidt writes: Agreed. The eMMC flashing with files on FAT is nothing goni specific. Others should be able to use this as well. I see three different parts here that can be separated: I agree. Since there is interest in DFU implementation this can and should be generalized. Great! 1) The DFU protocol implementation which lives in usb gadget with an interface for flashing backends 2) The flashing backends (no idea where to put them best). At the moment that would be the implementation here with eMMC and files on FAT, mine with raw NAND devices and in the future maybe others. Each flashing backend should be generic enough to allow different boards using it. 3) Board specific information like partitions or hints for the flashing backend. The information itself should be in the board file here and only used by the flashing backends. How does this sound to you? Andrzej? Sounds good to me. In my implementation the interface between dfu gadget and flashing backend is around the struct flash_entity. It contains a character string intended to provide a human-readable name, a void * context which is not interpreted by the gadget code, but is passed to the flashing backend and understood by it. The struct flash_entity also contains prepare-finish methods to be called before and after read/write operations, and the read_block- write_block pair. What do you think? I would need to use it for my NAND backend before I really can comment on it. I can only tell if I'm happy with and interface if I actually used it. :) Will do this when you send it a separate patch with only the DFU implementation with the flash entity as interface. See my roadmap proposal in the other mail. As far as generalization is concerned, in my flashing backend implementation I see these parts as candidates for generalization: 1) mbr/ebr reading 2) reading/writing mmc 3) read/write fat Agreed. That can be used by all kind of devices. 4) generic prepare/finish; not sure if fat-specific prepare can be generalized Would be good to get soem comments on the fs custodians around here. 5) read/write_block Agreed. 6) some more work should be put to create an interface between the board initialization routine, the flashing backend and the DFU gadget implementation. In my implementation the board initialization routine calls board-specific register_flash_areas, which, in turn, calls DFU gadget's register_flash_entities. What's your opinion? The interface between the configuration in the board file and the actual flashing backend is something I haven't wraped my mind around yet. Need to think about it. regards Stefan Schmidt ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello Stefan, On Thursday, November 03, 2011 2:32 PM Stefan Schmidt wrote: Sorry about that. I forgot to mention the reference. It is http://patchwork.ozlabs.org/patch/122080 Sorry again. I meant http://patchwork.ozlabs.org/patch/11122079 Hmm, applied this one but the DFU patch stills fails to apply in goni.c and goni.h. Seems there is something else applied that is not in mainline yet. Ok, here is the full story: this http://patchwork.ozlabs.org/patch/11122079 is the second patch in a series, the first being http://patchwork.ozlabs.org/patch/122080 and you actually need both. I pulled the latest git.denx.de tree and had to also apply a patch which Defines CONFIG_SYS_CACHELINE_SIZE for Goni target. It can be found here: http://article.gmane.org/gmane.comp.boot-loaders.u-boot/112755/match= I know this is confusing. Sorry about that. Andrzej ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello. On Thu, 2011-11-03 at 11:33, Andrzej Pietrasiewicz wrote: On Wednesday, November 02, 2011 9:07 PM Stefan Schmidt wrote: On Wed, 2011-11-02 at 11:00, Andrzej Pietrasiewicz wrote: diff --git a/common/Makefile b/common/Makefile index ae795e0..de926d9 100644 --- a/common/Makefile +++ b/common/Makefile @@ -162,6 +162,7 @@ COBJS-y += cmd_usb.o COBJS-y += usb.o COBJS-$(CONFIG_USB_STORAGE) += usb_storage.o endif +COBJS-$(CONFIG_CMD_DEVICE_FIRMWARE_UPGRADE) += cmd_dfu.o CONFIG_CMD_DFU instead? Looks a bit long to me. ACK. No problem with changing that. OK I already wrote a bit about my approach here. At OpenMoko we used a button to enter u-boot during startup when pressed. This offered a usbtty interface as usb gadget as well as the runtime descripto for DFU. With dfu-util it was possible to iniate the DFU download or upload procedure while being in the mode. Another option would be to directly jump into DFU mode when a button is pressed. Please see my comment in the other mail. A generic way to handle a button pressed seems necessary. Actually in my opinion just providing the dfu command is enough. If, in some hardware, initiating the runtime mode is desired by pressing some keys, then the board initialization code should check for such an event and if it happens then just programmatically run the dfu command. What do you think? Lets go with your dfu implementation and the dfu command. I will prepare patches against is if I see something missing from my patch. +/* #include usbstring.c */ Not needed, can be removed. I agree to coding style/reduntant code comments; will not specifically comment on them unless I disagree. Thanks. +static const struct dfu_function_descriptor dfu_func = { + .bLength = sizeof dfu_func, + .bDescriptorType = DFU_DT_FUNC, + .bmAttributes = DFU_BIT_WILL_DETACH | Here are you setting the WILL_DETACH bit. Is the device really detaching itself? Its not obvious to me from the code that it does. It seems it does not. The dfu-util session says Resetting USB, so I assume that resetting is initiated by the host, not by the device. To be removed. So far I also put this aside. Its only a 1.1 feature which can be added with another patch when the basic stuff is working and merged. +static char manufacturer[50]; +static const char longname[] = DFU Gadget; +/* default serial number takes at least two packets */ +static const char serial[] = 0123456789.0123456789.012345678; +static const char dfu_name[] = Device Firmware Upgrade; +static const char shortname[] = dfu; This surely should be come from the board configuration? Yes and no - depends on whether we want to version the gadget itself or the flashing backend, or both. As probably the last option is best, the answer is probably yes. We need some interface then. As I already stated this part is not worked out in my head yet. Feel free to propose something or we will fix it while working on it. +static bool is_runtime(struct dfu_dev *dev) +{ + return dev-dfu_state == DFU_STATE_appIDLE || + dev-dfu_state == DFU_STATE_appDETACH; +} Hmm, here you are checking if you are in run-time mode. In the introduction you wrote that it is in DFU mode when the dfu command is executed. When does it enter the run-time mode? After the first flashing? I think this is over-interpretation of what I wrote in introduction. I gave an example dfu-util session when the device is already in DFU mode. I didn't say that it only has DFU mode. It has both. Yeah, sorry for that. I indeed misread this part of your implementation. + /* send status response */ + dstat-bStatus = dev-dfu_status; + /* FIXME: set dstat-bwPollTimeout */ This really needs to be set. It was a nightmare with dfu-util not having it set with the initial u-boot implementation. setting the correct values here is not that easy though. It depends on the flash layer and if can get the information about the maximal time a flash write can take. Not sure what to do at this moment, either. If we don't have a back channel from the actual flashing subsystem we need to oriantate the values on the hardware spec on own measurements I fear. Soemthing that need to be set in the board file then. + case USB_REQ_DFU_DETACH: + /* Proprietary extension: 'detach' from idle mode and + * get back to runtime mode in case of USB Reset. As + * much as I dislike this, we just can't use every USB + * bus reset to switch back to runtime mode, since at + * least the Linux USB stack likes to send a number of + * resets in a row :( + */ OK, this makes it clear that you took the DFU state machine from Haralds implementation I'm also using. :)
Re: [U-Boot] [PATCH] dfu: initial implementation
Dear Andrzej Pietrasiewicz, In message 1320228007-8947-1-git-send-email-andrze...@samsung.com you wrote: Device Firmware Upgrade (DFU) is an extension to the USB specification. As of the time of this writing it is documented at http://www.usb.org/developers/devclass_docs/DFU_1.1.pdf ... Please refer to dfu-util documentation for details. The below ASCII-art presents a connection between the host and the device. +-+--- DFU ---+-+ | e.g. dfu-util |=== USB ===| / altsetting 0 | | host +-+ device - altsetting 1 | | file /| | \ ... | +-+ +-+ Sorry for asking some probably really dumb questions... If I understand correctly, there is no fundamental reason why the DFU protocol would only be possible to implement over a USB link. Or am I missing something here? Eventually it should be possible to run this protocol over Ethernet or even over a serial line? If my assumption is correct, then what would it take to split off protocol part and make it independent of the actual driver interface? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Today is the yesterday you worried about tomorrow. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello. This really comes as surprise to me as I'm working on exactly the same at the moment. I just realised that I only mentioned it here inside the fastboot patches thread. I started from the old patches Harald Welte wrote for u-boot during his work for OpenMoko. I worked with him during this time at OpenMoko and I'm the current maintainer of dfu-util. (Just as background). You seem to have started from scratch with this DFU implementation instead of using the older but available patches. Was there a specific reason for this? I found it not that complicated to update them on a current u-boot and adjust it accordingly. I have it working here with a beagleboard. Was about to send them out next week but you have been a small tick faster here. :) From a quick look I see some stuff missing in your implementation that is working in mine so there is some ground for working together on this. The main question is which of both implementation to use to build up from. I will give you some feedback on the design high level aspects further below. A complete review of the code and testing on the beagleboard will hopefully happen over the next days. Testing might be problematic as you flashing part is forced to be rewritten for every board here it seems. Need to investigate further before I can really comment on this. More comment inline. On Wed, 2011-11-02 at 11:00, Andrzej Pietrasiewicz wrote: http://www.usb.org/developers/devclass_docs/DFU_1.1.pdf Have you fully implemented 1.1? With the detahc logic inside the device implementation? The aim of DFU is to allow transferring data to/from selected areas of interest within a compliant device. In the DFU nomenclature the host-device direction is called download and the device-host direction is called upload. The areas are defined by the compliant device. In the DFU nomenclature the area of interest within the device is called an altsetting. The device is connected to the host through USB. On the host side compliant software must be used to initiate the data transfer. Example piece of such software is dfu-util package from the OpenMoko project: http://wiki.openmoko.org/wiki/Dfu-util. Better to use the official homepage here: http://dfu-util.gnumonks.org/ It misses some examples but has the current informations while the wiki is outdated. The DFU implementation on the device side remains in one of the two modes: the Run-Time mode and the DFU mode. The USB descriptor sets exported by the device depend on which mode is in force. While in DFU mode the device exports the descriptors corresponding to each available altsetting. An example dfu-util session when the device is in the DFU mode looks similar to this: # dfu-util -l dfu-util - (C) 2007-2008 by OpenMoko Inc. This program is Free Software and has ABSOLUTELY NO WARRANTY Found DFU: [0x0525:0x] devnum=46, cfg=0, intf=0, alt=0, name=bootldr Found DFU: [0x0525:0x] devnum=46, cfg=0, intf=0, alt=1, name=kernel Just curious. What version of dfu-util your are using for your tests? I released 0.5 earlier today. This u-boot implementation introduces a parameter-less dfu command. After the user finishes with dfu they can Ctrl-C to return to u-boot main menu. Hmm, that sounds like you only implemented the DFU mode but not the run-time mode yet? No addtional DFU descripto in the normal run-time operation mode like usbtty? If yes this is a real limitation as the update would only be possible when the user has serial access to start the dfu command and then flash the device. While the original purpose of DFU was to make updating USB device easy enough for end-users as well. The implementation from OpenMoko i forward ported and cleanup has this functionality and it works well enough. Did so on the Freerunner from OM already. In the end I would like to see different options available: 1) Entering DFU mode directly when a button is pressed on startup. An update mode if you want to call it like this. 2) Normal startup, but with DFU descripton to allow dfu-util detaching and entering the DFU mode for flashing. 3) Starting dfu mode form the command line as you did implement it. The implementation is split into two parts: the dfu gadget implementation and a flashing backend, the interface between the two being the struct flash_entity. The flashing backend's implementation is board-specific and is implemented on the Goni target. What is your reasoning behind this split? I have it working here with the normal nand_flashing routines taking care of bad blocks, etc. I agree that there are several aspects that are board specific. Partions can be read out dynamically and the alt setting generated though. Soemthing what bothers me is the different underlaying medias that can be flashed. Right now I only cover nand, but eMMC, USB and other are valid as well. That is not covered in my implementation at all right now. To make it clear, I'm happy to see somebody else working on this as well
Re: [U-Boot] [PATCH] dfu: initial implementation
On Wednesday 02 November 2011 06:00:07 Andrzej Pietrasiewicz wrote: Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- Dear All, This is Device Firmware Upgrade (DFU) implementation which supports data upload and download function to devices which are equipped with a UDC. this information belongs in the changelog (above the --- marker) are you working with the elinux.org guys ? http://elinux.org/Merge_DFU_support_into_mainline_U-Boot board/samsung/goni/Makefile |2 + board/samsung/goni/flash.c | 634 board/samsung/goni/flash.h | 28 ++ board/samsung/goni/goni.c | 17 + common/Makefile |1 + common/cmd_dfu.c| 50 +++ drivers/usb/gadget/Makefile |1 + drivers/usb/gadget/dfu.c| 920 drivers/usb/gadget/dfu.h| 171 include/configs/s5p_goni.h |6 + include/dfu.h | 31 ++ include/flash_entity.h | 39 ++ include/mbr.h | 49 +++ this should be split up into at least the dfu core and board-specific changes. although i'd wonder how much of the board/samsung/ stuff is really board specific and couldn't be generalized ... --- /dev/null +++ b/include/dfu.h +extern int usb_gadget_handle_interrupts(void); this doesn't belong in the dfu header --- /dev/null +++ b/include/flash_entity.h +struct flash_entity { + char*name; should probably be const --- /dev/null +++ b/include/mbr.h +/* + * Copyright (C) 2010 Samsung Electrnoics + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#include linux/compiler.h missing ifdef protection against multiple inclusion this also should be split out of the dfu core ... although it seems weird that u-boot doesn't already have mbr parsing code ... -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello. On Wed, 2011-11-02 at 11:16, Mike Frysinger wrote: On Wednesday 02 November 2011 06:00:07 Andrzej Pietrasiewicz wrote: are you working with the elinux.org guys ? http://elinux.org/Merge_DFU_support_into_mainline_U-Boot That would be me. As I stated in my other mail I was surprised seeing this patch coming in. I did not communicate my work on this broadly either. Wanted to show up with soem real code not only promises. Same as Andrzej it seems. :) A quick comparison shows me that there are areas that don't touch each other at all (mmc and images on fat as backend vs. raw nand flash as backend) and areas that are duplicated and needed to get sorted out. The DFU protocol implementation in this case. I hope Andrzej is willing to work toegtehr with me on a final implementation that contains pieces of both versions. Mine is missing a code split between DFU protocol and flashing backend for example but feature other parts missing here. board/samsung/goni/Makefile |2 + board/samsung/goni/flash.c | 634 board/samsung/goni/flash.h | 28 ++ board/samsung/goni/goni.c | 17 + common/Makefile |1 + common/cmd_dfu.c| 50 +++ drivers/usb/gadget/Makefile |1 + drivers/usb/gadget/dfu.c| 920 drivers/usb/gadget/dfu.h| 171 include/configs/s5p_goni.h |6 + include/dfu.h | 31 ++ include/flash_entity.h | 39 ++ include/mbr.h | 49 +++ this should be split up into at least the dfu core and board-specific changes. although i'd wonder how much of the board/samsung/ stuff is really board specific and couldn't be generalized ... Agreed. The eMMC flashing with files on FAT is nothing goni specific. Others should be able to use this as well. I see three different parts here that can be separated: 1) The DFU protocol implementation which lives in usb gadget with an interface for flashing backends 2) The flashing backends (no idea where to put them best). At the moment that would be the implementation here with eMMC and files on FAT, mine with raw NAND devices and in the future maybe others. Each flashing backend should be generic enough to allow different boards using it. 3) Board specific information like partitions or hints for the flashing backend. The information itself should be in the board file here and only used by the flashing backends. How does this sound to you? Andrzej? regards Stefan Schmidt signature.asc Description: Digital signature ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dfu: initial implementation
Hello. @Remy: One question I have for you is if the DFU implementation should be based on the re-written gadget layer from samsung or based on the current one? First, and only high level, review for the DFU part. Against which u-boot tree/branch/version is this patch? I tried to apply it against HEAD and it failed for me. Anything I miss? On Wed, 2011-11-02 at 11:00, Andrzej Pietrasiewicz wrote: The implementation is split into two parts: the dfu gadget implementation and a flashing backend, the interface between the two being the struct flash_entity. The flashing backend's implementation is board-specific and is implemented on the Goni target. I replied to Mike's mail with my ideas on this. Split between dfu and flashing backends is good and missng in my approach currently. I would not put it into the board file though but make it generic for other boards as well and only define the needed informations in the board file. Please see my other mail for details. Comments appreciated! diff --git a/common/Makefile b/common/Makefile index ae795e0..de926d9 100644 --- a/common/Makefile +++ b/common/Makefile @@ -162,6 +162,7 @@ COBJS-y += cmd_usb.o COBJS-y += usb.o COBJS-$(CONFIG_USB_STORAGE) += usb_storage.o endif +COBJS-$(CONFIG_CMD_DEVICE_FIRMWARE_UPGRADE) += cmd_dfu.o CONFIG_CMD_DFU instead? Looks a bit long to me. COBJS-$(CONFIG_CMD_XIMG) += cmd_ximg.o COBJS-$(CONFIG_YAFFS2) += cmd_yaffs2.o diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c V new file mode 100644 index 000..c85fbb1 --- /dev/null +++ b/common/cmd_dfu.c @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2011 Samsung Electrnoics + * author: Andrzej Pietrasiewicz andrze...@samsung.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#include common.h +#include command.h +#include dfu.h +#include flash_entity.h + +static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + board_dfu_init(); + dfu_init(); + while (1) { + int irq_res; + /* Handle control-c and timeouts */ + if (ctrlc()) { + printf(The remote end did not respond in time.\n); + goto fail; + } + + irq_res = usb_gadget_handle_interrupts(); + } +fail: + dfu_cleanup(); + board_dfu_cleanup(); + return -1; +} + +U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu, + Use the DFU [Device Firmware Upgrade], + dfu - device firmware upgrade +); + While I think a dfu command is usefull I don't like the need to execute it before any DFU interaction can happen. That may be an option during development but for field upgrades or receovery it is not. I already wrote a bit about my approach here. At OpenMoko we used a button to enter u-boot during startup when pressed. This offered a usbtty interface as usb gadget as well as the runtime descripto for DFU. With dfu-util it was possible to iniate the DFU download or upload procedure while being in the mode. Another option would be to directly jump into DFU mode when a button is pressed. diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c new file mode 100644 index 000..535e194 --- /dev/null +++ b/drivers/usb/gadget/dfu.c @@ -0,0 +1,920 @@ +/* + * dfu.c -- Device Firmware Update gadget + * + * Copyright (C) 2011 Samsung Electronics + * author: Andrzej Pietrasiewicz andrze...@samsung.com + * + * Based on gadget zero: + * Copyright (C) 2003-2007 David Brownell + * All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +