[U-Boot] [PATCH v3 11/31] blk: Call part_init() in the post_probe() method

[Hannes Schmelzer]

On 12/13/18 8:26 AM, Bin Meng wrote:
> Hi Hannes,
Hi Bin,
> On Thu, Dec 13, 2018 at 3:19 PM Hannes Schmelzer <hannes at schmelzer.or.at> wrote:
>>
>>
>> On 10/15/18 11:21 AM, Bin Meng wrote:
>>> part_init() is currently called in every DM BLK driver, either
>>> in its bind() or probe() method. However we can use the BLK
>>> uclass driver's post_probe() method to do it automatically.
>>>
>>> Update all DM BLK drivers to adopt this change.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>> ---
>>>
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>>    cmd/sata.c                        |  9 ---------
>>>    common/usb_storage.c              |  4 +---
>>>    drivers/block/blk-uclass.c        | 12 ++++++++++++
>>>    drivers/block/ide.c               |  2 --
>>>    drivers/block/sandbox.c           |  2 +-
>>>    drivers/mmc/mmc.c                 |  3 ---
>>>    drivers/nvme/nvme.c               |  1 -
>>>    drivers/scsi/scsi.c               |  1 -
>>>    lib/efi_driver/efi_block_device.c |  2 --
>>>    9 files changed, 14 insertions(+), 22 deletions(-)
>> Hi Bin,
>>
>> sorry for this very late report on this.
>> I Just merged the 'v2019.01-rc1' tag into my branch and noticed that
>> filesystem on my eMMC targets are not working anymore.
>> Bisect showed up, that this commit breaks the stuff.
>>
>> I reviewed the change a bit and i would say that all other block devices
>> should work as known, except MMC, NVME and EFI (i do not have one, so i
>> cannot test).
>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>> index 585951c..d6b9cdc 100644
>>> --- a/drivers/mmc/mmc.c
>>> +++ b/drivers/mmc/mmc.c
>>> @@ -2444,9 +2444,6 @@ static int mmc_startup(struct mmc *mmc)
>>>        bdesc->product[0] = 0;
>>>        bdesc->revision[0] = 0;
>>>    #endif
>>> -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBDISK_SUPPORT)
>>> -     part_init(bdesc);
>>> -#endif
>>>
>>>        return 0;
>>>    }
>> i changed this to:
>>
>> #if (!defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBDISK_SUPPORT)) && \
>>        !defined(CONFIG_BLK)
>>       part_init(bdesc);
>> #endif
> I don't understand. The patch above removed 3 lines. Did you say that
> you added back these 3 lines with additional "!defined(CONFIG_BLK)"?
Yes,
in case if CONFIG_BLK is disabled (meaning blk-uclass.c isn't compiled 
in and does therefore nothing),
the part_init(...) has to be done here.
>
>>> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
>>> index eb6fded..1ee0a0a 100644
>>> --- a/drivers/nvme/nvme.c
>>> +++ b/drivers/nvme/nvme.c
>>> @@ -664,7 +664,6 @@ static int nvme_blk_probe(struct udevice *udev)
>>>        sprintf(desc->vendor, "0x%.4x", pplat->vendor);
>>>        memcpy(desc->product, ndev->serial, sizeof(ndev->serial));
>>>        memcpy(desc->revision, ndev->firmware_rev, sizeof(ndev->firmware_rev));
>>> -     part_init(desc);
>>>
>>>        return 0;
>>>    }
>>>
>>> diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c
>>> index 7b71b4d..3f147cf 100644
>>> --- a/lib/efi_driver/efi_block_device.c
>>> +++ b/lib/efi_driver/efi_block_device.c
>>> @@ -173,8 +173,6 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
>>>                return ret;
>>>        EFI_PRINT("%s: block device '%s' created\n", __func__, bdev->name);
>>>
>>> -     ret = blk_prepare_device(bdev);
>>> -
>>>        /* Create handles for the partions of the block device */
>>>        disks = efi_bl_bind_partitions(handle, bdev);
>>>        EFI_PRINT("Found %d partitions\n", disks);
>> here i cannot see some alternative if CONFIG_BLK is enabled.
>>
>> I don't understand your comments here too...
With alternative i mean, that:
If CONFIG_BLK is disabled within the code here some part_init has to be 
done.
Otherwise Partitions and following filesystems cannot work.
>
>> I know, everything shall be converted to DM since the deadline is quite
>> close.
>> But i also think that upon this day everything should work as expected.
>>
> Regards,
> Bin
cheers,
Hannes