u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] disk: Use a helper function to reduce duplication
@ 2023-03-19 19:29 Simon Glass
  2023-03-20  7:31 ` Ilias Apalodimas
  2023-03-31 14:17 ` Tom Rini
  0 siblings, 2 replies; 7+ messages in thread
From: Simon Glass @ 2023-03-19 19:29 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Ilias Apalodimas, AKASHI Takahiro, Heinrich Schuchardt, Simon Glass

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;
-
 	return ops->read(parent, start, blkcnt, buffer);
 }
 
@@ -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


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH v2] disk: Use a helper function to reduce duplication
  2023-03-19 19:29 [PATCH v2] disk: Use a helper function to reduce duplication Simon Glass
@ 2023-03-20  7:31 ` Ilias Apalodimas
  2023-03-20  7:57   ` AKASHI Takahiro
  2023-03-31 14:17 ` Tom Rini
  1 sibling, 1 reply; 7+ messages in thread
From: Ilias Apalodimas @ 2023-03-20  7:31 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List, AKASHI Takahiro, Heinrich Schuchardt

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?

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;
> -
>  	return ops->read(parent, start, blkcnt, buffer);
>  }
>
> @@ -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
>

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2] disk: Use a helper function to reduce duplication
  2023-03-20  7:31 ` Ilias Apalodimas
@ 2023-03-20  7:57   ` AKASHI Takahiro
  0 siblings, 0 replies; 7+ messages in thread
From: AKASHI Takahiro @ 2023-03-20  7:57 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: Simon Glass, U-Boot Mailing List, Heinrich Schuchardt

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
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2] disk: Use a helper function to reduce duplication
  2023-03-19 19:29 [PATCH v2] disk: Use a helper function to reduce duplication Simon Glass
  2023-03-20  7:31 ` Ilias Apalodimas
@ 2023-03-31 14:17 ` Tom Rini
  2023-04-03  0:06   ` AKASHI Takahiro
  1 sibling, 1 reply; 7+ messages in thread
From: Tom Rini @ 2023-03-31 14:17 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Ilias Apalodimas, AKASHI Takahiro,
	Heinrich Schuchardt

[-- Attachment #1: Type: text/plain, Size: 309 bytes --]

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>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2] disk: Use a helper function to reduce duplication
  2023-03-31 14:17 ` Tom Rini
@ 2023-04-03  0:06   ` AKASHI Takahiro
  2023-04-03  1:16     ` Simon Glass
  2023-04-03 14:46     ` Tom Rini
  0 siblings, 2 replies; 7+ messages in thread
From: AKASHI Takahiro @ 2023-04-03  0:06 UTC (permalink / raw)
  To: Tom Rini
  Cc: Simon Glass, U-Boot Mailing List, Ilias Apalodimas, Heinrich Schuchardt

[-- Attachment #1: Type: text/plain, Size: 534 bytes --]

Hi Tom,

On Fri, Mar 31, 2023 at 10:17:03AM -0400, Tom Rini wrote:
> 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>
> 
> Applied to u-boot/next, thanks!

I think I said nak against this patch.
https://lists.denx.de/pipermail/u-boot/2023-March/512677.html

-Takahiro Akashi


> -- 
> Tom



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2] disk: Use a helper function to reduce duplication
  2023-04-03  0:06   ` AKASHI Takahiro
@ 2023-04-03  1:16     ` Simon Glass
  2023-04-03 14:46     ` Tom Rini
  1 sibling, 0 replies; 7+ messages in thread
From: Simon Glass @ 2023-04-03  1:16 UTC (permalink / raw)
  To: AKASHI Takahiro, Tom Rini, Simon Glass, U-Boot Mailing List,
	Ilias Apalodimas, Heinrich Schuchardt

Hi,

On Mon, 3 Apr 2023, 12:06 AKASHI Takahiro, <takahiro.akashi@linaro.org>
wrote:

> Hi Tom,
>
> On Fri, Mar 31, 2023 at 10:17:03AM -0400, Tom Rini wrote:
> > 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>
> >
> > Applied to u-boot/next, thanks!
>
> I think I said nak against this patch.
> https://lists.denx.de/pipermail/u-boot/2023-March/512677.html


Yes,sorry I did'nt get back to it. If this applied I wonder if there is
test coverage missing. Do you think you could add a test for it?

I am afk so cannot send a revert for a few days,if someone else can?

Regards,
Simon

>
>
> -Takahiro Akashi
>
>
> > --
> > Tom
>
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2] disk: Use a helper function to reduce duplication
  2023-04-03  0:06   ` AKASHI Takahiro
  2023-04-03  1:16     ` Simon Glass
@ 2023-04-03 14:46     ` Tom Rini
  1 sibling, 0 replies; 7+ messages in thread
From: Tom Rini @ 2023-04-03 14:46 UTC (permalink / raw)
  To: AKASHI Takahiro, Simon Glass, U-Boot Mailing List,
	Ilias Apalodimas, Heinrich Schuchardt

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

On Mon, Apr 03, 2023 at 09:06:13AM +0900, AKASHI Takahiro wrote:
> Hi Tom,
> 
> On Fri, Mar 31, 2023 at 10:17:03AM -0400, Tom Rini wrote:
> > 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>
> > 
> > Applied to u-boot/next, thanks!
> 
> I think I said nak against this patch.
> https://lists.denx.de/pipermail/u-boot/2023-March/512677.html

Ah I missed that, sorry, reverting.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-04-03 14:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-19 19:29 [PATCH v2] disk: Use a helper function to reduce duplication Simon Glass
2023-03-20  7:31 ` Ilias Apalodimas
2023-03-20  7:57   ` AKASHI Takahiro
2023-03-31 14:17 ` Tom Rini
2023-04-03  0:06   ` AKASHI Takahiro
2023-04-03  1:16     ` Simon Glass
2023-04-03 14:46     ` Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).