[U-Boot] [PATCH v2 2/4] block: ide: Fix block read/write with driver model

[Bin Meng]

Hi Heinrich,

On Mon, Sep 4, 2017 at 10:10 PM, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 09/04/2017 11:08 AM, Bin Meng wrote:
>> This converts the IDE driver to driver model so that block read and
>> write are fully functional.
>>
>> Fixes: b7c6baef ("x86: Convert MMC to driver model")
>> Reported-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>
>> ---
>>
>> Changes in v2:
>> - Fixed 'fatls ide 0' issue
>>
>>  drivers/block/blk-uclass.c |  2 +-
>>  drivers/block/ide.c        | 68 ++++++++++++++++++++++++++++++++++++++++++++++
>>  include/dm/uclass-id.h     |  1 +
>>  3 files changed, 70 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
>> index e5f00dc..8e58580 100644
>> --- a/drivers/block/blk-uclass.c
>> +++ b/drivers/block/blk-uclass.c
>> @@ -26,7 +26,7 @@ static const char *if_typename_str[IF_TYPE_COUNT] = {
>>  };
>>
>>  static enum uclass_id if_type_uclass_id[IF_TYPE_COUNT] = {
>> -     [IF_TYPE_IDE]           = UCLASS_INVALID,
>> +     [IF_TYPE_IDE]           = UCLASS_IDE,
>>       [IF_TYPE_SCSI]          = UCLASS_SCSI,
>>       [IF_TYPE_ATAPI]         = UCLASS_INVALID,
>>       [IF_TYPE_USB]           = UCLASS_MASS_STORAGE,
>> diff --git a/drivers/block/ide.c b/drivers/block/ide.c
>> index ce51153..58b295e 100644
>> --- a/drivers/block/ide.c
>> +++ b/drivers/block/ide.c
>> @@ -827,12 +827,20 @@ void ide_init(void)
>>               ide_ident(&ide_dev_desc[i]);
>>               dev_print(&ide_dev_desc[i]);
>>
>> +#ifndef CONFIG_BLK
>>               if ((ide_dev_desc[i].lba > 0) && (ide_dev_desc[i].blksz > 0)) {
>>                       /* initialize partition type */
>>                       part_init(&ide_dev_desc[i]);
>>               }
>> +#endif
>>       }
>>       WATCHDOG_RESET();
>> +
>> +#ifdef CONFIG_BLK
>> +     struct udevice *dev;
>> +
>> +     uclass_first_device(UCLASS_IDE, &dev);
>> +#endif
>>  }
>>
>>  /* We only need to swap data if we are running on a big endian cpu. */
>> @@ -1147,6 +1155,21 @@ int ide_device_present(int dev)
>>  #endif
>>
>>  #ifdef CONFIG_BLK
>> +static int ide_blk_probe(struct udevice *udev)
>> +{
>> +     struct blk_desc *desc = dev_get_uclass_platdata(udev);
>> +     int devnum = desc->devnum;
>> +
>> +     /* fill in device vendor/product/rev strings */
>> +     strncpy(desc->vendor, ide_dev_desc[devnum].vendor, BLK_VEN_SIZE);
>> +     strncpy(desc->product, ide_dev_desc[devnum].product, BLK_PRD_SIZE);
>> +     strncpy(desc->revision, ide_dev_desc[devnum].revision, BLK_REV_SIZE);
>> +
>> +     part_init(desc);
>> +
>> +     return 0;
>> +}
>> +
>>  static const struct blk_ops ide_blk_ops = {
>>       .read   = ide_read,
>>       .write  = ide_write,
>> @@ -1156,6 +1179,51 @@ U_BOOT_DRIVER(ide_blk) = {
>>       .name           = "ide_blk",
>>       .id             = UCLASS_BLK,
>>       .ops            = &ide_blk_ops,
>> +     .probe          = ide_blk_probe,
>> +};
>> +
>> +static int ide_probe(struct udevice *udev)
>> +{
>> +     struct udevice *blk_dev;
>> +     char name[20];
>> +     int blksz;
>> +     lbaint_t size;
>> +     int i;
>> +     int ret;
>> +
>> +     for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
>> +             if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
>> +                     sprintf(name, "blk#%d", i);
>> +
>> +                     blksz = ide_dev_desc[i].blksz;
>> +                     size = blksz * ide_dev_desc[i].lba;
>> +                     ret = blk_create_devicef(udev, "ide_blk", name,
>> +                                              IF_TYPE_IDE, i,
>> +                                              blksz, size, &blk_dev);
>> +                     if (ret)
>> +                             return ret;
>> +             }
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +U_BOOT_DRIVER(ide) = {
>> +     .name           = "ide",
>> +     .id             = UCLASS_IDE,
>> +     .probe          = ide_probe,
>> +};
>> +
>> +struct pci_device_id ide_supported[] = {
>> +     { PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_IDE << 8, 0xffff00) },
>> +     { }
>> +};
>> +
>> +U_BOOT_PCI_DEVICE(ide, ide_supported);
>> +
>> +UCLASS_DRIVER(ide) = {
>> +     .name           = "ide",
>> +     .id             = UCLASS_IDE,
>>  };
>>  #else
>>  U_BOOT_LEGACY_BLK(ide) = {
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 1a50199..3fc2083 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -41,6 +41,7 @@ enum uclass_id {
>>       UCLASS_I2C_EEPROM,      /* I2C EEPROM device */
>>       UCLASS_I2C_GENERIC,     /* Generic I2C device */
>>       UCLASS_I2C_MUX,         /* I2C multiplexer */
>> +     UCLASS_IDE,             /* IDE device */
>>       UCLASS_IRQ,             /* Interrupt controller */
>>       UCLASS_KEYBOARD,        /* Keyboard input device */
>>       UCLASS_LED,             /* Light-emitting diode (LED) */
>>
>
> Hello Bin,
>
> I assume the same sort of changes as in ide.c is needed in
> drivers/ata/sata.c too.
>
> When booting I see these error messages:
> ** Bad device scsi 0 **
> Error: Invalid Boot Flag (found 0x0000, expected 0xaa55)
>
> qemu-system-x86_64 -m 1G -bios denx/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 \
> -drive file=img,if=scsi,bus=0,unit=6
>

I never used QEMU to enable any SCSI device. Looks this should be a
SCSI device, but you referred to drivers/ata/sata.c. Is that the
correct driver for this device? Does the U-Boot driver work before
(without the commit: b7c6baef "x86: Convert MMC to driver model")?

Regards,
Bin