diff mbox

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

Message ID 1349479060-3211-1-git-send-email-swarren@wwwdotorg.org
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Stephen Warren Oct. 5, 2012, 11:17 p.m. UTC 
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(-)

Comments

Tom Rini Oct. 5, 2012, 11:27 p.m. UTC | #1 
-----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-----
Stephen Warren Oct. 5, 2012, 11:36 p.m. UTC | #2 
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.
Tom Rini Oct. 5, 2012, 11:39 p.m. UTC | #3 
-----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-----
Stephen Warren Oct. 5, 2012, 11:46 p.m. UTC | #4 
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.
Tom Rini Oct. 8, 2012, 6:47 p.m. UTC | #5 
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!
diff mbox

Patch

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;