From 5ff4609f855b9851af572f0ba9b66e8a5837fd8c Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Mon, 14 Aug 2023 01:46:41 +0200 Subject: disk: Drop always true conditional check if (device_get_uclass_id(dev) == UCLASS_PARTITION) is always true, because this disk_blk_read() function calls dev_get_blk() above and checks its return value for non-NULL. The dev_get_blk() performs the same device_get_uclass_id(dev) check and returns NULL if not UCLASS_PARTITION. Drop the duplicate check. Signed-off-by: Marek Vasut --- disk/disk-uclass.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'disk/disk-uclass.c') diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c index d32747e2242..5974dd8c2ec 100644 --- a/disk/disk-uclass.c +++ b/disk/disk-uclass.c @@ -186,10 +186,8 @@ unsigned long disk_blk_read(struct udevice *dev, lbaint_t start, 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; - } + part = dev_get_uclass_plat(dev); + start_in_disk += part->gpt_part_info.start; if (blkcache_read(desc->uclass_id, desc->devnum, start_in_disk, blkcnt, desc->blksz, buffer)) -- cgit v1.2.3 From 91d3066c907dedb3d45fd16e11f938bf46eafec7 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Mon, 14 Aug 2023 01:46:42 +0200 Subject: disk: Simplify disk_blk_read() using blk_read() The disk_blk_read() can be simplified using blk_read(), the only things which needs to be handled are the read offset based on the partition properties, and the block device ops which are coming from the parent udevice, not the partition udevice. The later is currently not implemented correctly as far as I can tell, since the current code extracts block device descriptor from the parent udevice which is OK, but extracts block device operations from the partition udevice, which does not seem OK. Switching to the blk_read() fixes that too. The dev_get_blk() usage is simplified using UCLASS_PARTITION check. Add non-confusing documentation what this really does. Signed-off-by: Marek Vasut --- disk/disk-uclass.c | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) (limited to 'disk/disk-uclass.c') diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c index 5974dd8c2ec..6daece1288f 100644 --- a/disk/disk-uclass.c +++ b/disk/disk-uclass.c @@ -168,36 +168,26 @@ static struct blk_desc *dev_get_blk(struct udevice *dev) return desc; } +/** + * disk_blk_read() - Read from a block device partition + * + * @dev: Device to read from (partition udevice) + * @start: Start block for the read (from start of partition) + * @blkcnt: Number of blocks to read (within the partition) + * @buffer: Place to put the data + * @return number of blocks read (which may be less than @blkcnt), + * or -ve on error. This never returns 0 unless @blkcnt is 0 + */ unsigned long disk_blk_read(struct udevice *dev, lbaint_t start, lbaint_t blkcnt, void *buffer) { - struct blk_desc *desc; - const struct blk_ops *ops; - struct disk_part *part; - lbaint_t start_in_disk; - ulong blks_read; - - desc = dev_get_blk(dev); - if (!desc) - return -ENOSYS; + struct disk_part *part = dev_get_uclass_plat(dev); - ops = blk_get_ops(dev); - if (!ops->read) + if (device_get_uclass_id(dev) != UCLASS_PARTITION) return -ENOSYS; - start_in_disk = start; - part = dev_get_uclass_plat(dev); - start_in_disk += part->gpt_part_info.start; - - if (blkcache_read(desc->uclass_id, desc->devnum, start_in_disk, blkcnt, - desc->blksz, buffer)) - return blkcnt; - blks_read = ops->read(dev, start, blkcnt, buffer); - if (blks_read == blkcnt) - blkcache_fill(desc->uclass_id, desc->devnum, start_in_disk, - blkcnt, desc->blksz, buffer); - - return blks_read; + return blk_read(dev_get_parent(dev), start + part->gpt_part_info.start, + blkcnt, buffer); } unsigned long disk_blk_write(struct udevice *dev, lbaint_t start, -- cgit v1.2.3 From 9161c2c90d00c46e944cfeb4091230ec460a9981 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Mon, 14 Aug 2023 01:46:43 +0200 Subject: disk: Simplify disk_blk_{write, erase}() using blk_{write, erase}() These two functions are basically identical, just call the blk_*() functions from disk_blk_*() functions. The only difference is that the disk_blk_*() functions have to use parent block device as the udevice implementing block device operations. Add documentation on what those functions really do. The documentation is not wrong even though it likely does look that way. The write/erase functions really do not take into account the partition offset. This will be fixed in the next patch. Signed-off-by: Marek Vasut --- disk/disk-uclass.c | 66 +++++++++++++++++++----------------------------------- 1 file changed, 23 insertions(+), 43 deletions(-) (limited to 'disk/disk-uclass.c') diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c index 6daece1288f..5cb1594e015 100644 --- a/disk/disk-uclass.c +++ b/disk/disk-uclass.c @@ -149,25 +149,6 @@ U_BOOT_DRIVER(blk_partition) = { /* * BLOCK IO APIs */ -static struct blk_desc *dev_get_blk(struct udevice *dev) -{ - struct blk_desc *desc; - - switch (device_get_uclass_id(dev)) { - /* - * We won't support UCLASS_BLK with dev_* interfaces. - */ - case UCLASS_PARTITION: - desc = dev_get_uclass_plat(dev_get_parent(dev)); - break; - default: - desc = NULL; - break; - } - - return desc; -} - /** * disk_blk_read() - Read from a block device partition * @@ -190,42 +171,41 @@ unsigned long disk_blk_read(struct udevice *dev, lbaint_t start, blkcnt, buffer); } +/** + * disk_blk_write() - Write to a block device + * + * @dev: Device to write to + * @start: Start block for the write + * @blkcnt: Number of blocks to write + * @buffer: Data to write + * @return number of blocks written (which may be less than @blkcnt), + * or -ve on error. This never returns 0 unless @blkcnt is 0 + */ unsigned long disk_blk_write(struct udevice *dev, lbaint_t start, lbaint_t blkcnt, const void *buffer) { - struct blk_desc *desc; - const struct blk_ops *ops; - - desc = dev_get_blk(dev); - if (!desc) - return -ENOSYS; - - ops = blk_get_ops(dev); - if (!ops->write) + if (device_get_uclass_id(dev) != UCLASS_PARTITION) return -ENOSYS; - blkcache_invalidate(desc->uclass_id, desc->devnum); - - return ops->write(dev, start, blkcnt, buffer); + return blk_write(dev_get_parent(dev), start, blkcnt, buffer); } +/** + * disk_blk_erase() - Erase part of a block device + * + * @dev: Device to erase + * @start: Start block for the erase + * @blkcnt: Number of blocks to erase + * @return number of blocks erased (which may be less than @blkcnt), + * or -ve on error. This never returns 0 unless @blkcnt is 0 + */ unsigned long disk_blk_erase(struct udevice *dev, lbaint_t start, lbaint_t blkcnt) { - struct blk_desc *desc; - const struct blk_ops *ops; - - desc = dev_get_blk(dev); - if (!desc) - return -ENOSYS; - - ops = blk_get_ops(dev); - if (!ops->erase) + if (device_get_uclass_id(dev) != UCLASS_PARTITION) return -ENOSYS; - blkcache_invalidate(desc->uclass_id, desc->devnum); - - return ops->erase(dev, start, blkcnt); + return blk_erase(dev_get_parent(dev), start, blkcnt); } UCLASS_DRIVER(partition) = { -- cgit v1.2.3 From 2bc0dfef9f27c03f24784f4f2382079caff05df1 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Mon, 14 Aug 2023 01:46:44 +0200 Subject: disk: Handle partition to block device offset conversion Convert the read/write/erase offset from one within a partition to one within a block device, to correctly access the data on the block device for both write and erase operations. Signed-off-by: Marek Vasut --- disk/disk-uclass.c | 68 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 16 deletions(-) (limited to 'disk/disk-uclass.c') diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c index 5cb1594e015..32722cf9176 100644 --- a/disk/disk-uclass.c +++ b/disk/disk-uclass.c @@ -17,6 +17,36 @@ #include #include +/** + * disk_blk_part_validate() - Check whether access to partition is within limits + * + * @dev: Device (partition udevice) + * @start: Start block for the access(from start of partition) + * @blkcnt: Number of blocks to access (within the partition) + * @return 0 on valid block range, or -ve on error. + */ +static int disk_blk_part_validate(struct udevice *dev, lbaint_t start, lbaint_t blkcnt) +{ + if (device_get_uclass_id(dev) != UCLASS_PARTITION) + return -ENOSYS; + + return 0; +} + +/** + * disk_blk_part_offset() - Compute offset from start of block device + * + * @dev: Device (partition udevice) + * @start: Start block for the access (from start of partition) + * @return Start block for the access (from start of block device) + */ +static lbaint_t disk_blk_part_offset(struct udevice *dev, lbaint_t start) +{ + struct disk_part *part = dev_get_uclass_plat(dev); + + return start + part->gpt_part_info.start; +} + int part_create_block_devices(struct udevice *blk_dev) { int part, count; @@ -162,21 +192,21 @@ U_BOOT_DRIVER(blk_partition) = { unsigned long disk_blk_read(struct udevice *dev, lbaint_t start, lbaint_t blkcnt, void *buffer) { - struct disk_part *part = dev_get_uclass_plat(dev); + int ret = disk_blk_part_validate(dev, start, blkcnt); - if (device_get_uclass_id(dev) != UCLASS_PARTITION) - return -ENOSYS; + if (ret) + return ret; - return blk_read(dev_get_parent(dev), start + part->gpt_part_info.start, + return blk_read(dev_get_parent(dev), disk_blk_part_offset(dev, start), blkcnt, buffer); } /** * disk_blk_write() - Write to a block device * - * @dev: Device to write to - * @start: Start block for the write - * @blkcnt: Number of blocks to write + * @dev: Device to write to (partition udevice) + * @start: Start block for the write (from start of partition) + * @blkcnt: Number of blocks to write (within the partition) * @buffer: Data to write * @return number of blocks written (which may be less than @blkcnt), * or -ve on error. This never returns 0 unless @blkcnt is 0 @@ -184,28 +214,34 @@ unsigned long disk_blk_read(struct udevice *dev, lbaint_t start, unsigned long disk_blk_write(struct udevice *dev, lbaint_t start, lbaint_t blkcnt, const void *buffer) { - if (device_get_uclass_id(dev) != UCLASS_PARTITION) - return -ENOSYS; + int ret = disk_blk_part_validate(dev, start, blkcnt); + + if (ret) + return ret; - return blk_write(dev_get_parent(dev), start, blkcnt, buffer); + return blk_write(dev_get_parent(dev), disk_blk_part_offset(dev, start), + blkcnt, buffer); } /** * disk_blk_erase() - Erase part of a block device * - * @dev: Device to erase - * @start: Start block for the erase - * @blkcnt: Number of blocks to erase + * @dev: Device to erase (partition udevice) + * @start: Start block for the erase (from start of partition) + * @blkcnt: Number of blocks to erase (within the partition) * @return number of blocks erased (which may be less than @blkcnt), * or -ve on error. This never returns 0 unless @blkcnt is 0 */ unsigned long disk_blk_erase(struct udevice *dev, lbaint_t start, lbaint_t blkcnt) { - if (device_get_uclass_id(dev) != UCLASS_PARTITION) - return -ENOSYS; + int ret = disk_blk_part_validate(dev, start, blkcnt); + + if (ret) + return ret; - return blk_erase(dev_get_parent(dev), start, blkcnt); + return blk_erase(dev_get_parent(dev), disk_blk_part_offset(dev, start), + blkcnt); } UCLASS_DRIVER(partition) = { -- cgit v1.2.3 From bfd98b9a634e5922ca60597f59d83afdaa7c99ad Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Mon, 14 Aug 2023 01:46:45 +0200 Subject: disk: Extend disk_blk_part_validate() with range checking Check whether access is out of bounds of the partition and return an error. This way there is no danger of esp. write or erase outside of the confines of partition. Signed-off-by: Marek Vasut --- disk/disk-uclass.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'disk/disk-uclass.c') diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c index 32722cf9176..f262105375b 100644 --- a/disk/disk-uclass.c +++ b/disk/disk-uclass.c @@ -27,9 +27,17 @@ */ static int disk_blk_part_validate(struct udevice *dev, lbaint_t start, lbaint_t blkcnt) { + struct disk_part *part = dev_get_uclass_plat(dev); + if (device_get_uclass_id(dev) != UCLASS_PARTITION) return -ENOSYS; + if (start >= part->gpt_part_info.size) + return -E2BIG; + + if ((start + blkcnt) > part->gpt_part_info.size) + return -ERANGE; + return 0; } -- cgit v1.2.3 From 30a12e080104dc7cbdead7e9adc4f5ec4f7a3c40 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Mon, 14 Aug 2023 01:46:46 +0200 Subject: disk: Switch part_blk_*() functions to disk_blk_*() The behavior of the part_blk_*() functions is now identical to disk_blk_*() functions, switch the former to the later. Signed-off-by: Marek Vasut --- disk/disk-uclass.c | 93 +++++++----------------------------------------------- 1 file changed, 12 insertions(+), 81 deletions(-) (limited to 'disk/disk-uclass.c') diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c index f262105375b..90a7c6f0f8a 100644 --- a/disk/disk-uclass.c +++ b/disk/disk-uclass.c @@ -103,87 +103,6 @@ int part_create_block_devices(struct udevice *blk_dev) return 0; } -static ulong part_blk_read(struct udevice *dev, lbaint_t start, - lbaint_t blkcnt, void *buffer) -{ - struct udevice *parent; - struct disk_part *part; - const struct blk_ops *ops; - - parent = dev_get_parent(dev); - ops = blk_get_ops(parent); - if (!ops->read) - return -ENOSYS; - - part = dev_get_uclass_plat(dev); - if (start >= part->gpt_part_info.size) - return 0; - - if ((start + blkcnt) > part->gpt_part_info.size) - blkcnt = part->gpt_part_info.size - start; - start += part->gpt_part_info.start; - - return ops->read(parent, start, blkcnt, buffer); -} - -static ulong part_blk_write(struct udevice *dev, lbaint_t start, - lbaint_t blkcnt, const void *buffer) -{ - struct udevice *parent; - struct disk_part *part; - const struct blk_ops *ops; - - parent = dev_get_parent(dev); - ops = blk_get_ops(parent); - if (!ops->write) - return -ENOSYS; - - part = dev_get_uclass_plat(dev); - if (start >= part->gpt_part_info.size) - return 0; - - if ((start + blkcnt) > part->gpt_part_info.size) - blkcnt = part->gpt_part_info.size - start; - start += part->gpt_part_info.start; - - return ops->write(parent, start, blkcnt, buffer); -} - -static ulong part_blk_erase(struct udevice *dev, lbaint_t start, - lbaint_t blkcnt) -{ - struct udevice *parent; - struct disk_part *part; - const struct blk_ops *ops; - - parent = dev_get_parent(dev); - ops = blk_get_ops(parent); - if (!ops->erase) - return -ENOSYS; - - part = dev_get_uclass_plat(dev); - if (start >= part->gpt_part_info.size) - return 0; - - if ((start + blkcnt) > part->gpt_part_info.size) - blkcnt = part->gpt_part_info.size - start; - start += part->gpt_part_info.start; - - return ops->erase(parent, start, blkcnt); -} - -static const struct blk_ops blk_part_ops = { - .read = part_blk_read, - .write = part_blk_write, - .erase = part_blk_erase, -}; - -U_BOOT_DRIVER(blk_partition) = { - .name = "blk_partition", - .id = UCLASS_PARTITION, - .ops = &blk_part_ops, -}; - /* * BLOCK IO APIs */ @@ -257,3 +176,15 @@ UCLASS_DRIVER(partition) = { .per_device_plat_auto = sizeof(struct disk_part), .name = "partition", }; + +static const struct blk_ops blk_part_ops = { + .read = disk_blk_read, + .write = disk_blk_write, + .erase = disk_blk_erase, +}; + +U_BOOT_DRIVER(blk_partition) = { + .name = "blk_partition", + .id = UCLASS_PARTITION, + .ops = &blk_part_ops, +}; -- cgit v1.2.3 From 804f7d63f26c00a64e2945fced4841abf200c0c0 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Mon, 14 Aug 2023 01:46:47 +0200 Subject: disk: Move part_create_block_devices() to blk uclass Move part_create_block_devices() to blk uclass and unexpose the function. This can now be internal to the block uclass. Signed-off-by: Marek Vasut --- disk/disk-uclass.c | 48 ------------------------------------------------ 1 file changed, 48 deletions(-) (limited to 'disk/disk-uclass.c') diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c index 90a7c6f0f8a..efe4bf1f949 100644 --- a/disk/disk-uclass.c +++ b/disk/disk-uclass.c @@ -55,54 +55,6 @@ static lbaint_t disk_blk_part_offset(struct udevice *dev, lbaint_t start) return start + part->gpt_part_info.start; } -int part_create_block_devices(struct udevice *blk_dev) -{ - int part, count; - struct blk_desc *desc = dev_get_uclass_plat(blk_dev); - struct disk_partition info; - struct disk_part *part_data; - char devname[32]; - struct udevice *dev; - int ret; - - if (!CONFIG_IS_ENABLED(PARTITIONS) || !blk_enabled()) - return 0; - - if (device_get_uclass_id(blk_dev) != UCLASS_BLK) - return 0; - - /* Add devices for each partition */ - for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) { - if (part_get_info(desc, part, &info)) - continue; - snprintf(devname, sizeof(devname), "%s:%d", blk_dev->name, - part); - - ret = device_bind_driver(blk_dev, "blk_partition", - strdup(devname), &dev); - if (ret) - return ret; - - part_data = dev_get_uclass_plat(dev); - part_data->partnum = part; - part_data->gpt_part_info = info; - count++; - - ret = device_probe(dev); - if (ret) { - debug("Can't probe\n"); - count--; - device_unbind(dev); - - continue; - } - } - debug("%s: %d partitions found in %s\n", __func__, count, - blk_dev->name); - - return 0; -} - /* * BLOCK IO APIs */ -- cgit v1.2.3