USB: Gadget & DFU related fixes
cover USB: Gadget & DFU related fixes
Various bugfixes, g_dnl & DFU updates.
Pantelis Antoniou (10):
usb: Remove obsolete header file
usb: Fix bug when both DFU & ETHER are defined
g_dnl: Issue connect/disconnect as appropriate
g_dnl: Properly terminate string list.
dfu: Only perform DFU board_usb_init() for TRATS
dfu: Fix crash when wrong number of arguments given
dfu: Send correct DFU response from composite_setup
dfu: Properly zero out timeout value
dfu: Add a partition type target
dfu: Support larger than memory transfers.
common/cmd_dfu.c | 5 +- drivers/dfu/dfu.c | 206 +++++++++++++++++++++++++++++++---------- drivers/dfu/dfu_mmc.c | 106 ++++++++++++++++----- drivers/usb/gadget/Makefile | 13 ++- drivers/usb/gadget/composite.c | 27 ++++++ drivers/usb/gadget/ep0.c | 1 + drivers/usb/gadget/f_dfu.c | 10 +- drivers/usb/gadget/g_dnl.c | 12 ++- include/dfu.h | 19 +++- include/g_dnl.h | 1 - 10 files changed, 314 insertions(+), 86 deletions(-)
Signature · 1 line
1.7.12
PATCH 1/10 usb: Remove obsolete header file
usbdescriptors.h conflicts with linux/usb/ch9.h
Remove it.
--- drivers/usb/gadget/f_dfu.c | 1 - include/g_dnl.h | 1 - 2 files changed, 2 deletions(-)
drivers/usb/gadget/f_dfu.c +0 -1
@@ -25,7 +25,6 @@#include <malloc.h> #include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> #include <linux/usb/composite.h>
include/g_dnl.h +0 -1
@@ -22,7 +22,6 @@#define __G_DOWNLOAD_H_#include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> int g_dnl_register (const char *s);
Signature · 1 line
1.7.12
PATCH 1/10 usb: Remove obsolete header file
Hi Pantelis,
Quoted 2 lines
usbdescriptors.hconflicts withlinux/usb/ch9.h
Remove it.
After applying this patch, I cannot build trats target anymore.
If I remember correctly both files (usbdescriptors.h and linux/usb/ch9.h) are necessary.
The usbdescriptors.h declares e.g.: struct usb_device_descriptor
Moreover after quick check, (I've applied all patches excluding the patch 01/10) the dfu is _NOT_ working properly anymore.
It writes u-boot.bin, but in a way that the board is bricked after flashing.
Regards,
Lukasz
BTW:
1. What is your target device? What is the output of dfu mmc 0 list command on your device?
On trats it is:
DFU alt settings list:
dev: eMMC alt: 0 name: u-boot layout: RAW_ADDR
dev: eMMC alt: 1 name: uImage layout: FAT
2. Please look into the TRATS board (especially the CONFIG_DFU_ALT constant) for reference.
3. What is yours dfu-util version? (Mine is 0.1+svnexported)
Quoted 31 lines
Signed-off-by Pantelis Antoniou <[email protected]>--- drivers/usb/gadget/f_dfu.c | 1 - include/g_dnl.h | 1 - 2 files changed, 2 deletions(-)drivers/usb/gadget/f_dfu.c +0
-1@@ -25,7 +25,6 @@#include <malloc.h> #include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> #include <linux/usb/composite.h> include/g_dnl.h +0
-1@@ -22,7 +22,6 @@#define __G_DOWNLOAD_H_#include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> int g_dnl_register (const char *s);
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 1/10 usb: Remove obsolete header file
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 5 lines · Lukasz Majewski
Hi Pantelis,
Quoted 1 line
usbdescriptors.hconflicts withlinux/usb/ch9.hRemove it.After applying this patch, I cannot build trats target anymore.
Had you also merged u-boot-usb? Locally, trats builds now (which is why this is merged to masgter) but wasn't after the u-boot-usb merge.
- --
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/
iQIcBAEBAgAGBQJQtjDuAAoJENk4IS6UOR1WiooP/3FZJupcB4F64VKFWN2QjO3Y MN3SnN7NFxL5/DqiZ/kg7YAz5f3Z7eKvGRuTZGfM5nyCgqDFm0IWlzzrHkHUK6ow Jlj3Gy0X1iQoDGCC5tCNHR84pka9f0NQ6PQrQbR7fSBGRw5iw/sFANiwWfY50FJz 49Z3rJMLbh0ocwepVHmKmXBlnv++kUxpFd7nMi8gIon70A0a/1X/y1/NIN/fZEE/ e40DHSSgocmrGfJjNb49/UNjhDMqyG9I4yvf7/7QTONkV8taaAHurprzcD/aGRpI zHlxsfVRlZ5TftfEqIyIK3fGHlW4qdCMqjzjiTxjANImlCgeUK3JHdSbafUA9j94 9fDBwE5OZp5v32npxv8d4a9/0JPjMQSAYVq3Lmb64MyWppFkgOKFtr4StbLg9+NE K443RM/NYhvX+bkIuDytc9OxbxKkfVvalCJow+2LosxJO9GovwZfD5BrOX7zul3j EwhzgiFV88Vpzn3SiyGtuhauFugkadrX+3hgC6Mmh1DMYG1MxSK/Ombw7G/nKdzx SUGRL9ij3eEU60QbZFb241TouQ3PZeB1v2P6b6er5veIbS6iUZjd6GdeRNRyWri2 Q9S+h6tHx0CSVSaRudHrAnhdtmbbCGvgzsPY7fHzMzu5hiV7Ng12Sg0Mr1TiDazu 4rd57SIVSIFLBmdNUnK3
=3b1u
-----END PGP SIGNATURE-----
PATCH 1/10 usb: Remove obsolete header file
Hi Tom,
Quoted 4 lines
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1On 11/28/12 09:40, Lukasz Majewski wrote:
Quoted 9 lines
Quoted 5 lines
Hi Pantelis,
Quoted 1 line
usbdescriptors.hconflicts withlinux/usb/ch9.hRemove it.After applying this patch, I cannot build trats target anymore.
Had you also merged u-boot-usb? Locally, trats builds now (which is why this is merged to masgter) but wasn't after the u-boot-usb merge.
Good point, I've rebased on master.
I will do the same with u-boot-usb
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 1/10 usb: Remove obsolete header file
In reply to Lukasz MajewskiHi Tom,
Quoted 5 lines
Hi Pantelis,
Quoted 2 lines
usbdescriptors.hconflicts withlinux/usb/ch9.h
Remove it.
After rebasing on u-boot-usb/next below comment apply:
After applying this patch, I cannot build trats target anymore.
With u-boot-usb/master I can compile the u-boot for trats board with no warnings and errors.
Unfortunately after flashing with dfu, the u-boot is _NOT_ working properly anymore.
It seems, that some parts of the binary weren't correct.
Quoted 2 lines
It writes u-boot.bin, but in a way that the board is bricked after flashing.
I need some time to perform the thorough review of core DFU patches (patches 7/10, 09/10, 10/10).
Quoted 48 lines
BTW:
1. What is your target device? What is the output of dfu mmc 0 list command on your device?On trats it is:
DFU alt settings list:
dev: eMMC alt: 0 name: u-boot layout:RAW_ADDR
dev: eMMC alt: 1 name: uImage layout: FAT2. Please look into the TRATS board (especially the
CONFIG_DFU_ALTconstant) for reference.3. What is yours dfu-util version? (Mine is 0.1+svnexported)
Quoted 31 lines
Signed-off-by Pantelis Antoniou <[email protected]>--- drivers/usb/gadget/f_dfu.c | 1 - include/g_dnl.h | 1 - 2 files changed, 2 deletions(-)drivers/usb/gadget/f_dfu.c +0
-1@@ -25,7 +25,6 @@#include <malloc.h> #include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> #include <linux/usb/composite.h> include/g_dnl.h +0
-1@@ -22,7 +22,6 @@#define __G_DOWNLOAD_H_#include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> int g_dnl_register (const char *s);
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 1/10 usb: Remove obsolete header file
Hi Lukasz,
Quoted 12 lines · Lukasz Majewski
Hi Tom,
Quoted 5 lines
Hi Pantelis,
Quoted 2 lines
usbdescriptors.hconflicts withlinux/usb/ch9.h
Remove it.After rebasing on u-boot-usb/next below comment apply:
After applying this patch, I cannot build trats target anymore.
Quoted 7 lines
With u-boot-usb/master I can compile the u-boot for trats board with no warnings and errors.
Unfortunately after flashing with dfu, the u-boot is _NOT_ working properly anymore.
It seems, that some parts of the binary weren't correct.
Are you writing to a file in a filesystem? I.e. FAT?
I'm in the middle of doing more extensive tests, but file write to an FS might have problems. I am using the raw mmc interface.
There could be something there that it's missed. I'm in the middle of doing more extensive tests.
Quoted 6 lines
Quoted 2 lines
It writes u-boot.bin, but in a way that the board is bricked after flashing.
I need some time to perform the thorough review of core DFU patches (patches 7/10, 09/10, 10/10).
OK.
Quoted 6 lines
Quoted 6 lines
BTW:
1. What is your target device? What is the output of dfu mmc 0 list command on your device?
I'm on am335x_evm target, actual board is a beaglebone.
Quoted 5 lines
Quoted 5 lines
On trats it is:
DFU alt settings list:
dev: eMMC alt: 0 name: u-boot layout:RAW_ADDR
dev: eMMC alt: 1 name: uImage layout: FAT
# setenv dfu_alt_info 'test part 0 3' # mmc part U-Boot# mmc part
Partition Map for MMC device 0 -- Partition Type: DOS
Part Start Sector Num Sectors UUID Type 1 63 112392 7a348599-01 0c Boot 2 112455 7501331 7a348599-02 83 3 7613786 12966 7a348599-03 83 # dfu mmc 0 list DFU alt settings list: dev: eMMC alt: 0 name: test layout: RAW_ADDR
Are you downloading u-boot.bin to the raw nand?
I.e. there's no boot partition?
All my tests have been downloading a small ext3 image to the mmc. I'm in the middle of doing more extensive tests but those takes a huge amount of time... :(
Quoted 3 lines
Quoted 3 lines
2. Please look into the TRATS board (especially the
CONFIG_DFU_ALTconstant) for reference.
Already looked there.
Quoted 2 lines
Quoted 2 lines
3. What is yours dfu-util version? (Mine is 0.1+svnexported)
Compiled from source git://gitorious.org/dfu-util/dfu-util.git
Current master.
Regards
-- Pantelis
Quoted 43 lines
Quoted 34 lines
Quoted 31 lines
Signed-off-by Pantelis Antoniou <[email protected]>---
drivers/usb/gadget/f_dfu.c| 1 -include/g_dnl.h| 1 -2 files changed, 2 deletions(-)drivers/usb/gadget/f_dfu.c +0
-1@@ -25,7 +25,6 @@#include <malloc.h> #include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> #include <linux/usb/composite.h> include/g_dnl.h +0
-1@@ -22,7 +22,6 @@#define __G_DOWNLOAD_H_#include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> int g_dnl_register (const char *s);Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 1/10 usb: Remove obsolete header file
Hi Pantelis,
Quoted 24 lines
Hi Lukasz,
Quoted 12 lines · Lukasz Majewski
Hi Tom,
Quoted 5 lines
Hi Pantelis,
Quoted 2 lines
usbdescriptors.hconflicts withlinux/usb/ch9.h
Remove it.After rebasing on u-boot-usb/next below comment apply:
After applying this patch, I cannot build trats target anymore.
Quoted 7 lines
With u-boot-usb/master I can compile the u-boot for trats board with no warnings and errors.
Unfortunately after flashing with dfu, the u-boot is _NOT_ working properly anymore.
It seems, that some parts of the binary weren't correct.
Quoted 5 lines
Are you writing to a file in a filesystem? I.e. FAT?
I'm in the middle of doing more extensive tests, but file write to an FS might have problems. I am using the raw mmc interface.
I've written u-boot to RAW eMMC (based on the LBA addressing). Moreover I was also able to write data to FAT and EXT4 fs (which uses standard fat/ext4 load commands).
Quoted 35 lines
There could be something there that it's missed. I'm in the middle of doing more extensive tests.
Quoted 6 lines
Quoted 2 lines
It writes u-boot.bin, but in a way that the board is bricked after flashing.
I need some time to perform the thorough review of core DFU patches (patches 7/10, 09/10, 10/10).
OK.
Quoted 6 lines
Quoted 6 lines
BTW:
1. What is your target device? What is the output of dfu mmc 0 list command on your device?I'm on
am335x_evmtarget, actual board is a beaglebone.Quoted 5 lines
Quoted 5 lines
On trats it is:
DFU alt settings list:
dev: eMMC alt: 0 name: u-boot layout:RAW_ADDR
dev: eMMC alt: 1 name: uImage layout: FAT# setenv dfu_alt_info 'test part 0 3' # mmc part U-Boot# mmc partPartition Map for MMC device 0 -- Partition Type: DOS
Quoted 8 lines
Part Start Sector Num Sectors UUID Type 1 63 112392 7a348599-01 0c Boot 2 112455 7501331 7a348599-02 83 3 7613786 12966 7a348599-03 83 # dfu mmc 0 list DFU alt settings list: dev: eMMC alt: 0 name: test layout: RAW_ADDR
Off topic:
It would be nice to have all partitions listed with dfu mmc 0 list :-) and then have access to it via dfu-util tool (as a separate alt settings). But this is a task for the future :-).
Quoted 2 lines
Are you downloading u-boot.bin to the raw nand?
I.e. there's no boot partition?
Yes, there isn't any partition for u-boot. I write to raw eMMC address.
Quoted 4 lines
All my tests have been downloading a small ext3 image to the mmc. I'm in the middle of doing more extensive tests but those takes a huge amount of time... :(
This is because of very low DFU transmission speed (It uses only EP0 with EPS=64B , so it is meant to transfer really small files).
Updating rootfs via DFU would take much time. It is OK, to transfer u-boot, uImage, some log data.
Quoted 62 lines
Quoted 4 lines
Quoted 4 lines
2. Please look into the TRATS board (especially the
CONFIG_DFU_ALTconstant) for reference.Already looked there.
Quoted 2 lines
Quoted 2 lines
3. What is yours dfu-util version? (Mine is 0.1+svnexported)
Compiled from source
git://gitorious.org/dfu-util/dfu-util.git
Current master.Regards
-- Pantelis
Quoted 43 lines
Quoted 34 lines
Quoted 31 lines
Signed-off-by Pantelis Antoniou <[email protected]>---
drivers/usb/gadget/f_dfu.c| 1 -include/g_dnl.h| 1 -2 files changed, 2 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_dfu.cindex 3ec4c65..10547e3 100644drivers/usb/gadget/f_dfu.c +0
-1@@ -25,7 +25,6 @@#include <malloc.h> #include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> #include <linux/usb/composite.h> include/g_dnl.h +0
-1@@ -22,7 +22,6 @@#define __G_DOWNLOAD_H_#include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> int g_dnl_register (const char *s);Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 1/10 usb: Remove obsolete header file
Hi Lukasz,
Quoted 28 lines · Lukasz Majewski
Hi Pantelis,
Quoted 26 lines
Hi Lukasz,
Quoted 12 lines · Lukasz Majewski
Hi Tom,
Quoted 5 lines
Hi Pantelis,
Quoted 2 lines
usbdescriptors.hconflicts withlinux/usb/ch9.h
Remove it.After rebasing on u-boot-usb/next below comment apply:
After applying this patch, I cannot build trats target anymore.
Quoted 7 lines
With u-boot-usb/master I can compile the u-boot for trats board with no warnings and errors.
Unfortunately after flashing with dfu, the u-boot is _NOT_ working properly anymore.
It seems, that some parts of the binary weren't correct.Are you writing to a file in a filesystem? I.e. FAT?
Quoted 8 lines
Quoted 3 lines
I'm in the middle of doing more extensive tests, but file write to an FS might have problems. I am using the raw mmc interface.
I've written u-boot to RAW eMMC (based on the LBA addressing). Moreover I was also able to write data to FAT and EXT4 fs (which uses standard fat/ext4 load commands).
I see. Note that ext4 writes won't work; there is no offset to the write file command. You will have to have use a large enough buffer to handle the file's worst case.
Quoted 59 lines
Quoted 35 lines
There could be something there that it's missed. I'm in the middle of doing more extensive tests.
Quoted 6 lines
Quoted 2 lines
It writes u-boot.bin, but in a way that the board is bricked after flashing.
I need some time to perform the thorough review of core DFU patches (patches 7/10, 09/10, 10/10).
OK.
Quoted 6 lines
Quoted 6 lines
BTW:
1. What is your target device? What is the output of dfu mmc 0 list command on your device?I'm on
am335x_evmtarget, actual board is a beaglebone.Quoted 5 lines
Quoted 5 lines
On trats it is:
DFU alt settings list:
dev: eMMC alt: 0 name: u-boot layout:RAW_ADDR
dev: eMMC alt: 1 name: uImage layout: FAT# setenv dfu_alt_info 'test part 0 3' # mmc part U-Boot# mmc partPartition Map for MMC device 0 -- Partition Type: DOS
Quoted 8 lines
Part Start Sector Num Sectors UUID Type
1 63 112392 7a348599-01 0c Boot 2 112455 7501331 7a348599-02 83 3 7613786 12966 7a348599-03 83 # dfu mmc 0 list DFU alt settings list: dev: eMMC alt: 0 name: test layout: RAW_ADDROff topic:
It would be nice to have all partitions listed with dfu mmc 0 list :-) and then have access to it via dfu-util tool (as a separate alt settings). But this is a task for the future :-).Quoted 2 lines
Are you downloading u-boot.bin to the raw nand?
I.e. there's no boot partition?Yes, there isn't any partition for u-boot. I write to raw eMMC address.
Quoted 4 lines
All my tests have been downloading a small ext3 image to the mmc. I'm in the middle of doing more extensive tests but those takes a huge amount of time... :(
Quoted 6 lines
This is because of very low DFU transmission speed (It uses only EP0 with EPS=64B , so it is meant to transfer really small files).
Updating rootfs via DFU would take much time. It is OK, to transfer u-boot, uImage, some log data.
We have customers that do want to use DFU for all of their initial programming.
The limitations are known, but the fact is that they find it very convenient for their use. And it's not like you're limited by the USB interface, most of the time is spent writing to the medium.
BTW, I've just confirmed that larger transfers get corrupted... :( Working on a fix now...
Quoted 71 lines
Quoted 62 lines
Quoted 4 lines
Quoted 4 lines
2. Please look into the TRATS board (especially the
CONFIG_DFU_ALTconstant) for reference.Already looked there.
Quoted 2 lines
Quoted 2 lines
3. What is yours dfu-util version? (Mine is 0.1+svnexported)
Compiled from source
git://gitorious.org/dfu-util/dfu-util.git
Current master.Regards
-- Pantelis
Quoted 43 lines
Quoted 34 lines
Quoted 31 lines
Signed-off-by Pantelis Antoniou <[email protected]>---
drivers/usb/gadget/f_dfu.c| 1 -include/g_dnl.h| 1 -2 files changed, 2 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_dfu.cindex 3ec4c65..10547e3 100644drivers/usb/gadget/f_dfu.c +0
-1@@ -25,7 +25,6 @@#include <malloc.h> #include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> #include <linux/usb/composite.h> include/g_dnl.h +0
-1@@ -22,7 +22,6 @@#define __G_DOWNLOAD_H_#include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> int g_dnl_register (const char *s);Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform GroupSignature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 1/10 usb: Remove obsolete header file
Hi Pantelis,
Quoted 36 lines
Hi Lukasz,
Quoted 32 lines · Lukasz Majewski
Hi Pantelis,
Quoted 29 lines
Hi Lukasz,
Quoted 12 lines · Lukasz Majewski
Hi Tom,
Quoted 5 lines
Hi Pantelis,
Quoted 2 lines
usbdescriptors.hconflicts withlinux/usb/ch9.h
Remove it.After rebasing on u-boot-usb/next below comment apply:
After applying this patch, I cannot build trats target anymore.
Quoted 7 lines
With u-boot-usb/master I can compile the u-boot for trats board with no warnings and errors.
Unfortunately after flashing with dfu, the u-boot is _NOT_ working properly anymore.
It seems, that some parts of the binary weren't correct.Are you writing to a file in a filesystem? I.e. FAT?
I'm in the middle of doing more extensive tests, but file write to an FS might have problems. I am using the raw mmc interface.
Quoted 8 lines
Quoted 4 lines
I've written u-boot to RAW eMMC (based on the LBA addressing). Moreover I was also able to write data to FAT and EXT4 fs (which uses standard fat/ext4 load commands).
I see. Note that ext4 writes won't work; there is no offset to the write file command. You will have to have use a large enough buffer to handle the file's worst case.
The 4 MiB buffer size was my wrong simplification. It shall be done as you proposed (with transfered data chopped to smaller chunks).
The ext4write command will write data from DRAM buffer (e.g. 0x44000000) to partition formatted with ext4 fs. (one just needs to remember that file size is given with hex number).
I've posted some fixes for ext4 recently to ML.
Quoted 64 lines
Quoted 64 lines
Quoted 36 lines
There could be something there that it's missed. I'm in the middle of doing more extensive tests.
Quoted 6 lines
Quoted 2 lines
It writes u-boot.bin, but in a way that the board is bricked after flashing.
I need some time to perform the thorough review of core DFU patches (patches 7/10, 09/10, 10/10).
OK.
Quoted 6 lines
Quoted 6 lines
BTW:
1. What is your target device? What is the output of dfu mmc 0 list command on your device?I'm on
am335x_evmtarget, actual board is a beaglebone.Quoted 5 lines
Quoted 5 lines
On trats it is:
DFU alt settings list:
dev: eMMC alt: 0 name: u-boot layout:RAW_ADDR
dev: eMMC alt: 1 name: uImage layout: FAT# setenv dfu_alt_info 'test part 0 3' # mmc part U-Boot# mmc partPartition Map for MMC device 0 -- Partition Type: DOS
Quoted 8 lines
Part Start Sector Num Sectors UUID Type
1 63 112392 7a348599-01 0c Boot 2 112455 7501331 7a348599-02 83 3 7613786 12966 7a348599-03 83 # dfu mmc 0 list DFU alt settings list: dev: eMMC alt: 0 name: test layout: RAW_ADDROff topic:
It would be nice to have all partitions listed with dfu mmc 0 list :-) and then have access to it via dfu-util tool (as a separate alt settings). But this is a task for the future :-).Quoted 2 lines
Are you downloading u-boot.bin to the raw nand?
I.e. there's no boot partition?Yes, there isn't any partition for u-boot. I write to raw eMMC address.
Quoted 4 lines
All my tests have been downloading a small ext3 image to the mmc. I'm in the middle of doing more extensive tests but those takes a huge amount of time... :(
This is because of very low DFU transmission speed (It uses only EP0 with EPS=64B , so it is meant to transfer really small files).
Quoted 6 lines
Quoted 3 lines
Updating rootfs via DFU would take much time. It is OK, to transfer u-boot, uImage, some log data.
We have customers that do want to use DFU for all of their initial programming.
Ok, I see.
Quoted 4 lines
The limitations are known, but the fact is that they find it very convenient for their use. And it's not like you're limited by the USB interface, most of the time is spent writing to the medium.
So in my opinion there would be nice to have some kind of caching of received data before writing to memory.
Then we could write memory asynchronously, when we "collect" e.g. 1 MiB data in a buffer (and in meantime collect next data).
Quoted 3 lines
BTW, I've just confirmed that larger transfers get corrupted... :( Working on a fix now...
Ok.
Quoted 73 lines
Quoted 72 lines
Quoted 63 lines
Quoted 5 lines
Quoted 5 lines
2. Please look into the TRATS board (especially the
CONFIG_DFU_ALTconstant) for reference.Already looked there.
Quoted 2 lines
Quoted 2 lines
3. What is yours dfu-util version? (Mine is 0.1+svnexported)
Compiled from source
git://gitorious.org/dfu-util/dfu-util.git
Current master.Regards
-- Pantelis
Quoted 43 lines
Quoted 34 lines
Quoted 31 lines
Signed-off-by Pantelis Antoniou <[email protected]>---
drivers/usb/gadget/f_dfu.c| 1 -include/g_dnl.h| 1 -2 files changed, 2 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_dfu.cindex 3ec4c65..10547e3 100644drivers/usb/gadget/f_dfu.c +0
-1@@ -25,7 +25,6 @@#include <malloc.h> #include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> #include <linux/usb/composite.h> include/g_dnl.h +0
-1@@ -22,7 +22,6 @@#define __G_DOWNLOAD_H_#include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> int g_dnl_register (const char *s);Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform GroupSignature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 1/10 usb: Remove obsolete header file
Hi Lukasz,
Quoted 47 lines · Lukasz Majewski
Hi Pantelis,
Quoted 36 lines
Hi Lukasz,
Quoted 32 lines · Lukasz Majewski
Hi Pantelis,
Quoted 29 lines
Hi Lukasz,
Quoted 12 lines · Lukasz Majewski
Hi Tom,
Quoted 5 lines
Hi Pantelis,
Quoted 2 lines
usbdescriptors.hconflicts withlinux/usb/ch9.h
Remove it.After rebasing on u-boot-usb/next below comment apply:
After applying this patch, I cannot build trats target anymore.
Quoted 7 lines
With u-boot-usb/master I can compile the u-boot for trats board with no warnings and errors.
Unfortunately after flashing with dfu, the u-boot is _NOT_ working properly anymore.
It seems, that some parts of the binary weren't correct.Are you writing to a file in a filesystem? I.e. FAT?
I'm in the middle of doing more extensive tests, but file write to an FS might have problems. I am using the raw mmc interface.
Quoted 8 lines
Quoted 4 lines
I've written u-boot to RAW eMMC (based on the LBA addressing). Moreover I was also able to write data to FAT and EXT4 fs (which uses standard fat/ext4 load commands).
I see. Note that ext4 writes won't work; there is no offset to the write file command. You will have to have use a large enough buffer to handle the file's worst case.
Quoted 8 lines
The 4 MiB buffer size was my wrong simplification. It shall be done as you proposed (with transfered data chopped to smaller chunks).
The ext4write command will write data from DRAM buffer (e.g.
0x44000000) to partition formatted with ext4 fs. (one just needs to remember that file size is given with hex number).
I've posted some fixes for ext4 recently to ML.
Nice. There is a big fat warning when you try to write an ext4 file at an offset other than 0; it should be easy to fix.
Quoted 78 lines
Quoted 64 lines
Quoted 64 lines
Quoted 36 lines
There could be something there that it's missed. I'm in the middle of doing more extensive tests.
Quoted 6 lines
Quoted 2 lines
It writes u-boot.bin, but in a way that the board is bricked after flashing.
I need some time to perform the thorough review of core DFU patches (patches 7/10, 09/10, 10/10).
OK.
Quoted 6 lines
Quoted 6 lines
BTW:
1. What is your target device? What is the output of dfu mmc 0 list command on your device?I'm on
am335x_evmtarget, actual board is a beaglebone.Quoted 5 lines
Quoted 5 lines
On trats it is:
DFU alt settings list:
dev: eMMC alt: 0 name: u-boot layout:RAW_ADDR
dev: eMMC alt: 1 name: uImage layout: FAT# setenv dfu_alt_info 'test part 0 3' # mmc part U-Boot# mmc partPartition Map for MMC device 0 -- Partition Type: DOS
Quoted 8 lines
Part Start Sector Num Sectors UUID Type
1 63 112392 7a348599-01 0c Boot 2 112455 7501331 7a348599-02 83 3 7613786 12966 7a348599-03 83 # dfu mmc 0 list DFU alt settings list: dev: eMMC alt: 0 name: test layout: RAW_ADDROff topic:
It would be nice to have all partitions listed with dfu mmc 0 list :-) and then have access to it via dfu-util tool (as a separate alt settings). But this is a task for the future :-).Quoted 2 lines
Are you downloading u-boot.bin to the raw nand?
I.e. there's no boot partition?Yes, there isn't any partition for u-boot. I write to raw eMMC address.
Quoted 4 lines
All my tests have been downloading a small ext3 image to the mmc. I'm in the middle of doing more extensive tests but those takes a huge amount of time... :(
This is because of very low DFU transmission speed (It uses only EP0 with EPS=64B , so it is meant to transfer really small files).
Quoted 6 lines
Quoted 3 lines
Updating rootfs via DFU would take much time. It is OK, to transfer u-boot, uImage, some log data.
We have customers that do want to use DFU for all of their initial programming.
Ok, I see.
Quoted 4 lines
The limitations are known, but the fact is that they find it very convenient for their use. And it's not like you're limited by the USB interface, most of the time is spent writing to the medium.
Quoted 6 lines
So in my opinion there would be nice to have some kind of caching of received data before writing to memory.
Then we could write memory asynchronously, when we "collect" e.g. 1 MiB data in a buffer (and in meantime collect next data).
I doubt that would work in a u-boot environment. There are no threads and I'm pretty sure most mass memory interfaces are PIO.
I'm completely up to date on latest u-boot happenings, maybe this has changed.
Regards
-- Pantelis
Quoted 89 lines
Quoted 4 lines
BTW, I've just confirmed that larger transfers get corrupted... :( Working on a fix now...
Ok.
Quoted 73 lines
Quoted 72 lines
Quoted 63 lines
Quoted 5 lines
Quoted 5 lines
2. Please look into the TRATS board (especially the
CONFIG_DFU_ALTconstant) for reference.Already looked there.
Quoted 2 lines
Quoted 2 lines
3. What is yours dfu-util version? (Mine is 0.1+svnexported)
Compiled from source
git://gitorious.org/dfu-util/dfu-util.git
Current master.Regards
-- Pantelis
Quoted 43 lines
Quoted 34 lines
Quoted 31 lines
Signed-off-by Pantelis Antoniou <[email protected]>---
drivers/usb/gadget/f_dfu.c| 1 -include/g_dnl.h| 1 -2 files changed, 2 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_dfu.cindex 3ec4c65..10547e3 100644drivers/usb/gadget/f_dfu.c +0
-1@@ -25,7 +25,6 @@#include <malloc.h> #include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> #include <linux/usb/composite.h> include/g_dnl.h +0
-1@@ -22,7 +22,6 @@#define __G_DOWNLOAD_H_#include <linux/usb/ch9.h> -#include <usbdescriptors.h> #include <linux/usb/gadget.h> int g_dnl_register (const char *s);Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform GroupSignature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform GroupSignature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 1/10 usb: Remove obsolete header file
In reply to Lukasz MajewskiHi Lukasz,
I did found out the problem with large transfers, and there is a patch that fixes it.
I'm pretty sure that the transfers are correct. Perhaps some other problem with your board?
Can you use the updated patch series and try with that, with debugging enabled?
Regards
-- Pantelis
PATCH 2/10 usb: Fix bug when both DFU & ETHER are defined
In reply to Pantelis AntoniouWhen both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined the makefile links objects twice.
Detect this and fix it with a not very elegant way in the makefile. Revisit and clean it later.
--- drivers/usb/gadget/Makefile | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
drivers/usb/gadget/Makefile +11 -2
@@ -26,14 +26,23 @@ include $(TOPDIR)/config.mk LIB := $(obj)libusb_gadget.o # new USB gadget layer dependencies + +# ugh; ugh; ugh common objects included twice +ifdef CONFIG_USB_GADGET + COBJS-y += epautoconf.o config.o usbstring.o +else + ifdef CONFIG_USB_ETHER + COBJS-y += epautoconf.o config.o usbstring.o + endif +endif + ifdef CONFIG_USB_GADGET -COBJS-y += epautoconf.o config.o usbstring.o COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o COBJS-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o COBJS-$(CONFIG_DFU_FUNCTION) += f_dfu.o endif ifdef CONFIG_USB_ETHER -COBJS-y += ether.o epautoconf.o config.o usbstring.o +COBJS-y += ether.o COBJS-$(CONFIG_USB_ETH_RNDIS) += rndis.o COBJS-$(CONFIG_MV_UDC) += mv_udc.o COBJS-$(CONFIG_CPU_PXA25X) += pxa25x_udc.o
Signature · 1 line
1.7.12
PATCH 2/10 usb: Fix bug when both DFU & ETHER are defined
Dear Pantelis Antoniou,
Quoted 7 lines
When both
CONFIG_USB_GADGET&CONFIG_USB_ETHERare defined the makefile links objects twice.Detect this and fix it with a not very elegant way in the makefile. Revisit and clean it later.
Signed-off-by Pantelis Antoniou <[email protected]>
[...]
This really needs some proper thinking and fixing.
Besides ... can you please annotate the patches with proper V2: and a proper changelog, see [1]?
[1] http://www.denx.de/wiki/U-Boot/Patches
Best regards,
Marek Vasut
PATCH 3/10 g_dnl: Issue connect/disconnect as appropriate
In reply to Pantelis AntoniouCall usb_gadget_connect/usb_gadget_disconnect in g_dnl_bind/g_dnl_unbind.
--- drivers/usb/gadget/g_dnl.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/usb/gadget/g_dnl.c +10 -1
@@ -83,7 +83,12 @@ static struct usb_gadget_strings *g_dnl_composite_strings[] = {static int g_dnl_unbind (struct usb_composite_dev *cdev) { -debug ("%s\n" , __func__); +struct usb_gadget *gadget = cdev->gadget; + +debug ("%s: calling usb_gadget_disconnect for " +"controller '%s'\n" , shortname, gadget->name); +usb_gadget_disconnect (gadget); +return 0 ; }
@@ -153,6 +158,10 @@ static int g_dnl_bind(struct usb_composite_dev *cdev) device_desc.bcdDevice =__constant_cpu_to_le16 (0x9999 ); } +debug ("%s: calling usb_gadget_connect for " +"controller '%s'\n" , shortname, gadget->name); +usb_gadget_connect (gadget); +return 0 ; error:
Signature · 1 line
1.7.12
PATCH 4/10 g_dnl: Properly terminate string list.
In reply to Pantelis AntoniouWell, not terminating the list causes very interesting crashes. As in changing the vendor & product ID crashes. Fun.
--- drivers/usb/gadget/g_dnl.c | 1 + 1 file changed, 1 insertion(+)
drivers/usb/gadget/g_dnl.c +1 -0
@@ -69,6 +69,7 @@ static struct usb_device_descriptor device_desc = {static struct usb_string g_dnl_string_defs[] = { {0 , manufacturer, }, {1 , product, }, + { }/* end of list */ };static struct usb_gadget_strings g_dnl_string_tab = {
Signature · 1 line
1.7.12
PATCH 4/10 g_dnl: Properly terminate string list.
Hi Pantelis,
Quoted 13 lines
Well, not terminating the list causes very interesting crashes. As in changing the vendor & product ID crashes. Fun.
Signed-off-by Pantelis Antoniou <[email protected]>--- drivers/usb/gadget/g_dnl.c | 1 + 1 file changed, 1 insertion(+)drivers/usb/gadget/g_dnl.c +0
-0
drivers/usb/gadget/g_dnl.c static struct usb_device_descriptor device_desc = { · line 72
Quoted 4 lines
drivers/usb/gadget/g_dnl.c +1
-0@@ -69,6 +69,7 @@ static struct usb_device_descriptor device_desc = {static struct usb_string g_dnl_string_defs[] = { {0 , manufacturer, }, {1 , product, }, + { }/* end of list */
Thanks for spotting.
drivers/usb/gadget/g_dnl.c static struct usb_device_descriptor device_desc = { · line 75
Quoted 3 lines
drivers/usb/gadget/g_dnl.c +0
-0};static struct usb_gadget_strings g_dnl_string_tab = {
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 4/10 g_dnl: Properly terminate string list.
In reply to Pantelis AntoniouDear Pantelis Antoniou,
Quoted 13 lines
Well, not terminating the list causes very interesting crashes. As in changing the vendor & product ID crashes. Fun.
Signed-off-by Pantelis Antoniou <[email protected]>--- drivers/usb/gadget/g_dnl.c | 1 + 1 file changed, 1 insertion(+)drivers/usb/gadget/g_dnl.c +0
-0
drivers/usb/gadget/g_dnl.c static struct usb_device_descriptor device_desc = { · line 72
Quoted 4 lines
drivers/usb/gadget/g_dnl.c +1
-0@@ -69,6 +69,7 @@ static struct usb_device_descriptor device_desc = {static struct usb_string g_dnl_string_defs[] = { {0 , manufacturer, }, {1 , product, }, + { }/* end of list */
So you're adding an uninited entry here? How'll this work?
drivers/usb/gadget/g_dnl.c static struct usb_device_descriptor device_desc = { · line 75
Quoted 3 lines
drivers/usb/gadget/g_dnl.c +0
-0};static struct usb_gadget_strings g_dnl_string_tab = {
Best regards,
Marek Vasut
PATCH 4/10 g_dnl: Properly terminate string list.
Hi Marek,
Quoted 10 lines · Marek Vasut
Dear Pantelis Antoniou,
Quoted 8 lines
Well, not terminating the list causes very interesting crashes. As in changing the vendor & product ID crashes. Fun.
Signed-off-by Pantelis Antoniou <[email protected]>---
drivers/usb/gadget/g_dnl.c| 1 +1 file changed, 1 insertion(+)
Quoted 12 lines
drivers/usb/gadget/g_dnl.c static struct usb_device_descriptor device_desc = { · line 72
Quoted 9 lines
drivers/usb/gadget/g_dnl.c +1
-0@@ -69,6 +69,7 @@ static struct usb_device_descriptor device_desc = { static struct usb_string[] = { {0 , manufacturer, }, {1 , product, }, + { }/* end of list */ So you're adding an uninited entry here? How'll this work?
This is very common idiom. It's not uninitialized, all the members are set to 0, including the char * members.
It is exactly the same method used in drivers/usb/gadget/ether.c
Quoted 6 lines
drivers/usb/gadget/g_dnl.c static struct usb_device_descriptor device_desc = { · line 75
Quoted 3 lines
drivers/usb/gadget/g_dnl.c +0
-0};static struct usb_gadget_strings g_dnl_string_tab = {Best regards,
Marek Vasut
Regards
-- Pantelis
PATCH 4/10 g_dnl: Properly terminate string list.
Dear Pantelis Antoniou,
Quoted 23 lines
Hi Marek,
Quoted 20 lines · Marek Vasut
Dear Pantelis Antoniou,
Quoted 18 lines
Well, not terminating the list causes very interesting crashes. As in changing the vendor & product ID crashes. Fun.
Signed-off-by Pantelis Antoniou <[email protected]>---
drivers/usb/gadget/g_dnl.c| 1 +1 file changed, 1 insertion(+)drivers/usb/gadget/g_dnl.c +0
-0@@ -69,6 +69,7 @@ static struct usb_device_descriptor device_desc = { static struct usb_string[] = { {0 , manufacturer, }, {1 , product, },
Quoted 8 lines
Quoted 3 lines
drivers/usb/gadget/g_dnl.c static struct usb_device_descriptor device_desc = { · line 72
Quoted 1 line
drivers/usb/gadget/g_dnl.c +1
-0+ { }/* end of list */ So you're adding an uninited entry here? How'll this work?
This is very common idiom. It's not uninitialized, all the members are set to 0, including the char * members.
It is exactly the same method used in
drivers/usb/gadget/ether.c
Blah, it's a matter of taste then (I prefer it explicit, but I don't really care).
Thanks for clearing this up.
Best regards,
Marek Vasut
PATCH 5/10 dfu: Only perform DFU board_usb_init() for TRATS
In reply to Pantelis AntoniouUSB initialization shouldn't happen for all the boards.
--- common/cmd_dfu.c | 3 +++ 1 file changed, 3 insertions(+)
common/cmd_dfu.c +3 -0
@@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) goto done; } +#ifdef CONFIG_TRATS board_usb_init (); +#endif +g_dnl_register (s);while (1 ) {if (ctrlc ())
Signature · 1 line
1.7.12
PATCH 5/10 dfu: Only perform DFU board_usb_init() for TRATS
Hi Pantelis,
Quoted 2 lines
USB initialization shouldn't happen for all the boards.
The board_usb_init() follows u-boot policy, that SoC IPs (USB) are enabled and configured just before their usage.
Quoted 10 lines
Signed-off-by Pantelis Antoniou <[email protected]>--- common/cmd_dfu.c | 3 +++ 1 file changed, 3 insertions(+)common/cmd_dfu.c +0
-0
common/cmd_dfu.c static int do_dfu(cmd_tbl_t *cmdtp, int flag, int · line 61
Quoted 7 lines
common/cmd_dfu.c +3
-0@@ -55,7 +55,10 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) goto done; } +#ifdef CONFIG_TRATS board_usb_init (); +#endif +
In mine opinion this #ifdef shall be removed and each target board using the DFU shall define board_usb_init() at board file.
Quoted 3 lines
common/cmd_dfu.c +0
-0g_dnl_register (s);while (1 ) {if (ctrlc ())
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 5/10 dfu: Only perform DFU board_usb_init() for TRATS
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 12 lines · Lukasz Majewski
Hi Pantelis,
Quoted 2 lines
USB initialization shouldn't happen for all the boards.
The
board_usb_init()follows u-boot policy, that SoC IPs (USB) are enabled and configured just before their usage.Quoted 3 lines
Signed-off-by Pantelis Antoniou <[email protected]>---
common/cmd_dfu.c| 3 +++ 1 file changed, 3 insertions(+)
Quoted 9 lines
Quoted 6 lines
common/cmd_dfu.c index renamed from common/cmd_dfu.c +0
-001d6b3a..327c738 100644 ---
a/common/cmd_dfu.c+++b/common/cmd_dfu.c@@ -55,7 +55,10 @@ static intdo_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])goto done; }+#ifdef
CONFIG_TRATSboard_usb_init(); +#endif +In mine opinion this #ifdef shall be removed and each target board using the DFU shall define
board_usb_init()at board file.
But this isn't a called-only-once place. What are you really doing here and are you sure it's needed every time DFU is called?
- --
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/
iQIcBAEBAgAGBQJQtjM2AAoJENk4IS6UOR1WOu8P/3253rY48k6+NgCefiNZf6GH Sw9ZEh7fNkC3QgSkKA8/Ifa52F455UFpslftjSVHIrGIwIVc+3TCM2lOdbaBZgMi bL0MsPKRRbujx6U69lY5A6eaFyrhPJJJcCryFoPkfzsYSuvazol/crKCs9BB24Mk j35nvd2juxmhh4paQ9+7UVkxI50haLPVBHU7A5v8yv3i9/Cig+qwqewt+GWvIhoE w6frRy4WyczTClWqF+KwlfT4bwJVtnHxzfl5d2qRn4C/McTzUpwVePT8xrGaS4zc 3p+VCQ269Po176sgwoud5EwJdvCNBVfFeHaTORW8UJ2zLzU4iDixx4VY9SQhTfHF MP7Ch5p2DIJRrlEUWuRAgQwO6pICBHD+f3jw5q06hg35JWmTnltyq53M5UILGyi/ Vz+SN0xF4YnMJRvKGT9lal0OiRxr/rO63k6fl2XybEfTED6AHhvUJKBV+yb1OxV0 CTCiBRqfKQwkProdFtAAJT6+CeexV4Im2WcHQwqcKxVNqgEQhM6MBsM3DkjfE+nj naz8ITF6Lal1C0K5dUbSPiY8KqgphXre11wJ28BFp5WSM/p/0wCrhImXxuOMeUd3 QtWT/BT2fJfKcr2bGVLTKdHVBHLWziJQK6IjO8rGnot2QbdSh+QS8n9ZrDf9rJYS +Ha0VT7jIVP/6uF1FXdj
=puWb
-----END PGP SIGNATURE-----
PATCH 5/10 dfu: Only perform DFU board_usb_init() for TRATS
Hi Tom,
Quoted 21 lines
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1Quoted 17 lines · Lukasz Majewski
Hi Pantelis,
Quoted 2 lines
USB initialization shouldn't happen for all the boards.
The
board_usb_init()follows u-boot policy, that SoC IPs (USB) are enabled and configured just before their usage.Quoted 8 lines
Signed-off-by Pantelis Antoniou <[email protected]>---
common/cmd_dfu.c| 3 +++ 1 file changed, 3 insertions(+)common/cmd_dfu.c index renamed from common/cmd_dfu.c +0
-001d6b3a..327c738 100644 ---
a/common/cmd_dfu.c+++b/common/cmd_dfu.c@@ -55,7 +55,10 @@ static intdo_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])goto done; }
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
+#ifdef
CONFIG_TRATSboard_usb_init(); +#endif +In mine opinion this #ifdef shall be removed and each target board using the DFU shall define
board_usb_init()at board file.But this isn't a called-only-once place. What are you really doing here and are you sure it's needed every time DFU is called?
Hmm, you are correct here.
But I don't have a good alternative for this.
One solution would be to define a static flag for it at do_dfu function to indicate if this was executed once (however I'm reluctant do this).
Any ideas?
Quoted 20 lines
- --
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/iQIcBAEBAgAGBQJQtjM2AAoJENk4IS6UOR1WOu8P/3253rY48k6+NgCefiNZf6GH Sw9ZEh7fNkC3QgSkKA8/Ifa52F455UFpslftjSVHIrGIwIVc+3TCM2lOdbaBZgMi bL0MsPKRRbujx6U69lY5A6eaFyrhPJJJcCryFoPkfzsYSuvazol/crKCs9BB24Mk j35nvd2juxmhh4paQ9+7UVkxI50haLPVBHU7A5v8yv3i9/Cig+qwqewt+GWvIhoE w6frRy4WyczTClWqF+KwlfT4bwJVtnHxzfl5d2qRn4C/McTzUpwVePT8xrGaS4zc 3p+VCQ269Po176sgwoud5EwJdvCNBVfFeHaTORW8UJ2zLzU4iDixx4VY9SQhTfHF MP7Ch5p2DIJRrlEUWuRAgQwO6pICBHD+f3jw5q06hg35JWmTnltyq53M5UILGyi/ Vz+SN0xF4YnMJRvKGT9lal0OiRxr/rO63k6fl2XybEfTED6AHhvUJKBV+yb1OxV0 CTCiBRqfKQwkProdFtAAJT6+CeexV4Im2WcHQwqcKxVNqgEQhM6MBsM3DkjfE+nj naz8ITF6Lal1C0K5dUbSPiY8KqgphXre11wJ28BFp5WSM/p/0wCrhImXxuOMeUd3 QtWT/BT2fJfKcr2bGVLTKdHVBHLWziJQK6IjO8rGnot2QbdSh+QS8n9ZrDf9rJYS +Ha0VT7jIVP/6uF1FXdj
=puWb
-----END PGP SIGNATURE-----
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 5/10 dfu: Only perform DFU board_usb_init() for TRATS
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 35 lines · Lukasz Majewski
Hi Tom,
Quoted 25 lines · Lukasz Majewski
Quoted 19 lines
Quoted 19 lines
Hi Pantelis,
Quoted 2 lines
USB initialization shouldn't happen for all the boards.
The
board_usb_init()follows u-boot policy, that SoC IPs (USB) are enabled and configured just before their usage.Quoted 10 lines
Signed-off-by: Pantelis Antoniou <[email protected]> ---
common/cmd_dfu.c| 3 +++ 1 file changed, 3 insertions(+)common/cmd_dfu.c index renamed from common/cmd_dfu.c +0
-001d6b3a..327c738 100644 ---
a/common/cmd_dfu.c+++b/common/cmd_dfu.c@@ -55,7 +55,10 @@ static intdo_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])goto done; }Quoted 5 lines
Quoted 5 lines
Quoted 1 line
+#ifdef
CONFIG_TRATSboard_usb_init(); +#endif +In mine opinion this #ifdef shall be removed and each target board using the DFU shall define
board_usb_init()at board file.Quoted 4 lines
But this isn't a called-only-once place. What are you really doing here and are you sure it's needed every time DFU is called?
Hmm, you are correct here.
Quoted 8 lines
But I don't have a good alternative for this.
One solution would be to define a static flag for it at
do_dfufunction to indicate if this was executed once (however I'm reluctant do this).Any ideas?
I think the answer, and it's what we do on am335x is that arch_misc_init() is what calls the equiv of s3c_udc_probe(...) under the logic of "if we are built with usb gadget support, we want to use it, so init it".
- --
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/
iQIcBAEBAgAGBQJQt56WAAoJENk4IS6UOR1WznYP/23g0QuMB2slIC41OLTeGKfh 11zybSEVZYZmSPfgjEsXqEWh1cYryQNyiNyKIzNfPPyH/ZAA2PuMH7mKMmdp5St6 p7IIhmFwO+phkLGgpLVSJ6PsCGfY68N1r1FU04JJhpteoNmSPtutBWrb2bJ8tib/ 5HHSjUEUSYIgE1OHHVouGUx4KzNwWgyr0nds9WyfJ/X9OnQ22WRuVlkOIpy74NCz r9QSIEOSbmqY6uU+YFFOorgp0Ox97okRJAH0KAsBNxq6PE2NmZard0Qg2m2Ism7L NFbBvlfeF+/m9cicnrnuygyVkkNRcsX5NjWzVilzXQCfYmwBSH2YKPZbpRb3XGmr wNSNqbfSEWG3Oxa+g0NnqI8SPqsTNVXR8X1QsF/f7zIOHlYZfXlbqsDEzITm1YoI S1OEmpYXQQI1kZEOaxfXyJYbMXnA1/y8uItX8Bl/JUMWDQqQMFeJMVS711khGYuR EUVL8YQam6N7Xgzk89sN8UPyOfAbxxOgB5fNyKeuSL+sz0vBaAkmv69gNsdPsfIr vFvfyUKwyMtqhWZO+cG0VU4jzI0S0SMHdh52GtrU6P/3r77MC6zrhVja2EylXqvD p8pSi7eEdeBUMbJ6uMgLd0kxYwh3NWy5NTTR10yKDyTXi8kh/grG89syI5Eiczwj /CW6UuwG8R7T2l2+d1X3
=mdL4
-----END PGP SIGNATURE-----
PATCH 5/10 dfu: Only perform DFU board_usb_init() for TRATS
Hi Tom,
Quoted 46 lines
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1Quoted 42 lines · Lukasz Majewski
Hi Tom,
Quoted 25 lines · Lukasz Majewski
Quoted 19 lines
Quoted 19 lines
Hi Pantelis,
Quoted 2 lines
USB initialization shouldn't happen for all the boards.
The
board_usb_init()follows u-boot policy, that SoC IPs (USB) are enabled and configured just before their usage.Quoted 10 lines
Signed-off-by: Pantelis Antoniou <[email protected]> ---
common/cmd_dfu.c| 3 +++ 1 file changed, 3 insertions(+)common/cmd_dfu.c index renamed from common/cmd_dfu.c +0
-001d6b3a..327c738 100644 ---
a/common/cmd_dfu.c+++b/common/cmd_dfu.c@@ -55,7 +55,10 @@ static intdo_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])goto done; }Quoted 5 lines
Quoted 5 lines
Quoted 1 line
+#ifdef
CONFIG_TRATSboard_usb_init(); +#endif +In mine opinion this #ifdef shall be removed and each target board using the DFU shall define
board_usb_init()at board file.Quoted 4 lines
But this isn't a called-only-once place. What are you really doing here and are you sure it's needed every time DFU is called?
Hmm, you are correct here.
But I don't have a good alternative for this.
One solution would be to define a static flag for it at
do_dfufunction to indicate if this was executed once (however I'm reluctant do this).
Quoted 6 lines
Quoted 1 line
Any ideas?
I think the answer, and it's what we do on am335x is that
arch_misc_init()is what calls the equiv ofs3c_udc_probe(...)under the logic of "if we are built with usb gadget support, we want to use it, so init it".
I've understood the policy differently:
"We are build with gadget support and we _might_ use it, so enable low level code only when (or just before) we use it".
What's about the power consumption? Why IP block which will be used from time to time shall be enabled and operational?
Quoted 25 lines
- --
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/iQIcBAEBAgAGBQJQt56WAAoJENk4IS6UOR1WznYP/23g0QuMB2slIC41OLTeGKfh 11zybSEVZYZmSPfgjEsXqEWh1cYryQNyiNyKIzNfPPyH/ZAA2PuMH7mKMmdp5St6 p7IIhmFwO+phkLGgpLVSJ6PsCGfY68N1r1FU04JJhpteoNmSPtutBWrb2bJ8tib/ 5HHSjUEUSYIgE1OHHVouGUx4KzNwWgyr0nds9WyfJ/X9OnQ22WRuVlkOIpy74NCz r9QSIEOSbmqY6uU+YFFOorgp0Ox97okRJAH0KAsBNxq6PE2NmZard0Qg2m2Ism7L NFbBvlfeF+/m9cicnrnuygyVkkNRcsX5NjWzVilzXQCfYmwBSH2YKPZbpRb3XGmr wNSNqbfSEWG3Oxa+g0NnqI8SPqsTNVXR8X1QsF/f7zIOHlYZfXlbqsDEzITm1YoI S1OEmpYXQQI1kZEOaxfXyJYbMXnA1/y8uItX8Bl/JUMWDQqQMFeJMVS711khGYuR EUVL8YQam6N7Xgzk89sN8UPyOfAbxxOgB5fNyKeuSL+sz0vBaAkmv69gNsdPsfIr vFvfyUKwyMtqhWZO+cG0VU4jzI0S0SMHdh52GtrU6P/3r77MC6zrhVja2EylXqvD p8pSi7eEdeBUMbJ6uMgLd0kxYwh3NWy5NTTR10yKDyTXi8kh/grG89syI5Eiczwj /CW6UuwG8R7T2l2+d1X3
=mdL4
-----END PGP SIGNATURE-----
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH 5/10 dfu: Only perform DFU board_usb_init() for TRATS
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 47 lines · Lukasz Majewski
Hi Tom,
Quoted 44 lines · Lukasz Majewski
Quoted 44 lines
Quoted 44 lines
Hi Tom,
Quoted 27 lines · Lukasz Majewski
Quoted 21 lines
Quoted 21 lines
Hi Pantelis,
Quoted 3 lines
USB initialization shouldn't happen for all the
boards.The
board_usb_init()follows u-boot policy, that SoC IPs (USB) are enabled and configured just before their
usage.Quoted 10 lines
Signed-off-by: Pantelis Antoniou <[email protected]> ---
common/cmd_dfu.c
| 3 +++ 1 file changed, 3 insertions(+)common/cmd_dfu.c +0
-0+++ b/common/cmd_dfu.c @@ -55,7 +55,10 @@ staticint do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char
* const argv[]) goto done; }Quoted 5 lines
Quoted 5 lines
Quoted 1 line
+#ifdef
CONFIG_TRATSboard_usb_init(); +#endif +In mine opinion this #ifdef shall be removed and each target board using the DFU shall define
board_usb_init()at board file.Quoted 4 lines
But this isn't a called-only-once place. What are you really doing here and are you sure it's needed every time DFU is called?
Hmm, you are correct here.
But I don't have a good alternative for this.
One solution would be to define a static flag for it at
do_dfufunction to indicate if this was executed once (however I'm reluctant do this).
Quoted 14 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Any ideas?
I think the answer, and it's what we do on am335x is that
arch_misc_init()is what calls the equiv ofs3c_udc_probe(...)under the logic of "if we are built with usb gadget support, we want to use it, so init it".Quoted 7 lines
I've understood the policy differently:
"We are build with gadget support and we _might_ use it, so enable low level code only when (or just before) we use it".
What's about the power consumption? Why IP block which will be used from time to time shall be enabled and operational?
Frankly, I think this shows we don't have a good setup right now, in general. Saying the gadget needs to whack and re-whack the device into on state (but isn't really turning it off once done) isn't ideal.
There are also indeed drawbacks to saying "gadget support enabled at
boot-time, let me set it up". Right now, I don't really have a good answer. It also needs to cover things like gadget ethernet where we don't really want to disappear immediately after each command.
- --
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/
iQIcBAEBAgAGBQJQuBJzAAoJENk4IS6UOR1WWncP/29E2kP1ErctSmBPUkJZASEk qmW+rYCwxpz3aesC6Dcnc0Urad+MwO9SPbzUFfQcB4CMVXXAXMcahH+4FY8FzTnV p8QqQjZ0o54xYxEI/evkaEWsTyM3nBBXtMq9II2gwYPMxP88i2JwL1NMOF6o2z+Y VVNyGO1685JIGU+WMzpoXp4WYjfd/8qv8JHhQ+moGoFe0p7p5c5kokYsbXdcxvyk rkeJrog8SV+vJQLnu0nUh8Q/pBYftiMDD+U8upQOiYNF1CPgGugX33XJydtYMoA8 rv6an/xh4PXUSJkAJbwp8vrxQqrHD0MYWtZEPwTEToFmTxvvaS2zJ0uOnNLkjanv dp8RrD38HIczsgMtAVXR9nhFsW89MB2EiXc7N/EoO0BA4E0pac1/+OUfUMwELj16 IKJIHa4BXkHfg/ybQXCxr+GDZEc8YwLGXxzyIw8cS3PffNSkR1fkJcjZwFg190y0 1IjmlF0Dhg0CGweej0WTFuzi3OmHQOvMJcvT0doiI35iVbFvGwhjNUt8dJeDr0Yr xkhWPhR3+iMXmzle+me+ZUxUe5g2vGqPC6HeGUO/fo0ntqZfITjzznMHKpN8Uxwv D6Njg2e785ZIgTxcMXKn/Q2xtPBF78/stWeStyIwIAxYOEDw/C+NipWAHEpJdGTp k8mhP+GKWgatyZ5yqDBA
=01FA
-----END PGP SIGNATURE-----
PATCH 5/10 dfu: Only perform DFU board_usb_init() for TRATS
In reply to Lukasz MajewskiQuoted 7 lines · Lukasz Majewski
Hi Pantelis,
Quoted 2 lines
USB initialization shouldn't happen for all the boards.
The
board_usb_init()follows u-boot policy, that SoC IPs (USB) are enabled and configured just before their usage.
Going back to this old thread as I dig at things again. How much of s3c_udc_probe is actually enabling hardware as opposed to registering that such hardware exists and can be enabled when we call usb_gadget_register_driver ? It looks like all of the clocking (and pin muxing?) is already being done.
Signature · 1 line
Tom
PATCH 5/10 dfu: Only perform DFU board_usb_init() for TRATS
Hi Tom,
Quoted 7 lines · Lukasz Majewski
Quoted 6 lines
Hi Pantelis,
Quoted 2 lines
USB initialization shouldn't happen for all the boards.
Quoted 9 lines
Quoted 2 lines
The
board_usb_init()follows u-boot policy, that SoC IPs (USB) are enabled and configured just before their usage.Going back to this old thread as I dig at things again. How much of
s3c_udc_probeis actually enabling hardware as opposed to registering that such hardware exists and can be enabled when we callusb_gadget_register_driver? It looks like all of the clocking (and pin muxing?) is already being done.
I will check if s3c_udc_probe can be removed totally and replaced by usb_gadget_register_driver (since some time has passed from original UDC driver posting).
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 5/10 dfu: Only perform DFU board_usb_init() for TRATS
Quoted 19 lines · Lukasz Majewski
Hi Tom,
Quoted 7 lines · Lukasz Majewski
Quoted 6 lines
Hi Pantelis,
Quoted 2 lines
USB initialization shouldn't happen for all the boards.
Quoted 9 lines
Quoted 2 lines
The
board_usb_init()follows u-boot policy, that SoC IPs (USB) are enabled and configured just before their usage.Going back to this old thread as I dig at things again. How much of
s3c_udc_probeis actually enabling hardware as opposed to registering that such hardware exists and can be enabled when we callusb_gadget_register_driver? It looks like all of the clocking (and pin muxing?) is already being done.
Quoted 3 lines
I will check if
s3c_udc_probecan be removed totally and replaced byusb_gadget_register_driver(since some time has passed from original UDC driver posting).
Well, not so much replace but just, if there's no actual HW init, move the "probe" (which isn't a probe, it's registering board infos) call into arch_misc_init or similar.
--
Tom
cover USB: Gadget & DFU related fixes
In reply to Pantelis AntoniouDear Pantelis Antoniou,
Quoted 13 lines
Various bugfixes,
g_dnl& DFU updates.Pantelis Antoniou (10):
usb: Remove obsolete header file
usb: Fix bug when both DFU & ETHER are defined
g_dnl: Issue connect/disconnect as appropriate
g_dnl: Properly terminate string list.
dfu: Only perform DFUboard_usb_init()for TRATS
dfu: Fix crash when wrong number of arguments given
dfu: Send correct DFU response fromcomposite_setup
dfu: Properly zero out timeout value
dfu: Add a partition type target
dfu: Support larger than memory transfers.
[...]
So this is a V2 patchset? Anyway, can you please rebase on top of u-boot- usb/master and repost? I rebased it on top of u-boot/master and pushed.
Best regards,
Marek Vasut
cover USB: Gadget & DFU related fixes
Hi Marek,
Quoted 15 lines · Marek Vasut
Dear Pantelis Antoniou,
Quoted 13 lines
Various bugfixes,
g_dnl& DFU updates.Pantelis Antoniou (10):
usb: Remove obsolete header file
usb: Fix bug when both DFU & ETHER are defined
g_dnl: Issue connect/disconnect as appropriate
g_dnl: Properly terminate string list.
dfu: Only perform DFUboard_usb_init()for TRATS
dfu: Fix crash when wrong number of arguments given
dfu: Send correct DFU response fromcomposite_setup
dfu: Properly zero out timeout value
dfu: Add a partition type target
dfu: Support larger than memory transfers.
Quoted 5 lines
[...]
So this is a V2 patchset? Anyway, can you please rebase on top of u-boot- usb/master and repost? I rebased it on top of u-boot/master and pushed.
Yeah, it's V2. I've rebased, and the patch series applies without any conflicts or fudges.
Quoted 2 lines
Best regards,
Marek Vasut
Regards
-- Pantelis
PATCH 6/10 dfu: Fix crash when wrong number of arguments given
In reply to Pantelis AntoniouFix obvious crash when not enough arguments are given to the dfu command.
--- common/cmd_dfu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
common/cmd_dfu.c +1 -1
@@ -50,7 +50,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])if (ret)return CMD_RET_FAILURE ; -if (strcmp (argv[3 ],"list" ) ==0 ) { +if (argc >3 &&strcmp (argv[3 ],"list" ) ==0 ) {dfu_show_entities (); goto done; }
Signature · 1 line
1.7.12
PATCH 6/10 dfu: Fix crash when wrong number of arguments given
Hi Pantelis,
Quoted 13 lines
Fix obvious crash when not enough arguments are given to the dfu command.
Signed-off-by Pantelis Antoniou <[email protected]>--- common/cmd_dfu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)common/cmd_dfu.c +0
-0
common/cmd_dfu.c static int do_dfu(cmd_tbl_t *cmdtp, int flag, int · line 53
Quoted 5 lines
common/cmd_dfu.c +1
-1@@ -50,7 +50,7 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (ret)return CMD_RET_FAILURE ; -if (strcmp (argv[3 ],"list" ) ==0 ) { +if (argc >3 &&strcmp (argv[3 ],"list" ) ==0 ) {
I'd add the (argc > 4) to the very beginning check:
if (argc < 3 || argc > 4)
return CMD_RET_USAGE;
and leave below code unchanged:
if (strcmp(argv[3], "list") == 0) {
dfu_show_entities();
goto done;
}Quoted 3 lines
common/cmd_dfu.c +0
-0dfu_show_entities (); goto done; }
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 7/10 dfu: Send correct DFU response from composite_setup
In reply to Pantelis AntoniouDFU is a bit peculiar. It needs to hook to composite setup and return it's function descriptor.
So when get-descriptor request comes with a type of DFU_DT_FUNC we iterate over the configs, and functions, and when we find the DFU function we call the setup method which is prepared to return the appropriate function descriptor.
--- drivers/usb/gadget/composite.c | 27 +++++++++++++++++++++++++++ drivers/usb/gadget/ep0.c | 1 + drivers/usb/gadget/f_dfu.c | 6 ++++-- 3 files changed, 32 insertions(+), 2 deletions(-)
drivers/usb/gadget/composite.c +27 -0
@@ -773,6 +773,33 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)if (value >=0 ) value =min (w_length, (u16 ) value);break ; + +#ifdef CONFIG_DFU_FUNCTION +/* DFU is mighty weird */ +case DFU_DT_FUNC : + w_value &=0xff ; +list_for_each_entry (c, &cdev->configs, list) { +if (w_value !=0 ) { + w_value--; +continue ; + } + +list_for_each_entry (f, &c->functions, list) { + +/* DFU function only */ +if (strcmp (f->name,"dfu" ) !=0 ) +continue ; + + value = f->setup (f, ctrl); + goto dfu_func_done; + } + } +dfu_func_done: +if (value >=0 ) + value =min (w_length, (u16 ) value); +break ; +#endif +default : goto unknown; }
drivers/usb/gadget/ep0.c +1 -0
@@ -221,6 +221,7 @@ static int ep0_get_descriptor (struct usb_device_instance *device,break ;case USB_DESCRIPTOR_TYPE_CONFIGURATION : +case USB_DESCRIPTOR_TYPE_OTHER_SPEED_CONFIGURATION : {struct usb_configuration_descriptor *configuration_descriptor;
drivers/usb/gadget/f_dfu.c +4 -2
@@ -534,8 +534,10 @@ dfu_handle(struct usb_function *f, const struct usb_ctrlrequest *ctrl) value =min (len, (u16 )sizeof (dfu_func));memcpy (req->buf, &dfu_func, value); } - }else /* DFU specific request */ - value = dfu_state[f_dfu->dfu_state] (f_dfu, ctrl, gadget, req); +return value; + } + + value = dfu_state[f_dfu->dfu_state] (f_dfu, ctrl, gadget, req);if (value >=0 ) { req->length = value;
Signature · 1 line
1.7.12
PATCH 8/10 dfu: Properly zero out timeout value
In reply to Pantelis AntoniouZero out timeout value; letting it filled with undefined values ends up with the dfu host hanging.
--- drivers/usb/gadget/f_dfu.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/usb/gadget/f_dfu.c +3 -0
@@ -164,6 +164,9 @@ static void handle_getstatus(struct usb_request *req)/* send status response */ dstat->bStatus = f_dfu->dfu_status; + dstat->bwPollTimeout[0 ] =0 ; + dstat->bwPollTimeout[1 ] =0 ; + dstat->bwPollTimeout[2 ] =0 ; dstat->bState = f_dfu->dfu_state; dstat->iString =0 ; }
Signature · 1 line
1.7.12
PATCH 8/10 dfu: Properly zero out timeout value
Hi Pantelis,
Quoted 13 lines
Zero out timeout value; letting it filled with undefined values ends up with the dfu host hanging.
Signed-off-by Pantelis Antoniou <[email protected]>--- drivers/usb/gadget/f_dfu.c | 3 +++ 1 file changed, 3 insertions(+)drivers/usb/gadget/f_dfu.c +0
-0
drivers/usb/gadget/f_dfu.c static void handle_getstatus(struct usb_request · line 172
Quoted 9 lines
drivers/usb/gadget/f_dfu.c +3
-0@@ -164,6 +164,9 @@ static void handle_getstatus(struct usb_request *req)/* send status response */ dstat->bStatus = f_dfu->dfu_status; + dstat->bwPollTimeout[0 ] =0 ; + dstat->bwPollTimeout[1 ] =0 ; + dstat->bwPollTimeout[2 ] =0 ; dstat->bState = f_dfu->dfu_state; dstat->iString =0 ; }
Signature · 5 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center | Linux Platform Group
PATCH 9/10 dfu: Add a partition type target
In reply to Pantelis AntoniouDealing with raw block numbers with the dfu is very annoying. Introduce a partition method.
--- drivers/dfu/dfu_mmc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
drivers/dfu/dfu_mmc.c +31 -0
@@ -21,6 +21,7 @@#include <common.h> #include <malloc.h> +#include <errno.h> #include <dfu.h> enum dfu_mmc_op {
@@ -153,6 +154,10 @@ int dfu_read_medium_mmc(struct dfu_entity *dfu, void *buf, long *len)int dfu_fill_entity_mmc (struct dfu_entity *dfu,char *s) { +int dev, part; +struct mmc *mmc; +block_dev_desc_t *blk_dev; +disk_partition_t partinfo;char *st; dfu->dev_type =DFU_DEV_MMC ;
@@ -166,8 +171,34 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s) dfu->layout =DFU_FS_FAT ; }else if (!strcmp (st,"ext4" )) { dfu->layout =DFU_FS_EXT4 ; + }else if (!strcmp (st,"part" )) { + + dfu->layout =DFU_RAW_ADDR ; + + dev =simple_strtoul (s, &s,10 ); + s++; + part =simple_strtoul (s, &s,10 ); + + mmc =find_mmc_device (dev); +if (mmc ==NULL ||mmc_init (mmc)) { +printf ("%s: could not find mmc device #%d!\n" , __func__, dev); +return -ENODEV ; + } + + blk_dev = &mmc->block_dev; +if (get_partition_info (blk_dev, part, &partinfo) !=0 ) { +printf ("%s: could not find partition #%d on mmc device #%d!\n" , + __func__, part, dev); +return -ENODEV ; + } + + dfu->data.mmc.lba_start = partinfo.start; + dfu->data.mmc.lba_size = partinfo.size; + dfu->data.mmc.lba_blk_size = partinfo.blksz; + }else {printf ("%s: Memory layout (%s) not supported!\n" , __func__, st); +return -ENODEV ; }if (dfu->layout ==DFU_FS_EXT4 || dfu->layout ==DFU_FS_FAT ) {
Signature · 1 line
1.7.12
PATCH 10/10 dfu: Support larger than memory transfers.
In reply to Pantelis AntoniouWe didn't support upload/download larger than available memory. This is pretty bad when you have to update your root filesystem for example.
This patch removes the limitation (and the crashes when you transfered any file larger than 4MB).
On top of that reduces the huge dfu buffer from 4MB to just 64K, which was over the top.
--- drivers/dfu/dfu.c | 206 ++++++++++++++++++++++++++++++++++++++------------ drivers/dfu/dfu_mmc.c | 79 ++++++++++++------- include/dfu.h | 19 ++++- 3 files changed, 224 insertions(+), 80 deletions(-)
drivers/dfu/dfu.c +156 -50
@@ -44,90 +44,196 @@ static int dfu_find_alt_num(const char *s)static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE) dfu_buf[DFU_DATA_BUF_SIZE ]; +static int dfu_write_buffer_flush (struct dfu_entity *dfu) +{ +long w_size; +int ret; + +/* flush size? */ + w_size = dfu->i_buf - dfu->i_buf_start; +if (w_size ==0 ) +return 0 ; + +/* update CRC32 */ + dfu->crc =crc32 (dfu->crc, dfu->i_buf_start, w_size); + + ret = dfu->write_medium (dfu, dfu->offset, dfu->i_buf_start, &w_size); +if (ret) +debug ("%s: Write error!\n" , __func__); + +/* point back */ + dfu->i_buf = dfu->i_buf_start; + +/* update offset */ + dfu->offset += w_size; + +puts ("#" ); + +return ret; +} +int dfu_write (struct dfu_entity *dfu,void *buf,int size,int blk_seq_num) { -static unsigned char *i_buf; -static int i_blk_seq_num; -long w_size =0 ;int ret =0 ; +int tret; -debug ("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: 0x%p\n" , - __func__, dfu->name, buf, size, blk_seq_num, i_buf); +debug ("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x " +"offset: 0x%llx bufoffset: 0x%x\n" , + __func__, dfu->name, buf, size, blk_seq_num, dfu->offset, + dfu->i_buf - dfu->i_buf_start);if (blk_seq_num ==0 ) { - i_buf = dfu_buf; - i_blk_seq_num =0 ; +/* initial state */ + dfu->crc =0 ; + dfu->offset =0 ; + dfu->i_blk_seq_num =0 ; + dfu->i_buf_start = dfu_buf; + dfu->i_buf_end = dfu_buf +sizeof (dfu_buf); + dfu->i_buf = dfu->i_buf_start; } -if (i_blk_seq_num++ != blk_seq_num) { +if (dfu->i_blk_seq_num != blk_seq_num) {printf ("%s: Wrong sequence number! [%d] [%d]\n" , - __func__, i_blk_seq_num, blk_seq_num); + __func__, dfu->i_blk_seq_num, blk_seq_num);return -1 ; } + dfu->i_blk_seq_num++; + +/* flush buffer if overflow */ +if ((dfu->i_buf + size) > dfu->i_buf_end) { + tret =dfu_write_buffer_flush (dfu); +if (ret ==0 ) + ret = tret; + } -memcpy (i_buf, buf, size); - i_buf += size; +memcpy (dfu->i_buf, buf, size); + dfu->i_buf += size; +/* if end or if buffer full flush */ +if (size ==0 || (dfu->i_buf + size) > dfu->i_buf_end) { + tret =dfu_write_buffer_flush (dfu); +if (ret ==0 ) + ret = tret; + } + +/* end? */ if (size ==0 ) { -/* Integrity check (if needed) */ -debug ("%s: %s %d [B] CRC32: 0x%x\n" , __func__, dfu->name, - i_buf - dfu_buf,crc32 (0 , dfu_buf, i_buf - dfu_buf)); +debug ("%s: DFU complete CRC32: 0x%x\n" , __func__, dfu->crc); - w_size = i_buf - dfu_buf; - ret = dfu->write_medium (dfu, dfu_buf, &w_size); -if (ret) -debug ("%s: Write error!\n" , __func__); +puts ("\nDownload complete\n" ); - i_blk_seq_num =0 ; - i_buf =NULL ; -return ret; +/* clear everything */ + dfu->crc =0 ; + dfu->offset =0 ; + dfu->i_blk_seq_num =0 ; + dfu->i_buf_start = dfu_buf; + dfu->i_buf_end = dfu_buf +sizeof (dfu_buf); + dfu->i_buf = dfu->i_buf_start; } -return ret; +return ret =0 ? size : ret; +} + +static int dfu_read_buffer_fill (struct dfu_entity *dfu,void *buf,int size) +{ +long chunk; +int ret, readn; + + readn =0 ; +while (size >0 ) { + +/* get chunk that can be read */ + chunk =min (size, dfu->b_left); +/* consume */ +if (chunk >0 ) { +memcpy (buf, dfu->i_buf, chunk); + dfu->crc =crc32 (dfu->crc, buf, chunk); + dfu->i_buf += chunk; + dfu->b_left -= chunk; + size -= chunk; + buf += chunk; + readn += chunk; + } + +/* all done */ +if (size >0 ) { +/* no more to read */ +if (dfu->r_left ==0 ) +break ; + + dfu->i_buf = dfu->i_buf_start; + dfu->b_left = dfu->i_buf_end - dfu->i_buf_start; + +/* got to read, but buffer is empty */ +if (dfu->b_left > dfu->r_left) + dfu->b_left = dfu->r_left; + ret = dfu->read_medium (dfu, dfu->offset, dfu->i_buf, + &dfu->b_left); +if (ret !=0 ) { +debug ("%s: Read error!\n" , __func__); +return ret; + } + dfu->offset += dfu->b_left; + dfu->r_left -= dfu->b_left; + +puts ("#" ); + } + } + +return readn; }int dfu_read (struct dfu_entity *dfu,void *buf,int size,int blk_seq_num) { -static unsigned char *i_buf; -static int i_blk_seq_num; -static long r_size; -static u32 crc;int ret =0 ;debug ("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: 0x%p\n" , - __func__, dfu->name, buf, size, blk_seq_num, i_buf); + __func__, dfu->name, buf, size, blk_seq_num, dfu->i_buf);if (blk_seq_num ==0 ) { - i_buf = dfu_buf; - ret = dfu->read_medium (dfu, i_buf, &r_size); -debug ("%s: %s %ld [B]\n" , __func__, dfu->name, r_size); - i_blk_seq_num =0 ; -/* Integrity check (if needed) */ - crc =crc32 (0 , dfu_buf, r_size); + ret = dfu->read_medium (dfu,0 ,NULL , &dfu->r_left); +if (ret !=0 ) { +debug ("%s: failed to get r_left\n" , __func__); +return ret; + } + +debug ("%s: %s %ld [B]\n" , __func__, dfu->name, dfu->r_left); + + dfu->i_blk_seq_num =0 ; + dfu->crc =0 ; + dfu->offset =0 ; + dfu->i_buf_start = dfu_buf; + dfu->i_buf_end = dfu_buf +sizeof (dfu_buf); + dfu->i_buf = dfu->i_buf_start; + dfu->b_left =0 ; } -if (i_blk_seq_num++ != blk_seq_num) { +if (dfu->i_blk_seq_num != blk_seq_num) {printf ("%s: Wrong sequence number! [%d] [%d]\n" , - __func__, i_blk_seq_num, blk_seq_num); + __func__, dfu->i_blk_seq_num, blk_seq_num);return -1 ; } + dfu->i_blk_seq_num++; -if (r_size >= size) { -memcpy (buf, i_buf, size); - i_buf += size; - r_size -= size; -return size; - }else { -memcpy (buf, i_buf, r_size); - i_buf += r_size; -debug ("%s: %s CRC32: 0x%x\n" , __func__, dfu->name, crc); -puts ("UPLOAD ... done\nCtrl+C to exit ...\n" ); - - i_buf =NULL ; - i_blk_seq_num =0 ; - crc =0 ; -return r_size; + ret =dfu_read_buffer_fill (dfu, buf, size); +if (ret <0 ) { +printf ("%s: Failed to fill buffer\n" , __func__); +return -1 ; } + +if (ret < size) { +debug ("%s: %s CRC32: 0x%x\n" , __func__, dfu->name, dfu->crc); +puts ("\nUPLOAD ... done\nCtrl+C to exit ...\n" ); + + dfu->i_blk_seq_num =0 ; + dfu->crc =0 ; + dfu->offset =0 ; + dfu->i_buf_start = dfu_buf; + dfu->i_buf_end = dfu_buf +sizeof (dfu_buf); + dfu->i_buf = dfu->i_buf_start; + dfu->b_left =0 ; + } +return ret; }
drivers/dfu/dfu_mmc.c +52 -27
168 lines not on this page · raw message
include/dfu.h +16 -3
@@ -59,7 +59,7 @@ static inline unsigned int get_mmc_blk_size(int dev)#define DFU_NAME_SIZE 32#define DFU_CMD_BUF_SIZE 128 -#define DFU_DATA_BUF_SIZE (1024*1024*4)/* 4 MiB */ +#define DFU_DATA_BUF_SIZE (1024*64)/* 64 KB (the huge buffer is overkill) */ struct dfu_entity {char name[DFU_NAME_SIZE ];
@@ -73,10 +73,23 @@ struct dfu_entity {struct mmc_internal_data mmc; } data; -int (*read_medium)(struct dfu_entity *dfu,void *buf,long *len); -int (*write_medium)(struct dfu_entity *dfu,void *buf,long *len); +int (*read_medium)(struct dfu_entity *dfu, +u64 offset,void *buf,long *len); + +int (*write_medium)(struct dfu_entity *dfu, +u64 offset,void *buf,long *len);struct list_head list; + +/* on the fly state */ +u32 crc; +u64 offset; +int i_blk_seq_num; +u8 *i_buf; +u8 *i_buf_start; +u8 *i_buf_end; +long r_left; +long b_left; };int dfu_config_entities (char *s,char *interface,int num);
Signature · 1 line
1.7.12
cover USB: Gadget & DFU related fixes
In reply to Pantelis AntoniouDear Pantelis Antoniou,
Quoted 13 lines
Various bugfixes,
g_dnl& DFU updates.Pantelis Antoniou (10):
usb: Remove obsolete header file
usb: Fix bug when both DFU & ETHER are defined
g_dnl: Issue connect/disconnect as appropriate
g_dnl: Properly terminate string list.
dfu: Only perform DFUboard_usb_init()for TRATS
dfu: Fix crash when wrong number of arguments given
dfu: Send correct DFU response fromcomposite_setup
dfu: Properly zero out timeout value
dfu: Add a partition type target
dfu: Support larger than memory transfers.
[...]
Please address the few issues I pointed out and send a proper V2. Thanks!
Best regards,
Marek Vasut