[U-Boot] [RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer

[U-Boot] [RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer

[Bo Shen]

The value of dfu->r_left need decrease along with the transfer

Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
 drivers/dfu/dfu.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 56b21c7..65c6984 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -229,6 +229,7 @@ static int dfu_read_buffer_fill(struct dfu_entity *dfu, void *buf, int size)
 			dfu->crc = crc32(dfu->crc, buf, chunk);
 			dfu->i_buf += chunk;
 			dfu->b_left -= chunk;
+			dfu->r_left -= chunk;
 			size -= chunk;
 			buf += chunk;
 			readn += chunk;
-- 
1.7.9.5

[U-Boot] [RFC PATCH 2/2] usb: dfu: correct dfu buffer inited value

[Bo Shen]

After dfu buffer is initialized, the buffer should be all available,
while not 0. Initialize its value to min(dfu_buf_size, dfu->r_left).

Signed-off-by: Bo Shen <voice.shen@atmel.com>

---
 drivers/dfu/dfu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index 65c6984..b8c8aa4 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -288,7 +288,7 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
 		dfu->offset = 0;
 		dfu->i_buf_end = dfu_get_buf() + dfu_buf_size;
 		dfu->i_buf = dfu->i_buf_start;
-		dfu->b_left = 0;
+		dfu->b_left = min(dfu_buf_size, dfu->r_left);
 
 		dfu->bad_skip = 0;
 
-- 
1.7.9.5

[U-Boot] [RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer

[Marek Vasut]

Hi,

> The value of dfu->r_left need decrease along with the transfer
> 
> Signed-off-by: Bo Shen <voice.shen@atmel.com>

Lucasz/Heiko , can you ACK/NAK this and 2/2 ? Thanks!

> ---
>  drivers/dfu/dfu.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 56b21c7..65c6984 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -229,6 +229,7 @@ static int dfu_read_buffer_fill(struct dfu_entity *dfu,
> void *buf, int size) dfu->crc = crc32(dfu->crc, buf, chunk);
>  			dfu->i_buf += chunk;
>  			dfu->b_left -= chunk;
> +			dfu->r_left -= chunk;
>  			size -= chunk;
>  			buf += chunk;
>  			readn += chunk;

Best regards,
Marek Vasut

[U-Boot] [RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer

[Lukasz Majewski]

Hi Marek,

> Hi,
> 
> > The value of dfu->r_left need decrease along with the transfer
> > 
> > Signed-off-by: Bo Shen <voice.shen@atmel.com>
> 
> Lucasz/Heiko , can you ACK/NAK this and 2/2 ? Thanks!

For me it looks like a more difficult problem. 
I've got an impression that patches proposed by Bo would break eMMC.

Unfortunately I'm now a bit short of time to go deeper to it. Before
ELCE I shall have got a bit more time.

> 
> > ---
> >  drivers/dfu/dfu.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> > index 56b21c7..65c6984 100644
> > --- a/drivers/dfu/dfu.c
> > +++ b/drivers/dfu/dfu.c
> > @@ -229,6 +229,7 @@ static int dfu_read_buffer_fill(struct
> > dfu_entity *dfu, void *buf, int size) dfu->crc = crc32(dfu->crc,
> > buf, chunk); dfu->i_buf += chunk;
> >  			dfu->b_left -= chunk;
> > +			dfu->r_left -= chunk;
> >  			size -= chunk;
> >  			buf += chunk;
> >  			readn += chunk;
> 
> Best regards,
> Marek Vasut

Regards,
Lukasz
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20131022/7623890f/attachment.pgp>

[U-Boot] [RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer

[Marek Vasut]

Hi Lukasz,

> Hi Marek,
> 
> > Hi,
> > 
> > > The value of dfu->r_left need decrease along with the transfer
> > > 
> > > Signed-off-by: Bo Shen <voice.shen@atmel.com>
> > 
> > Lucasz/Heiko , can you ACK/NAK this and 2/2 ? Thanks!
> 
> For me it looks like a more difficult problem.
> I've got an impression that patches proposed by Bo would break eMMC.
> 
> Unfortunately I'm now a bit short of time to go deeper to it. Before
> ELCE I shall have got a bit more time.

OK, thanks!

Best regards,
Marek Vasut

[U-Boot] [RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer

[Bo Shen]

Hi Lukasz,

On 10/22/2013 10:42, Marek Vasut wrote:
> Hi Lukasz,
>
>> Hi Marek,
>>
>>> Hi,
>>>
>>>> The value of dfu->r_left need decrease along with the transfer
>>>>
>>>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>>>
>>> Lucasz/Heiko , can you ACK/NAK this and 2/2 ? Thanks!
>>
>> For me it looks like a more difficult problem.
>> I've got an impression that patches proposed by Bo would break eMMC.
>>
>> Unfortunately I'm now a bit short of time to go deeper to it. Before
>> ELCE I shall have got a bit more time.

Would it be possible for you to check these two RFC patch?

> OK, thanks!
>
> Best regards,
> Marek Vasut
>

Best Regards,
Bo Shen

[U-Boot] [RFC PATCH 2/2] usb: dfu: correct dfu buffer inited value

[Lukasz Majewski]

Hi Bo,

> After dfu buffer is initialized, the buffer should be all available,
> while not 0. Initialize its value to min(dfu_buf_size, dfu->r_left).
> 
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> 
> ---
>  drivers/dfu/dfu.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 65c6984..b8c8aa4 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -288,7 +288,7 @@ int dfu_read(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) dfu->offset = 0;
>  		dfu->i_buf_end = dfu_get_buf() + dfu_buf_size;
>  		dfu->i_buf = dfu->i_buf_start;
> -		dfu->b_left = 0;
> +		dfu->b_left = min(dfu_buf_size, dfu->r_left);
>  

I've testd in on Trats. It causes dfu read to be performed two times.

Could you write a more verbose message to explain the problem that you
are trying to solve? I can _only_ suppose that you want to read/write
data from/to NAND memory.

So, I'm curious why dfu-util breaks with your setup but works at am335x.
Both chips are supposed to use dfu_nand.c for performing NAND
read/write.


>  		dfu->bad_skip = 0;
>  



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] [RFC PATCH 2/2] usb: dfu: correct dfu buffer inited value

[Bo Shen]

Hi Lukasz,

On 11/4/2013 18:17, Lukasz Majewski wrote:
> Hi Bo,
>
>> After dfu buffer is initialized, the buffer should be all available,
>> while not 0. Initialize its value to min(dfu_buf_size, dfu->r_left).
>>
>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>>
>> ---
>>   drivers/dfu/dfu.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
>> index 65c6984..b8c8aa4 100644
>> --- a/drivers/dfu/dfu.c
>> +++ b/drivers/dfu/dfu.c
>> @@ -288,7 +288,7 @@ int dfu_read(struct dfu_entity *dfu, void *buf,
>> int size, int blk_seq_num) dfu->offset = 0;
>>   		dfu->i_buf_end = dfu_get_buf() + dfu_buf_size;
>>   		dfu->i_buf = dfu->i_buf_start;
>> -		dfu->b_left = 0;
>> +		dfu->b_left = min(dfu_buf_size, dfu->r_left);
>>
>
> I've testd in on Trats. It causes dfu read to be performed two times.

Have you apply these two patches together:
[RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer
[RFC PATCH 2/2] usb: dfu: correct dfu buffer inited value

I test this with dfu mmc.

Without these two patches, it will read file two times.
--->8---
U-Boot> dfu mmc 0
GADGET DRIVER: usb_dnl_dfu
reading image.bin
3049120 bytes read in 628 ms (4.6 MiB/s)
reading image.bin
3049120 bytes read in 628 ms (4.6 MiB/s)
---8<---

with these two patches, it only read file one time.
--->8---
U-Boot> dfu mmc 0
GADGET DRIVER: usb_dnl_dfu
reading image.bin
3049120 bytes read in 628 ms (4.6 MiB/s)
---8<---

The result is opposite.

> Could you write a more verbose message to explain the problem that you
> are trying to solve? I can _only_ suppose that you want to read/write
> data from/to NAND memory.
>
> So, I'm curious why dfu-util breaks with your setup but works at am335x.
> Both chips are supposed to use dfu_nand.c for performing NAND
> read/write.

For the NAND upload, if without patch: [U-Boot,RFC] usb: dfu: make nand 
upload working (http://patchwork.ozlabs.org/patch/283886/)
It doesn't work at my side. more information as following:
--->8---
$ ./dfu-util -l
dfu-util 0.7

Copyright 2005-2008 Weston Schmidt, Harald Welte and OpenMoko Inc.
Copyright 2010-2012 Tormod Volden and Stefan Schmidt
This program is Free Software and has ABSOLUTELY NO WARRANTY
Please report bugs to dfu-util at lists.gnumonks.org

Found Runtime: [413c:8187] devnum=0, cfg=1, intf=3, alt=0, name="UNDEFINED"
Found DFU: [03eb:6156] devnum=0, cfg=1, intf=0, alt=0, name="sama5d34ek.dtb"
Found DFU: [03eb:6156] devnum=0, cfg=1, intf=0, alt=1, name="uImage"
Found DFU: [03eb:6156] devnum=0, cfg=1, intf=0, alt=2, name="rootfs.ubi"

$ ./dfu-util -d 03eb:6156 -U kernel.image -a 1
dfu-util 0.7

Copyright 2005-2008 Weston Schmidt, Harald Welte and OpenMoko Inc.
Copyright 2010-2012 Tormod Volden and Stefan Schmidt
This program is Free Software and has ABSOLUTELY NO WARRANTY
Please report bugs to dfu-util at lists.gnumonks.org

Filter on vendor = 0x03eb product = 0x6156
Opening DFU capable USB device... ID 03eb:6156
WARNING: Can not find cached DFU functional descriptor
Warning: Assuming DFU version 1.0
Run-time device DFU version 0100
Found DFU: [03eb:6156] devnum=0, cfg=1, intf=0, alt=1, name="uImage"
Claiming USB DFU Interface...
Setting Alternate Setting #1 ...
Determining device status: state = dfuUPLOAD-IDLE, status = 0
aborting previous incomplete transfer
Determining device status: state = dfuIDLE, status = 0
dfuIDLE, continuing
Error obtaining cached DFU functional descriptor
DFU mode device DFU version 0110
Device returned transfer size 4096
bytes_per_hash=4096
Copying data from DFU device to PC
Starting upload: [] finished!
---8<---

It doesn't transfer anything.

After apply the patch for NAND upload, it works OK. And only perform one 
time reading.

>
>>   		dfu->bad_skip = 0;
>>

Best Regards,
Bo Shen

[U-Boot] [RFC PATCH 2/2] usb: dfu: correct dfu buffer inited value

[Lukasz Majewski]

Hi Bo,

First of all, sorry for a little delay.

> Hi Lukasz,
> 
> On 11/4/2013 18:17, Lukasz Majewski wrote:
> > Hi Bo,
> >
> >> After dfu buffer is initialized, the buffer should be all
> >> available, while not 0. Initialize its value to min(dfu_buf_size,
> >> dfu->r_left).
> >>
> >> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> >>
> >> ---
> >>   drivers/dfu/dfu.c |    2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> >> index 65c6984..b8c8aa4 100644
> >> --- a/drivers/dfu/dfu.c
> >> +++ b/drivers/dfu/dfu.c
> >> @@ -288,7 +288,7 @@ int dfu_read(struct dfu_entity *dfu, void *buf,
> >> int size, int blk_seq_num) dfu->offset = 0;
> >>   		dfu->i_buf_end = dfu_get_buf() + dfu_buf_size;
> >>   		dfu->i_buf = dfu->i_buf_start;
> >> -		dfu->b_left = 0;
> >> +		dfu->b_left = min(dfu_buf_size, dfu->r_left);
> >>
> >
> > I've testd in on Trats. It causes dfu read to be performed two
> > times.
> 
> Have you apply these two patches together:
> [RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer
> [RFC PATCH 2/2] usb: dfu: correct dfu buffer inited value
> 

I did a "little" mistake when applying those patches. When both patches
are applied dfu works fine.

I've tested it on Trats (Exynos4210).

For:
[RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer
[RFC PATCH 2/2] usb: dfu: correct dfu buffer inited value

Tested-by: Lukasz Majewski <l.majewski@samsung.com>
Acked-by: Lukasz Majewski <l.majewski@samsung.com>

@ Marek,

Since I'm still struggling with internal firewall - Marek could you
apply those two patches to u-boot-usb tree?

Also please apply Heiko's patch:

[PATCH v3 3/5] usb, g_dnl: make iSerialNumber board configurable

Thanks in advance.


> I test this with dfu mmc.
> 
> Without these two patches, it will read file two times.
> --->8---
> U-Boot> dfu mmc 0
> GADGET DRIVER: usb_dnl_dfu
> reading image.bin
> 3049120 bytes read in 628 ms (4.6 MiB/s)
> reading image.bin
> 3049120 bytes read in 628 ms (4.6 MiB/s)
> ---8<---
> 
> with these two patches, it only read file one time.
> --->8---
> U-Boot> dfu mmc 0
> GADGET DRIVER: usb_dnl_dfu
> reading image.bin
> 3049120 bytes read in 628 ms (4.6 MiB/s)
> ---8<---
> 
> The result is opposite.
> 
> > Could you write a more verbose message to explain the problem that
> > you are trying to solve? I can _only_ suppose that you want to
> > read/write data from/to NAND memory.
> >
> > So, I'm curious why dfu-util breaks with your setup but works at
> > am335x. Both chips are supposed to use dfu_nand.c for performing
> > NAND read/write.
> 
> For the NAND upload, if without patch: [U-Boot,RFC] usb: dfu: make
> nand upload working (http://patchwork.ozlabs.org/patch/283886/)
> It doesn't work at my side. more information as following:
> --->8---
> $ ./dfu-util -l
> dfu-util 0.7
> 
> Copyright 2005-2008 Weston Schmidt, Harald Welte and OpenMoko Inc.
> Copyright 2010-2012 Tormod Volden and Stefan Schmidt
> This program is Free Software and has ABSOLUTELY NO WARRANTY
> Please report bugs to dfu-util at lists.gnumonks.org
> 
> Found Runtime: [413c:8187] devnum=0, cfg=1, intf=3, alt=0,
> name="UNDEFINED" Found DFU: [03eb:6156] devnum=0, cfg=1, intf=0,
> alt=0, name="sama5d34ek.dtb" Found DFU: [03eb:6156] devnum=0, cfg=1,
> intf=0, alt=1, name="uImage" Found DFU: [03eb:6156] devnum=0, cfg=1,
> intf=0, alt=2, name="rootfs.ubi"
> 
> $ ./dfu-util -d 03eb:6156 -U kernel.image -a 1
> dfu-util 0.7
> 
> Copyright 2005-2008 Weston Schmidt, Harald Welte and OpenMoko Inc.
> Copyright 2010-2012 Tormod Volden and Stefan Schmidt
> This program is Free Software and has ABSOLUTELY NO WARRANTY
> Please report bugs to dfu-util at lists.gnumonks.org
> 
> Filter on vendor = 0x03eb product = 0x6156
> Opening DFU capable USB device... ID 03eb:6156
> WARNING: Can not find cached DFU functional descriptor
> Warning: Assuming DFU version 1.0
> Run-time device DFU version 0100
> Found DFU: [03eb:6156] devnum=0, cfg=1, intf=0, alt=1, name="uImage"
> Claiming USB DFU Interface...
> Setting Alternate Setting #1 ...
> Determining device status: state = dfuUPLOAD-IDLE, status = 0
> aborting previous incomplete transfer
> Determining device status: state = dfuIDLE, status = 0
> dfuIDLE, continuing
> Error obtaining cached DFU functional descriptor
> DFU mode device DFU version 0110
> Device returned transfer size 4096
> bytes_per_hash=4096
> Copying data from DFU device to PC
> Starting upload: [] finished!
> ---8<---
> 
> It doesn't transfer anything.
> 
> After apply the patch for NAND upload, it works OK. And only perform
> one time reading.
> 
> >
> >>   		dfu->bad_skip = 0;
> >>
> 
> Best Regards,
> Bo Shen
> 



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] [RFC PATCH 2/2] usb: dfu: correct dfu buffer inited value

[Marek Vasut]

Dear Lukasz Majewski,

> Hi Bo,
> 
> First of all, sorry for a little delay.
> 
> > Hi Lukasz,
> > 
> > On 11/4/2013 18:17, Lukasz Majewski wrote:
> > > Hi Bo,
> > > 
> > >> After dfu buffer is initialized, the buffer should be all
> > >> available, while not 0. Initialize its value to min(dfu_buf_size,
> > >> dfu->r_left).
> > >> 
> > >> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> > >> 
> > >> ---
> > >> 
> > >>   drivers/dfu/dfu.c |    2 +-
> > >>   1 file changed, 1 insertion(+), 1 deletion(-)
> > >> 
> > >> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> > >> index 65c6984..b8c8aa4 100644
> > >> --- a/drivers/dfu/dfu.c
> > >> +++ b/drivers/dfu/dfu.c
> > >> @@ -288,7 +288,7 @@ int dfu_read(struct dfu_entity *dfu, void *buf,
> > >> int size, int blk_seq_num) dfu->offset = 0;
> > >> 
> > >>   		dfu->i_buf_end = dfu_get_buf() + dfu_buf_size;
> > >>   		dfu->i_buf = dfu->i_buf_start;
> > >> 
> > >> -		dfu->b_left = 0;
> > >> +		dfu->b_left = min(dfu_buf_size, dfu->r_left);
> > > 
> > > I've testd in on Trats. It causes dfu read to be performed two
> > > times.
> > 
> > Have you apply these two patches together:
> > [RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer
> > [RFC PATCH 2/2] usb: dfu: correct dfu buffer inited value
> 
> I did a "little" mistake when applying those patches. When both patches
> are applied dfu works fine.
> 
> I've tested it on Trats (Exynos4210).
> 
> For:
> [RFC PATCH 1/2] usb: dfu: decrease dfu->r_left along with the transfer
> [RFC PATCH 2/2] usb: dfu: correct dfu buffer inited value
> 
> Tested-by: Lukasz Majewski <l.majewski@samsung.com>
> Acked-by: Lukasz Majewski <l.majewski@samsung.com>
> 
> @ Marek,
> 
> Since I'm still struggling with internal firewall - Marek could you
> apply those two patches to u-boot-usb tree?
> 
> Also please apply Heiko's patch:
> 
> [PATCH v3 3/5] usb, g_dnl: make iSerialNumber board configurable
> 
> Thanks in advance.

Roger that, all done, thanks!

Best regards,
Marek Vasut