Re: [U-Boot] [PATCH] fs: fat: fix bug when write size is less than a sector size
Hi, Dear Wolfgang Denk
On 7/19/2013 7:20 PM, Wolfgang Denk wrote:
Dear Josh Wu,
In message 51e90a3d.6000...@atmel.com you wrote:
Why would this be needed? The case of a partial write is covered
further down below in the code, in the if (size % mydata-sect_size)
part...
The call of disk_write() will pass the sector size as 0. That will cause
the mmc driver stalled.
I think instead of sector size you mean the number of sectors. The
sector size is fixed.
yes, I mean the number of sectors.
Big question: why should we call disk_write() at all when the count is
zero? Maybe a simple
if ((size / mydata-sect_size) 0) {
if (disk_write(startsect, size / mydata-sect_size, buffer)
0) {
...
}
}
would fix this issue?
I agree with above changes. It make the logical clearer:
Step 1. write buffer in sectors. If buffer size is less than one
sector we don't need to call disk_write() with zero sector.
Step 2. write remain buffer in one sector.
So I think I will send a v2 patch according to your suggestion. Thanks.
Best regards,
Wolfgang Denk
Best Regards,
Josh Wu
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: fix bug when write size is less than a sector size
Dear Wolfgang Denk,
Thank you for the quick reply.
On 7/18/2013 4:24 PM, Wolfgang Denk wrote:
Dear Josh Wu,
In message 1374134870-10154-1-git-send-email-josh...@atmel.com you wrote:
Tested in at91sam9x5ek and sama5d3x-ek in mmc fat.
Signed-off-by: Josh Wu josh...@atmel.com
---
When I debugging this issue, I also find the thread talking about this:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/158698
But finally no patch is send for this bug. So I send this and cc all the
people in that thread. Hope it can be merged in 2013.07.
Definitely not. This needs thurough testing before it can go
mainline.
got it.
- __u32 startsect;
+ __u32 startsect, nr_sectors;
if (clustnum 0)
startsect = mydata-data_begin +
@@ -571,7 +571,11 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer,
debug(clustnum: %d, startsect: %d\n, clustnum, startsect);
- if (disk_write(startsect, size / mydata-sect_size, buffer) 0) {
+ nr_sectors = size / mydata-sect_size;
+ if (nr_sectors == 0)
+ nr_sectors = 1;
Why would this be needed? The case of a partial write is covered
further down below in the code, in the if (size % mydata-sect_size)
part...
The call of disk_write() will pass the sector size as 0. That will cause
the mmc driver stalled.
And after a search, the fix patch is already sent by Ruud Commandeur and
merge in mainline:
mmc write bug fix
commit a586c0aa211fb79ecaa06aee3299bfdd81329876
Author: Ruud Commandeur rcommand...@clb.nl
Date: Wed May 22 13:19:43 2013 +0200
So please forget this patch. Since it is fixed in mmc driver layer. I
will test it in my board.
Sorry for the noise.
Best regards,
Wolfgang Denk
Best Regards,
Josh Wu
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: fix bug when write size is less than a sector size
Dear Josh Wu,
In message 51e90a3d.6000...@atmel.com you wrote:
Why would this be needed? The case of a partial write is covered
further down below in the code, in the if (size % mydata-sect_size)
part...
The call of disk_write() will pass the sector size as 0. That will cause
the mmc driver stalled.
I think instead of sector size you mean the number of sectors. The
sector size is fixed.
Big question: why should we call disk_write() at all when the count is
zero? Maybe a simple
if ((size / mydata-sect_size) 0) {
if (disk_write(startsect, size / mydata-sect_size, buffer)
0) {
...
}
}
would fix this issue?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Der Irrtum wiederholt sich immerfort in der Tat. Deshalb muß man das
Wahre unermüdlich in Worten wiederholen. - Goethe
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] fs: fat: fix bug when write size is less than a sector size
Tested in at91sam9x5ek and sama5d3x-ek in mmc fat.
Signed-off-by: Josh Wu josh...@atmel.com
---
When I debugging this issue, I also find the thread talking about this:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/158698
But finally no patch is send for this bug. So I send this and cc all the
people in that thread. Hope it can be merged in 2013.07.
fs/fat/fat_write.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 5829adf..b7ba8ea 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -561,7 +561,7 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer,
unsigned long size)
{
int idx = 0;
- __u32 startsect;
+ __u32 startsect, nr_sectors;
if (clustnum 0)
startsect = mydata-data_begin +
@@ -571,7 +571,11 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer,
debug(clustnum: %d, startsect: %d\n, clustnum, startsect);
- if (disk_write(startsect, size / mydata-sect_size, buffer) 0) {
+ nr_sectors = size / mydata-sect_size;
+ if (nr_sectors == 0)
+ nr_sectors = 1;
+
+ if (disk_write(startsect, nr_sectors, buffer) 0) {
debug(Error writing data\n);
return -1;
}
--
1.7.9.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: fix bug when write size is less than a sector size
Dear Josh Wu,
In message 1374134870-10154-1-git-send-email-josh...@atmel.com you wrote:
Tested in at91sam9x5ek and sama5d3x-ek in mmc fat.
Signed-off-by: Josh Wu josh...@atmel.com
---
When I debugging this issue, I also find the thread talking about this:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/158698
But finally no patch is send for this bug. So I send this and cc all the
people in that thread. Hope it can be merged in 2013.07.
Definitely not. This needs thurough testing before it can go
mainline.
- __u32 startsect;
+ __u32 startsect, nr_sectors;
if (clustnum 0)
startsect = mydata-data_begin +
@@ -571,7 +571,11 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer,
debug(clustnum: %d, startsect: %d\n, clustnum, startsect);
- if (disk_write(startsect, size / mydata-sect_size, buffer) 0) {
+ nr_sectors = size / mydata-sect_size;
+ if (nr_sectors == 0)
+ nr_sectors = 1;
Why would this be needed? The case of a partial write is covered
further down below in the code, in the if (size % mydata-sect_size)
part...
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
I still miss my ex-wife, but my aim is getting better.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
