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