Hi Takahiro, On Wed, 28 Sept 2022 at 18:51, AKASHI Takahiro <takahiro.aka...@linaro.org> wrote: > > 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.aka...@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.
Yes that's right. > > 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. Yes I agree, but you already have that with your dev_read() API. I just want to rename it a bit. > > > 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". The code seems to be almost the same between the two so I think it may be possible to move the common code into a shared function. > > > Also should have comments in the > > header file about what the functions do (and what type of device they > > accept). > > Yeah. Regards, Simon