[PATCH 14/45] dm: blk: Add udevice functions

[AKASHI Takahiro]

Hi Simon,

On Wed, Sep 28, 2022 at 04:20:56AM -0600, Simon Glass wrote:
> Hi Takahiro,
> 
> On Sun, 25 Sept 2022 at 18:17, AKASHI Takahiro
> <takahiro.akashi at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Sun, Sep 25, 2022 at 09:02:17AM -0600, Simon Glass wrote:
> > > At present we have functions called blk_dread(), etc., which take a
> > > struct blk_desc * to refer to the block device. Add some functions which
> > > use udevice instead, since this is more in keeping with how driver model
> > > is supposed to work.
> >
> > Unfortunately, NAK.
> > I have already added similar functions in disk/disk-uclass.c
> > with my commit 59da9d4782cd ("dm: disk: add read/write interfaces with
> > udevice"). dev_read()/dev_write() works well with UCLASS_BLK (as intended).
> >
> > I remember that you also ack'ed that patch.
> 
> You have a better memory than me!
> 
> How about we make those functions call my new ones?

So do you want to have two distinct API's for BLK and PARTITION?
You seem to have a policy that each DM class should have its own
API's.

I don't have a strong opinion here, but form user's point of view,
block-level accessing has no difference between BLK and PARTITION.
Most common users of such accessors are file systems.
In stead of using the following code at every corner,
   cid = device_get_uclass_id(udev);
   if (cid == UCLASS_BLK)
       blk_read(udev, ...);
   else if (cid == UCLASS_PARTITION)
       disk_part(udev, ...);
it may be more convenient to have a single API.

> Also I think we should rename your functions to avoid using the
> dev_read prefix, since this is for reading from the device tree.

Okay.

> Perhaps disk_read()? Also it seems that we could rationalise the code
> between disk_read() and part_read() ?

I'm not sure what you mean by "rationalise".

> Also should have comments in the
> header file about what the functions do (and what type of device they
> accept).

Yeah.

-Takahiro Akashi

> Regards,
> Simon
> [..]