fs/fat: add a parameter: allow_whole_dev to fat_register_device()

PATCH New Josh Wu 4 participants 10 messages Jun 12, 2014 – Jun 14, 2014 VFAT/FAT/msdos filesystem

Share & tools

Josh Wu

author

2014-06-12 05:57 UTC #0 lore raw

PATCH 1/1 fs/fat: add a parameter: allow_whole_dev to fat_register_device()

For SPL in FAT and envrionment load/save in FAT, to support no partition table device (whole device is FAT partition). We need specify the partition number as 0.

But in FAT SPL and environment case, when we specify the partition number as 1, it is reasonable to support the device has no partition table and only a FAT partition.

This patch add a parameter: allow_whole_dev for fat_register_device(). If allow_whole_dev is true, it will enable no partition table device.

In FAT SPL and FAT file environment, we call fat_register_device() with allow_whole_dev as true. Other cases we call it with false.

Signed-off-by Josh Wu <[email protected]>

---
 board/cm5200/fwupdate.c        |    2 +-
 board/esd/common/auto_update.c |    3 +--
 board/mcc200/auto_update.c     |    4 ++--
 common/env_fat.c               |    4 ++--
 common/spl/spl_fat.c           |    2 +-
 fs/fat/fat.c                   |   15 +++++++++++----
 include/fat.h                  |    3 ++-
 7 files changed, 20 insertions(+), 13 deletions(-)

board/cm5200/fwupdate.c +1 -1

index 06d5023..801d82a 100644
@@ -120,7 +120,7 @@ static int load_rescue_image(ulong addr)
 	/* Detect partition */
 	for (partno = -1, i = 0; i < 6; i++) {
 		if (get_partition_info(stor_dev, i, &info) == 0) {
-			if (fat_register_device(stor_dev, i) == 0) {
+			if (fat_register_device(stor_dev, i, false) == 0) {
 				/* Check if rescue image is present */
 				FW_DEBUG("Looking for firmware directory '%s'"
 					" on partition %d\n", fwdir, i);

board/esd/common/auto_update.c +1 -2

index 85c3567..d2dd76a 100644
@@ -30,7 +30,6 @@ extern int N_AU_IMAGES;
 #define MAX_LOADSZ 0x1c00000
 
 /* externals */
-extern int fat_register_device(block_dev_desc_t *, int);
 extern int file_fat_detectfs(void);
 extern long file_fat_read(const char *, void *, unsigned long);
 long do_fat_read (const char *filename, void *buffer,

@@ -390,7 +389,7 @@ int do_auto_update(void)
 		}
 	}
 
-	if (fat_register_device (stor_dev, 1) != 0) {
+	if (fat_register_device(stor_dev, 1, false) != 0) {
 		debug ("Unable to register ide disk 0:1\n");
 		return -1;
 	}

board/mcc200/auto_update.c +2 -2

index 43173ce..6cc12d5 100644
@@ -106,7 +106,7 @@ ulong totsize;
 #define KEYPAD_MASK_HI	((1<<(KEYPAD_COL-1+(KEYPAD_ROW*3-3)))>>8)
 
 /* externals */
-extern int fat_register_device(block_dev_desc_t *, int);
+extern int fat_register_device(block_dev_desc_t *, int, bool);
 extern int file_fat_detectfs(void);
 extern long file_fat_read(const char *, void *, unsigned long);
 extern int i2c_read (unsigned char, unsigned int, int , unsigned char* , int);

@@ -373,7 +373,7 @@ int do_auto_update(void)
 		res = -1;
 		goto xit;
 	}
-	if (fat_register_device(stor_dev, 1) != 0) {
+	if (fat_register_device(stor_dev, 1, false) != 0) {
 		debug ("Unable to use USB %d:%d for fatls\n",
 			au_usb_stor_curr_dev, 1);
 		res = -1;

common/env_fat.c +2 -2

index aad0487..b36e08e 100644
@@ -67,7 +67,7 @@ int saveenv(void)
 		return 1;
 	}
 
-	err = fat_register_device(dev_desc, part);
+	err = fat_register_device(dev_desc, part, true);
 	if (err) {
 		printf("Failed to register %s%d:%d\n",
 			FAT_ENV_INTERFACE, dev, part);

@@ -117,7 +117,7 @@ void env_relocate_spec(void)
 		return;
 	}
 
-	err = fat_register_device(dev_desc, part);
+	err = fat_register_device(dev_desc, part, true);
 	if (err) {
 		printf("Failed to register %s%d:%d\n",
 			FAT_ENV_INTERFACE, dev, part);

common/spl/spl_fat.c +1 -1

index 56be943..c7a4fd6 100644
@@ -25,7 +25,7 @@ static int spl_register_fat_device(block_dev_desc_t *block_dev, int partition)
 	if (fat_registered)
 		return err;
 
-	err = fat_register_device(block_dev, partition);
+	err = fat_register_device(block_dev, partition, true);
 	if (err) {
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		printf("%s: fat register err - %d\n", __func__, err);

fs/fat/fat.c +11 -4

index 54f42ea..ad08cde 100644
@@ -81,7 +81,8 @@ int fat_set_blk_dev(block_dev_desc_t *dev_desc, disk_partition_t *info)
 	return -1;
 }
 
-int fat_register_device(block_dev_desc_t *dev_desc, int part_no)
+int fat_register_device(block_dev_desc_t *dev_desc, int part_no,
+			bool allow_whole_dev)
 {
 	disk_partition_t info;

@@ -91,9 +92,15 @@ int fat_register_device(block_dev_desc_t *dev_desc, int part_no)
 	/* Read the partition table, if present */
 	if (get_partition_info(dev_desc, part_no, &info)) {
 		if (part_no != 0) {
-			printf("** Partition %d not valid on device %d **\n",
-					part_no, dev_desc->dev);
-			return -1;
+			/*
+			 * If part_no = 1, we will support whole partition
+			 * device if allow_whole_dev is true
+			 */
+			if (!(allow_whole_dev && part_no == 1)) {
+				printf("** Partition %d not valid on device %d **\n",
+				       part_no, dev_desc->dev);
+				return -1;
+			}
 		}
 
 		info.start = 0;

include/fat.h +2 -1

index 63cf787..df15945 100644
@@ -203,7 +203,8 @@ long file_fat_read_at(const char *filename, unsigned long pos, void *buffer,
 long file_fat_read(const char *filename, void *buffer, unsigned long maxsize);
 const char *file_getfsname(int idx);
 int fat_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info);
-int fat_register_device(block_dev_desc_t *dev_desc, int part_no);
+int fat_register_device(block_dev_desc_t *dev_desc, int part_no,
+			bool allow_whole_dev);
 
 int file_fat_write(const char *filename, void *buffer, unsigned long maxsize);
 int fat_read_file(const char *filename, void *buf, int offset, int len);

Signature · 1 line

1.7.9.5

Wolfgang Denk

2014-06-12 06:26 UTC #1 lore raw

PATCH 1/1 fs/fat: add a parameter: allow_whole_dev to fat_register_device()

Dear Josh Wu,

Quoted 3 lines · In message <[email protected]> you wrote:

For SPL in FAT and envrionment load/save in FAT, to support no partition table device (whole device is FAT partition). We need specify the partition number as 0.

Sorry, I cannot parse this. What exactly do you mean here?

Quoted 3 lines

But in FAT SPL and environment case, when we specify the partition number as 1, it is reasonable to support the device has no partition table and only a FAT partition.

Why would the expectations in SPL be different from other use cases?

include/fat.h long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, · line 207

Quoted 2 lines

include/fat.h +2 -0

+int fat_register_device(block_dev_desc_t *dev_desc, int part_no,
+			bool allow_whole_dev);

Please make this an "int" type, and use 0 and 1.

Thanks.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The Wright Bothers weren't the first to fly. They were just the first
not to crash.

Josh Wu

author

2014-06-12 07:04 UTC #2 lore raw

PATCH 1/1 fs/fat: add a parameter: allow_whole_dev to fat_register_device()

Dear Wolfgang

Thanks for the review.

Quoted 7 lines · Wolfgang Denk

Dear Josh Wu,
Quoted 3 lines · In message <[email protected]> you wrote:
For SPL in FAT and envrionment load/save in FAT, to support no partition table device (whole device is FAT partition). We need specify the partition number as 0.
Sorry, I cannot parse this. What exactly do you mean here?

Sorry, Let me try to explain it a little bit:
In U-Boot when we access a partition of a device, we use 'ifname dev:part' format.
For instance: 'mmc 0:1' means the MMC card's #1 partition of the device #0.

But for a case if the mmc device has no partition table (MBR), it only has one FAT partition.
To support that case, we need to access by using 'mmc 0:0'.

So the problem is: if we specify mmc 0:0 then I cannot access the mmc device if it has a partition table.
And if we specify mmc 0:1 then I cannot access if it has no partition table.

For the fs layer this case is solved by use 'mmc 0', or 'mmc 0:auto' by commit:
10a37fd7a4 (disk: get_device_and_partition() "auto" partition and cleanup)

But for env fat and SPL fat, we don't use the function in above commit as we use a simpler function fat_register_device().
So this patch make this function works too.

Quoted 5 lines

Quoted 4 lines
But in FAT SPL and environment case, when we specify the partition number as 1, it is reasonable to support the device has no partition table and only a FAT partition.
Why would the expectations in SPL be different from other use cases?

For example, when I use SPL binary in mmc card, I want it to load the
file: u-boot.img from the first partition.
I expect it should work even if the mmc device has no partition table.

But when I define CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION as 1. it cannot work when the mmc has no partition table.

same thing happens for saving environment to a FAT file in MMC.

Quoted 4 lines

include/fat.h long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, · line 207
Quoted 3 lines
include/fat.h +2 -0
+int fat_register_device(block_dev_desc_t *dev_desc, int part_no,
+			bool allow_whole_dev);
Please make this an "int" type, and use 0 and 1.

Is there any special concern for that? like cause machine compatiable issue?

Best Regards,
Josh Wu

Quoted 7 lines

Thanks.
Best regards,
Wolfgang Denk

Wolfgang Denk

2014-06-12 08:52 UTC #3 lore raw

PATCH 1/1 fs/fat: add a parameter: allow_whole_dev to fat_register_device()

Dear Josh Wu,

Quoted 4 lines · In message <[email protected]> you wrote:

In U-Boot when we access a partition of a device, we use 'ifname dev:part' format.
For instance: 'mmc 0:1' means the MMC card's #1 partition of the device #0.

Don;t we also support plain "ifname dev", i. e. without partition specification?

Quoted 2 lines

But for a case if the mmc device has no partition table (MBR), it only has one FAT partition.

Um... No. Without a partition table, there are no partitins at all, so there can be no FAT partitions. I guess you mean there is a FAT file system on the device?

Quoted 1 line

To support that case, we need to access by using 'mmc 0:0'.

I feel that should be "mmc 0".

Quoted 2 lines

So the problem is: if we specify mmc 0:0 then I cannot access the mmc device if it has a partition table.

Well, if it _has_ a partition table, then you should select the correct partition number, and not use the (nonexitent) number 0.

Quoted 1 line

And if we specify mmc 0:1 then I cannot access if it has no partition table.

Correct. Because no partition 1 exists.

Quoted 7 lines

For the fs layer this case is solved by use 'mmc 0', or 'mmc 0:auto' by commit:
10a37fd7a4 (disk: get_device_and_partition() "auto" partition and cleanup)
But for env fat and SPL fat, we don't use the function in above commit as we use a simpler function fat_register_device().
So this patch make this function works too.

I may be wrong, but I think with your patch we do not implement the same behaviour as for get_device_and_partition() [see the commit message for the aforementioned commit how it is supposed to work].

I think, we should implemnt consistent behaviour here.

Quoted 9 lines

Quoted 5 lines
Quoted 4 lines
But in FAT SPL and environment case, when we specify the partition number as 1, it is reasonable to support the device has no partition table and only a FAT partition.
Why would the expectations in SPL be different from other use cases?
For example, when I use SPL binary in mmc card, I want it to load the
file: u-boot.img from the first partition.
I expect it should work even if the mmc device has no partition table.

Please see the description in the commit message how it is supposed to work. Note that it's not just the first partition, but actully the first _bootable_ partition which should get used.

Quoted 4 lines

But when I define CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION as 1. it cannot work when the mmc has no partition table.
same thing happens for saving environment to a FAT file in MMC.

This is even more problematic, as we do not have any definition where the environment would be located. Is it the first partition? Or the first bootable partition? Or a specifically defined one?

I think it is dangerous to just "make it work" without clearly specifying first what the expected behaviour should be.

Quoted 6 lines

Quoted 4 lines
include/fat.h long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, · line 207
Quoted 3 lines
include/fat.h +2 -0
+int fat_register_device(block_dev_desc_t *dev_desc, int part_no,
+			bool allow_whole_dev);
Please make this an "int" type, and use 0 and 1.
Is there any special concern for that? like cause machine compatiable issue?

Boolean values in C are 1 and 0. Hiding these under other names (like "true" and "false") doesn't buy anything.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Everyting looks interesting until you do it. Then you find it's  just
another job.                     - Terry Pratchett, _Moving Pictures_

Josh Wu

author

2014-06-13 03:33 UTC #4 lore raw

PATCH 1/1 fs/fat: add a parameter: allow_whole_dev to fat_register_device()

Dear Wolfgang

Quoted 8 lines · Wolfgang Denk

Dear Josh Wu,
Quoted 3 lines · In message <[email protected]> you wrote:
In U-Boot when we access a partition of a device, we use 'ifname dev:part' format.
For instance: 'mmc 0:1' means the MMC card's #1 partition of the device #0.
Don;t we also support plain "ifname dev", i. e. without partition specification?

The problem is we only support "ifname dev" on command line mode or the filesystem call which calls get_device_and_partition().

For environment save/load and SPL load on FAT, which use fat_register_device() instead of calling get_device_and_partition(), we need specify the partition number. It DOESN'T support "ifname dev" without partition number.
for example:
1. to enable FAT env save/load, we need define:
FAT_ENV_INTERFACE(=mmc), FAT_ENV_DEVICE(=0), FAT_ENV_PART(=1)
2. to enable FAT SPL load, we need fine:
CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION(=1)

Quoted 6 lines

Quoted 3 lines
But for a case if the mmc device has no partition table (MBR), it only has one FAT partition.
Um... No. Without a partition table, there are no partitins at all, so there can be no FAT partitions. I guess you mean there is a FAT file system on the device?

yes, if we cannot find a partition table, then the first sector is assumed as FAT file system's first sector.

Quoted 16 lines

Quoted 2 lines
To support that case, we need to access by using 'mmc 0:0'.
I feel that should be "mmc 0".
Quoted 2 lines
So the problem is: if we specify mmc 0:0 then I cannot access the mmc device if it has a partition table.
Well, if it _has_ a partition table, then you should select the correct partition number, and not use the (nonexitent) number 0.
Quoted 1 line
And if we specify mmc 0:1 then I cannot access if it has no partition table.
Correct. Because no partition 1 exists.
Quoted 4 lines
For the fs layer this case is solved by use 'mmc 0', or 'mmc 0:auto' by commit:
10a37fd7a4 (disk: get_device_and_partition() "auto" partition and cleanup)

Quoted 8 lines

Quoted 3 lines
But for env fat and SPL fat, we don't use the function in above commit as we use a simpler function fat_register_device().
So this patch make this function works too.
I may be wrong, but I think with your patch we do not implement the same behaviour as for get_device_and_partition() [see the commit message for the aforementioned commit how it is supposed to work].
I think, we should implemnt consistent behaviour here.

I agree. I will read the get_device_and_partition() code to understand it.

Quoted 15 lines

Quoted 8 lines
Quoted 5 lines
Quoted 4 lines
But in FAT SPL and environment case, when we specify the partition number as 1, it is reasonable to support the device has no partition table and only a FAT partition.
Why would the expectations in SPL be different from other use cases?
For example, when I use SPL binary in mmc card, I want it to load the
file: u-boot.img from the first partition.
I expect it should work even if the mmc device has no partition table.
Please see the description in the commit message how it is supposed to work. Note that it's not just the first partition, but actully the first _bootable_ partition which should get used.
Quoted 3 lines
But when I define CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION as 1. it cannot work when the mmc has no partition table.

Quoted 7 lines

Quoted 1 line
same thing happens for saving environment to a FAT file in MMC.
This is even more problematic, as we do not have any definition where the environment would be located. Is it the first partition? Or the first bootable partition? Or a specifically defined one?
I think it is dangerous to just "make it work" without clearly specifying first what the expected behaviour should be.

I understand your concern. Thanks a lot for your comments. After the discussing I get a further think of this problem. Let me summary here:

The problem I met is FAT env save/load and FAT spl load use fat_register_device() instead of get_device_and_partition(). So that means I must specify a partition number.

I think for usually user case, we don't want to specify the partition number. that means:
1. if has partition table, please use partition #1.
2. if no partition table, assume the whole device is a FAT file system.

if we agree with above, then the solution should be implement a way to support the case we don't specify a partition number.
1. use get_device_and_partition().
2. add a unspecify partition number support in fat_register_device()

I think #1 is the best way be cause we don't have to implement same things in two place. But I am doubt that the FAT spl can use it. I'll check this.

What do think of that?

Quoted 7 lines

Quoted 5 lines
Quoted 4 lines
include/fat.h long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, · line 207
Quoted 3 lines
include/fat.h +2 -0
+int fat_register_device(block_dev_desc_t *dev_desc, int part_no,
+			bool allow_whole_dev);
Please make this an "int" type, and use 0 and 1.
Is there any special concern for that? like cause machine compatiable issue?
Boolean values in C are 1 and 0. Hiding these under other names (like "true" and "false") doesn't buy anything.

Okay. I just think use bool will be more readable. That also can make people less use an integer number, which in some case it's hard to understand it.

Quoted 5 lines

Best regards,
Wolfgang Denk

Best Regards,
Josh Wu

Stephen Warren

2014-06-13 04:07 UTC #5 lore raw

PATCH 1/1 fs/fat: add a parameter: allow_whole_dev to fat_register_device()

Quoted 10 lines · Josh Wu

Dear Wolfgang
Quoted 7 lines · Wolfgang Denk
Dear Josh Wu,
Quoted 4 lines · In message <[email protected]> you wrote:
In U-Boot when we access a partition of a device, we use 'ifname dev:part' format.
For instance: 'mmc 0:1' means the MMC card's #1 partition of the device #0.

Quoted 5 lines

Quoted 2 lines
Don;t we also support plain "ifname dev", i. e. without partition specification?
The problem is we only support "ifname dev" on command line mode or the filesystem call which calls get_device_and_partition().

Why not just replace the calls to fat_register_device() with calls to get_device_and_partition() (perhaps some wrapper is needed to make the APIs match). That way, someone (U-Boot config file author I suppose) can always specify exactly what they want, without needing any non-deterministic fallbacks.

Josh Wu

author

2014-06-13 04:54 UTC #6 lore raw

PATCH 1/1 fs/fat: add a parameter: allow_whole_dev to fat_register_device()

Hi, Stephen

Quoted 13 lines · Stephen Warren

Quoted 12 lines · Josh Wu
Dear Wolfgang
Quoted 9 lines · Wolfgang Denk
Dear Josh Wu,
Quoted 4 lines · In message <[email protected]> you wrote:
In U-Boot when we access a partition of a device, we use 'ifname dev:part' format.
For instance: 'mmc 0:1' means the MMC card's #1 partition of the device #0.
Don;t we also support plain "ifname dev", i. e. without partition specification?

Quoted 7 lines

Quoted 2 lines
The problem is we only support "ifname dev" on command line mode or the filesystem call which calls get_device_and_partition().
Why not just replace the calls to fat_register_device() with calls to get_device_and_partition() (perhaps some wrapper is needed to make the APIs match). That way, someone (U-Boot config file author I suppose) can always specify exactly what they want, without needing any non-deterministic fallbacks.

yes, that is the exactly I will do. Thanks.

Best Regards,
Josh Wu

Tom Rini

2014-06-14 20:07 UTC #7 lore raw

PATCH 1/1 fs/fat: add a parameter: allow_whole_dev to fat_register_device()

Quoted 16 lines · Josh Wu

Hi, Stephen
Quoted 13 lines · Stephen Warren
Quoted 12 lines · Josh Wu
Dear Wolfgang
Quoted 9 lines · Wolfgang Denk
Dear Josh Wu,
Quoted 4 lines · In message <[email protected]> you wrote:
In U-Boot when we access a partition of a device, we use 'ifname dev:part' format.
For instance: 'mmc 0:1' means the MMC card's #1 partition of the device #0.
Don;t we also support plain "ifname dev", i. e. without partition specification?

Quoted 8 lines

Quoted 7 lines
Quoted 2 lines
The problem is we only support "ifname dev" on command line mode or the filesystem call which calls get_device_and_partition().
Why not just replace the calls to fat_register_device() with calls to get_device_and_partition() (perhaps some wrapper is needed to make the APIs match). That way, someone (U-Boot config file author I suppose) can always specify exactly what they want, without needing any non-deterministic fallbacks.
yes, that is the exactly I will do. Thanks.

Yes, I also agree, thanks!

Signature · 8 lines

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

Wolfgang Denk

2014-06-13 04:38 UTC #8 lore raw

PATCH 1/1 fs/fat: add a parameter: allow_whole_dev to fat_register_device()

In reply to Josh Wu

Dear Josh Wu,

Quoted 11 lines · In message <[email protected]> you wrote:

Quoted 3 lines
Don;t we also support plain "ifname dev", i. e. without partition specification?
The problem is we only support "ifname dev" on command line mode or the filesystem call which calls get_device_and_partition().
For environment save/load and SPL load on FAT, which use fat_register_device() instead of calling get_device_and_partition(), we need specify the partition number. It DOESN'T support "ifname dev" without partition number.

OK. so we have indentified the problem. It seems we should fix that?

Quoted 3 lines

Quoted 1 line
I think, we should implemnt consistent behaviour here.
I agree. I will read the get_device_and_partition() code to understand it.

The commit message of the patch you referred to has a nice description of how it is supposed to work.

Quoted 10 lines

After the discussing I get a further think of this problem. Let me summary here:
The problem I met is FAT env save/load and FAT spl load use fat_register_device() instead of get_device_and_partition(). So that means I must specify a partition number.
I think for usually user case, we don't want to specify the partition number. that means:
1. if has partition table, please use partition #1.

This is different from the non-SPL case then, which uses the first bootable partition instead, which may or may bnot be # 1.

Quoted 8 lines

if we agree with above, then the solution should be implement a way to support the case we don't specify a partition number.
1. use get_device_and_partition().
2. add a unspecify partition number support in fat_register_device()
I think #1 is the best way be cause we don't have to implement same things in two place. But I am doubt that the FAT spl can use it. I'll check this.

Using the same code is also a good way to make sure the behaviour is the same :-)

Quoted 1 line

What do think of that?

Sounds good to me.

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
Quoted 1 line
Please make this an "int" type, and use 0 and 1.
Is there any special concern for that? like cause machine compatiable issue?
Boolean values in C are 1 and 0. Hiding these under other names (like "true" and "false") doesn't buy anything.
Okay. I just think use bool will be more readable. That also can make people less use an integer number, which in some case it's hard to understand it.

None of the related code uses ant bool types so far, so please do not introduce it now.

Thanks.

Best regards,

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There is is no reason for any individual to have a computer in  their
home.      -- Ken Olsen (President of Digital Equipment Corporation),
              Convention of the World Future Society, in Boston, 1977

Tom Rini

2014-06-13 13:40 UTC #9 lore raw

PATCH 1/1 fs/fat: add a parameter: allow_whole_dev to fat_register_device()

In reply to Josh Wu

Quoted 6 lines · Josh Wu

Dear Wolfgang
Quoted 3 lines · Wolfgang Denk
Dear Josh Wu,
In message <[email protected]> you wrote:

I will read and think about the rest of this, but:

[snip]

Quoted 10 lines

Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
include/fat.h long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, · line 207
Quoted 2 lines
include/fat.h +2 -0
+int fat_register_device(block_dev_desc_t *dev_desc, int part_no,
+			bool allow_whole_dev);
Please make this an "int" type, and use 0 and 1.
Is there any special concern for that? like cause machine compatiable issue?
Boolean values in C are 1 and 0. Hiding these under other names (like "true" and "false") doesn't buy anything.
Okay. I just think use bool will be more readable. That also can make people less use an integer number, which in some case it's hard to understand it.

We have bool and make use of bool when things are a boolean, your way here is fine.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140613/601926a4/attachment.pgp>