Hi Takahiro,
On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
In include/blk.h, Simon suggested: ===> /*
- These functions should take struct udevice instead of struct blk_desc,
- but this is convenient for migration to driver model. Add a 'd' prefix
- to the function operations, so that blk_read(), etc. can be reserved for
- functions with the correct arguments.
*/ unsigned long blk_dread(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt, void *buffer); unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt, const void *buffer); unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt); <===
So new interfaces are provided with this patch.
They are expected to be used everywhere in U-Boot at the end. The exceptions are block device drivers, partition drivers and efi_disk which should know details of blk_desc structure.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/block/blk-uclass.c | 91 ++++++++++++++++++++++++++++++++++++++ include/blk.h | 6 +++ 2 files changed, 97 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 6ba11a8fa7f7..8fbec8779e1e 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -482,6 +482,97 @@ unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start, return ops->erase(dev, start, blkcnt); }
+static struct blk_desc *dev_get_blk(struct udevice *dev) +{
struct blk_desc *block_dev;switch (device_get_uclass_id(dev)) {case UCLASS_BLK:block_dev = dev_get_uclass_plat(dev);break;case UCLASS_PARTITION:block_dev = dev_get_uclass_plat(dev_get_parent(dev));break;default:block_dev = NULL;break;}return block_dev;+}
+unsigned long blk_read(struct udevice *dev, lbaint_t start,
lbaint_t blkcnt, void *buffer)+{
struct blk_desc *block_dev;const struct blk_ops *ops;struct disk_part *part;lbaint_t start_in_disk;ulong blks_read;block_dev = dev_get_blk(dev);if (!block_dev)return -ENOSYS;
IMO blk_read() should take a block device. That is how all other DM APIs work. I think it is too confusing to use the parent device here.
So how about changing these functions to take a blk device, then adding new ones for what you want here, e.g.
int media_read(struct udevice *dev... { struct udevice *blk;
blk = dev_get_blk(dev); if (!blk) return -ENOSYS;
ret = blk_read(blk, ... }
Also can we use blk instead of block_dev?
ops = blk_get_ops(dev);if (!ops->read)return -ENOSYS;start_in_disk = start;if (device_get_uclass_id(dev) == UCLASS_PARTITION) {part = dev_get_uclass_plat(dev);start_in_disk += part->gpt_part_info.start;}if (blkcache_read(block_dev->if_type, block_dev->devnum,start_in_disk, blkcnt, block_dev->blksz, buffer))return blkcnt;blks_read = ops->read(dev, start, blkcnt, buffer);if (blks_read == blkcnt)blkcache_fill(block_dev->if_type, block_dev->devnum,start_in_disk, blkcnt, block_dev->blksz, buffer);return blks_read;+}
+unsigned long blk_write(struct udevice *dev, lbaint_t start,
lbaint_t blkcnt, const void *buffer)+{
struct blk_desc *block_dev;const struct blk_ops *ops;block_dev = dev_get_blk(dev);if (!block_dev)return -ENOSYS;ops = blk_get_ops(dev);if (!ops->write)return -ENOSYS;blkcache_invalidate(block_dev->if_type, block_dev->devnum);return ops->write(dev, start, blkcnt, buffer);+}
+unsigned long blk_erase(struct udevice *dev, lbaint_t start,
lbaint_t blkcnt)+{
struct blk_desc *block_dev;const struct blk_ops *ops;block_dev = dev_get_blk(dev);if (!block_dev)return -ENOSYS;ops = blk_get_ops(dev);if (!ops->erase)return -ENOSYS;blkcache_invalidate(block_dev->if_type, block_dev->devnum);return ops->erase(dev, start, blkcnt);+}
int blk_get_from_parent(struct udevice *parent, struct udevice **devp) { struct udevice *dev; diff --git a/include/blk.h b/include/blk.h index 3d883eb1db64..f5fdd6633a09 100644 --- a/include/blk.h +++ b/include/blk.h @@ -284,6 +284,12 @@ unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start, unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt);
+unsigned long blk_read(struct udevice *dev, lbaint_t start,
lbaint_t blkcnt, void *buffer);+unsigned long blk_write(struct udevice *dev, lbaint_t start,
lbaint_t blkcnt, const void *buffer);+unsigned long blk_erase(struct udevice *dev, lbaint_t start,
lbaint_t blkcnt);
Please add full comments to these.
/**
- blk_find_device() - Find a block device
-- 2.33.0
Regards, Simon