Re: [U-Boot] [PATCH v1] DOS_PBR block type is also valid dos block type.

Hi Stephen,
On Tue, Mar 12, 2013 at 11:09 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 03/11/2013 08:57 PM, Sonic Zhang wrote:
Hi Stephen,
On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote:
From: Sonic Zhang sonic.zhang@analog.com
- Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos().
What problem does this solve?
I don't believe this change is correct. The purpose of test_part_dos() is to determine whether a block device contains an MS-DOS partition table.
Such a partition table is present in an MBR, but not a PBR. A PBR contains a *FAT file-system, and does not include a partition table.
The SD card formated by windows 7 into one FAT partition can't be initialized correct in u-boot function init_part() after you reuses function test_block_type() in function test_part_dos(). So, files on that partition can't be displayed when run command "fatls mmc 0".
The only difference in your change is to mark dos partition with flag DOS_PBR invalid.
I did test a raw FAT filesystem on an SD card without any partition table, and it worked fine. Admittedly I created the layout/filesystem with Linux rather than Windows, but I don't think the layout would be any difference. What if you "fatls mmc 0:0" rather than "fatls mmc 0"; does that make any difference?
"fatls mmc 0:0" makes no difference.
Regards,
Sonic

On 03/12/2013 08:58 PM, Sonic Zhang wrote:
Hi Stephen,
On Tue, Mar 12, 2013 at 11:09 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 03/11/2013 08:57 PM, Sonic Zhang wrote:
Hi Stephen,
On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote:
From: Sonic Zhang sonic.zhang@analog.com
- Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos().
What problem does this solve?
I don't believe this change is correct. The purpose of test_part_dos() is to determine whether a block device contains an MS-DOS partition table.
Such a partition table is present in an MBR, but not a PBR. A PBR contains a *FAT file-system, and does not include a partition table.
The SD card formated by windows 7 into one FAT partition can't be initialized correct in u-boot function init_part() after you reuses function test_block_type() in function test_part_dos(). So, files on that partition can't be displayed when run command "fatls mmc 0".
The only difference in your change is to mark dos partition with flag DOS_PBR invalid.
I did test a raw FAT filesystem on an SD card without any partition table, and it worked fine. Admittedly I created the layout/filesystem with Linux rather than Windows, but I don't think the layout would be any difference. What if you "fatls mmc 0:0" rather than "fatls mmc 0"; does that make any difference?
"fatls mmc 0:0" makes no difference.
I have reproduced this. However, I believe it's not a simple "the code is wrong" issue, but rather some kind of issue with stale state sticking around.
In other words, the following works just fine:
======== Reset the board
Insert an SD card with a raw FAT filesystem; no partitions
Tegra20 (SeaBoard) # ls mmc 1 3637576 zimage 0 raw-fat-no-partititions
2 file(s), 0 dir(s) ========
(I have a file named raw-fat-no-partitions on the card so I can easily identify it)
However, the following fails:
======== Reset the board
Insert an SD card with DOS partition table, two partitions, each containing a FAT filesystem
Tegra20 (SeaBoard) # ls mmc 1 3637576 zimage 0 part-id-1
2 file(s), 0 dir(s)
Insert an SD card with a raw FAT filesystem; no partitions
Tegra20 (SeaBoard) # mmc rescan 1 Tegra20 (SeaBoard) # ls mmc 1 ** Can't read partition table on 1:0 ** ** Invalid partition 1 ** ========
(Again, I have a file named part-id-1 on the other card so I can easily identify it)
This reproduces all the way back to at least commit d1efb64 "disk: part_dos: don't claim whole-disk FAT filesystems", which is where I fixed raw-FAT-filesystem-without-partition-tables, and is the change you wanted to revert.
I'll see if I can track down what's going on.

On 03/13/2013 10:51 AM, Stephen Warren wrote:
On 03/12/2013 08:58 PM, Sonic Zhang wrote:
Hi Stephen,
On Tue, Mar 12, 2013 at 11:09 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 03/11/2013 08:57 PM, Sonic Zhang wrote:
Hi Stephen,
On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote:
From: Sonic Zhang sonic.zhang@analog.com
- Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos().
What problem does this solve?
I don't believe this change is correct. The purpose of test_part_dos() is to determine whether a block device contains an MS-DOS partition table.
Such a partition table is present in an MBR, but not a PBR. A PBR contains a *FAT file-system, and does not include a partition table.
The SD card formated by windows 7 into one FAT partition can't be initialized correct in u-boot function init_part() after you reuses function test_block_type() in function test_part_dos(). So, files on that partition can't be displayed when run command "fatls mmc 0".
The only difference in your change is to mark dos partition with flag DOS_PBR invalid.
I did test a raw FAT filesystem on an SD card without any partition table, and it worked fine. Admittedly I created the layout/filesystem with Linux rather than Windows, but I don't think the layout would be any difference. What if you "fatls mmc 0:0" rather than "fatls mmc 0"; does that make any difference?
"fatls mmc 0:0" makes no difference.
I have reproduced this. However, I believe it's not a simple "the code is wrong" issue, but rather some kind of issue with stale state sticking around.
Oh, actually perhaps I haven't. What is the exact error message that you see?
If I apply your patch, it doesn't solve the problem that I described; I suspect the problem you're seeing is something different.
participants (2)
-
Sonic Zhang
-
Stephen Warren