From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?= <benoit.thebaudeau@advansee.com>
Date: Sun, 23 Sep 2012 19:46:15 +0200 (CEST)
Subject: [U-Boot] [PATCH] FAT: split block device interactions from
 filesystem logic
In-Reply-To: <1784319.OGak1elLNI@merom>
Message-ID: <671074222.5001803.1348422375021.JavaMail.root@advansee.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Pavel,

On Sunday, September 23, 2012 5:28:31 PM, Pavel Herrmann wrote:
> add some CCs
> 
> On Wednesday 19 September 2012 16:34:02 Pavel Herrmann wrote:
> > Put block device interaction code into separate file from
> > filesystem logic.
> > This makes it easier to change block device API, and is similar to
> > what
> > other filesystems do.
> > Cleanup some logic inconsistencies as well.

Sounds good. See my comments below.

This might need some light rework to make it applicable after
http://patchwork.ozlabs.org/patch/184793/, which should be applied soon.

> > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> > ---
> >  fs/fat/Makefile    |   4 +-
> >  fs/fat/dev.c       | 191
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++ fs/fat/fat.c
> >       |
> > 175 ++++++------------------------------------------
> > fs/fat/fat_write.c |
> > 21 +++---
> >  include/fat.h      |   7 ++
> >  5 files changed, 228 insertions(+), 170 deletions(-)
> >  create mode 100644 fs/fat/dev.c
> > 
> > diff --git a/fs/fat/Makefile b/fs/fat/Makefile
> > index 9635d36..5d10b24 100644
> > --- a/fs/fat/Makefile
> > +++ b/fs/fat/Makefile
> > @@ -24,8 +24,8 @@ include $(TOPDIR)/config.mk
> >  LIB	= $(obj)libfat.o
> > 
> >  AOBJS	=
> > -COBJS-$(CONFIG_CMD_FAT)	:= fat.o
> > -COBJS-$(CONFIG_FAT_WRITE):= fat_write.o
> > +COBJS-$(CONFIG_CMD_FAT)	:= fat.o dev.o
> > +COBJS-$(CONFIG_FAT_WRITE) := fat_write.o dev.o
> > 
> >  ifndef CONFIG_SPL_BUILD
> >  COBJS-$(CONFIG_CMD_FAT)	+= file.o
> > diff --git a/fs/fat/dev.c b/fs/fat/dev.c
> > new file mode 100644
> > index 0000000..d5ff0c5
> > --- /dev/null
> > +++ b/fs/fat/dev.c
> > @@ -0,0 +1,191 @@
> > +/*
> > + * (C) Copyright 2012
> > + * Pavel Herrmann <morpheus.ibis@gmail.com>

Since 99% of the code here comes from fat.c and fat_write.c, the copyright
information from these files should probably be added here too.

> > + *
> > + * See file CREDITS for list of people who contributed to this
> > + * project.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public
> > License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> > + * MA 02111-1307 USA
> > + */
> > +
> > +#include <common.h>
> > +#include <fat.h>
> > +#include <part.h>
> > +#include <errno.h>
> > +
> > +#define DOS_BOOT_MAGIC_OFFSET   0x1fe
> > +#define DOS_FS_TYPE_OFFSET      0x36
> > +#define DOS_FS32_TYPE_OFFSET    0x52
> > +
> > +static block_dev_desc_t *cur_dev;
> > +static unsigned int cur_part_nr;
> > +static disk_partition_t cur_part_info;
> > +
> > +int fat_disk_read(__u32 block, __u32 nr_blocks, void *buf)
> > +{
> > +	if (!cur_dev || !cur_dev->block_read)
> > +		return -1;
> > +
> > +	return cur_dev->block_read(cur_dev->dev,
> > +		cur_part_info.start + block, nr_blocks, buf);
> > +}
> > +
> > +int fat_disk_write(__u32 block, __u32 nr_blocks, void *buf)
> > +{
> > +	if (!cur_dev || !cur_dev->block_read)
> > +		return -1;
> > +
> > +	return cur_dev->block_read(cur_dev->dev,
> > +		cur_part_info.start + block, nr_blocks, buf);
> > +}

block_write...

Please, split this patch into:
 - moved code + renaming,
 - behavior changes (one patch per behavior change).
That will make things clearer and easier to review.

> > +
> > +int fat_get_blksz(void)

cur_dev->blksz is ulong, so long might be better here, even if it does not seem
strictly necessary.

> > +{
> > +	if (cur_dev == NULL)
> > +		return -EINVAL;
> > +	return cur_dev->blksz;

Or cur_part_info.blksize? Your calls to this function replace both usages, so
please make sure that this won't cause any trouble.

> > +}
> > +
> > +int fat_get_partsize(void)

long?

> > +{
> > +	if (cur_dev == NULL)
> > +		return -EINVAL;

This test looks unrelated to the line below and changes the behavior where this
function is called. Please clarify.

> > +	return cur_part_info.size;
> > +}
> > +
> > +int fat_register_device(block_dev_desc_t *dev_desc, int part_no)
> > +{
> > +	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
> > +
> > +	/* First close any currently found FAT filesystem */
> > +	cur_dev = NULL;
> > +
> > +#if (defined(CONFIG_CMD_IDE) || \
> > +	defined(CONFIG_CMD_SATA) || \
> > +	defined(CONFIG_CMD_SCSI) || \
> > +	defined(CONFIG_CMD_USB) || \
> > +	defined(CONFIG_MMC) || \
> > +	defined(CONFIG_SYSTEMACE))
> > +
> > +	/* Read the partition table, if present */
> > +	if (!get_partition_info(dev_desc, part_no, &cur_part_info)) {
> > +		cur_dev = dev_desc;
> > +		cur_part_nr = part_no;
> > +	}
> > +#endif
> > +
> > +	/* Otherwise it might be a superfloppy (whole-disk FAT
> > filesystem) */
> > +	if (!cur_dev) {
> > +		if (part_no != 1) {
> > +			printf("** Partition %d not valid on device %d **\n",
> > +			part_no, dev_desc->dev);
> > +			return -1;
> > +		}
> > +
> > +		cur_dev = dev_desc;
> > +		cur_part_nr = 1;
> > +		cur_part_info.start = 0;
> > +		cur_part_info.size = dev_desc->lba;
> > +		cur_part_info.blksz = dev_desc->blksz;
> > +		memset(cur_part_info.name, 0, sizeof(cur_part_info.name));
> > +		memset(cur_part_info.type, 0, sizeof(cur_part_info.type));
> > +	}
> > +
> > +	/* Make sure it has a valid FAT header */
> > +	if (fat_disk_read(0, 1, buffer) != 1) {
> > +		cur_dev = NULL;
> > +		return -1;
> > +	}
> > +
> > +	/* Check if it's actually a DOS volume */
> > +	if (memcmp(buffer + DOS_BOOT_MAGIC_OFFSET, "\x55\xAA", 2)) {
> > +		cur_dev = NULL;
> > +		return -1;
> > +	}
> > +
> > +	/* Check for FAT12/FAT16/FAT32 filesystem */
> > +	if (!memcmp(buffer + DOS_FS_TYPE_OFFSET, "FAT", 3))
> > +		return 0;
> > +	if (!memcmp(buffer + DOS_FS32_TYPE_OFFSET, "FAT32", 5))
> > +		return 0;
> > +
> > +	cur_dev = NULL;
> > +	return -1;
> > +}
> > +
> > +int file_fat_detectfs(void)
> > +{
> > +	boot_sector bs;
> > +	volume_info volinfo;
> > +	int fatsize;
> > +	char vol_label[12];
> > +
> > +	if (cur_dev == NULL) {
> > +		printf("No current device\n");
> > +		return 1;
> > +	}
> > +
> > +#if defined(CONFIG_CMD_IDE) || \
> > +	defined(CONFIG_CMD_SATA) || \
> > +	defined(CONFIG_CMD_SCSI) || \
> > +	defined(CONFIG_CMD_USB) || \
> > +	defined(CONFIG_MMC)
> > +	printf("Interface:  ");
> > +	switch (cur_dev->if_type) {
> > +	case IF_TYPE_IDE:
> > +		printf("IDE");
> > +		break;
> > +	case IF_TYPE_SATA:
> > +		printf("SATA");
> > +		break;
> > +	case IF_TYPE_SCSI:
> > +		printf("SCSI");
> > +		break;
> > +	case IF_TYPE_ATAPI:
> > +		printf("ATAPI");
> > +		break;
> > +	case IF_TYPE_USB:
> > +		printf("USB");
> > +		break;
> > +	case IF_TYPE_DOC:
> > +		printf("DOC");
> > +		break;
> > +	case IF_TYPE_MMC:
> > +		printf("MMC");
> > +		break;
> > +	default:
> > +		printf("Unknown");
> > +	}
> > +
> > +	printf("\n  Device %d: ", cur_dev->dev);
> > +	dev_print(cur_dev);
> > +#endif
> > +
> > +	if (fat_read_bootsectandvi(&bs, &volinfo, &fatsize)) {
> > +		printf("\nNo valid FAT fs found\n");
> > +		return 1;
> > +	}
> > +
> > +	memcpy(vol_label, volinfo.volume_label, 11);
> > +	vol_label[11] = '\0';
> > +	volinfo.fs_type[5] = '\0';
> > +
> > +	printf("Partition %d: Filesystem: %s \"%s\"\n", cur_part_nr,
> > +		volinfo.fs_type, vol_label);
> > +
> > +	return 0;
> > +}
> > +
> > diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> > index 19f6a8c..a13c62b 100644
> > --- a/fs/fat/fat.c
> > +++ b/fs/fat/fat.c
> > @@ -45,84 +45,6 @@ static void downcase(char *str)
> >  	}
> >  }
> > 
> > -static block_dev_desc_t *cur_dev;
> > -static unsigned int cur_part_nr;
> > -static disk_partition_t cur_part_info;
> > -
> > -#define DOS_BOOT_MAGIC_OFFSET	0x1fe
> > -#define DOS_FS_TYPE_OFFSET	0x36
> > -#define DOS_FS32_TYPE_OFFSET	0x52
> > -
> > -static int disk_read(__u32 block, __u32 nr_blocks, void *buf)
> > -{
> > -	if (!cur_dev || !cur_dev->block_read)
> > -		return -1;
> > -
> > -	return cur_dev->block_read(cur_dev->dev,
> > -			cur_part_info.start + block, nr_blocks, buf);
> > -}
> > -
> > -int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
> > -{
> > -	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
> > -
> > -	/* First close any currently found FAT filesystem */
> > -	cur_dev = NULL;
> > -
> > -#if (defined(CONFIG_CMD_IDE) || \
> > -     defined(CONFIG_CMD_SATA) || \
> > -     defined(CONFIG_CMD_SCSI) || \
> > -     defined(CONFIG_CMD_USB) || \
> > -     defined(CONFIG_MMC) || \
> > -     defined(CONFIG_SYSTEMACE) )
> > -
> > -	/* Read the partition table, if present */
> > -	if (!get_partition_info(dev_desc, part_no, &cur_part_info)) {
> > -		cur_dev = dev_desc;
> > -		cur_part_nr = part_no;
> > -	}
> > -#endif
> > -
> > -	/* Otherwise it might be a superfloppy (whole-disk FAT
> > filesystem) */
> > -	if (!cur_dev) {
> > -		if (part_no != 1) {
> > -			printf("** Partition %d not valid on device %d **\n",
> > -					part_no, dev_desc->dev);
> > -			return -1;
> > -		}
> > -
> > -		cur_dev = dev_desc;
> > -		cur_part_nr = 1;
> > -		cur_part_info.start = 0;
> > -		cur_part_info.size = dev_desc->lba;
> > -		cur_part_info.blksz = dev_desc->blksz;
> > -		memset(cur_part_info.name, 0, sizeof(cur_part_info.name));
> > -		memset(cur_part_info.type, 0, sizeof(cur_part_info.type));
> > -	}
> > -
> > -	/* Make sure it has a valid FAT header */
> > -	if (disk_read(0, 1, buffer) != 1) {
> > -		cur_dev = NULL;
> > -		return -1;
> > -	}
> > -
> > -	/* Check if it's actually a DOS volume */
> > -	if (memcmp(buffer + DOS_BOOT_MAGIC_OFFSET, "\x55\xAA", 2)) {
> > -		cur_dev = NULL;
> > -		return -1;
> > -	}
> > -
> > -	/* Check for FAT12/FAT16/FAT32 filesystem */
> > -	if (!memcmp(buffer + DOS_FS_TYPE_OFFSET, "FAT", 3))
> > -		return 0;
> > -	if (!memcmp(buffer + DOS_FS32_TYPE_OFFSET, "FAT32", 5))
> > -		return 0;
> > -
> > -	cur_dev = NULL;
> > -	return -1;
> > -}
> > -
> > -
> >  /*
> >   * Get the first occurence of a directory delimiter ('/' or '\')
> >   in a
> > string. * Return index into string if found, -1 otherwise.
> > @@ -212,7 +134,7 @@ static __u32 get_fatent(fsdata *mydata, __u32
> > entry)
> > 
> >  		startblock += mydata->fat_sect;	/* Offset from start of disk */
> > 
> > -		if (disk_read(startblock, getsize, bufptr) < 0) {
> > +		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
> >  			debug("Error reading FAT blocks\n");
> >  			return ret;
> >  		}
> > @@ -290,7 +212,7 @@ get_cluster(fsdata *mydata, __u32 clustnum,
> > __u8
> > *buffer, unsigned long size) printf("FAT: Misaligned buffer address
> > (%p)\n", buffer);
> > 
> >  		while (size >= mydata->sect_size) {
> > -			ret = disk_read(startsect++, 1, tmpbuf);
> > +			ret = fat_disk_read(startsect++, 1, tmpbuf);
> >  			if (ret != 1) {
> >  				debug("Error reading data (got %d)\n", ret);
> >  				return -1;
> > @@ -302,7 +224,7 @@ get_cluster(fsdata *mydata, __u32 clustnum,
> > __u8
> > *buffer, unsigned long size) }
> >  	} else {
> >  		idx = size / mydata->sect_size;
> > -		ret = disk_read(startsect, idx, buffer);
> > +		ret = fat_disk_read(startsect, idx, buffer);
> >  		if (ret != idx) {
> >  			debug("Error reading data (got %d)\n", ret);
> >  			return -1;
> > @@ -315,7 +237,7 @@ get_cluster(fsdata *mydata, __u32 clustnum,
> > __u8
> > *buffer, unsigned long size) if (size) {
> >  		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
> > 
> > -		ret = disk_read(startsect, 1, tmpbuf);
> > +		ret = fat_disk_read(startsect, 1, tmpbuf);
> >  		if (ret != 1) {
> >  			debug("Error reading data (got %d)\n", ret);
> >  			return -1;
> > @@ -702,25 +624,28 @@ static dir_entry *get_dentfromdir(fsdata
> > *mydata, int
> > startsect, /*
> >   * Read boot sector and volume info from a FAT filesystem
> >   */
> > -static int
> > -read_bootsectandvi(boot_sector *bs, volume_info *volinfo, int
> > *fatsize)
> > +int
> > +fat_read_bootsectandvi(boot_sector *bs, volume_info *volinfo, int
> > *fatsize)
> > {
> >  	__u8 *block;
> >  	volume_info *vistart;
> >  	int ret = 0;
> > +	int blksz;
> > 
> > -	if (cur_dev == NULL) {
> > +	blksz = fat_get_blksz();
> > +	if (blksz <= 0) {
> > +		/*this means the device is NULL, or otherwise unavailable*/
> >  		debug("Error: no device selected\n");
> >  		return -1;
> >  	}
> > 
> > -	block = memalign(ARCH_DMA_MINALIGN, cur_dev->blksz);
> > +	block = memalign(ARCH_DMA_MINALIGN, blksz);
> >  	if (block == NULL) {
> >  		debug("Error: allocating block\n");
> >  		return -1;
> >  	}
> > 
> > -	if (disk_read(0, 1, block) < 0) {
> > +	if (fat_disk_read(0, 1, block) < 0) {
> >  		debug("Error: reading block\n");
> >  		goto fail;
> >  	}
> > @@ -792,11 +717,14 @@ do_fat_read(const char *filename, void
> > *buffer,
> > unsigned long maxsize, int dols) __u32 root_cluster = 0;
> >  	int rootdir_size = 0;
> >  	int j;
> > +	int blksz;
> > 
> > -	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
> > +	if (fat_read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
> >  		debug("Error: reading boot sector\n");
> >  		return -1;
> >  	}
> > +	/*this will always succeed, because it did in above call*/
> > +	blksz = fat_get_blksz();
> > 
> >  	if (mydata->fatsize == 32) {
> >  		root_cluster = bs.root_cluster;
> > @@ -812,9 +740,9 @@ do_fat_read(const char *filename, void *buffer,
> > unsigned
> > long maxsize, int dols)
> > 
> >  	mydata->sect_size = (bs.sector_size[1] << 8) + bs.sector_size[0];
> >  	mydata->clust_size = bs.cluster_size;
> > -	if (mydata->sect_size != cur_part_info.blksz) {
> > -		printf("Error: FAT sector size mismatch (fs=%hu, dev=%lu)\n",
> > -				mydata->sect_size, cur_part_info.blksz);
> > +	if (mydata->sect_size != blksz) {

cur_part_info vs. cur_dev OK?

> > +		printf("Error: FAT sector size mismatch (fs=%hu, dev=%d)\n",
> > +				mydata->sect_size, blksz);
> >  		return -1;
> >  	}
> > 
> > @@ -884,7 +812,7 @@ do_fat_read(const char *filename, void *buffer,
> > unsigned
> > long maxsize, int dols) debug("FAT read sect=%d, clust_size=%d,
> > DIRENTSPERBLOCK=%zd\n", cursect, mydata->clust_size,
> > DIRENTSPERBLOCK);
> > 
> > -			if (disk_read(cursect,
> > +			if (fat_disk_read(cursect,
> >  					(mydata->fatsize == 32) ?
> >  					(mydata->clust_size) :
> >  					PREFETCH_BLOCKS,
> > @@ -1124,69 +1052,6 @@ exit:
> >  	return ret;
> >  }
> > 
> > -int file_fat_detectfs(void)
> > -{
> > -	boot_sector bs;
> > -	volume_info volinfo;
> > -	int fatsize;
> > -	char vol_label[12];
> > -
> > -	if (cur_dev == NULL) {
> > -		printf("No current device\n");
> > -		return 1;
> > -	}
> > -
> > -#if defined(CONFIG_CMD_IDE) || \
> > -    defined(CONFIG_CMD_SATA) || \
> > -    defined(CONFIG_CMD_SCSI) || \
> > -    defined(CONFIG_CMD_USB) || \
> > -    defined(CONFIG_MMC)
> > -	printf("Interface:  ");
> > -	switch (cur_dev->if_type) {
> > -	case IF_TYPE_IDE:
> > -		printf("IDE");
> > -		break;
> > -	case IF_TYPE_SATA:
> > -		printf("SATA");
> > -		break;
> > -	case IF_TYPE_SCSI:
> > -		printf("SCSI");
> > -		break;
> > -	case IF_TYPE_ATAPI:
> > -		printf("ATAPI");
> > -		break;
> > -	case IF_TYPE_USB:
> > -		printf("USB");
> > -		break;
> > -	case IF_TYPE_DOC:
> > -		printf("DOC");
> > -		break;
> > -	case IF_TYPE_MMC:
> > -		printf("MMC");
> > -		break;
> > -	default:
> > -		printf("Unknown");
> > -	}
> > -
> > -	printf("\n  Device %d: ", cur_dev->dev);
> > -	dev_print(cur_dev);
> > -#endif
> > -
> > -	if (read_bootsectandvi(&bs, &volinfo, &fatsize)) {
> > -		printf("\nNo valid FAT fs found\n");
> > -		return 1;
> > -	}
> > -
> > -	memcpy(vol_label, volinfo.volume_label, 11);
> > -	vol_label[11] = '\0';
> > -	volinfo.fs_type[5] = '\0';
> > -
> > -	printf("Partition %d: Filesystem: %s \"%s\"\n", cur_part_nr,
> > -		volinfo.fs_type, vol_label);
> > -
> > -	return 0;
> > -}
> > -
> >  int file_fat_ls(const char *dir)
> >  {
> >  	return do_fat_read(dir, NULL, 0, LS_YES);
> > diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> > index a6181e7..2e5bda9 100644
> > --- a/fs/fat/fat_write.c
> > +++ b/fs/fat/fat_write.c

Please get rid of the '#include "fat.c"'. This is the perfect opportunity to do
so.

> > @@ -43,17 +43,12 @@ static void uppercase(char *str, int len)
> >  static int total_sector;
> >  static int disk_write(__u32 block, __u32 nr_blocks, void *buf)
> >  {
> > -	if (!cur_dev || !cur_dev->block_write)
> > -		return -1;
> > -
> > -	if (cur_part_info.start + block + nr_blocks >
> > -		cur_part_info.start + total_sector) {
> > +	if (block + nr_blocks > total_sector) {
> >  		printf("error: overflow occurs\n");
> >  		return -1;
> >  	}
> > 
> > -	return cur_dev->block_write(cur_dev->dev,
> > -			cur_part_info.start + block, nr_blocks,	buf);
> > +	return fat_disk_write(block, nr_blocks, buf);
> >  }

Why don't you move that to dev.c as well?

> > 
> >  /*
> > @@ -196,7 +191,7 @@ static __u32 get_fatent_value(fsdata *mydata,
> > __u32
> > entry) return -1;
> >  		}
> > 
> > -		if (disk_read(startblock, getsize, bufptr) < 0) {
> > +		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
> >  			debug("Error reading FAT blocks\n");
> >  			return ret;
> >  		}
> > @@ -509,7 +504,7 @@ static int set_fatent_value(fsdata *mydata,
> > __u32 entry,
> > __u32 entry_value) return -1;
> >  		}
> > 
> > -		if (disk_read(startblock, getsize, bufptr) < 0) {
> > +		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
> >  			debug("Error reading FAT blocks\n");
> >  			return -1;
> >  		}
> > @@ -793,7 +788,7 @@ static int check_overflow(fsdata *mydata, __u32
> > clustnum, unsigned long size) if (size % mydata->sect_size)
> >  		sect_num++;
> > 
> > -	if (startsect + sect_num > cur_part_info.start + total_sector)
> > +	if (startsect + sect_num > total_sector)

Are you certain? It depends on if mydata->data_begin and mydata->rootdir_sect
are disk- or partition-relative (I did not check).

Anyway, this is a change of behavior, so that should go into a separate patch.

> >  		return -1;
> > 
> >  	return 0;
> > @@ -932,14 +927,14 @@ static int do_fat_write(const char *filename,
> > void
> > *buffer,
> > 
> >  	dir_curclust = 0;
> > 
> > -	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
> > +	if (fat_read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
> >  		debug("error: reading boot sector\n");
> >  		return -1;
> >  	}
> > 
> >  	total_sector = bs.total_sect;
> >  	if (total_sector == 0)
> > -		total_sector = cur_part_info.size;
> > +		total_sector = fat_get_partsize();

cur_dev was not tested here in the original code.

> > 
> >  	if (mydata->fatsize == 32)
> >  		mydata->fatlength = bs.fat32_length;
> > @@ -977,7 +972,7 @@ static int do_fat_write(const char *filename,
> > void
> > *buffer, return -1;
> >  	}
> > 
> > -	if (disk_read(cursect,
> > +	if (fat_disk_read(cursect,
> >  		(mydata->fatsize == 32) ?
> >  		(mydata->clust_size) :
> >  		PREFETCH_BLOCKS, do_fat_read_block) < 0) {
> > diff --git a/include/fat.h b/include/fat.h
> > index f1b4a0d..98f77a5 100644
> > --- a/include/fat.h
> > +++ b/include/fat.h
> > @@ -213,4 +213,11 @@ const char *file_getfsname(int idx);
> >  int fat_register_device(block_dev_desc_t *dev_desc, int part_no);
> > 
> >  int file_fat_write(const char *filename, void *buffer, unsigned
> >  long
> > maxsize); +
> > +/*internal functions for fs/blockdev API abstraction*/
> > +int fat_disk_read(__u32 block, __u32 nr_blocks, void *buf);
> > +int fat_disk_write(__u32 block, __u32 nr_blocks, void *buf);
> > +int fat_read_bootsectandvi(boot_sector *bs, volume_info *vi, int
> > *fatsize);
> > +int fat_get_blksz(void);
> > +int fat_get_partsize(void);
> >  #endif /* _FAT_H_ */

Since these are FAT-private functions, shouldn't their declarations go to some
private header file in the fs/fat/ folder?

Best regards,
Beno?t