dfu: make data buffer size configurable
Dfu transfer uses a buffer before writing data to the raw storage device. Make the size (in bytes) of this buffer configurable.
--- README | 5 +++++ drivers/dfu/dfu.c | 2 +- include/dfu.h | 4 +++-
3 Dateien ge?ndert, 9 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)
README +5 -0
@@ -1360,6 +1360,11 @@ The following options need to be configured: CONFIG_DFU_NAND This enables support for exposing NAND devices via DFU. + CONFIG_SYS_DFU_DATA_BUF_SIZE + Dfu transfer uses a buffer before writing data to the + raw storage device. Make the size (in bytes) of this buffer + configurable. + CONFIG_SYS_DFU_MAX_FILE_SIZE When updating files rather than the raw storage device, we use a static buffer to copy the file into and then write
drivers/dfu/dfu.c +1 -1
@@ -42,7 +42,7 @@ static int dfu_find_alt_num(const char *s) }static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE) - dfu_buf[DFU_DATA_BUF_SIZE ]; + dfu_buf[CONFIG_SYS_DFU_DATA_BUF_SIZE ];static int dfu_write_buffer_drain (struct dfu_entity *dfu) {
include/dfu.h +3 -1
@@ -68,7 +68,9 @@ 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*8)/* 8 MiB */ +#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE +#define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#endif #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE #define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20)/* 4 MiB */ #endif
Signature · 1 line
1.7.11.7
Hi Heiko,
Just thinking out loud here. Can we have an extra option that allocates the buffer dynamically based on an env variable?
Regards
-- Pantelis
Quoted 63 lines · Heiko Schocher
Dfu transfer uses a buffer before writing data to the raw storage device. Make the size (in bytes) of this buffer configurable.
Signed-off-by Heiko Schocher <[email protected]>Cc Pantelis Antoniou <[email protected]>Cc Tom Rini <[email protected]>Cc Lukasz Majewski <[email protected]>Cc Kyungmin Park <[email protected]>Cc Marek Vasut <[email protected]>---
README | 5 +++++ drivers/dfu/dfu.c | 2 +- include/dfu.h | 4 +++-3 Dateien ge?ndert, 9 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)
README +5
-0@@ -1360,6 +1360,11 @@ The following options need to be configured: CONFIG_DFU_NAND This enables support for exposing NAND devices via DFU. + CONFIG_SYS_DFU_DATA_BUF_SIZE + Dfu transfer uses a buffer before writing data to the + raw storage device. Make the size (in bytes) of this buffer + configurable. + CONFIG_SYS_DFU_MAX_FILE_SIZE When updating files rather than the raw storage device, we use a static buffer to copy the file into and then writedrivers/dfu/dfu.c +1
-1@@ -42,7 +42,7 @@ static int dfu_find_alt_num(const char *s) }static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE) - dfu_buf[DFU_DATA_BUF_SIZE ]; + dfu_buf[CONFIG_SYS_DFU_DATA_BUF_SIZE ];static int dfu_write_buffer_drain (struct dfu_entity *dfu) {include/dfu.h +3
-1@@ -68,7 +68,9 @@ 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*8)/* 8 MiB */ +#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE +#define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#endif #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE #define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20)/* 4 MiB */ #endif Signature · 1 line
1.7.11.7
Hello Pantelis,
Am 04.06.2013 12:08, schrieb Pantelis Antoniou:
Quoted 4 lines
Hi Heiko,
Just thinking out loud here. Can we have an extra option that allocates the buffer dynamically based on an env variable?
Hmm.. also a possibility... I have here no preferences ...
Name: "dfu_data_buf_size" if not defined, or env invalid, use default CONFIG_SYS_DFU_DATA_BUF_SIZE size?
But this can be done in a second step, right?
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Heiko,
Quoted 8 lines · Heiko Schocher
Hello Pantelis,
Am 04.06.2013 12:08, schrieb Pantelis Antoniou:
Quoted 4 lines
Hi Heiko,
Just thinking out loud here. Can we have an extra option that allocates the buffer dynamically based on an env variable?
Quoted 7 lines
Hmm.. also a possibility... I have here no preferences ...
Name: "
dfu_data_buf_size" if not defined, or env invalid, use defaultCONFIG_SYS_DFU_DATA_BUF_SIZEsize?But this can be done in a second step, right?
Yes, only as a second step please.
Quoted 5 lines
bye,
HeikoSignature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Regards
-- Pantelis
Dear Pantelis Antoniou,
Quoted 11 lines
Heiko,
Quoted 8 lines · Heiko Schocher
Hello Pantelis,
Am 04.06.2013 12:08, schrieb Pantelis Antoniou:
Quoted 4 lines
Hi Heiko,
Just thinking out loud here. Can we have an extra option that allocates the buffer dynamically based on an env variable?
Quoted 8 lines
Quoted 6 lines
Hmm.. also a possibility... I have here no preferences ...
Name: "
dfu_data_buf_size" if not defined, or env invalid, use defaultCONFIG_SYS_DFU_DATA_BUF_SIZEsize?But this can be done in a second step, right?
Yes, only as a second step please.
Why not do it like that right away?
btw. sorry, I'm playing dead beetle until next week :-(
Best regards,
Marek Vasut
Hello Marek,
Am 09.06.2013 22:01, schrieb Marek Vasut:
Quoted 15 lines
Dear Pantelis Antoniou,
Quoted 13 lines
Heiko,
Quoted 10 lines · Heiko Schocher
Hello Pantelis,
Am 04.06.2013 12:08, schrieb Pantelis Antoniou:
Quoted 4 lines
Hi Heiko,
Just thinking out loud here. Can we have an extra option that allocates the buffer dynamically based on an env variable?
Hmm.. also a possibility... I have here no preferences ...
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Name: "
dfu_data_buf_size" if not defined, or env invalid, use defaultCONFIG_SYS_DFU_DATA_BUF_SIZEsize?But this can be done in a second step, right?
Yes, only as a second step please.
Why not do it like that right away?
Ok, I can change this. Envvar name "dfu_data_buf_size" is ok?
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Hi Heiko,
Quoted 21 lines
Hello Marek,
Am 09.06.2013 22:01, schrieb Marek Vasut:
Quoted 18 lines
Dear Pantelis Antoniou,
Quoted 16 lines
Heiko,
Quoted 13 lines · Heiko Schocher
Hello Pantelis,
Am 04.06.2013 12:08, schrieb Pantelis Antoniou:
Quoted 4 lines
Hi Heiko,
Just thinking out loud here. Can we have an extra option that allocates the buffer dynamically based on an env variable?
Hmm.. also a possibility... I have here no preferences ...
Name: "
dfu_data_buf_size" if not defined, or env invalid, use defaultCONFIG_SYS_DFU_DATA_BUF_SIZEsize?
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
But this can be done in a second step, right?
Yes, only as a second step please.
Why not do it like that right away?
Ok, I can change this. Envvar name "
dfu_data_buf_size" is ok?
For me it's fine.
Quoted 3 lines
bye,
Heiko
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Dear Heiko Schocher,
Quoted 2 lines · In message <[email protected]> you wrote:
Ok, I can change this. Envvar name "
dfu_data_buf_size" is ok?
Such long names are a paint to type. As we can't buffer anything else but data, we should be able to omit this, i. e. what about
dfu_bufsiz
["bufsiz" as used for example as BUFSIZ in C89 stdio.h]
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Fascinating is a word I use for the unexpected. -- Spock, "The Squire of Gothos", stardate 2124.5
Quoted 4 lines · Wolfgang Denk
Dear Heiko Schocher,
Quoted 1 line · In message <[email protected]> you wrote:
Quoted 8 lines
Quoted 1 line
Ok, I can change this. Envvar name "
dfu_data_buf_size" is ok?Such long names are a paint to type. As we can't buffer anything else but data, we should be able to omit this, i. e. what about
dfu_bufsiz["bufsiz" as used for example as BUFSIZ in C89
stdio.h]
Works for me.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130610/5a58b873/attachment.pgp>
Dear Tom Rini,
Quoted 6 lines · Wolfgang Denk
Quoted 5 lines
Dear Heiko Schocher,
Quoted 1 line · In message <[email protected]> you wrote:
Ok, I can change this. Envvar name "
dfu_data_buf_size" is ok?
Quoted 8 lines
Quoted 6 lines
Such long names are a paint to type. As we can't buffer anything else but data, we should be able to omit this, i. e. what about
dfu_bufsiz["bufsiz" as used for example as BUFSIZ in C89
stdio.h]Works for me.
WFM, but I need to read the discussion around here. I just stopped playing dead beetle. Actually, I'm trying to find out why such a variable is needed at all. Can the buffer not be allocated dynamically (and thus dependant only on malloc area size)?
Please ignore this if it's been answered in one of the emails I didn't read yet.
Best regards,
Marek Vasut
Hello Marek,
Am 12.06.2013 10:36, schrieb Marek Vasut:
Quoted 15 lines
Dear Tom Rini,
Quoted 13 lines · Wolfgang Denk
Quoted 11 lines
Dear Heiko Schocher,
Quoted 1 line · In message <[email protected]> you wrote:
Ok, I can change this. Envvar name "
dfu_data_buf_size" is ok?Such long names are a paint to type. As we can't buffer anything else but data, we should be able to omit this, i. e. what about
dfu_bufsiz["bufsiz" as used for example as BUFSIZ in C89
stdio.h]
Quoted 8 lines
Quoted 1 line
Works for me.
WFM, but I need to read the discussion around here. I just stopped playing dead beetle. Actually, I'm trying to find out why such a variable is needed at all. Can the buffer not be allocated dynamically (and thus dependant only on malloc area size)?
Please ignore this if it's been answered in one of the emails I didn't read yet.
posted a v2 for this here:
http://patchwork.ozlabs.org/patch/250665/
which allocates the buffer in the malloc area, dependend on the environemnt variable "dfu_bufsiz".
A reason why we should have at last a config option see here:
http://lists.denx.de/pipermail/u-boot/2013-June/155924.html
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Lukasz Majewski
Acked-byHi Heiko,
Quoted 4 lines
Dfu transfer uses a buffer before writing data to the raw storage device. Make the size (in bytes) of this buffer configurable.
Quoted 57 lines
Signed-off-by Heiko Schocher <[email protected]>Cc Pantelis Antoniou <[email protected]>Cc Tom Rini <[email protected]>Cc Lukasz Majewski <[email protected]>Cc Kyungmin Park <[email protected]>Cc Marek Vasut <[email protected]>--- README | 5 +++++ drivers/dfu/dfu.c | 2 +- include/dfu.h | 4 +++-3 Dateien ge?ndert, 9 Zeilen hinzugef?gt(+), 2 Zeilen entfernt(-)
README +0
-0@@ -1360,6 +1360,11 @@ The following options need to be configured: CONFIG_DFU_NAND This enables support for exposing NAND devices viaDFU.
README +3
-0+ CONFIG_SYS_DFU_DATA_BUF_SIZE + Dfu transfer uses a buffer before writing data to the + raw storage device. Make the size (in bytes) of thisbuffer
README +2
-0+ configurable. + CONFIG_SYS_DFU_MAX_FILE_SIZE When updating files rather than the raw storagedevice, we use a static buffer to copy the file into and then write
drivers/dfu/dfu.c +1
-1@@ -42,7 +42,7 @@ static int dfu_find_alt_num(const char *s) }static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE) - dfu_buf[DFU_DATA_BUF_SIZE ]; + dfu_buf[CONFIG_SYS_DFU_DATA_BUF_SIZE ];static int dfu_write_buffer_drain (struct dfu_entity *dfu) {include/dfu.h +0
-1@@ -68,7 +68,9 @@ 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*8)/* 8 MiB */ +#ifndef
CONFIG_SYS_DFU_DATA_BUF_SIZE
+#defineCONFIG_SYS_DFU_DATA_BUF_SIZE
(1024*1024*8) /* 8 MiB */ +#endif
#ifndefCONFIG_SYS_DFU_MAX_FILE_SIZE
#defineCONFIG_SYS_DFU_MAX_FILE_SIZE(4 << 20) /* 4
MiB */ #endif
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Poland Institute (SRPOL) | Linux Platform Group
Tom Rini
NAKQuoted 3 lines · Heiko Schocher
Dfu transfer uses a buffer before writing data to the raw storage device. Make the size (in bytes) of this buffer configurable.
NAK.
README The following options need to be configured: · line 1370
Quoted 8 lines
README +5
-0+ CONFIG_SYS_DFU_DATA_BUF_SIZE + Dfu transfer uses a buffer before writing data to the + raw storage device. Make the size (in bytes) of this buffer + configurable. + CONFIG_SYS_DFU_MAX_FILE_SIZE When updating files rather than the raw storage device, we use a static buffer to copy the file into and then write
The point of the buffer being configurable is to allow for larger files, right? We need to fix CONFIG_SYS_DFU_MAX_FILE_SIZE so that..
include/dfu.h static inline unsigned int get_mmc_blk_size(int dev) · line 76
Quoted 7 lines
include/dfu.h +3
-1-#define DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE +#define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#endif #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE #define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20)/* 4 MiB */ #endif
We use one variable for both spots. Or is there some case I'm missing where we need to buffer 8MiB at a time for raw writes? In which case we still need to make CONFIG_SYS_DFU_MAX_FILE_SIZE be used :)
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130604/9690fcdf/attachment.pgp>
Hello Tom,
Am 04.06.2013 22:04, schrieb Tom Rini:
Quoted 7 lines · Heiko Schocher
Quoted 4 lines
Dfu transfer uses a buffer before writing data to the raw storage device. Make the size (in bytes) of this buffer configurable.
NAK.
:-(
Quoted 11 lines
README The following options need to be configured: · line 1370
Quoted 8 lines
README +5
-0+ CONFIG_SYS_DFU_DATA_BUF_SIZE + Dfu transfer uses a buffer before writing data to the + raw storage device. Make the size (in bytes) of this buffer + configurable. + CONFIG_SYS_DFU_MAX_FILE_SIZE When updating files rather than the raw storage device, we use a static buffer to copy the file into and then writeThe point of the buffer being configurable is to allow for larger files, right? We need to fix
CONFIG_SYS_DFU_MAX_FILE_SIZEso that..
In current code CONFIG_SYS_DFU_MAX_FILE_SIZE is not used in dfu_nand.c, as if buffer is full, it is immediately flushed to nand. Also default value from CONFIG_SYS_DFU_MAX_FILE_SIZE is smaller (4MiB) as default value of CONFIG_SYS_DFU_DATA_BUF_SIZE (8MiB) ...
I used on my upcoming board port a CONFIG_SYS_DFU_DATA_BUF_SIZE from 1MiB and that worked perfectly, when transferring a file > 200MB. The default value from 8MiB sometimes caused an error on the host:
[]# date;dfu-util -a rootfs -D dxr2-org/dxr2.xx-release-image-UNKNOWN-dxr2.ubi;date
Di 28. Mai 14:20:44 CEST 2013
dfu-util 0.5
[...]
Copying data from PC to DFU device
Starting download: [#############################################dfu_download: libusb_control_transfer returned -7 Error during download
Why we have a buffersize from 8MiB for raw writes, but a max file size from 4MiB only?
Quoted 11 lines
include/dfu.h static inline unsigned int get_mmc_blk_size(int dev) · line 76
Quoted 7 lines
include/dfu.h +3
-1-#define DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE +#define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#endif #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE #define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20)/* 4 MiB */ #endif We use one variable for both spots. Or is there some case I'm missing where we need to buffer 8MiB at a time for raw writes? In which case we still need to make
CONFIG_SYS_DFU_MAX_FILE_SIZEbe used :)
I do not really know, why we have 2 defines here!
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Quoted 25 lines · Heiko Schocher
Hello Tom,
Am 04.06.2013 22:04, schrieb Tom Rini:
Quoted 7 lines · Heiko Schocher
Quoted 4 lines
Dfu transfer uses a buffer before writing data to the raw storage device. Make the size (in bytes) of this buffer configurable.
NAK.
:-(
Quoted 11 lines
README The following options need to be configured: · line 1370
Quoted 8 lines
README +5
-0+ CONFIG_SYS_DFU_DATA_BUF_SIZE + Dfu transfer uses a buffer before writing data to the + raw storage device. Make the size (in bytes) of this buffer + configurable. + CONFIG_SYS_DFU_MAX_FILE_SIZE When updating files rather than the raw storage device, we use a static buffer to copy the file into and then writeThe point of the buffer being configurable is to allow for larger files, right? We need to fix
CONFIG_SYS_DFU_MAX_FILE_SIZEso that..
Quoted 1 line
In current code
CONFIG_SYS_DFU_MAX_FILE_SIZEis not used indfu_nand.c,
Nor anywhere else. As I said in the DFU + UBI thread, there's a bug here :)
Quoted 3 lines
as if buffer is full, it is immediately flushed to nand. Also default value from
CONFIG_SYS_DFU_MAX_FILE_SIZEis smaller (4MiB) as default value ofCONFIG_SYS_DFU_DATA_BUF_SIZE(8MiB) ...
Right, and the commit that did it was about increasing the size of the kernel that could be sent over.
Quoted 12 lines
I used on my upcoming board port a
CONFIG_SYS_DFU_DATA_BUF_SIZEfrom 1MiB and that worked perfectly, when transferring a file > 200MB. The default value from 8MiB sometimes caused an error on the host:[]# date;dfu-util -a rootfs -D dxr2-org/dxr2.xx-release-image-UNKNOWN-dxr2.ubi;date
Di 28. Mai 14:20:44 CEST 2013
dfu-util 0.5
[...]
Copying data from PC to DFU device
Starting download: [#############################################dfu_download:libusb_control_transferreturned -7 Error during download
Quoted 2 lines
Why we have a buffersize from 8MiB for raw writes, but a max file size from 4MiB only?
Then we need to poke around the code here a bit more and see what's going on, and fix things so that we can both do larger (say, 8MiB) filesystem transfers and not have dfu-util get mad sometimes.
Quoted 12 lines
Quoted 11 lines
include/dfu.h static inline unsigned int get_mmc_blk_size(int dev) · line 76
Quoted 7 lines
include/dfu.h +3
-1-#define DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE +#define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#endif #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE #define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20)/* 4 MiB */ #endif We use one variable for both spots. Or is there some case I'm missing where we need to buffer 8MiB at a time for raw writes? In which case we still need to make
CONFIG_SYS_DFU_MAX_FILE_SIZEbe used :)
Quoted 1 line
I do not really know, why we have 2 defines here!
File size vs buffer size? I'm not quite certain it was the right way to go either.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130605/bc2e7e74/attachment.pgp>
Hello Tom,
Am 05.06.2013 14:43, schrieb Tom Rini:
Quoted 5 lines · Heiko Schocher
Quoted 4 lines
Hello Tom,
Am 04.06.2013 22:04, schrieb Tom Rini:
Quoted 1 line · Heiko Schocher
[...]
Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
README The following options need to be configured: · line 1370
Quoted 8 lines
README +5
-0+ CONFIG_SYS_DFU_DATA_BUF_SIZE + Dfu transfer uses a buffer before writing data to the + raw storage device. Make the size (in bytes) of this buffer + configurable. + CONFIG_SYS_DFU_MAX_FILE_SIZE When updating files rather than the raw storage device, we use a static buffer to copy the file into and then writeThe point of the buffer being configurable is to allow for larger files, right? We need to fix
CONFIG_SYS_DFU_MAX_FILE_SIZEso that..
Quoted 4 lines
Quoted 1 line
In current code
CONFIG_SYS_DFU_MAX_FILE_SIZEis not used indfu_nand.c,Nor anywhere else. As I said in the DFU + UBI thread, there's a bug here :)
CONFIG_SYS_DFU_MAX_FILE_SIZE is used in ./drivers/dfu/dfu_mmc.c ...
Quoted 6 lines
Quoted 3 lines
as if buffer is full, it is immediately flushed to nand. Also default value from
CONFIG_SYS_DFU_MAX_FILE_SIZEis smaller (4MiB) as default value ofCONFIG_SYS_DFU_DATA_BUF_SIZE(8MiB) ...Right, and the commit that did it was about increasing the size of the kernel that could be sent over.
Hmm.. the CONFIG_SYS_DFU_DATA_BUF_SIZE limits not the size of a file that could be loaded into a partition. It specifies only the size of one chunk, that get burned into the raw nand ...
And this should be a configurable size ...
Quoted 12 lines
Quoted 12 lines
I used on my upcoming board port a
CONFIG_SYS_DFU_DATA_BUF_SIZEfrom 1MiB and that worked perfectly, when transferring a file > 200MB. The default value from 8MiB sometimes caused an error on the host:[]# date;dfu-util -a rootfs -D dxr2-org/dxr2.xx-release-image-UNKNOWN-dxr2.ubi;date
Di 28. Mai 14:20:44 CEST 2013
dfu-util 0.5
[...]
Copying data from PC to DFU device
Starting download: [#############################################dfu_download:libusb_control_transferreturned -7 Error during download
Quoted 6 lines
Quoted 2 lines
Why we have a buffersize from 8MiB for raw writes, but a max file size from 4MiB only?
Then we need to poke around the code here a bit more and see what's going on, and fix things so that we can both do larger (say, 8MiB) filesystem transfers and not have dfu-util get mad sometimes.
Timeout in libusb_control_transfer while the target writes the 8MiB into the nand ... ?
I try to find out something ...
Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
include/dfu.h static inline unsigned int get_mmc_blk_size(int dev) · line 76
Quoted 7 lines
include/dfu.h +3
-1-#define DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE +#define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#endif #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE #define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20)/* 4 MiB */ #endif We use one variable for both spots. Or is there some case I'm missing where we need to buffer 8MiB at a time for raw writes? In which case we still need to make
CONFIG_SYS_DFU_MAX_FILE_SIZEbe used :)
Quoted 4 lines
Quoted 1 line
I do not really know, why we have 2 defines here!
File size vs buffer size? I'm not quite certain it was the right way to go either.
Yeah, but why is the file size < buffer size as default?
In dfu_mmc:
If raw partition, if dfu_buf (size of CONFIG_SYS_DFU_DATA_BUF_SIZE) full -> write it to the mmc. Same for nand.
If FAT or EXT4 partition (mmc only), write the dfu_buffer through mmc_file_buffer() to dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE] ... this seems buggy to me, but maybe I oversee something, I could not try it ... and if the hole file is transfered, the dfu_file_buf gets flushed to the partition ...
The CONFIG_SYS_DFU_MAX_FILE_SIZE is only needed as we can only write a complete image to FAT, EXT4 (also UBI) partitions, I think.
So we have in the dfu subsystem following problems:
a) we have no common API to add image types. Currently all dfu_{device_type} has to program it.
b) we have no possibility to write image types (FAT, EXT4 or UBI) in chunks -> CONFIG_SYS_DFU_MAX_FILE_SIZE introduced
c) CONFIG_SYS_DFU_DATA_BUF_SIZE > CONFIG_SYS_DFU_MAX_FILE_SIZE which is in my eyes buggy ...
d) sporadic problems with CONFIG_SYS_DFU_DATA_BUF_SIZE = 8MiB Currently i get always an error ... try to find out why ...
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
On Wed, Jun 05, 2013 at 04:04:46PM +0200, Heiko Schocher wrote:
[snip]
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
In current code
CONFIG_SYS_DFU_MAX_FILE_SIZEis not used indfu_nand.c,Nor anywhere else. As I said in the DFU + UBI thread, there's a bug here :)
CONFIG_SYS_DFU_MAX_FILE_SIZEis used in./drivers/dfu/dfu_mmc.c...
Ah yes, right.
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
as if buffer is full, it is immediately flushed to nand. Also default value from
CONFIG_SYS_DFU_MAX_FILE_SIZEis smaller (4MiB) as default value ofCONFIG_SYS_DFU_DATA_BUF_SIZE(8MiB) ...
Quoted 8 lines
Quoted 2 lines
Right, and the commit that did it was about increasing the size of the kernel that could be sent over.
Hmm.. the
CONFIG_SYS_DFU_DATA_BUF_SIZElimits not the size of a file that could be loaded into a partition. It specifies only the size of one chunk, that get burned into the raw nand ...And this should be a configurable size ...
Or a saner, smaller size? I think we've got a few things being done in a confusing and/or incorrect manner, see below.
Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
I used on my upcoming board port a
CONFIG_SYS_DFU_DATA_BUF_SIZEfrom 1MiB and that worked perfectly, when transferring a file > 200MB. The default value from 8MiB sometimes caused an error on the host:[]# date;dfu-util -a rootfs -D dxr2-org/dxr2.xx-release-image-UNKNOWN-dxr2.ubi;date
Di 28. Mai 14:20:44 CEST 2013
dfu-util 0.5
[...]
Copying data from PC to DFU device
Starting download: [#############################################dfu_download:libusb_control_transferreturned -7 Error during downloadWhy we have a buffersize from 8MiB for raw writes, but a max file size from 4MiB only?
Quoted 8 lines
Quoted 3 lines
Then we need to poke around the code here a bit more and see what's going on, and fix things so that we can both do larger (say, 8MiB) filesystem transfers and not have dfu-util get mad sometimes.
Timeout in
libusb_control_transferwhile the target writes the 8MiB into the nand ... ?I try to find out something ...
Well, it ought to be fine, but we're pushing some boundaries here, and I don't know if we have a good reason for it. We aren't changing the size of the chunks being passed along.
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
include/dfu.h static inline unsigned int get_mmc_blk_size(int dev) · line 76
Quoted 7 lines
include/dfu.h +3
-1-#define DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE +#define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#endif #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE #define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20)/* 4 MiB */ #endif We use one variable for both spots. Or is there some case I'm missing where we need to buffer 8MiB at a time for raw writes? In which case we still need to make
CONFIG_SYS_DFU_MAX_FILE_SIZEbe used :)
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
I do not really know, why we have 2 defines here!
File size vs buffer size? I'm not quite certain it was the right way to go either.
Yeah, but why is the file size < buffer size as default?
A bug, I'm pretty sure. The change that made buffer > file was with the comment to allow for bigger files. I think it might not have been completely tested :)
Quoted 3 lines
In
dfu_mmc:
If raw partition, ifdfu_buf(size ofCONFIG_SYS_DFU_DATA_BUF_SIZE) full -> write it to the mmc. Same for nand.
Right. Since we want to buffer up some amount of data, flush, repeat until done.
Quoted 5 lines
If FAT or EXT4 partition (mmc only), write the
dfu_bufferthroughmmc_file_buffer()to dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE] ... this seems buggy to me, but maybe I oversee something, I could not try it ... and if the hole file is transfered, thedfu_file_bufgets flushed to the partition ...
The need here is that since we can't append to files, transfer the whole file, then write. We will not be told the filesize ahead of time, so we have to transfer up to the buffer and if we would exceed, error out at that point, otherwise continue. Now I'm pretty sure, but not 100% sure that there's a reason we can't use dfu_buf in both places. That needs re-checking.
Quoted 2 lines
The
CONFIG_SYS_DFU_MAX_FILE_SIZEis only needed as we can only write a complete image to FAT, EXT4 (also UBI) partitions, I think.
It'll be needed for any file write, rather than block write. The question is, can it ever be valid for MAX_FILE_SIZE to be smaller than BUF_SIZE ? Maybe.
Quoted 4 lines
So we have in the dfu subsystem following problems:
a) we have no common API to add image types. Currently all dfu_{device_type} has to program it.
We also have no common image types.
Quoted 2 lines
b) we have no possibility to write image types (FAT, EXT4 or UBI) in chunks ->
CONFIG_SYS_DFU_MAX_FILE_SIZEintroduced
Correct.
Quoted 2 lines
c)
CONFIG_SYS_DFU_DATA_BUF_SIZE>CONFIG_SYS_DFU_MAX_FILE_SIZEwhich is in my eyes buggy ...
Or at least very odd, given the current defaults for both.
Quoted 2 lines
d) sporadic problems with
CONFIG_SYS_DFU_DATA_BUF_SIZE= 8MiB Currently i get always an error ... try to find out why ...
Maybe we don't need to go so large for the buffer.
If you've got a beaglebone (and I know there's one in denx :)) or am335x gp evm, you can test filesystem writes on SD cards with DFU.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130606/9af3e6b5/attachment.pgp>
Hello Tom,
Am 06.06.2013 17:55, schrieb Tom Rini:
Quoted 19 lines · Heiko Schocher
[snip]
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
In current code
CONFIG_SYS_DFU_MAX_FILE_SIZEis not used indfu_nand.c,Nor anywhere else. As I said in the DFU + UBI thread, there's a bug here :)
CONFIG_SYS_DFU_MAX_FILE_SIZEis used in./drivers/dfu/dfu_mmc.c...Ah yes, right.
Quoted 7 lines
Quoted 6 lines
Quoted 3 lines
as if buffer is full, it is immediately flushed to nand. Also default value from
CONFIG_SYS_DFU_MAX_FILE_SIZEis smaller (4MiB) as default value ofCONFIG_SYS_DFU_DATA_BUF_SIZE(8MiB) ...Right, and the commit that did it was about increasing the size of the kernel that could be sent over.
Quoted 8 lines
Quoted 5 lines
Hmm.. the
CONFIG_SYS_DFU_DATA_BUF_SIZElimits not the size of a file that could be loaded into a partition. It specifies only the size of one chunk, that get burned into the raw nand ...And this should be a configurable size ...
Or a saner, smaller size? I think we've got a few things being done in a confusing and/or incorrect manner, see below.
Yes, smaller size is maybe the way to go ... see below
Quoted 19 lines
Quoted 19 lines
Quoted 18 lines
Quoted 12 lines
I used on my upcoming board port a
CONFIG_SYS_DFU_DATA_BUF_SIZEfrom 1MiB and that worked perfectly, when transferring a file > 200MB. The default value from 8MiB sometimes caused an error on the host:[]# date;dfu-util -a rootfs -D dxr2-org/dxr2.xx-release-image-UNKNOWN-dxr2.ubi;date
Di 28. Mai 14:20:44 CEST 2013
dfu-util 0.5
[...]
Copying data from PC to DFU device
Starting download: [#############################################dfu_download:libusb_control_transferreturned -7 Error during downloadQuoted 2 lines
Why we have a buffersize from 8MiB for raw writes, but a max file size from 4MiB only?
Then we need to poke around the code here a bit more and see what's going on, and fix things so that we can both do larger (say, 8MiB) filesystem transfers and not have dfu-util get mad sometimes.
Quoted 8 lines
Quoted 4 lines
Timeout in
libusb_control_transferwhile the target writes the 8MiB into the nand ... ?I try to find out something ...
Well, it ought to be fine, but we're pushing some boundaries here, and I don't know if we have a good reason for it. We aren't changing the size of the chunks being passed along.
My suspicion that the problem is a timeout was the right idea!
I tried following patch in the current dfu-util sources:
(git clone git://gitorious.org/dfu-util/dfu-util.git
current head: f66634406e9397e67c34033c3c0c26dde486528c)
src/main.c +2 -1
@@ -290,7 +290,8 @@ int main(int argc, char **argv)exit (0 ); } -dfu_init (5000 ); +printf ("Using 20 sec timeout\n" ); +dfu_init (20000 ); num_devs =count_dfu_devices (ctx, dif);if (num_devs ==0 ) {
and I see no longer the above error! So I see two solutions for my problem:
- make DFU_DATA_BUF_SIZE in U-Boot smaller or configurable
- make the timeout in dfu-util bigger or configurable
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
Quoted 11 lines
Review
Quoted 7 lines
(quoted diff) +3
-1-#define DFU_DATA_BUF_SIZE (1024*1024*8) /* 8 MiB */ +#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE +#define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8) /* 8 MiB */ +#endif #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE #define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20) /* 4 MiB */ #endifWe use one variable for both spots. Or is there some case I'm missing where we need to buffer 8MiB at a time for raw writes? In which case we still need to make
CONFIG_SYS_DFU_MAX_FILE_SIZEbe used :)I do not really know, why we have 2 defines here!
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
File size vs buffer size? I'm not quite certain it was the right way to go either.
Yeah, but why is the file size < buffer size as default?
A bug, I'm pretty sure. The change that made buffer > file was with the comment to allow for bigger files. I think it might not have been completely tested :)
Maybe. I don't know.
Quoted 6 lines
Quoted 3 lines
In
dfu_mmc:
If raw partition, ifdfu_buf(size ofCONFIG_SYS_DFU_DATA_BUF_SIZE) full -> write it to the mmc. Same for nand.Right. Since we want to buffer up some amount of data, flush, repeat until done.
Yep.
Quoted 6 lines
Quoted 5 lines
If FAT or EXT4 partition (mmc only), write the
dfu_bufferthroughmmc_file_buffer()to dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE] ... this seems buggy to me, but maybe I oversee something, I could not try it ... and if the hole file is transfered, thedfu_file_bufgets flushed to the partition ...
Quoted 6 lines
The need here is that since we can't append to files, transfer the whole file, then write. We will not be told the filesize ahead of time, so we have to transfer up to the buffer and if we would exceed, error out at that point, otherwise continue. Now I'm pretty sure, but not 100% sure that there's a reason we can't use
dfu_bufin both places. That needs re-checking.
Ok.
Quoted 6 lines
Quoted 2 lines
The
CONFIG_SYS_DFU_MAX_FILE_SIZEis only needed as we can only write a complete image to FAT, EXT4 (also UBI) partitions, I think.It'll be needed for any file write, rather than block write. The question is, can it ever be valid for
MAX_FILE_SIZEto be smaller thanBUF_SIZE? Maybe.
I would say no ...
Quoted 6 lines
Quoted 4 lines
So we have in the dfu subsystem following problems:
a) we have no common API to add image types. Currently all dfu_{device_type} has to program it.
We also have no common image types.
Then we should introduce it ... oh, maybe we have them:drivers/dfu/dfu_mmc.c in dfu_write_medium_mmc()i
switch(dfu->layout)DFU_RAW_ADDR:DFU_FS_FAT:DFU_FS_EXT4:
?
Quoted 8 lines
Quoted 2 lines
b) we have no possibility to write image types (FAT, EXT4 or UBI) in chunks ->
CONFIG_SYS_DFU_MAX_FILE_SIZEintroducedCorrect.
Quoted 2 lines
c)
CONFIG_SYS_DFU_DATA_BUF_SIZE>CONFIG_SYS_DFU_MAX_FILE_SIZEwhich is in my eyes buggy ...
Quoted 6 lines
Or at least very odd, given the current defaults for both.
Quoted 2 lines
d) sporadic problems with
CONFIG_SYS_DFU_DATA_BUF_SIZE= 8MiB Currently i get always an error ... try to find out why ...Maybe we don't need to go so large for the buffer.
Yep, with 1 or 2 MiB I see no problems with current dfu-util.
Quoted 2 lines
If you've got a beaglebone (and I know there's one in denx :)) or am335x gp evm, you can test filesystem writes on SD cards with DFU.
Ah! Ok.
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Dear Heiko,
Quoted 6 lines · In message <[email protected]> you wrote:
and I see no longer the above error! So I see two solutions for my problem:
- make
DFU_DATA_BUF_SIZEin U-Boot smaller or configurable
- make the timeout in dfu-util bigger or configurable
I think we have just learned that the one-size-fits-all approach does not work for everybody. Actually both parameters should be configurable.
Best regards,
Wolfgang Denk
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Artificial Intelligence is no match for natural stupidity.
PATCH v2 1/1 dfu: make data buffer size configurable
In reply to Heiko SchocherDfu transfer uses a buffer before writing data to the raw storage device. Make the size (in bytes) of this buffer configurable through environment variable "dfu_bufsiz". Defaut value is configurable through CONFIG_SYS_DFU_DATA_BUF_SIZE
---
- changes for v2:
make the dfu_buf size configurable through the environment variable "dfu_bufsiz".
README | 6 ++++++ drivers/dfu/dfu.c | 49 +++++++++++++++++++++++++++++++++++++++++-------- include/dfu.h | 4 +++-
3 Dateien ge?ndert, 50 Zeilen hinzugef?gt(+), 9 Zeilen entfernt(-)
README +6 -0
@@ -1376,6 +1376,12 @@ The following options need to be configured: CONFIG_DFU_NAND This enables support for exposing NAND devices via DFU. + CONFIG_SYS_DFU_DATA_BUF_SIZE + Dfu transfer uses a buffer before writing data to the + raw storage device. Make the size (in bytes) of this buffer + configurable. The size of this buffer is also configurable + through the "dfu_bufsiz" environment variable. + CONFIG_SYS_DFU_MAX_FILE_SIZE When updating files rather than the raw storage device, we use a static buffer to copy the file into and then write
drivers/dfu/dfu.c +41 -8
93 lines not on this page · raw message
include/dfu.h +3 -1
@@ -68,7 +68,9 @@ 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*8)/* 8 MiB */ +#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE +#define CONFIG_SYS_DFU_DATA_BUF_SIZE (1024*1024*8)/* 8 MiB */ +#endif #ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE #define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20)/* 4 MiB */ #endif
Signature · 1 line
1.7.11.7
PATCH v2 1/1 dfu: make data buffer size configurable
Quoted 5 lines · Heiko Schocher
Dfu transfer uses a buffer before writing data to the raw storage device. Make the size (in bytes) of this buffer configurable through environment variable "
dfu_bufsiz". Defaut value is configurable throughCONFIG_SYS_DFU_DATA_BUF_SIZE
Quoted 7 lines
Signed-off-by Heiko Schocher <[email protected]>Cc Pantelis Antoniou <[email protected]>Cc Tom Rini <[email protected]>Cc Lukasz Majewski <[email protected]>Cc Kyungmin Park <[email protected]>Cc Marek Vasut <[email protected]>Cc Wolfgang Denk <[email protected]>
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130612/900ccb73/attachment.pgp>
Marek Vasut
AppliedPATCH v2 1/1 dfu: make data buffer size configurable
In reply to Heiko SchocherDear Heiko Schocher,
Quoted 5 lines
Dfu transfer uses a buffer before writing data to the raw storage device. Make the size (in bytes) of this buffer configurable through environment variable "
dfu_bufsiz". Defaut value is configurable throughCONFIG_SYS_DFU_DATA_BUF_SIZE
Quoted 7 lines
Signed-off-by Heiko Schocher <[email protected]>Cc Pantelis Antoniou <[email protected]>Cc Tom Rini <[email protected]>Cc Lukasz Majewski <[email protected]>Cc Kyungmin Park <[email protected]>Cc Marek Vasut <[email protected]>Cc Wolfgang Denk <[email protected]>
Applied, thanks!
Best regards,
Marek Vasut