[PATCH v2] disk: Use a helper function to reduce duplication
AKASHI Takahiro
takahiro.akashi at linaro.org
Mon Mar 20 08:57:12 CET 2023
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 at 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
> >
More information about the U-Boot
mailing list