Replace the FAT filesystem code

The existing FAT filesystem implementation in U-Boot has some bugs that are tricky to fix cleanly without significant rework of the code. For example, see:

http://lists.denx.de/pipermail/u-boot/2015-July/221054.html
[PATCH] fat: handle paths that include ../

This series replaces U-Boot's FAT filesystem implementation with the "ff" project; http://elm-chan.org/fsw/ff/00index_e.html. This project appears to be actively maintained, does not have at least the bug referenced in the patch link above, and is quite easy to integrate into U-Boot. An earlier version is also used in Barebox, so presumably it's had some level of testing/exposure there.

Note that this series is based on u-boot/master without the patch linked above applied. I wonder if it's a little risky to apply this series after -rc1; perhaps the patch above should be applied to v2015.10, and this series immediately after the release?

v2:
* buildman passes for everything it can build:
* Implemented FAT functions used only by SPL code.
* Fixed a warning in ff.c.
* Fixed types in integer.h so that >64KiB files are loaded correctly.
* ./test/fs/fs-test.sh passes.
* Tested on a Raspberry Pi as well as sandbox.
* Use a define for a hard-coded array size in ff-uboot.c.

Binary size changes due to this series:
* Text size increases around 1-6KiB depending on target.
* Data size sometimes goes up, sometimes down, depending on target.
* Nearly 200KiB of BSS is saved.

Stephen Warren (10):
disk: support host devices in dev_print()
fat: move to a different directory
fat: add ff.c implementation
fat: apply upstream bugfix to ff.c
fat: ff: add ifdef to avoid unused function warning
fat: ffconf.h changes for U-Boot port
fat: port integer.h to U-Boot types
fat: add U-Boot to ff.c API conversion wrapper
fat: switch to new FAT implementation
fat: remove old implementation

 disk/part.c        |    3 +
 fs/fat/Makefile    |    8 +-
 fs/fat/diskio.h    |   80 +
 fs/fat/fat.c       | 1323 ---------------
 fs/fat/fat_write.c | 1115 -------------
 fs/fat/ff-uboot.c  |  403 +++++
 fs/fat/ff.c        | 4634 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/fat/ff.h        |  350 ++++
 fs/fat/ffconf.h    |  277 ++++
 fs/fat/file.c      |  183 ---
 fs/fat/integer.h   |   35 +
 11 files changed, 5784 insertions(+), 2627 deletions(-)
 create mode 100644 fs/fat/diskio.h
 delete mode 100644 fs/fat/fat.c
 delete mode 100644 fs/fat/fat_write.c
 create mode 100644 fs/fat/ff-uboot.c
 create mode 100644 fs/fat/ff.c
 create mode 100644 fs/fat/ff.h
 create mode 100644 fs/fat/ffconf.h
 delete mode 100644 fs/fat/file.c
 create mode 100644 fs/fat/integer.h

1.9.1

Add a case statement for IF_TYPE_HOST in dev_print() so that it prints the device type rather than the "device type unknown" message.

---
 disk/part.c | 3 +++
 1 file changed, 3 insertions(+)

index 43485c9148b0..4e72257434b6 100644
@@ -179,6 +179,9 @@ void dev_print (block_dev_desc_t *dev_desc)
 	case IF_TYPE_DOC:
 		puts("device type DOC\n");
 		return;
+	case IF_TYPE_HOST:
+		puts("host device\n");
+		break;
 	case IF_TYPE_UNKNOWN:
 		puts("device type unknown\n");
 		return;

1.9.1

A future patch will add a new implementation of the FAT filesystem in fat/. Move the old version to fat_orig/ so that the patches which add the new implementation can add it directly in the final location.

---
 fs/Kconfig                       | 2 +-
 fs/Makefile                      | 4 ++--
 fs/{fat => fat_orig}/Kconfig     | 0
 fs/{fat => fat_orig}/Makefile    | 0
 fs/{fat => fat_orig}/fat.c       | 0
 fs/{fat => fat_orig}/fat_write.c | 0
 fs/{fat => fat_orig}/file.c      | 0
 7 files changed, 3 insertions(+), 3 deletions(-)
 rename fs/{fat => fat_orig}/Kconfig (100%)
 rename fs/{fat => fat_orig}/Makefile (100%)
 rename fs/{fat => fat_orig}/fat.c (100%)
 rename fs/{fat => fat_orig}/fat_write.c (100%)
 rename fs/{fat => fat_orig}/file.c (100%)

index 41bb0b9f3a47..6017a0136012 100644
@@ -8,7 +8,7 @@ source "fs/ext4/Kconfig"
 
 source "fs/reiserfs/Kconfig"
 
-source "fs/fat/Kconfig"
+source "fs/fat_orig/Kconfig"
 
 source "fs/jffs2/Kconfig"
 

index 51d06fccb61e..1614650fb6fe 100644
@@ -7,7 +7,7 @@
 #
 
 ifdef CONFIG_SPL_BUILD
-obj-$(CONFIG_SPL_FAT_SUPPORT) += fat/
+obj-$(CONFIG_SPL_FAT_SUPPORT) += fat_orig/
 obj-$(CONFIG_SPL_EXT_SUPPORT) += ext4/
 else
 obj-y				+= fs.o

@@ -15,7 +15,7 @@ obj-y				+= fs.o
 obj-$(CONFIG_CMD_CBFS) += cbfs/
 obj-$(CONFIG_CMD_CRAMFS) += cramfs/
 obj-$(CONFIG_FS_EXT4) += ext4/
-obj-y += fat/
+obj-y += fat_orig/
 obj-$(CONFIG_CMD_JFFS2) += jffs2/
 obj-$(CONFIG_CMD_REISER) += reiserfs/
 obj-$(CONFIG_SANDBOX) += sandbox/

similarity index 100%
rename from fs/fat/Kconfig
rename to fs/fat_orig/Kconfig

similarity index 100%
rename from fs/fat/Makefile
rename to fs/fat_orig/Makefile

similarity index 100%
rename from fs/fat/fat.c
rename to fs/fat_orig/fat.c

similarity index 100%
rename from fs/fat/fat_write.c
rename to fs/fat_orig/fat_write.c

similarity index 100%
rename from fs/fat/file.c
rename to fs/fat_orig/file.c

1.9.1

These files come from http://elm-chan.org/fsw/ff/ff11.zip, downloaded on 20150804. This is linked from http://elm-chan.org/fsw/ff/00index_e.html.
This commit adds unmodified versions of these files, except that dos2unix has been executed on them all.

---
 fs/fat/diskio.h  |   80 +
 fs/fat/ff.c      | 4635 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/fat/ff.h      |  350 +++++
 fs/fat/ffconf.h  |  271 ++++
 fs/fat/integer.h |   33 +
 5 files changed, 5369 insertions(+)
 create mode 100644 fs/fat/diskio.h
 create mode 100644 fs/fat/ff.c
 create mode 100644 fs/fat/ff.h
 create mode 100644 fs/fat/ffconf.h
 create mode 100644 fs/fat/integer.h

new file mode 100644
index 000000000000..9ca0c7e10689
@@ -0,0 +1,80 @@
+/*-----------------------------------------------------------------------/
+/  Low level disk interface modlue include file   (C)ChaN, 2014          /
+/-----------------------------------------------------------------------*/
+
+#ifndef _DISKIO_DEFINED
+#define _DISKIO_DEFINED
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define _USE_WRITE	1	/* 1: Enable disk_write function */
+#define _USE_IOCTL	1	/* 1: Enable disk_ioctl fucntion */
+
+#include "integer.h"
+
+
+/* Status of Disk Functions */
+typedef BYTE	DSTATUS;
+
+/* Results of Disk Functions */
+typedef enum {
+	RES_OK = 0,		/* 0: Successful */
+	RES_ERROR,		/* 1: R/W Error */
+	RES_WRPRT,		/* 2: Write Protected */
+	RES_NOTRDY,		/* 3: Not Ready */
+	RES_PARERR		/* 4: Invalid Parameter */
+} DRESULT;
+
+
+/*---------------------------------------*/
+/* Prototypes for disk control functions */
+
+
+DSTATUS disk_initialize (BYTE pdrv);
+DSTATUS disk_status (BYTE pdrv);
+DRESULT disk_read (BYTE pdrv, BYTE* buff, DWORD sector, UINT count);
+DRESULT disk_write (BYTE pdrv, const BYTE* buff, DWORD sector, UINT count);
+DRESULT disk_ioctl (BYTE pdrv, BYTE cmd, void* buff);
+
+
+/* Disk Status Bits (DSTATUS) */
+
+#define STA_NOINIT		0x01	/* Drive not initialized */
+#define STA_NODISK		0x02	/* No medium in the drive */
+#define STA_PROTECT		0x04	/* Write protected */
+
+
+/* Command code for disk_ioctrl fucntion */
+
+/* Generic command (Used by FatFs) */
+#define CTRL_SYNC			0	/* Complete pending write process (needed at _FS_READONLY == 0) */
+#define GET_SECTOR_COUNT	1	/* Get media size (needed at _USE_MKFS == 1) */
+#define GET_SECTOR_SIZE		2	/* Get sector size (needed at _MAX_SS != _MIN_SS) */
+#define GET_BLOCK_SIZE		3	/* Get erase block size (needed at _USE_MKFS == 1) */
+#define CTRL_TRIM			4	/* Inform device that the data on the block of sectors is no longer used (needed at _USE_TRIM == 1) */
+
+/* Generic command (Not used by FatFs) */
+#define CTRL_POWER			5	/* Get/Set power status */
+#define CTRL_LOCK			6	/* Lock/Unlock media removal */
+#define CTRL_EJECT			7	/* Eject media */
+#define CTRL_FORMAT			8	/* Create physical format on the media */
+
+/* MMC/SDC specific ioctl command */
+#define MMC_GET_TYPE		10	/* Get card type */
+#define MMC_GET_CSD			11	/* Get CSD */
+#define MMC_GET_CID			12	/* Get CID */
+#define MMC_GET_OCR			13	/* Get OCR */
+#define MMC_GET_SDSTAT		14	/* Get SD status */
+
+/* ATA/CF specific ioctl command */
+#define ATA_GET_REV			20	/* Get F/W revision */
+#define ATA_GET_MODEL		21	/* Get model name */
+#define ATA_GET_SN			22	/* Get serial number */
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif

new file mode 100644
index 000000000000..f254b2a02808

1.9.1

Part of the diff is not shown here. View the raw message.

This is a v0.11 post-release patch #1 (March 9, 2015) downloaded from:
http://elm-chan.org/fsw/ff/patches.html on 20150804.

---
 fs/fat/ff.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

index 30d2e714c8d2..fe7764d6daa2 100644
@@ -2329,14 +2329,11 @@ FRESULT validate (	/* FR_OK(0): The object is valid, !=0: Invalid */
 	FIL *fil = (FIL*)obj;	/* Assuming offset of .fs and .id in the FIL/DIR structure is identical */
 
 
-	if (!fil || !fil->fs || !fil->fs->fs_type || fil->fs->id != fil->id)
+	if (!fil || !fil->fs || !fil->fs->fs_type || fil->fs->id != fil->id || (disk_status(fil->fs->drv) & STA_NOINIT))
 		return FR_INVALID_OBJECT;
 
 	ENTER_FF(fil->fs);		/* Lock file system */
 
-	if (disk_status(fil->fs->drv) & STA_NOINIT)
-		return FR_NOT_READY;
-
 	return FR_OK;
 }
 

1.9.1

Without this, any board that doesn't enable read-write FAT triggers:
fs/fat/ff.c:1331:6: warning: ?gen_numname? defined but not used [-Wunused-function]

... since gen_numname() is not referenced from dir_register() is ifdef'd out.

---
v2: New patch.

---
 fs/fat/ff.c | 2 ++
 1 file changed, 2 insertions(+)

index fe7764d6daa2..c2527d50a506 100644
@@ -1326,6 +1326,7 @@ void fit_lfn (
 /*-----------------------------------------------------------------------*/
 /* Create numbered name                                                  */
 /*-----------------------------------------------------------------------*/
+#if !_FS_READONLY
 #if _USE_LFN
 static
 void gen_numname (

@@ -1378,6 +1379,7 @@ void gen_numname (
 	} while (j < 8);
 }
 #endif
+#endif
 
 
 

1.9.1

Turn on _FS_NORTC: This means we don't have to implement get_fattime() which simplifies life for now.

Automatically set _FS_READONLY based on CONFIG_FAT_WRITE. This requires including <config.h> since we reference CONFIG_*.

Set _USE_LFN to enable long filename handling; an essential feature.

Set _FS_RPATH so that paths components "." and ".." are correctly parsed and handled. This enables paths such as "/extlinux/../foo" to work.

Set _USE_LABEL so that volume labels can be read.

---
 fs/fat/ffconf.h | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

index 7ac4f4443d9c..9f7952a1f8ea 100644
@@ -1,3 +1,5 @@
+#include <config.h>
+
 /*---------------------------------------------------------------------------/
 /  FatFs - FAT file system module configuration file  R0.11 (C)ChaN, 2015
 /---------------------------------------------------------------------------*/

@@ -16,7 +18,11 @@
 /  data transfer. */
 
 
+#ifdef CONFIG_FAT_WRITE
 #define _FS_READONLY	0
+#else
+#define _FS_READONLY	1
+#endif
 /* This option switches read-only configuration. (0:Read/Write or 1:Read-only)
 /  Read-only configuration removes writing API functions, f_write(), f_sync(),
 /  f_unlink(), f_mkdir(), f_chmod(), f_rename(), f_truncate(), f_getfree()

@@ -55,7 +61,7 @@
 /* This option switches fast seek feature. (0:Disable or 1:Enable) */
 
 
-#define _USE_LABEL		0
+#define _USE_LABEL		1
 /* This option switches volume label functions, f_getlabel() and f_setlabel().
 /  (0:Disable or 1:Enable) */
 

@@ -93,7 +99,7 @@
 */
 
 
-#define	_USE_LFN	0
+#define	_USE_LFN	1
 #define	_MAX_LFN	255
 /* The _USE_LFN option switches the LFN feature.
 /

@@ -127,7 +133,7 @@
 /  When _LFN_UNICODE is 0, this option has no effect. */
 
 
-#define _FS_RPATH	0
+#define _FS_RPATH	1
 /* This option configures relative path feature.
 /
 /   0: Disable relative path feature and remove related functions.

@@ -195,7 +201,7 @@
 / System Configurations
 /---------------------------------------------------------------------------*/
 
-#define _FS_NORTC	0
+#define _FS_NORTC	1
 #define _NORTC_MON	2
 #define _NORTC_MDAY	1
 #define _NORTC_YEAR	2015

1.9.1

Hmm, is there some way to wrap volume label support in Kconfig? I'm sure this is useful in some cases, but maybe something that boards that are already on the cusp of their size limit would want off?

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150814/6d1ebbb3/attachment.sig>

It's certainly possible; ffconf.h's definition of _FS_READONLY is already driven by CONFIG_FAT_WRITE in this series.

I'm not sure if it's worth it though; on the RPi, turning off volume label support only saves ~0.5K of .text, 0 .data, and 8 bytes .bss. If so, I can certainly throw in another patch to allow it to be configured.

Yeah, OK, that's not a big enough savings to bother with I think. On the boards where we're seeing a bigger size growth can you investigate a little? Thanks!

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150815/c6dca460/attachment.sig>

I haven't built a lot of boards, but I think that large size increase may have been limited to sandbox. For some reason, its config.mk doesn't contain:

PLATFORM_RELFLAGS += -ffunction-sections -fvisibility=hidden

If I add that to arch/sandbox/config.mk, then I see a much smaller size increase when switching to the new FAT implementation.

There are obviously a few functions that aren't used in ff.c, but aren't ifdef'd out.

ff.c uses a bunch of custom typedefs. Define these in terms of standard types from <linux/types.h>, so they'll automatically be the correct size for any build of U-Boot.

---
v2: Make INT and UINT 32-bit types e.g. so that f_read() can read (or at the least, report reading) more than 64KiB-1 bytes. Now, ./test/fs/fs-test.sh passes.

---
 fs/fat/integer.h | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

index f254b2a02808..15ed7e06dd7c 100644
@@ -5,6 +5,8 @@
 #ifndef _FF_INTEGER
 #define _FF_INTEGER
 
+#include <linux/types.h>
+
 #ifdef _WIN32	/* FatFs development platform */
 
 #include <windows.h>

@@ -13,20 +15,20 @@
 #else			/* Embedded platform */
 
 /* This type MUST be 8 bit */
-typedef unsigned char	BYTE;
+typedef uint8_t		BYTE;
 
 /* These types MUST be 16 bit */
-typedef short			SHORT;
-typedef unsigned short	WORD;
-typedef unsigned short	WCHAR;
+typedef int16_t		SHORT;
+typedef uint16_t	WORD;
+typedef uint16_t	WCHAR;
 
 /* These types MUST be 16 bit or 32 bit */
-typedef int				INT;
-typedef unsigned int	UINT;
+typedef int32_t		INT;
+typedef uint32_t	UINT;
 
 /* These types MUST be 32 bit */
-typedef long			LONG;
-typedef unsigned long	DWORD;
+typedef int32_t		LONG;
+typedef uint32_t	DWORD;
 
 #endif
 

1.9.1

Add a file that converts all the FAT-related APIs used by U-Boot into ff.c calls. This allows ff.c to provide the FAT implementation for U-Boot.

---
v2:
* Implement fat_register_device(), file_fat_read() for SPL FAT usage.
* Use _MAX_LFN to define size of lfname[] in file_fat_ls().

---
 fs/fat/Kconfig    |   0
 fs/fat/Makefile   |   7 +
 fs/fat/ff-uboot.c | 403 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 410 insertions(+)
 create mode 100644 fs/fat/Kconfig
 create mode 100644 fs/fat/Makefile
 create mode 100644 fs/fat/ff-uboot.c

new file mode 100644
index 000000000000..e69de29bb2d1

new file mode 100644
index 000000000000..c527fc2bb401
@@ -0,0 +1,7 @@
+#
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+obj-$(CONFIG_FS_FAT)	+= ff.o
+obj-$(CONFIG_FS_FAT)	+= ff-uboot.o

1.9.1

Modify the U-Boot build system to build the new FAT implementation in fs/fat/ rather than the old implementation in fs/fat_orig/.

---
 fs/Kconfig  | 2 +-
 fs/Makefile | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

index 6017a0136012..41bb0b9f3a47 100644
@@ -8,7 +8,7 @@ source "fs/ext4/Kconfig"
 
 source "fs/reiserfs/Kconfig"
 
-source "fs/fat_orig/Kconfig"
+source "fs/fat/Kconfig"
 
 source "fs/jffs2/Kconfig"
 

index 1614650fb6fe..51d06fccb61e 100644
@@ -7,7 +7,7 @@
 #
 
 ifdef CONFIG_SPL_BUILD
-obj-$(CONFIG_SPL_FAT_SUPPORT) += fat_orig/
+obj-$(CONFIG_SPL_FAT_SUPPORT) += fat/
 obj-$(CONFIG_SPL_EXT_SUPPORT) += ext4/
 else
 obj-y				+= fs.o

@@ -15,7 +15,7 @@ obj-y				+= fs.o
 obj-$(CONFIG_CMD_CBFS) += cbfs/
 obj-$(CONFIG_CMD_CRAMFS) += cramfs/
 obj-$(CONFIG_FS_EXT4) += ext4/
-obj-y += fat_orig/
+obj-y += fat/
 obj-$(CONFIG_CMD_JFFS2) += jffs2/
 obj-$(CONFIG_CMD_REISER) += reiserfs/
 obj-$(CONFIG_SANDBOX) += sandbox/

1.9.1

Now that we've switched to the new FAT filesystem implementation, delete the old implementation.

---
 fs/fat_orig/Kconfig     |    0
 fs/fat_orig/Makefile    |   11 -
 fs/fat_orig/fat.c       | 1323 -----------------------------------------------
 fs/fat_orig/fat_write.c | 1115 ---------------------------------------
 fs/fat_orig/file.c      |  183 -------
 5 files changed, 2632 deletions(-)
 delete mode 100644 fs/fat_orig/Kconfig
 delete mode 100644 fs/fat_orig/Makefile
 delete mode 100644 fs/fat_orig/fat.c
 delete mode 100644 fs/fat_orig/fat_write.c
 delete mode 100644 fs/fat_orig/file.c

deleted file mode 100644
index e69de29bb2d1..000000000000

deleted file mode 100644
index b60e8486c48d..000000000000
@@ -1,11 +0,0 @@
-#
-#
-# SPDX-License-Identifier:	GPL-2.0+
-#
-
-obj-$(CONFIG_FS_FAT)	:= fat.o
-obj-$(CONFIG_FAT_WRITE):= fat_write.o
-
-ifndef CONFIG_SPL_BUILD
-obj-$(CONFIG_FS_FAT)	+= file.o
-endif

1.9.1

Hi Stephen,

This seems like a good idea to me. Are there some standard tests for FAT that we can bring in also? Perhaps using sandbox? At present we have fs-test.sh which is a start.

Sounds good.

Regards,
Simon

This series does pass fs-test.sh, which caught a couple of bugs. It would be nice to ensure that the image fs-test.sh creates cause all of FAT12/16/32 to be parsed. It'd also be nice to extend it with a check for the bug that triggered me to start fiddling with FAT; /foo/../xxx where xxx is past the first sector (cluster?) of the root directory. We could likely test for volume ID retrieval, ls output, ... too. Many options to extend the test; all will take time though.

So, yes. I can see this being a useful way forward for dealing with FAT.

Yes, it's too late for v2015.10 but we can put this in for the next release assuming there's enough testing and agreement and so forth.

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150814/35f284d4/attachment.sig>