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

[Simon Glass]

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!

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