[U-Boot] [PATCH 1/2] FAT: check for partition 0 not 1 for whole-disk fs

[U-Boot] [PATCH 1/2] FAT: check for partition 0 not 1 for whole-disk fs

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

The recent switch to use get_device_and_partition() from do_fat_ls()
broke the ability to access a FAT filesystem directly on a whole device;
FAT only works within a partition on a device.

This change makes e.g. "fatls mmc 0:0" work; explicitly requesting
partition ID 0 is something that get_device_and_partition() fully
supports. However, fat_register_device() expects partition ID 1 to be
used in the full-disk case; partition ID 1 was previously implicitly
specified when the user didn't actually specify a partition ID. Update
fat_register_device() to expect the correct ID.

This change does imply that if a user explicitly executes "fatls mmc 0:1"
then this will fail, and may be a change in behaviour.

Note that this still prevents "fatls mmc 0:auto" from working. The next
patch will fix that.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Tom, this series is really a bug-fix and should go in before the 9-long
series I posted earlier today. I'll need to rebase that other series on
top of this and repost, once any comments are addressed.
---
 fs/fat/fat.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 41ae15e..80156c8 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -85,7 +85,7 @@ int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
 
 	/* Otherwise it might be a superfloppy (whole-disk FAT filesystem) */
 	if (!cur_dev) {
-		if (part_no != 1) {
+		if (part_no != 0) {
 			printf("** Partition %d not valid on device %d **\n",
 					part_no, dev_desc->dev);
 			return -1;
-- 
1.7.0.4

[U-Boot] [PATCH 2/2] disk: part_dos: don't claim whole-disk FAT filesystems

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Logically, a disk that contains a raw FAT filesystem does not in fact
have a partition table. However, test_part_dos() was claiming that such
disks did in fact have a DOS-style partition table. This caused
get_device_and_partition() not to return a whole-disk disk_partition_t,
since part_type != PART_TYPE_UNKNOWN.

part_dos.c's print_partition_extended() detected the raw FAT filesystem
condition and printed a fake partition table that encompassed the whole
disk.

However, part_dos.c's get_partition_info_extended() did not return any
valid partitions in this case. This combination caused
get_device_and_partition() not to find any valid partitions, and hence
to return an error.

Fix test_part_dos() not to claim that raw FAT filesystems are DOS
partition tables. In turn, this causes get_device_and_partition() to
return a whole-disk disk_partition_t, and hence the following commands
work:

fatls mmc 0 /
fatls mmc 0:auto /

An alternative would be to modify print_partition_extended() to detect
raw FAT filesystems, just like print_partition_extended() does, and to
return a fake partition in this case. However, this seems logically
incorrect, and also duplicates code, since get_device_and_partition()
falls back to returning a whole-disk partition when there is no partition
table on the device.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 disk/part_dos.c |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/disk/part_dos.c b/disk/part_dos.c
index c9a3e2b..5c454e6 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -94,12 +94,13 @@ int test_part_dos (block_dev_desc_t *dev_desc)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
 
-	if ((dev_desc->block_read(dev_desc->dev, 0, 1, (ulong *) buffer) != 1) ||
-	    (buffer[DOS_PART_MAGIC_OFFSET + 0] != 0x55) ||
-	    (buffer[DOS_PART_MAGIC_OFFSET + 1] != 0xaa) ) {
-		return (-1);
-	}
-	return (0);
+	if (dev_desc->block_read(dev_desc->dev, 0, 1, (ulong *) buffer) != 1)
+		return -1;
+
+	if (test_block_type(buffer) != DOS_MBR)
+		return -1;
+
+	return 0;
 }
 
 /*  Print a partition that is relative to its Extended partition table
@@ -117,17 +118,13 @@ static void print_partition_extended (block_dev_desc_t *dev_desc, int ext_part_s
 		return;
 	}
 	i=test_block_type(buffer);
-	if(i==-1) {
+	if (i != DOS_MBR) {
 		printf ("bad MBR sector signature 0x%02x%02x\n",
 			buffer[DOS_PART_MAGIC_OFFSET],
 			buffer[DOS_PART_MAGIC_OFFSET + 1]);
 		return;
 	}
-	if(i==DOS_PBR) {
-		printf ("    1\t\t         0\t%10ld\t%2x\n",
-			dev_desc->lba, buffer[DOS_PBR_MEDIA_TYPE_OFFSET]);
-		return;
-	}
+
 	/* Print all primary/logical partitions */
 	pt = (dos_partition_t *) (buffer + DOS_PART_TBL_OFFSET);
 	for (i = 0; i < 4; i++, pt++) {
-- 
1.7.0.4

[U-Boot] [U-Boot, 2/2] disk: part_dos: don't claim whole-disk FAT filesystems

[Tom Rini]

On Fri, Oct 05, 2012 at 01:17:40PM -0000, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> Logically, a disk that contains a raw FAT filesystem does not in fact
> have a partition table. However, test_part_dos() was claiming that such
> disks did in fact have a DOS-style partition table. This caused
> get_device_and_partition() not to return a whole-disk disk_partition_t,
> since part_type != PART_TYPE_UNKNOWN.
> 
> part_dos.c's print_partition_extended() detected the raw FAT filesystem
> condition and printed a fake partition table that encompassed the whole
> disk.
> 
> However, part_dos.c's get_partition_info_extended() did not return any
> valid partitions in this case. This combination caused
> get_device_and_partition() not to find any valid partitions, and hence
> to return an error.
> 
> Fix test_part_dos() not to claim that raw FAT filesystems are DOS
> partition tables. In turn, this causes get_device_and_partition() to
> return a whole-disk disk_partition_t, and hence the following commands
> work:
> 
> fatls mmc 0 /
> fatls mmc 0:auto /
> 
> An alternative would be to modify print_partition_extended() to detect
> raw FAT filesystems, just like print_partition_extended() does, and to
> return a fake partition in this case. However, this seems logically
> incorrect, and also duplicates code, since get_device_and_partition()
> falls back to returning a whole-disk partition when there is no partition
> table on the device.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Applied to u-boot/master, thanks!

-- 
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/20121008/cdc79a84/attachment.pgp>

[U-Boot] [PATCH 1/2] FAT: check for partition 0 not 1 for whole-disk fs

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/05/12 16:17, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> The recent switch to use get_device_and_partition() from
> do_fat_ls() broke the ability to access a FAT filesystem directly
> on a whole device; FAT only works within a partition on a device.
> 
> This change makes e.g. "fatls mmc 0:0" work; explicitly requesting 
> partition ID 0 is something that get_device_and_partition() fully 
> supports. However, fat_register_device() expects partition ID 1 to
> be used in the full-disk case; partition ID 1 was previously
> implicitly specified when the user didn't actually specify a
> partition ID. Update fat_register_device() to expect the correct
> ID.
> 
> This change does imply that if a user explicitly executes "fatls
> mmc 0:1" then this will fail, and may be a change in behaviour.

So wait, you can't list device 0, bus 1 after this patch?

> Note that this still prevents "fatls mmc 0:auto" from working. The
> next patch will fix that.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- Tom, this
> series is really a bug-fix and should go in before the 9-long 
> series I posted earlier today. I'll need to rebase that other
> series on top of this and repost, once any comments are addressed.

Should this also go in the release?

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQb2zkAAoJENk4IS6UOR1WbOoP/jHEC/4KcnWN8xPES0T35Ji1
GwJUdc2O+myjoxNgZaANPWBc8Mcy8Qq+QSSx/gti2BhhCww8ly6IuoVBr7unHAF2
syPoRGw3MP6av67MCPItcycIH6Ym/Y1ZZg+SdZHMJzDIAt7L8u7t5R21sym74iS9
xdamVX98ew6ZwUFHv5AhCt+47CqzAjugU3H1lYuer8RXcgX+kcHeNDVUHBmqNA69
HuysditT3UQWtE2Wcmq98xI3Mvd7mRFUdx6+BFla+lfh2C9/OfWoVzipMP84d9xI
k61VAT3ZpMd5maL4SLy+k7E92bMGN0wvxD2cvlCLnEeKrcd7c6+CZqDMFRmdtiPI
43rklb9ZX1K2vjQzIZL3GOOEGEHQvJ7mTcWkQCHeyyrJaRkx3uChPpIyaar3y1/Y
8ezqO4iVqhiY5YJyUd2v1Kj28S0ilCqxKVl/dt5yzde9EGzsOpDoCHmMCQTzfOiD
NhZyiyMtg8IzEhCaaqKL975O6JlsHPMNFmsESAkFExl+LYRYHq1itCZUPkrH400p
gV/72J7K5q+Z8o+iCp68glL2BMLIj6Vz3aGDngQIziKJJQtksVHcshpuFyDYqaB4
ebgE3AMQJnOBllLPlf+BZXAZpCCL5zDY1kO8pEpp7MRNgEEsUIOKb//E6wDP5PUL
7+kooEgl/EbnUpQZmfEb
=goTb
-----END PGP SIGNATURE-----

[U-Boot] [PATCH 1/2] FAT: check for partition 0 not 1 for whole-disk fs

[Stephen Warren]

On 10/05/2012 05:27 PM, Tom Rini wrote:
> On 10/05/12 16:17, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
> 
>> The recent switch to use get_device_and_partition() from 
>> do_fat_ls() broke the ability to access a FAT filesystem
>> directly on a whole device; FAT only works within a partition on
>> a device.
> 
>> This change makes e.g. "fatls mmc 0:0" work; explicitly
>> requesting partition ID 0 is something that
>> get_device_and_partition() fully supports. However,
>> fat_register_device() expects partition ID 1 to be used in the
>> full-disk case; partition ID 1 was previously implicitly
>> specified when the user didn't actually specify a partition ID.
>> Update fat_register_device() to expect the correct ID.
> 
>> This change does imply that if a user explicitly executes "fatls 
>> mmc 0:1" then this will fail, and may be a change in behaviour.
> 
> So wait, you can't list device 0, bus 1 after this patch?

That's partition 1 not bus 1.

In the context of having a raw FAT filesystem on a device with no
partition table, the partition specification "0:1" doesn't work before
or after this patch; I believe (if it worked at all ever before) it
was broken by the previous get_device_and_partition() rework.

If you do have a partition table, then "0:1" works just fine
with/without this patch.

>> Note that this still prevents "fatls mmc 0:auto" from working.
>> The next patch will fix that.
> 
>> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- Tom, this 
>> series is really a bug-fix and should go in before the 9-long 
>> series I posted earlier today. I'll need to rebase that other 
>> series on top of this and repost, once any comments are
>> addressed.
> 
> Should this also go in the release?

I imagine so; that's why I rebased it below the previous series I sent
which I assume isn't going into this release.

[U-Boot] [PATCH 1/2] FAT: check for partition 0 not 1 for whole-disk fs

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/05/12 16:36, Stephen Warren wrote:
> On 10/05/2012 05:27 PM, Tom Rini wrote:
>> On 10/05/12 16:17, Stephen Warren wrote:
>>> From: Stephen Warren <swarren@nvidia.com>
>> 
>>> The recent switch to use get_device_and_partition() from 
>>> do_fat_ls() broke the ability to access a FAT filesystem 
>>> directly on a whole device; FAT only works within a partition
>>> on a device.
>> 
>>> This change makes e.g. "fatls mmc 0:0" work; explicitly 
>>> requesting partition ID 0 is something that 
>>> get_device_and_partition() fully supports. However, 
>>> fat_register_device() expects partition ID 1 to be used in the 
>>> full-disk case; partition ID 1 was previously implicitly 
>>> specified when the user didn't actually specify a partition
>>> ID. Update fat_register_device() to expect the correct ID.
>> 
>>> This change does imply that if a user explicitly executes
>>> "fatls mmc 0:1" then this will fail, and may be a change in
>>> behaviour.
>> 
>> So wait, you can't list device 0, bus 1 after this patch?
> 
> That's partition 1 not bus 1.

Er yes, thinko there.

> In the context of having a raw FAT filesystem on a device with no 
> partition table, the partition specification "0:1" doesn't work
> before or after this patch; I believe (if it worked at all ever
> before) it was broken by the previous get_device_and_partition()
> rework.
> 
> If you do have a partition table, then "0:1" works just fine 
> with/without this patch.

OK, so the behavior change here, potentially involves 2 partitions
without a partition table?  What is the case where fatls mmc 0:1 would
fail now?

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQb2/IAAoJENk4IS6UOR1WKbEQALH+HYDHhJeA0EYf8iypop65
2FIekgWaT2lrlQNEowEVGHp7kenHPVgQ8EjbE3TzymWS3QLdD7V9gCJqOXa5ozqQ
Nw/sFTcw8j/u83yy7mby0scwxkc3ce0PNly1JH4+L1axKuPUdcXWa52ITy4W1XmZ
348xamJQktrWOUaaUJRMaQP7qCWUNiBZniCTKUIhJhZITEch/JHoF7QkY4PM082X
pZB3wDjQJ8CdnsdutzeegiHLwLRm4bWjfy159uvTQePoSx0NJdSsJpU/Zxf+0t6i
CCf8rvYz1vKiH2u7D2cWuB61Cg/VwCfhN+wEGL4FSvKSh1zUsKa9fJtHtpw/ueB8
/iAi8dKA1cGzr5Ym0aUrz9UL2ZZ6IO4HTrNW5xGRK8TsbgL+kcgSzF8C9V5T2WF9
J++Ol5yGIjgqHPf/H7pulQ0uJ5NU70JbUB5AYLc5FWW+rULJP8rECQpSC74F3fo0
4HfoOrry/IwCBRSHE1crWIyvP+DmnDEHKX7XEhh8P3nqagwhF9LlSQwcEhATjrBp
wEeqZD8A7hViqjuz8aG5J7qVxUPOB1VfzwTzNM6zKMgHbizahmbokcYLFHpORoQ8
YRNjdxgksWAtVSizJlfxOa4D7/GV4pLXUJhqfUYIk8afkbt48YuOUbtwApkLSNLN
QM3oQOs6y1RK8qy+wBHq
=bUng
-----END PGP SIGNATURE-----

[U-Boot] [PATCH 1/2] FAT: check for partition 0 not 1 for whole-disk fs

[Stephen Warren]

On 10/05/2012 05:39 PM, Tom Rini wrote:
> On 10/05/12 16:36, Stephen Warren wrote:
>> On 10/05/2012 05:27 PM, Tom Rini wrote:
>>> On 10/05/12 16:17, Stephen Warren wrote:
>>>> From: Stephen Warren <swarren@nvidia.com>
>>> 
>>>> The recent switch to use get_device_and_partition() from 
>>>> do_fat_ls() broke the ability to access a FAT filesystem 
>>>> directly on a whole device; FAT only works within a
>>>> partition on a device.
>>> 
>>>> This change makes e.g. "fatls mmc 0:0" work; explicitly 
>>>> requesting partition ID 0 is something that 
>>>> get_device_and_partition() fully supports. However, 
>>>> fat_register_device() expects partition ID 1 to be used in
>>>> the full-disk case; partition ID 1 was previously implicitly
>>>>  specified when the user didn't actually specify a partition 
>>>> ID. Update fat_register_device() to expect the correct ID.
>>> 
>>>> This change does imply that if a user explicitly executes 
>>>> "fatls mmc 0:1" then this will fail, and may be a change in 
>>>> behaviour.
>>> 
>>> So wait, you can't list device 0, bus 1 after this patch?
> 
>> That's partition 1 not bus 1.
> 
> Er yes, thinko there.
> 
>> In the context of having a raw FAT filesystem on a device with no
>>  partition table, the partition specification "0:1" doesn't work 
>> before or after this patch; I believe (if it worked at all ever 
>> before) it was broken by the previous get_device_and_partition() 
>> rework.
> 
>> If you do have a partition table, then "0:1" works just fine 
>> with/without this patch.
> 
> OK, so the behavior change here, potentially involves 2 partitions 
> without a partition table?  What is the case where fatls mmc 0:1
> would fail now?

The failure case involves zero partitions. The issue is that the
following works just fine:

fdisk /dev/sda
mkfs.dos /dev/sdaN    for any N

but the following fails:

mkfs.dos /dev/sda (i.e. raw on the disk with no partition table)

In the latter case, I /think/ either of the following might have
worked in U-Boot until recently:

(1) fatls mmc 0 /       # Surely this worked
(2) fatls mmc 0:1 /     # I'm not sure if this works

This patch makes (1) above work.

Patch 2 in this series makes the new equivalent of (2) work, namely:

fatls mmc 0:0 /

The second 0 there is "partition 0" which means "the whole disk".
IIRC, I called out that behaviour change in the
get_device_and_partition() series anyway.

[U-Boot] [U-Boot, 1/2] FAT: check for partition 0 not 1 for whole-disk fs

[Tom Rini]

On Fri, Oct 05, 2012 at 01:17:39PM -0000, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> The recent switch to use get_device_and_partition() from do_fat_ls()
> broke the ability to access a FAT filesystem directly on a whole device;
> FAT only works within a partition on a device.
> 
> This change makes e.g. "fatls mmc 0:0" work; explicitly requesting
> partition ID 0 is something that get_device_and_partition() fully
> supports. However, fat_register_device() expects partition ID 1 to be
> used in the full-disk case; partition ID 1 was previously implicitly
> specified when the user didn't actually specify a partition ID. Update
> fat_register_device() to expect the correct ID.
> 
> This change does imply that if a user explicitly executes "fatls mmc 0:1"
> then this will fail, and may be a change in behaviour.
> 
> Note that this still prevents "fatls mmc 0:auto" from working. The next
> patch will fix that.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> ---
> Tom, this series is really a bug-fix and should go in before the 9-long
> series I posted earlier today. I'll need to rebase that other series on
> top of this and repost, once any comments are addressed.

Applied to u-boot/master, thanks!

-- 
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/20121008/333d45e1/attachment.pgp>