[U-Boot] [PATCH 01/17] fs: fat: extend get_fs_info() for write use

[Heinrich Schuchardt]

On 07/20/2018 08:06 PM, Heinrich Schuchardt wrote:
> On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
>> get_fs_info() was introduced in major re-work of read operation by Rob.
>> We want to reuse this function in write operation by extending it with
>> additional members in fsdata structure.
>>
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>> ---
>>  fs/fat/fat.c  | 3 +++
>>  include/fat.h | 2 ++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>> index 4efe8a3eda..b48f48a751 100644
>> --- a/fs/fat/fat.c
>> +++ b/fs/fat/fat.c
>> @@ -556,6 +556,9 @@ static int get_fs_info(fsdata *mydata)
>>  		return ret;
>>  	}
>>  
>> +	mydata->bs_total_sect = bs.total_sect;

Please, check here if sectors is non-zero and in this case use that
value. Do not rely on total_sect = 0 in later coding to read sectors.
Anyway there should be only one place in the coding where we determine
the sector count.

The same logic is used in the Linux kernel:

fs/fat/inode.c:1766:   total_sectors = bpb.fat_sectors;
fs/fat/inode.c:1767:   if (total_sectors == 0)
fs/fat/inode.c:1768:           total_sectors = bpb.fat_total_sect;

>> +	mydata->bs_fats = bs.fats;
>> +
>>  	if (mydata->fatsize == 32) {
>>  		mydata->fatlength = bs.fat32_length;
>>  	} else {
>> diff --git a/include/fat.h b/include/fat.h
>> index 09e1423685..0c88b59a4a 100644
>> --- a/include/fat.h
>> +++ b/include/fat.h
>> @@ -173,6 +173,8 @@ typedef struct {
>>  	int	fatbufnum;	/* Used by get_fatent, init to -1 */
>>  	int	rootdir_size;	/* Size of root dir for non-FAT32 */
>>  	__u32	root_cluster;	/* First cluster of root dir for FAT32 */
>> +	__u32	bs_total_sect;	/* Boot sector's number of sectors */

Please, use

__u32 total_sect; /* Number of sectors */

> 
> How can one sector have multiple sectors?
> Please, reword the comment to something more obvious.
> 
> Best regards
> 
> Heinrich
> 
>> +	__u8	bs_fats;	/* Boot sector's number of FATs */

and

__u8 fats; /* Number of FATs */

This is not information about the boot sector but about the partition.

Best regards

Heinich

>>  } fsdata;
>>  
>>  static inline u32 clust_to_sect(fsdata *fsdata, u32 clust)
>>
> 
>