| Message ID | 20211001050228.55183-20-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi_loader: more tightly integrate UEFI disks to device model | expand |
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
===>
/*
* 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;
+
+ 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);
/**
* blk_find_device() - Find a block device
*