On 10/25/21 21:03, Simon Glass wrote:
Hi Heinrich,
On Mon, 25 Oct 2021 at 12:41, Heinrich Schuchardt heinrich.schuchardt@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@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@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@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@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!
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