On 08/31/2017 02:55 PM, Bin Meng wrote:
> Hi Simon,
>
> On Thu, Aug 31, 2017 at 8:52 PM, Simon Glass <s...@chromium.org> wrote:
>> Hi Bin,
>>
>> On 31 August 2017 at 10:53, Bin Meng <bmeng...@gmail.com> wrote:
>>> Hi Heinrich,
>>>
>>> On Thu, Aug 31, 2017 at 5:19 AM, Heinrich Schuchardt <xypron.g...@gmx.de>
>>> wrote:
>>>> On 08/30/2017 06:37 AM, Heinrich Schuchardt wrote:
>>>>>
>>>>>
>>>>> On 08/30/2017 03:54 AM, Bin Meng wrote:
>>>>>> Hi Heinrich,
>>>>>>
>>>>>> On Wed, Aug 30, 2017 at 4:26 AM, Heinrich Schuchardt
>>>>>> <xypron.g...@gmx.de> wrote:
>>>>>>> Hello Simon,
>>>>>>>
>>>>>>> U-Boot HEAD qemu-86_defconfig cannot discover an IDE disk with one FAT
>>>>>>> partition in qemu-system-x86_64.
>>>>>>>
>>>>>>> By bisection I found this patch.
>>>>>>>
>>>>>>> b7c6baef2891ce8978cbfddb66e944943473ac21
>>>>>>> x86: Convert MMC to driver model
>>>>>>>
>>>>>>> With this patch I get
>>>>>>>
>>>>>>> IDE: Bus 0: OK Bus 1: OK
>>>>>>> Device 0: Model: QEMU HARDDISK Firm: 2.5+ Ser#: QM00001
>>>>>>> Type: Hard Disk
>>>>>>> Supports 48-bit addressing
>>>>>>> Capacity: 128.0 MB = 0.1 GB (262144 x 512)
>>>>>>> ** Can't read Driver Desriptor Block **
>>>>>>> Device 1: not available
>>>>>>> Device 2: Model: QEMU Firm: 2.5+ Ser#: QEMU DVD-ROM
>>>>>>> Type: Removable CD ROM
>>>>>>> Capacity: not available
>>>>>>> Device 3: not available
>>>>>>>
>>>>>>> => ide info
>>>>>>> =>
>>>>>>>
>>>>>>> Without the patch I get=> ide info
>>>>>>> Device 0: Model: QEMU HARDDISK Firm: 2.5+ Ser#: QM00001
>>>>>>> Type: Hard Disk
>>>>>>> Supports 48-bit addressing
>>>>>>> Capacity: 128.0 MB = 0.1 GB (262144 x 512)
>>>>>>> Device 2: Model: QEMU Firm: 2.5+ Ser#: QEMU DVD-ROM
>>>>>>> Type: Removable CD ROM
>>>>>>> Capacity: not available
>>>>>>>
>>>>>>> I think we observe two independent errors here:
>>>>>>>
>>>>>>> - The hard disk Device 0 is not read.
>>>>>>> - The ide command stops at the first device that is not available.
>>>>>>>
>>>>>>> I guess only the first is caused by your patch.
>>>>>>
>>>>>> Both logs look fine to me. The "Can't read Driver Desriptor Block"
>>>>>> comes from part_mac.c. Did you verify the actual IDE read/write fails
>>>>>> with current HEAD?
>>>>>>
>>>>>> Regards,
>>>>>> Bin
>>>>>>
>>>>>
>>>>> Hello Bin,
>>>>>
>>>>> I have not checked block level read but used the shell commands for
>>>>> testing.
>>>>>
>>>>> Before the patch I can read the directory of the drive:
>>>>>
>>>>> => ide info
>>>>> Device 0: Model: QEMU HARDDISK Firm: 2.5+ Ser#: QM00001
>>>>> Type: Hard Disk
>>>>> Supports 48-bit addressing
>>>>> Capacity: 128.0 MB = 0.1 GB (262144 x 512)
>>>>> Device 2: Model: QEMU Firm: 2.5+ Ser#: QEMU DVD-ROM
>>>>> Type: Removable CD ROM
>>>>> Capacity: not available
>>>>> => fat2ls ide 0:1
>>>>> Unknown command 'fat2ls' - try 'help'
>>>>> => fatls ide 0:1
>>>>> 164768 snp.efi
>>>>> 0 file1
>>>>> 0 file2
>>>>>
>>>>>
>>>>> After the patch (including HEAD) I cannot read the directory and cannot
>>>>> load the file snp.efi either:
>>>>>
>>>>> => ide info
>>>>> => fatls ide 0:1
>>>>> ** Bad device ide 0 **
>>>>> =>
>>>>> => fatls mmc 0:1
>>>>> ** Bad device mmc 0 **
>>>>>
>>>>>
>>>>> In both cases I have loaded the same image with:
>>>>>
>>>>> export BUILD_ROM=y
>>>>> make distclean && make qemu-x86_defconfig && make -j6
>>>>>
>>>>> qemu-system-x86_64 -m 1G -bios u-boot.rom -nographic \
>>>>> -netdev \
>>>>> user,id=eth0,tftp=tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \
>>>>> -device e1000,netdev=eth0 -machine pc-i440fx-2.8 -hda img
>>>>>
>>>>> Best regards
>>>>>
>>>>> Heinrich
>>>>>
>>>>
>>>> Hello Bin, hello Simon,
>>>>
>>>> I think the bug is in functions ide_init (drivers/block/ide.c).
>>>>
>>>> Platform X86 implies CONFIG_BLK=y.
>>>>
>>>> So we should initialize ide_dev_desc[i].bdev.
>>>>
>>>> We don't, so blk_dread fails after finding no read operation with ENOSYS
>>>> when called from part_test_dos.
>>>>
>>>> The following comment in include/blk.h confirms that bdev has to be filled:
>>>>
>>>> #if CONFIG_IS_ENABLED(BLK)
>>>> /*
>>>> * For now we have a few functions which take struct blk_desc as a
>>>> * parameter. This field allows them to look up the associated
>>>> * device. Once these functions are removed we can drop this field.
>>>> */
>>>> struct udevice *bdev;
>>>> #else
>>>>
>>>> I assume that we have the same issue in in __sata_initialize
>>>> (drivers/ata/sata.c) but have not tested.
>>>>
>>>
>>> Thanks for the testing!
>>>
>>> Simon, are you going to fix this?
>>
>> I am not going to race you to it, if you are thinking of fixing it. I
>> am back from travels in a few days but have a busy week ahead and the
>> release is imminent :-(
>>
>
> Or maybe Heinrich, do you plan to work on a fix? If not, I will put it
> on my todo list.
>
> Regards,
> Bin
>
Hello hello Bin,
I am aware that this bug is release critical. But unfortunately I am
departing on vacation tomorrow.
I guess there should be CONFIG_SYS_IDE_MAXDEVICE times an
U_BOOT_IDE_DEVICE(ide_disk[i]) which should refer to driver
U_BOOT_DRIVER(ide_blk).
Just as prove of concept I append a patch to this mail.
With the patch the IDE partitions are discovered in disk/part.c.
But they are not usable to access the disk because the devices are not
registered in a uclass.
Hopefully you will find the time to fix the problem.
Regards
Heinrich
From 1d00518e03cff89f7e98237500a1a5f857842fb0 Mon Sep 17 00:00:00 2001
From: Heinrich Schuchardt <xypron.g...@gmx.de>
Date: Thu, 31 Aug 2017 19:16:05 +0200
Subject: [PATCH 1/1] foo
Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>
---
drivers/block/ide.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index edcf87b8c1..6d97fdc121 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -34,6 +34,8 @@ static int ide_bus_ok[CONFIG_SYS_IDE_MAXBUS];
struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
+struct udevice ide_dev[CONFIG_SYS_IDE_MAXBUS];
+
#define IDE_TIME_OUT 2000 /* 2 sec timeout */
#define ATAPI_TIME_OUT 7000 /* 7 sec timeout (5 sec seems to work...) */
@@ -771,6 +773,7 @@ void ide_init(void)
{
unsigned char c;
int i, bus;
+ const struct driver *driver = DM_GET_DRIVER(ide_blk);
#ifdef CONFIG_IDE_PREINIT
WATCHDOG_RESET();
@@ -865,7 +868,11 @@ void ide_init(void)
ide_dev_desc[i].log2blksz =
LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
ide_dev_desc[i].lba = 0;
-#ifndef CONFIG_BLK
+#ifdef CONFIG_BLK
+ ide_dev[i].driver = driver;
+ ide_dev[i].uclass_platdata = &ide_dev[i];
+ ide_dev_desc[i].bdev = &ide_dev[i];
+#else
ide_dev_desc[i].block_read = ide_read;
ide_dev_desc[i].block_write = ide_write;
#endif
--
2.11.0
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
