[U-Boot] [PATCH 00/10] 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(-)
Dear Pantelis Antoniou,
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.
[...]
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
Hi Marek,
On Nov 29, 2012, at 8:32 AM, Marek Vasut wrote:
Dear Pantelis Antoniou,
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.
[...]
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.
Best regards, Marek Vasut
Regards
-- Pantelis
usbdescriptors.h conflicts with linux/usb/ch9.h Remove it.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- drivers/usb/gadget/f_dfu.c | 1 - include/g_dnl.h | 1 - 2 files changed, 2 deletions(-)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 3ec4c65..10547e3 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -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>
diff --git a/include/g_dnl.h b/include/g_dnl.h index 0ec7440..f47395f 100644 --- a/include/g_dnl.h +++ b/include/g_dnl.h @@ -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);
Hi Pantelis,
usbdescriptors.h conflicts with linux/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)
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/f_dfu.c | 1 - include/g_dnl.h | 1 - 2 files changed, 2 deletions(-)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 3ec4c65..10547e3 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -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>
diff --git a/include/g_dnl.h b/include/g_dnl.h index 0ec7440..f47395f 100644 --- a/include/g_dnl.h +++ b/include/g_dnl.h @@ -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);
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/28/12 09:40, Lukasz Majewski wrote:
Hi Pantelis,
usbdescriptors.h conflicts with linux/usb/ch9.h Remove 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
Hi Tom,
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/28/12 09:40, Lukasz Majewski wrote:
Hi Pantelis,
usbdescriptors.h conflicts with linux/usb/ch9.h Remove 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
Hi Tom,
Hi Pantelis,
usbdescriptors.h conflicts with linux/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.
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).
BTW:
- 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
- Please look into the TRATS board (especially the CONFIG_DFU_ALT
constant) for reference.
- What is yours dfu-util version? (Mine is 0.1+svnexported)
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/f_dfu.c | 1 - include/g_dnl.h | 1 - 2 files changed, 2 deletions(-)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 3ec4c65..10547e3 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -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>
diff --git a/include/g_dnl.h b/include/g_dnl.h index 0ec7440..f47395f 100644 --- a/include/g_dnl.h +++ b/include/g_dnl.h @@ -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);
Hi Lukasz,
On Nov 28, 2012, at 6:01 PM, Lukasz Majewski wrote:
Hi Tom,
Hi Pantelis,
usbdescriptors.h conflicts with linux/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.
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.
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.
BTW:
- 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.
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... :(
- Please look into the TRATS board (especially the CONFIG_DFU_ALT
constant) for reference.
Already looked there.
- 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
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/f_dfu.c | 1 - include/g_dnl.h | 1 - 2 files changed, 2 deletions(-)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 3ec4c65..10547e3 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -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>
diff --git a/include/g_dnl.h b/include/g_dnl.h index 0ec7440..f47395f 100644 --- a/include/g_dnl.h +++ b/include/g_dnl.h @@ -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);
-- Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
Hi Pantelis,
Hi Lukasz,
On Nov 28, 2012, at 6:01 PM, Lukasz Majewski wrote:
Hi Tom,
Hi Pantelis,
usbdescriptors.h conflicts with linux/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.
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).
There could be something there that it's missed. I'm in the middle of doing more extensive tests.
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.
BTW:
- 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.
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
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 :-).
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.
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.
- Please look into the TRATS board (especially the CONFIG_DFU_ALT
constant) for reference.
Already looked there.
- 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
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/f_dfu.c | 1 - include/g_dnl.h | 1 - 2 files changed, 2 deletions(-)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 3ec4c65..10547e3 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -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>
diff --git a/include/g_dnl.h b/include/g_dnl.h index 0ec7440..f47395f 100644 --- a/include/g_dnl.h +++ b/include/g_dnl.h @@ -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);
-- Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
Hi Lukasz,
On Nov 28, 2012, at 7:46 PM, Lukasz Majewski wrote:
Hi Pantelis,
Hi Lukasz,
On Nov 28, 2012, at 6:01 PM, Lukasz Majewski wrote:
Hi Tom,
Hi Pantelis,
usbdescriptors.h conflicts with linux/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.
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).
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.
There could be something there that it's missed. I'm in the middle of doing more extensive tests.
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.
BTW:
- 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.
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
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 :-).
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.
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.
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...
- Please look into the TRATS board (especially the CONFIG_DFU_ALT
constant) for reference.
Already looked there.
- 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
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/f_dfu.c | 1 - include/g_dnl.h | 1 - 2 files changed, 2 deletions(-)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 3ec4c65..10547e3 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -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>
diff --git a/include/g_dnl.h b/include/g_dnl.h index 0ec7440..f47395f 100644 --- a/include/g_dnl.h +++ b/include/g_dnl.h @@ -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);
-- Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
-- Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
Hi Pantelis,
Hi Lukasz,
On Nov 28, 2012, at 7:46 PM, Lukasz Majewski wrote:
Hi Pantelis,
Hi Lukasz,
On Nov 28, 2012, at 6:01 PM, Lukasz Majewski wrote:
Hi Tom,
Hi Pantelis,
usbdescriptors.h conflicts with linux/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.
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).
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.
There could be something there that it's missed. I'm in the middle of doing more extensive tests.
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.
BTW:
- 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.
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
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 :-).
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.
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.
We have customers that do want to use DFU for all of their initial programming.
Ok, I see.
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).
BTW, I've just confirmed that larger transfers get corrupted... :( Working on a fix now...
Ok.
- Please look into the TRATS board (especially the
CONFIG_DFU_ALT constant) for reference.
Already looked there.
- 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
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/f_dfu.c | 1 - include/g_dnl.h | 1 - 2 files changed, 2 deletions(-)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 3ec4c65..10547e3 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -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>
diff --git a/include/g_dnl.h b/include/g_dnl.h index 0ec7440..f47395f 100644 --- a/include/g_dnl.h +++ b/include/g_dnl.h @@ -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);
-- Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
-- Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
Hi Lukasz,
On Nov 29, 2012, at 10:13 AM, Lukasz Majewski wrote:
Hi Pantelis,
Hi Lukasz,
On Nov 28, 2012, at 7:46 PM, Lukasz Majewski wrote:
Hi Pantelis,
Hi Lukasz,
On Nov 28, 2012, at 6:01 PM, Lukasz Majewski wrote:
Hi Tom,
Hi Pantelis,
> usbdescriptors.h conflicts with linux/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.
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).
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.
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.
There could be something there that it's missed. I'm in the middle of doing more extensive tests.
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.
BTW:
- 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.
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
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 :-).
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.
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.
We have customers that do want to use DFU for all of their initial programming.
Ok, I see.
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).
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
BTW, I've just confirmed that larger transfers get corrupted... :( Working on a fix now...
Ok.
- Please look into the TRATS board (especially the
CONFIG_DFU_ALT constant) for reference.
Already looked there.
- 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
> > Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com > --- > drivers/usb/gadget/f_dfu.c | 1 - > include/g_dnl.h | 1 - > 2 files changed, 2 deletions(-) > > diff --git a/drivers/usb/gadget/f_dfu.c > b/drivers/usb/gadget/f_dfu.c index 3ec4c65..10547e3 100644 > --- a/drivers/usb/gadget/f_dfu.c > +++ b/drivers/usb/gadget/f_dfu.c > @@ -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> > > diff --git a/include/g_dnl.h b/include/g_dnl.h > index 0ec7440..f47395f 100644 > --- a/include/g_dnl.h > +++ b/include/g_dnl.h > @@ -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);
-- Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
-- Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
-- Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
Hi 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
When 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.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- drivers/usb/gadget/Makefile | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index 040eaba..15206cd 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -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
Dear Pantelis Antoniou,
When 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.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
[...]
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
Call usb_gadget_connect/usb_gadget_disconnect in g_dnl_bind/g_dnl_unbind.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- drivers/usb/gadget/g_dnl.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 7d87050..25da733 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -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:
Well, not terminating the list causes very interesting crashes. As in changing the vendor & product ID crashes. Fun.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- drivers/usb/gadget/g_dnl.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 25da733..a5a4c1f 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -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 = {
Hi Pantelis,
Well, not terminating the list causes very interesting crashes. As in changing the vendor & product ID crashes. Fun.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/g_dnl.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 25da733..a5a4c1f 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -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.
};
static struct usb_gadget_strings g_dnl_string_tab = {
Acked-by: Lukasz Majewski l.majewski@samsung.com
Dear Pantelis Antoniou,
Well, not terminating the list causes very interesting crashes. As in changing the vendor & product ID crashes. Fun.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/g_dnl.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 25da733..a5a4c1f 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -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?
};
static struct usb_gadget_strings g_dnl_string_tab = {
Best regards, Marek Vasut
Hi Marek,
On Nov 29, 2012, at 10:20 AM, Marek Vasut wrote:
Dear Pantelis Antoniou,
Well, not terminating the list causes very interesting crashes. As in changing the vendor & product ID crashes. Fun.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/g_dnl.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 25da733..a5a4c1f 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -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
};
static struct usb_gadget_strings g_dnl_string_tab = {
Best regards, Marek Vasut
Regards
-- Pantelis
Dear Pantelis Antoniou,
Hi Marek,
On Nov 29, 2012, at 10:20 AM, Marek Vasut wrote:
Dear Pantelis Antoniou,
Well, not terminating the list causes very interesting crashes. As in changing the vendor & product ID crashes. Fun.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/g_dnl.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 25da733..a5a4c1f 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -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
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
USB initialization shouldn't happen for all the boards.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- common/cmd_dfu.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 01d6b3a..327c738 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -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())
Hi Pantelis,
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.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
common/cmd_dfu.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 01d6b3a..327c738 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -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.
g_dnl_register(s); while (1) { if (ctrlc())
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/28/12 09:47, Lukasz Majewski wrote:
Hi Pantelis,
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.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- common/cmd_dfu.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 01d6b3a..327c738 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -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.
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
Hi Tom,
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/28/12 09:47, Lukasz Majewski wrote:
Hi Pantelis,
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.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- common/cmd_dfu.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 01d6b3a..327c738 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -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.
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?
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-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/28/12 11:08, Lukasz Majewski wrote:
Hi Tom,
On 11/28/12 09:47, Lukasz Majewski wrote:
Hi Pantelis,
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.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- common/cmd_dfu.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 01d6b3a..327c738 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -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.
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?
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
Hi Tom,
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/28/12 11:08, Lukasz Majewski wrote:
Hi Tom,
On 11/28/12 09:47, Lukasz Majewski wrote:
Hi Pantelis,
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.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- common/cmd_dfu.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 01d6b3a..327c738 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -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.
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?
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".
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?
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/29/12 18:14, Lukasz Majewski wrote:
Hi Tom,
On 11/28/12 11:08, Lukasz Majewski wrote:
Hi Tom,
On 11/28/12 09:47, Lukasz Majewski wrote:
> Hi Pantelis, > >> 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. > > >> Signed-off-by: Pantelis Antoniou >> panto@antoniou-consulting.com --- common/cmd_dfu.c >> | 3 +++ 1 file changed, 3 insertions(+) >> >> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c >> index 01d6b3a..327c738 100644 --- a/common/cmd_dfu.c >> +++ b/common/cmd_dfu.c @@ -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. >
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?
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".
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
On Wed, Nov 28, 2012 at 03:47:43PM +0100, Lukasz Majewski wrote:
Hi Pantelis,
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.
Hi Tom,
On Wed, Nov 28, 2012 at 03:47:43PM +0100, Lukasz Majewski wrote:
Hi Pantelis,
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.
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).
On Mon, Dec 17, 2012 at 12:37 PM, Lukasz Majewski l.majewski@samsung.com wrote:
Hi Tom,
On Wed, Nov 28, 2012 at 03:47:43PM +0100, Lukasz Majewski wrote:
Hi Pantelis,
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.
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).
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
Fix obvious crash when not enough arguments are given to the dfu command.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- common/cmd_dfu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 327c738..83ef324 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -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; }
Hi Pantelis,
Fix obvious crash when not enough arguments are given to the dfu command.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
common/cmd_dfu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 327c738..83ef324 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -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; }
dfu_show_entities(); goto done;}
DFU 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.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- 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(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index ebb5131..6496436 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -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; } diff --git a/drivers/usb/gadget/ep0.c b/drivers/usb/gadget/ep0.c index aa8f916..971d846 100644 --- a/drivers/usb/gadget/ep0.c +++ b/drivers/usb/gadget/ep0.c @@ -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; diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 10547e3..6494f5e 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -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;
Zero out timeout value; letting it filled with undefined values ends up with the dfu host hanging.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- drivers/usb/gadget/f_dfu.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 6494f5e..c15585c 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -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; }
Hi Pantelis,
Zero out timeout value; letting it filled with undefined values ends up with the dfu host hanging.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com
drivers/usb/gadget/f_dfu.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index 6494f5e..c15585c 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -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;
}
Acked-by: Lukasz Majewski l.majewski@samsung.com
Dealing with raw block numbers with the dfu is very annoying. Introduce a partition method.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- drivers/dfu/dfu_mmc.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 5d504df..083d745 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -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) {
We 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.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com --- drivers/dfu/dfu.c | 206 ++++++++++++++++++++++++++++++++++++++------------ drivers/dfu/dfu_mmc.c | 79 ++++++++++++------- include/dfu.h | 19 ++++- 3 files changed, 224 insertions(+), 80 deletions(-)
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index e8477fb..1260c55 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -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; }
diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 083d745..4c8994b 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -22,6 +22,7 @@ #include <common.h> #include <malloc.h> #include <errno.h> +#include <div64.h> #include <dfu.h>
enum dfu_mmc_op { @@ -30,35 +31,48 @@ enum dfu_mmc_op { };
static int mmc_block_op(enum dfu_mmc_op op, struct dfu_entity *dfu, - void *buf, long *len) + u64 offset, void *buf, long *len) { char cmd_buf[DFU_CMD_BUF_SIZE]; + u32 blk_start, blk_count;
- sprintf(cmd_buf, "mmc %s 0x%x %x %x", - op == DFU_OP_READ ? "read" : "write", - (unsigned int) buf, - dfu->data.mmc.lba_start, - dfu->data.mmc.lba_size); - - if (op == DFU_OP_READ) + /* if buf == NULL return total size of the area */ + if (buf == NULL) { *len = dfu->data.mmc.lba_blk_size * dfu->data.mmc.lba_size; + return 0; + } + + blk_start = dfu->data.mmc.lba_start + + (u32)lldiv(offset, dfu->data.mmc.lba_blk_size); + blk_count = *len / dfu->data.mmc.lba_blk_size; + if (blk_start + blk_count > + dfu->data.mmc.lba_start + dfu->data.mmc.lba_size) { + debug("%s: block_op out of bounds\n", __func__); + return -1; + } + + sprintf(cmd_buf, "mmc %s %p %x %x", + op == DFU_OP_READ ? "read" : "write", + buf, blk_start, blk_count);
debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf); return run_command(cmd_buf, 0); }
-static inline int mmc_block_write(struct dfu_entity *dfu, void *buf, long *len) +static inline int mmc_block_write(struct dfu_entity *dfu, + u64 offset, void *buf, long *len) { - return mmc_block_op(DFU_OP_WRITE, dfu, buf, len); + return mmc_block_op(DFU_OP_WRITE, dfu, offset, buf, len); }
-static inline int mmc_block_read(struct dfu_entity *dfu, void *buf, long *len) +static inline int mmc_block_read(struct dfu_entity *dfu, + u64 offset, void *buf, long *len) { - return mmc_block_op(DFU_OP_READ, dfu, buf, len); + return mmc_block_op(DFU_OP_READ, dfu, offset, buf, len); }
static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu, - void *buf, long *len) + u64 offset, void *buf, long *len) { char cmd_buf[DFU_CMD_BUF_SIZE]; char *str_env; @@ -66,12 +80,17 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
switch (dfu->layout) { case DFU_FS_FAT: - sprintf(cmd_buf, "fat%s mmc %d:%d 0x%x %s %lx", + sprintf(cmd_buf, "fat%s mmc %d:%d 0x%x %s %lx %llx", op == DFU_OP_READ ? "load" : "write", dfu->data.mmc.dev, dfu->data.mmc.part, - (unsigned int) buf, dfu->name, *len); + (unsigned int) buf, dfu->name, *len, offset); break; case DFU_FS_EXT4: + if (offset != 0) { + debug("%s: Offset value %llx != 0 not supported!\n", + __func__, offset); + return -1; + } sprintf(cmd_buf, "ext4%s mmc %d:%d /%s 0x%x %ld", op == DFU_OP_READ ? "load" : "write", dfu->data.mmc.dev, dfu->data.mmc.part, @@ -80,6 +99,7 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu, default: printf("%s: Layout (%s) not (yet) supported!\n", __func__, dfu_get_layout(dfu->layout)); + return -1; }
debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf); @@ -102,27 +122,30 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu, return ret; }
-static inline int mmc_file_write(struct dfu_entity *dfu, void *buf, long *len) +static inline int mmc_file_write(struct dfu_entity *dfu, + u64 offset, void *buf, long *len) { - return mmc_file_op(DFU_OP_WRITE, dfu, buf, len); + return mmc_file_op(DFU_OP_WRITE, dfu, offset, buf, len); }
-static inline int mmc_file_read(struct dfu_entity *dfu, void *buf, long *len) +static inline int mmc_file_read(struct dfu_entity *dfu, + u64 offset, void *buf, long *len) { - return mmc_file_op(DFU_OP_READ, dfu, buf, len); + return mmc_file_op(DFU_OP_READ, dfu, offset, buf, len); }
-int dfu_write_medium_mmc(struct dfu_entity *dfu, void *buf, long *len) +int dfu_write_medium_mmc(struct dfu_entity *dfu, + u64 offset, void *buf, long *len) { int ret = -1;
switch (dfu->layout) { case DFU_RAW_ADDR: - ret = mmc_block_write(dfu, buf, len); + ret = mmc_block_write(dfu, offset, buf, len); break; case DFU_FS_FAT: case DFU_FS_EXT4: - ret = mmc_file_write(dfu, buf, len); + ret = mmc_file_write(dfu, offset, buf, len); break; default: printf("%s: Layout (%s) not (yet) supported!\n", __func__, @@ -132,17 +155,17 @@ int dfu_write_medium_mmc(struct dfu_entity *dfu, void *buf, long *len) return ret; }
-int dfu_read_medium_mmc(struct dfu_entity *dfu, void *buf, long *len) +int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf, long *len) { int ret = -1;
switch (dfu->layout) { case DFU_RAW_ADDR: - ret = mmc_block_read(dfu, buf, len); + ret = mmc_block_read(dfu, offset, buf, len); break; case DFU_FS_FAT: case DFU_FS_EXT4: - ret = mmc_file_read(dfu, buf, len); + ret = mmc_file_read(dfu, offset, buf, len); break; default: printf("%s: Layout (%s) not (yet) supported!\n", __func__, @@ -181,13 +204,15 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
mmc = find_mmc_device(dev); if (mmc == NULL || mmc_init(mmc)) { - printf("%s: could not find mmc device #%d!\n", __func__, dev); + 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", + printf("%s: could not find partition #%d " + "on mmc device #%d!\n", __func__, part, dev); return -ENODEV; } diff --git a/include/dfu.h b/include/dfu.h index 5350d79..b662488 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -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);
Dear Pantelis Antoniou,
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.
[...]
Please address the few issues I pointed out and send a proper V2. Thanks!
Best regards, Marek Vasut
participants (6)
-
Lukasz Majewski -
Lukasz Majewski -
Marek Vasut -
Pantelis Antoniou -
Tom Rini -
Tom Rini