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

[Hannes Schmelzer]

On 12/13/18 8:40 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.
> OK, now I understand. But you should convert your board to use DM MMC
> instead. It's the right timing now.
OK, thats already on my plan to do this since the deadline becomes closer.
So we let the "bug" as it is and force people to convert more quickly ;-)

>
>>>>> 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.
> No, this one I still don't understand. The EFI block device is a DM
> driver. It cannot work when CONFIG_BLK is disabled.
OK. I don't catched that the EFI cannot work without DM,
i just took notice that there is no "non-DM" alternative.
So it is OK as it is.