On Wed, Nov 20, 2024 at 02:55:48PM +0100, Caleb Connolly wrote:
Hi both,
Varadarajan: Thanks for sending this patch.
On 19/11/2024 16:44, Tom Rini wrote:
On Tue, Nov 19, 2024 at 01:00:48PM +0530, Varadarajan Narayanan wrote:
Do FAT read and write based on the device sector size instead of the size recorded in the FAT meta data. This helps to overcome the 'FAT sector size mismatch' error and enables U-Boot to read/write those partitions.
Does this ignore the filesystem sector size or account for it?
Accounts for it.
There's a whole lot of logic added below which isn't really explained here.
Will post a new version with additional explanation.
Signed-off-by: Varadarajan Narayanan quic_varada@quicinc.com
fs/fat/fat.c | 227 ++++++++++++++++++++++++++++++++++++++++++--- fs/fat/fat_write.c | 19 ---- 2 files changed, 214 insertions(+), 32 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index e2570e8167..f4bad99335 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -44,24 +44,223 @@ static void downcase(char *str, size_t len)
static struct blk_desc *cur_dev; static struct disk_partition cur_part_info; +int fat_sect_size;
This variable should be static, no harm in 0 initializing it here either.
Sure.
[...]
+int disk_write(__u32 sect, __u32 nr_sect, void *buf) +{
- int ret;
- __u8 *block = NULL;
- __u32 rem, size;
- __u32 s, n;
- rem = nr_sect * fat_sect_size;
- /*
* block N block N + 1 block N + 2* +-+-+--+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+* | | | | |s|e|c|t|o|r| | |s|e|c|t|o|r| | |s|e|c|t|o|r| | | | |* +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+* . . . | | | | . . .* ------+---------------+---------------+---------------+------* |<--- FAT reads in sectors --->|** | part 1 | part 2 | part 3 |**/- /* Do part 1 */
- if (fat_sect_size) {
__u32 offset;/* Read one block and overwrite the leading sectors */block = malloc_cache_aligned(cur_dev->blksz);if (!block) {printf("Error: allocating block: %lu\n", cur_dev->blksz);return -1;}s = sect_to_block(sect, &offset);offset = offset * fat_sect_size;ret = blk_dread(cur_dev, cur_part_info.start + s, 1, block);if (ret != 1) {ret = -1;goto exit;}if (rem > (cur_part_info.blksz - offset))size = cur_part_info.blksz - offset;elsesize = rem;memcpy(block + offset, buf, size);ret = blk_dwrite(cur_dev, cur_part_info.start + s, 1, block);if (ret != 1) {ret = -1;goto exit;}rem -= size;buf += size;s++;- } else {
/** fat_sect_size not being set implies, this is the first read* to partition. The first sector is being read to get the* FS meta data. The FAT sector size is got from this meta data.*/This is the disk_write() function though, I don't think this behaviour is correct?
Will clean this.
Thanks for the inputs.
-Varada
ret = blk_dread(cur_dev, cur_part_info.start + s, 1, buf);if (ret != 1)return -1;- }
- /* Do part 2, write directly from the given buffer */
- if (rem > cur_part_info.blksz) {
n = rem / cur_part_info.blksz;ret = blk_dwrite(cur_dev, cur_part_info.start + s, n, buf);if (ret != n) {ret = -1;goto exit;}buf += n * cur_part_info.blksz;rem = rem % cur_part_info.blksz;s += n;- }
- /* Do part 3, read a block and copy the trailing sectors */
- if (rem) {
ret = blk_dread(cur_dev, cur_part_info.start + s, 1, block);if (ret != 1) {ret = -1;goto exit;} else {memcpy(block, buf, rem);}ret = blk_dwrite(cur_dev, cur_part_info.start + s, 1, block);if (ret != 1) {ret = -1;goto exit;}- }
+exit:
- if (block)
free(block);- return (ret == -1) ? -1 : nr_sect;
}
[...]
Hi Tom,
With this patch, we now have https://patchwork.ozlabs.org/project/uboot/patch/20241113044956.1836896-1-ca...
I suspect my patch would corrupt storage on write, I'll give it another test and see.
and https://patchwork.ozlabs.org/project/uboot/patch/20230730111516.v2.1.Ia13846... for seemingly the same issue. Can you please try these other two patches and report which ones do / don't handle your use case as well? Thanks!
Kind regards,
-- // Caleb (they/them)