On Fri, Apr 12, 2013 at 09:39:19PM +0200, Beno??t Th??baudeau wrote: > Hi Tom, Ruud, Mats, > > On Friday, April 12, 2013 5:12:31 PM, Tom Rini wrote: > > On Fri, Apr 12, 2013 at 05:06:35PM +0200, Ruud Commandeur wrote: > > > Hi Mats, > > > > > > Thanks a lot, this seems to solve my problem. Nothing more actualy than > > > adding a "if(size)" around the code block of set_sector( ). I could have > > > thought of that myself, but was not sure if anything else should be done > > > in this case... > > > > Since your change sounds slightly different, can you confirm that it > > also solves the problem and if so post it as patch with Signed-off-by > > and so forth? Thanks! > > > > > > > > Regards, > > > > > > Ruud > > > > > > -----Oorspronkelijk bericht----- > > > Van: Mats K?rrman [mailto:mats.karr...@tritech.se] > > > Verzonden: vrijdag 12 april 2013 16:11 > > > Aan: Ruud Commandeur > > > CC: u-boot@lists.denx.de > > > Onderwerp: RE: fatwrite problem > > > > > > Hi Ruud, > > > > > > Ruud Commandeur wrote: > > > > Once the size of the set_cluster call equals 0, the mmc command is > > > > incomplete and times out. In the earlier reported problem, a patch is > > > > mentioned, but not available for dowload here. Also in the latest > > > > versions of the git repository I could not find a patch for this > > > > problem. Can anyone tell me if there is a fix for this problem? > > > > > > I asked Damien Huang (back then) and got the following reply (I think > > > there > > > was > > > some character encoding problem so his mail never was accepted by the > > > list). > > > I have not further analyzed the contents, anyway it wasn't the solution to > > > my problem. > > > BR // Mats > > > > > > Damien Huang wrote: > > > As requested from Mats, I am resending this email. The patch is given > > > below: > > > > > > diff -cr ./u-boot-original/u-boot/fs/fat/fat_write.c > > > ./u-boot-test/u-boot/fs/fat/fat_write.c > > > *** ./u-boot-original/u-boot/fs/fat/fat_write.c 2013-02-07 > > > 14:47:33.314732999 +1100 > > > --- ./u-boot-test/u-boot/fs/fat/fat_write.c 2013-02-28 > > > 15:36:24.551861920 +1100 > > > *************** > > > *** 562,596 **** > > > { > > > int idx = 0; > > > __u32 startsect; > > > ! > > > ! if (clustnum > 0) > > > ! startsect = mydata->data_begin + > > > ! clustnum * mydata->clust_size; > > > ! else > > > ! startsect = mydata->rootdir_sect; > > > ! > > > ! debug("clustnum: %d, startsect: %d\n", clustnum, startsect); > > > ! > > > ! if (disk_write(startsect, size / mydata->sect_size, buffer) < 0) { > > > ! debug("Error writing data\n"); > > > ! return -1; > > > ! } > > > ! > > > ! if (size % mydata->sect_size) { > > > ! __u8 tmpbuf[mydata->sect_size]; > > > ! > > > ! idx = size / mydata->sect_size; > > > ! buffer += idx * mydata->sect_size; > > > ! memcpy(tmpbuf, buffer, size % mydata->sect_size); > > > ! > > > ! if (disk_write(startsect + idx, 1, tmpbuf) < 0) { > > > ! debug("Error writing data\n"); > > > ! return -1; > > > ! } > > > ! > > > ! return 0; > > > ! } > > > ! > > > return 0; > > > } > > > > > > --- 562,595 ---- > > > { > > > int idx = 0; > > > __u32 startsect; > > > ! if(size) //if there are data to be set > > > ! { > > > ! if (clustnum > 0) > > > ! startsect = mydata->data_begin + > > > ! clustnum * mydata->clust_size; > > > ! else > > > ! startsect = mydata->rootdir_sect; > > > ! > > > ! debug("clustnum: %d, startsect: %d\n", clustnum, startsect); > > > ! > > > ! if (disk_write(startsect, size / mydata->sect_size, buffer) < 0) > > > { > > > ! debug("Error writing data\n"); > > > ! return -1; > > > ! } > > > ! > > > ! if (size % mydata->sect_size) { > > > ! __u8 tmpbuf[mydata->sect_size]; > > > ! > > > ! idx = size / mydata->sect_size; > > > ! buffer += idx * mydata->sect_size; > > > ! memcpy(tmpbuf, buffer, size % mydata->sect_size); > > > ! > > > ! if (disk_write(startsect + idx, 1, tmpbuf) < 0) { > > > ! debug("Error writing data\n"); > > > ! return -1; > > > ! } > > > ! } > > > ! }//end if data to be set > > > return 0; > > > } > > > > > > diff -cr ./u-boot-original/u-boot/include/configs/am335x_evm.h > > > ./u-boot-test/u-boot/include/configs/am335x_evm.h > > > *** ./u-boot-original/u-boot/include/configs/am335x_evm.h 2013-02-07 > > > 14:47:35.754702325 +1100 > > > --- ./u-boot-test/u-boot/include/configs/am335x_evm.h 2013-03-01 > > > 12:25:23.942104474 +1100 > > > *************** > > > *** 143,148 **** > > > --- 143,149 ---- > > > #define CONFIG_CMD_MMC > > > #define CONFIG_DOS_PARTITION > > > #define CONFIG_FS_FAT > > > + #define CONFIG_FAT_WRITE > > > #define CONFIG_FS_EXT4 > > > #define CONFIG_CMD_FAT > > > #define CONFIG_CMD_EXT2 > > > > Benoit, what do you think? Thanks! > > This is fine as a workaround, but it does not fix the root cause of the issue: > set_clusster() should not have been called with size set to 0 in the first > place. This is hiding some cluster mishandling. It may mean that a cluster is > wasted at EoF in some cases, which is unexpected and may trigger abnormal > behaviors on systems reading back those files. > > What kind of action triggered this issue for you: > - writing an empty file? > - writing a file with a size equal to an integer multiple of the cluster > size? > - something else? > > This can be caused only be lines 713, 724 or 739. Looking closer at the code, > only line 713 triggers this issue, so an 'if' could be added here rather than > in > set_cluster(). > > The code for writing an empty file is broken: It should set both the start > cluster and the size to 0 in the corresponding directory entry, but it > actually > allocates a single cluster (see find_empty_cluster() called from > do_fat_write()). > > So 2 fixes are required here: > - 'if' either in set_cluster(), or on line 713 to discard empty sizes, or > make > the underlying MMC driver return without doing anything if size is 0, the > latter being perhaps the most robust solution if it does not cause other > issues for this driver,
I prefer updating set_cluster since he sees this in one MMC driver and I in another (meaning we would have to go whack all of them, or start poking drivers/mmc/mmc.c). I can confirm that size != 0 in that test fixes the problem here. > - empty file creation. I took a stab at this and ended up killing my test FAT partition, which is not a big deal, but can you take a stab at this please? Thanks! -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
