[PATCH 1/1] blk: simplify blk_get_devnum_by_typename()

[Heinrich Schuchardt]

On 10/25/21 21:37, Heinrich Schuchardt wrote:
> 
> 
> On 10/25/21 21:03, Simon Glass wrote:
>> Hi Heinrich,
>>
>> On Mon, 25 Oct 2021 at 12:41, Heinrich Schuchardt
>> <heinrich.schuchardt at canonical.com> wrote:
>>>
>>>
>>>
>>> On 10/25/21 19:29, Simon Glass wrote:
>>>> Hi Heinrich,
>>>>
>>>> On Mon, 25 Oct 2021 at 09:44, Heinrich Schuchardt
>>>> <heinrich.schuchardt at canonical.com> wrote:
>>>>>
>>>>> On 10/25/21 17:18, Simon Glass wrote:
>>>>>> Hi Heinrich,
>>>>>>
>>>>>> On Mon, 25 Oct 2021 at 02:00, Heinrich Schuchardt
>>>>>> <heinrich.schuchardt at canonical.com> wrote:
>>>>>>>
>>>>>>> On 10/25/21 09:54, Heinrich Schuchardt wrote:
>>>>>>>> On 10/24/21 21:54, Simon Glass wrote:
>>>>>>>>> Hi Heinrich,
>>>>>>>>>
>>>>>>>>> On Sat, 23 Oct 2021 at 08:06, Heinrich Schuchardt
>>>>>>>>> <heinrich.schuchardt at canonical.com> wrote:
>>>>>>>>>>
>>>>>>>>>> The block descriptor contains the if_type. There is no need to 
>>>>>>>>>> first
>>>>>>>>>> look
>>>>>>>>>> up the uclass for the if_type and then to check the parent 
>>>>>>>>>> device's
>>>>>>>>>> uclass
>>>>>>>>>> to know if the device has the correct if_type.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Heinrich Schuchardt 
>>>>>>>>>> <heinrich.schuchardt at canonical.com>
>>>>>>>>>> ---
>>>>>>>>>>      drivers/block/blk-uclass.c | 35 
>>>>>>>>>> +----------------------------------
>>>>>>>>>>      1 file changed, 1 insertion(+), 34 deletions(-)
>>>>>>>>>
>>>>>>>>> This seems to be heading in the wrong direction though.
>>>>>>>>>
>>>>>>>>> The IF_TYPE should really go away and be replaced with the 
>>>>>>>>> UCLASS ID,
>>>>>>>>> I think.
>>>>>>>>>
>>>>>>>>> At present we have lots of calls to blk_create_device_f() which
>>>>>>>>> specify the type. I think we should drop the IF_TYPE_.. arg to 
>>>>>>>>> that
>>>>>>>>> function and have it figured out from the uclass, in the interim.
>>>>>>>>>
>>>>>>>>> But why do we need IF_TYPE now?
>>>>>>>>
>>>>>>>> I admit that this patch is just an intermediate step.
>>>>>>>>
>>>>>>>> We can replace IF_TYPE by ULASS ID once SPL block devices are 
>>>>>>>> always
>>>>>>>> using the driver model. Have we reached this point yet? I have 
>>>>>>>> not seen
>>>>>>>> drivers/block/blk_legacy.c being deleted.
>>>>>>>
>>>>>>> qemu_malta64, qemu_malta64el, qemu_malta, qemu_maltael are 
>>>>>>> examples of
>>>>>>> defconfigs requiring drivers/block/blk_legacy.c
>>>>>>
>>>>>> Yes, we seem to have BLK on only for MMC and USB, but malta64 (for
>>>>>> example) uses IDE.
>>>>>>
>>>>>> The problem seems to be HAVE_BLOCK_DEVICE which should not be used.
>>>>>>
>>>>>> +Tom Rini
>>>>>>
>>>>>> Should we remove HAVE_BLOCK_DEVICE at this point, or make it 
>>>>>> depend on BROKEN?
>>>>>>
>>>>>>>
>>>>>>> Best regards
>>>>>>>
>>>>>>> Heinrich
>>>>>>>
>>>>>>>>
>>>>>>>> Removing if_type_uclass_id[] is anyway on the right path.
>>>>>>
>>>>>> Are you sure? Instead, could we use the uclass ID in more places?
>>>>>
>>>>> Yes, you want to get rid of if_type. This means the table will become
>>>>> obsolete.
>>>>
>>>> Yes.
>>>>
>>>>> Once the legacy drivers are removed we can replace all occurrences of
>>>>> if_type by uclass_id. That uclass_id we should not take from blk_desc
>>>>> but from udevice.
>>>>
>>>> How do we get from a blk_desc to a udevice, though?
>>>>
>>>> Could you instead look at moving from using blk_desc to using udevice?
>>>> If we had that, I think I can see your point with this patch. At
>>>> present, it looks like a backwards step because you are reducing the
>>>> usage of the uclass and in fact removing it altogether.
>>>>
>>>> Can you see what I am getting at? Let's move towards migration
>>>> incrementally, not destroy the bridges we have built towards the goal.
>>>
>>> Once you move from if_type to uclass_id you will simply replace
>>>
>>>     if (desc->if_type != if_type)
>>>
>>> by
>>>
>>>     if (device_get_uclass_id(dev->uclass->uclass_id) != uclass_id)
>>>
>>> Except for this line this patch brings you to the final code.
>>
>> device_get_uclass_id() BTW
>>
>> But where does uclass_id come from?! You are removing the function
>> that produces it!
> 
> In future you will pass uclass_id as parameter instead of if_type 
> because if_type will be replaced by uclass_id!

uclass_get_by_name() should be able to find the uclass_id if you pass in 
an interface name.

Best regards

Heinrich

> 
>>
>> How about, instead, you update blk_create_devicef() to drop the
>> if_type parameter and just use the device's uclass? That would
>> actually head forwards towards migration, rather than away from it.
>>
>> Regards,
>> Simon
>>