Hi Takahiro,
On Wed, 28 Sept 2022 at 18:51, AKASHI Takahiro takahiro.akashi@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.akashi@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