On Mon, Mar 20, 2023 at 09:31:58AM +0200, Ilias Apalodimas wrote:

Hi Simon,

Patch looks good, but isn't the new function name a bit misleading? Something like blk_part_find_start() sounds a bit more descriptive, or am I missing something?

I don't think that the helper function works as my original code does.

Cheers /Ilias

On Mon, Mar 20, 2023 at 08:29:57AM +1300, Simon Glass wrote:

...

Reduce the duplicated code slightly by using a helper function to handle the common code.

This reduces the code size very slightly.

Signed-off-by: Simon Glass sjg@chromium.org

Changes in v2:

• Rebase to -next

disk/disk-uclass.c | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c index d32747e2242d..7f1fd80b2248 100644 --- a/disk/disk-uclass.c +++ b/disk/disk-uclass.c @@ -65,26 +65,38 @@ int part_create_block_devices(struct udevice *blk_dev) return 0; }

+static int blk_part_setup(struct udevice *dev, lbaint_t *startp,

• 	  lbaint_t blkcnt)
  

+{

• struct disk_part *part;
• part = dev_get_uclass_plat(dev);
• if (*startp >= part->gpt_part_info.size)

• return -E2BIG;
  

• if (*startp + blkcnt > part->gpt_part_info.size)

• blkcnt = part->gpt_part_info.size - *startp;
  

• *startp += part->gpt_part_info.start;
• 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;

• int ret;

  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)

• ret = blk_part_setup(dev, &start, blkcnt);
• if (ret) return 0;

• if ((start + blkcnt) > part->gpt_part_info.size)

• blkcnt = part->gpt_part_info.size - start;
  

• start += part->gpt_part_info.start;

We cannot read out more blocks than the size of the partition.

...

return ops->read(parent, start, blkcnt, buffer);

So blkcnt must also be adjusted, otherwise we may see extra blocks beyond the partition boundary.

-Takahiro Akashi

...

}

@@ -92,22 +104,18 @@ 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;

• int ret;

  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)

• ret = blk_part_setup(dev, &start, blkcnt);
• if (ret) 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);

}

@@ -115,22 +123,18 @@ 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;

• int ret;

  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)

• ret = blk_part_setup(dev, &start, blkcnt);
• if (ret) 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);

}

-- 2.40.0.rc1.284.g88254d51c5-goog