diff mbox series

[v6,08/25] spl: Refactor spl_load_info->read to use units of bytes

Message ID 20231106022603.3405551-9-seanga2@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show
Series spl: Use common function for loading/parsing images | expand

Commit Message

Sean Anderson Nov. 6, 2023, 2:25 a.m. UTC 
Simplify things a bit for callers of spl_load_info->read by refactoring it
to use units of bytes instead of bl_len. This generally simplifies the
logic, as MMC is the only loader which actually works in sectors. It will
also allow further refactoring to remove the special-case handling of
filename.  spl_load_legacy_img already works in units of bytes (oops) so it
doesn't need to be changed.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

Changes in v6:
- New

 arch/arm/mach-imx/spl_imx_romapi.c | 16 +++-------
 common/spl/spl_fit.c               | 51 +++++++++++++++---------------
 common/spl/spl_imx_container.c     | 45 +++++++++++---------------
 common/spl/spl_mmc.c               | 24 ++++++++------
 common/spl/spl_nand.c              |  8 ++---
 include/spl.h                      | 16 ++++++----
 test/image/spl_load_os.c           | 13 ++++----
 7 files changed, 80 insertions(+), 93 deletions(-)

Comments

Xavier Drudis Ferran Nov. 6, 2023, 12:35 p.m. UTC | #1 
Thanks for your work. I'm still reading... but...


El Sun, Nov 05, 2023 at 09:25:46PM -0500, Sean Anderson deia:
> diff --git a/include/spl.h b/include/spl.h
> index 951e136b9ea..ecfc50e0095 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -297,10 +297,10 @@ struct spl_load_info {
>  	 * read() - Read from device
>  	 *
>  	 * @load: Information about the load state
> -	 * @sector: Sector number to read from (each @load->bl_len bytes)
> -	 * @count: Number of sectors to read
> +	 * @offset: Offset to read from in bytes, in multiples of @load->bl_len
> +	 * @count: Number of bytes to read, in multiples of @load->bl_len

I'm no native English speaker, but would it be easier to understand? :

+	 * @offset: Offset to read from in bytes, a multiple of @load->bl_len
+	 * @count: Number of bytes to read, a multiple of @load->bl_len
Sean Anderson Nov. 6, 2023, 1:54 p.m. UTC | #2 
On 11/6/23 07:35, Xavier Drudis Ferran wrote:
> Thanks for your work. I'm still reading... but...
> 
> 
> El Sun, Nov 05, 2023 at 09:25:46PM -0500, Sean Anderson deia:
>> diff --git a/include/spl.h b/include/spl.h
>> index 951e136b9ea..ecfc50e0095 100644
>> --- a/include/spl.h
>> +++ b/include/spl.h
>> @@ -297,10 +297,10 @@ struct spl_load_info {
>>   	 * read() - Read from device
>>   	 *
>>   	 * @load: Information about the load state
>> -	 * @sector: Sector number to read from (each @load->bl_len bytes)
>> -	 * @count: Number of sectors to read
>> +	 * @offset: Offset to read from in bytes, in multiples of @load->bl_len
>> +	 * @count: Number of bytes to read, in multiples of @load->bl_len
> 
> I'm no native English speaker, but would it be easier to understand? :
> 
> +	 * @offset: Offset to read from in bytes, a multiple of @load->bl_len
> +	 * @count: Number of bytes to read, a multiple of @load->bl_len
> 
> 
> 

I think it would have to be worded

@offset: Offset to read from in bytes, as a multiple of @load->bl_len

but to me these both mean the same thing.

--Sean
Xavier Drudis Ferran Nov. 7, 2023, 8:49 a.m. UTC | #3 
El Mon, Nov 06, 2023 at 08:54:03AM -0500, Sean Anderson deia:
> On 11/6/23 07:35, Xavier Drudis Ferran wrote:
> > Thanks for your work. I'm still reading... but...
> > 
> > 
> > El Sun, Nov 05, 2023 at 09:25:46PM -0500, Sean Anderson deia:
> > > diff --git a/include/spl.h b/include/spl.h
> > > index 951e136b9ea..ecfc50e0095 100644
> > > --- a/include/spl.h
> > > +++ b/include/spl.h
> > > @@ -297,10 +297,10 @@ struct spl_load_info {
> > >   	 * read() - Read from device
> > >   	 *
> > >   	 * @load: Information about the load state
> > > -	 * @sector: Sector number to read from (each @load->bl_len bytes)
> > > -	 * @count: Number of sectors to read
> > > +	 * @offset: Offset to read from in bytes, in multiples of @load->bl_len
> > > +	 * @count: Number of bytes to read, in multiples of @load->bl_len
> > 
> > I'm no native English speaker, but would it be easier to understand? :
> > 
> > +	 * @offset: Offset to read from in bytes, a multiple of @load->bl_len
> > +	 * @count: Number of bytes to read, a multiple of @load->bl_len
> > 
> > 
> > 
> 
> I think it would have to be worded
> 
> @offset: Offset to read from in bytes, as a multiple of @load->bl_len
> 
> but to me these both mean the same thing.
> 
> --Sean

Ah, OK. I doubted on whether it should be a comma or "as". Apparently
it was both.

They may mean the same, but I got confused by having "in bytes", and
"in multiples" so close.

"in bytes" means the value should be the correct answer to the
question "how many bytes" (not a number of bits, not a number of
sectors)

but "in multiples of..." doesn't mean the value should be the correct
answer to "how many of multiples". Should the value be a number of
sectors (first multiple of bl_len) a number of dozens of sectors
(another multiple of bl_len) or ...?

In the first part "in" introduces a unit of measure, but in the second
the same word introduces a constraint on a value. That was confusing
to me.

Not saying it doesn't mean the same or even that you should change it,
just elaborating why I said it.
Sean Anderson Nov. 8, 2023, 3:59 p.m. UTC | #4 
On 11/7/23 03:49, Xavier Drudis Ferran wrote:
> El Mon, Nov 06, 2023 at 08:54:03AM -0500, Sean Anderson deia:
>> On 11/6/23 07:35, Xavier Drudis Ferran wrote:
>>> Thanks for your work. I'm still reading... but...
>>>
>>>
>>> El Sun, Nov 05, 2023 at 09:25:46PM -0500, Sean Anderson deia:
>>>> diff --git a/include/spl.h b/include/spl.h
>>>> index 951e136b9ea..ecfc50e0095 100644
>>>> --- a/include/spl.h
>>>> +++ b/include/spl.h
>>>> @@ -297,10 +297,10 @@ struct spl_load_info {
>>>>    	 * read() - Read from device
>>>>    	 *
>>>>    	 * @load: Information about the load state
>>>> -	 * @sector: Sector number to read from (each @load->bl_len bytes)
>>>> -	 * @count: Number of sectors to read
>>>> +	 * @offset: Offset to read from in bytes, in multiples of @load->bl_len
>>>> +	 * @count: Number of bytes to read, in multiples of @load->bl_len
>>>
>>> I'm no native English speaker, but would it be easier to understand? :
>>>
>>> +	 * @offset: Offset to read from in bytes, a multiple of @load->bl_len
>>> +	 * @count: Number of bytes to read, a multiple of @load->bl_len
>>>
>>>
>>>
>>
>> I think it would have to be worded
>>
>> @offset: Offset to read from in bytes, as a multiple of @load->bl_len
>>
>> but to me these both mean the same thing.
>>
>> --Sean
> 
> Ah, OK. I doubted on whether it should be a comma or "as". Apparently
> it was both.
> 
> They may mean the same, but I got confused by having "in bytes", and
> "in multiples" so close.
> 
> "in bytes" means the value should be the correct answer to the
> question "how many bytes" (not a number of bits, not a number of
> sectors)
> 
> but "in multiples of..." doesn't mean the value should be the correct
> answer to "how many of multiples". Should the value be a number of
> sectors (first multiple of bl_len) a number of dozens of sectors
> (another multiple of bl_len) or ...?
> 
> In the first part "in" introduces a unit of measure, but in the second
> the same word introduces a constraint on a value. That was confusing
> to me.
> 
> Not saying it doesn't mean the same or even that you should change it,
> just elaborating why I said it.
> 

OK, what about

@offset: Offset to read from in bytes. This must be a multiple of @load->bl_len.

--Sean
Xavier Drudis Ferran Nov. 9, 2023, 6:24 a.m. UTC | #5 
El Wed, Nov 08, 2023 at 10:59:13AM -0500, Sean Anderson deia:
> 
> OK, what about
> 
> @offset: Offset to read from in bytes. This must be a multiple of @load->bl_len.
> 
> --Sean

Great. Thank you.
diff mbox series

Patch

diff --git a/arch/arm/mach-imx/spl_imx_romapi.c b/arch/arm/mach-imx/spl_imx_romapi.c
index 93d48e56aca..d7f6cb4b5ba 100644
--- a/arch/arm/mach-imx/spl_imx_romapi.c
+++ b/arch/arm/mach-imx/spl_imx_romapi.c
@@ -53,16 +53,10 @@  static int is_boot_from_stream_device(u32 boot)
 }
 
 static ulong spl_romapi_read_seekable(struct spl_load_info *load,
-				      ulong sector, ulong count,
+				      ulong offset, ulong byte,
 				      void *buf)
 {
-	u32 pagesize = *(u32 *)load->priv;
-	ulong byte = count * pagesize;
-	u32 offset;
-
-	offset = sector * pagesize;
-
-	return spl_romapi_raw_seekable_read(offset, byte, buf) / pagesize;
+	return spl_romapi_raw_seekable_read(offset, byte, buf);
 }
 
 static int spl_romapi_load_image_seekable(struct spl_image_info *spl_image,
@@ -109,8 +103,7 @@  static int spl_romapi_load_image_seekable(struct spl_image_info *spl_image,
 		memset(&load, 0, sizeof(load));
 		load.bl_len = pagesize;
 		load.read = spl_romapi_read_seekable;
-		load.priv = &pagesize;
-		return spl_load_simple_fit(spl_image, &load, offset / pagesize, header);
+		return spl_load_simple_fit(spl_image, &load, offset, header);
 	} else if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER) &&
 		   valid_container_hdr((void *)header)) {
 		struct spl_load_info load;
@@ -118,9 +111,8 @@  static int spl_romapi_load_image_seekable(struct spl_image_info *spl_image,
 		memset(&load, 0, sizeof(load));
 		load.bl_len = pagesize;
 		load.read = spl_romapi_read_seekable;
-		load.priv = &pagesize;
 
-		ret = spl_load_imx_container(spl_image, &load, offset / pagesize);
+		ret = spl_load_imx_container(spl_image, &load, offset);
 	} else {
 		/* TODO */
 		puts("Can't support legacy image\n");
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 6084ead0919..ce7ef0efd0d 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -180,7 +180,7 @@  static int get_aligned_image_offset(struct spl_load_info *info, int offset)
 	if (info->filename)
 		return offset & ~(ARCH_DMA_MINALIGN - 1);
 
-	return offset / info->bl_len;
+	return ALIGN_DOWN(offset, info->bl_len);
 }
 
 static int get_aligned_image_overhead(struct spl_load_info *info, int offset)
@@ -205,7 +205,7 @@  static int get_aligned_image_size(struct spl_load_info *info, int data_size,
 	if (info->filename)
 		return data_size;
 
-	return (data_size + info->bl_len - 1) / info->bl_len;
+	return ALIGN(data_size, info->bl_len);
 }
 
 /**
@@ -222,7 +222,7 @@  static int get_aligned_image_size(struct spl_load_info *info, int data_size,
  *
  * Return:	0 on success or a negative error number.
  */
-static int load_simple_fit(struct spl_load_info *info, ulong sector,
+static int load_simple_fit(struct spl_load_info *info, ulong fit_offset,
 			   const struct spl_fit_info *ctx, int node,
 			   struct spl_image_info *image_info)
 {
@@ -234,7 +234,6 @@  static int load_simple_fit(struct spl_load_info *info, ulong sector,
 	void *load_ptr;
 	void *src;
 	ulong overhead;
-	int nr_sectors;
 	uint8_t image_comp = -1, type = -1;
 	const void *data;
 	const void *fit = ctx->fit;
@@ -291,11 +290,12 @@  static int load_simple_fit(struct spl_load_info *info, ulong sector,
 		length = len;
 
 		overhead = get_aligned_image_overhead(info, offset);
-		nr_sectors = get_aligned_image_size(info, length, offset);
+		size = get_aligned_image_size(info, length, offset);
 
 		if (info->read(info,
-			       sector + get_aligned_image_offset(info, offset),
-			       nr_sectors, src_ptr) != nr_sectors)
+			       fit_offset +
+			       get_aligned_image_offset(info, offset), size,
+			       src_ptr) != size)
 			return -EIO;
 
 		debug("External data: dst=%p, offset=%x, size=%lx\n",
@@ -380,7 +380,7 @@  __weak int board_spl_fit_append_fdt_skip(const char *name)
 }
 
 static int spl_fit_append_fdt(struct spl_image_info *spl_image,
-			      struct spl_load_info *info, ulong sector,
+			      struct spl_load_info *info, ulong offset,
 			      const struct spl_fit_info *ctx)
 {
 	struct spl_image_info image_info;
@@ -414,7 +414,7 @@  static int spl_fit_append_fdt(struct spl_image_info *spl_image,
 		spl_image->fdt_addr = map_sysmem(image_info.load_addr, size);
 		memcpy(spl_image->fdt_addr, gd->fdt_blob, size);
 	} else {
-		ret = load_simple_fit(info, sector, ctx, node, &image_info);
+		ret = load_simple_fit(info, offset, ctx, node, &image_info);
 		if (ret < 0)
 			return ret;
 
@@ -465,7 +465,7 @@  static int spl_fit_append_fdt(struct spl_image_info *spl_image,
 					      __func__);
 			}
 			image_info.load_addr = (ulong)tmpbuffer;
-			ret = load_simple_fit(info, sector, ctx, node,
+			ret = load_simple_fit(info, offset, ctx, node,
 					      &image_info);
 			if (ret < 0)
 				break;
@@ -642,7 +642,7 @@  static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
 }
 
 static int spl_fit_load_fpga(struct spl_fit_info *ctx,
-			     struct spl_load_info *info, ulong sector)
+			     struct spl_load_info *info, ulong offset)
 {
 	int node, ret;
 
@@ -657,7 +657,7 @@  static int spl_fit_load_fpga(struct spl_fit_info *ctx,
 	warn_deprecated("'fpga' property in config node. Use 'loadables'");
 
 	/* Load the image and set up the fpga_image structure */
-	ret = load_simple_fit(info, sector, ctx, node, &fpga_image);
+	ret = load_simple_fit(info, offset, ctx, node, &fpga_image);
 	if (ret) {
 		printf("%s: Cannot load the FPGA: %i\n", __func__, ret);
 		return ret;
@@ -667,11 +667,10 @@  static int spl_fit_load_fpga(struct spl_fit_info *ctx,
 }
 
 static int spl_simple_fit_read(struct spl_fit_info *ctx,
-			       struct spl_load_info *info, ulong sector,
+			       struct spl_load_info *info, ulong offset,
 			       const void *fit_header)
 {
 	unsigned long count, size;
-	int sectors;
 	void *buf;
 
 	/*
@@ -690,13 +689,13 @@  static int spl_simple_fit_read(struct spl_fit_info *ctx,
 	 * For FIT with data embedded, data is loaded as part of FIT image.
 	 * For FIT with external data, data is not loaded in this step.
 	 */
-	sectors = get_aligned_image_size(info, size, 0);
-	buf = board_spl_fit_buffer_addr(size, sectors, info->bl_len);
+	size = get_aligned_image_size(info, size, 0);
+	buf = board_spl_fit_buffer_addr(size, size, 1);
 
-	count = info->read(info, sector, sectors, buf);
+	count = info->read(info, offset, size, buf);
 	ctx->fit = buf;
-	debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu, size=0x%lx\n",
-	      sector, sectors, buf, count, size);
+	debug("fit read offset %lx, size=%lu, dst=%p, count=%lu\n",
+	      offset, size, buf, count);
 
 	return (count == 0) ? -EIO : 0;
 }
@@ -728,7 +727,7 @@  static int spl_simple_fit_parse(struct spl_fit_info *ctx)
 }
 
 int spl_load_simple_fit(struct spl_image_info *spl_image,
-			struct spl_load_info *info, ulong sector, void *fit)
+			struct spl_load_info *info, ulong offset, void *fit)
 {
 	struct spl_image_info image_info;
 	struct spl_fit_info ctx;
@@ -737,7 +736,7 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 	int index = 0;
 	int firmware_node;
 
-	ret = spl_simple_fit_read(&ctx, info, sector, fit);
+	ret = spl_simple_fit_read(&ctx, info, offset, fit);
 	if (ret < 0)
 		return ret;
 
@@ -752,7 +751,7 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 		return ret;
 
 	if (IS_ENABLED(CONFIG_SPL_FPGA))
-		spl_fit_load_fpga(&ctx, info, sector);
+		spl_fit_load_fpga(&ctx, info, offset);
 
 	/*
 	 * Find the U-Boot image using the following search order:
@@ -782,7 +781,7 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 	}
 
 	/* Load the image and set up the spl_image structure */
-	ret = load_simple_fit(info, sector, &ctx, node, spl_image);
+	ret = load_simple_fit(info, offset, &ctx, node, spl_image);
 	if (ret)
 		return ret;
 
@@ -800,7 +799,7 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 	 * We allow this to fail, as the U-Boot image might embed its FDT.
 	 */
 	if (os_takes_devicetree(spl_image->os)) {
-		ret = spl_fit_append_fdt(spl_image, info, sector, &ctx);
+		ret = spl_fit_append_fdt(spl_image, info, offset, &ctx);
 		if (ret < 0 && spl_image->os != IH_OS_U_BOOT)
 			return ret;
 	}
@@ -823,7 +822,7 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 			continue;
 
 		image_info.load_addr = 0;
-		ret = load_simple_fit(info, sector, &ctx, node, &image_info);
+		ret = load_simple_fit(info, offset, &ctx, node, &image_info);
 		if (ret < 0) {
 			printf("%s: can't load image loadables index %d (ret = %d)\n",
 			       __func__, index, ret);
@@ -837,7 +836,7 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 			debug("Loadable is %s\n", genimg_get_os_name(os_type));
 
 		if (os_takes_devicetree(os_type)) {
-			spl_fit_append_fdt(&image_info, info, sector, &ctx);
+			spl_fit_append_fdt(&image_info, info, offset, &ctx);
 			spl_image->fdt_addr = image_info.fdt_addr;
 		}
 
diff --git a/common/spl/spl_imx_container.c b/common/spl/spl_imx_container.c
index 1cc51782766..ad89a99fb23 100644
--- a/common/spl/spl_imx_container.c
+++ b/common/spl/spl_imx_container.c
@@ -19,11 +19,10 @@  static struct boot_img_t *read_auth_image(struct spl_image_info *spl_image,
 					  struct spl_load_info *info,
 					  struct container_hdr *container,
 					  int image_index,
-					  u32 container_sector)
+					  ulong container_offset)
 {
 	struct boot_img_t *images;
-	ulong sector;
-	u32 sectors;
+	ulong offset, overhead, size;
 
 	if (image_index > container->num_images) {
 		debug("Invalid image number\n");
@@ -39,16 +38,14 @@  static struct boot_img_t *read_auth_image(struct spl_image_info *spl_image,
 		return NULL;
 	}
 
-	sectors = ALIGN(images[image_index].size, info->bl_len) /
-		info->bl_len;
-	sector = images[image_index].offset / info->bl_len +
-		container_sector;
+	size = ALIGN(images[image_index].size, info->bl_len);
+	offset = images[image_index].offset + container_offset;
 
-	debug("%s: container: %p sector: %lu sectors: %u\n", __func__,
-	      container, sector, sectors);
-	if (info->read(info, sector, sectors,
-		       map_sysmem(images[image_index].dst,
-				  images[image_index].size)) != sectors) {
+	debug("%s: container: %p offset: %lu size: %lu\n", __func__,
+	      container, offset, size);
+	if (info->read(info, offset, size,
+		       map_sysmem(images[image_index].dst - overhead,
+				  images[image_index].size)) != size) {
 		printf("%s wrong\n", __func__);
 		return NULL;
 	}
@@ -62,15 +59,13 @@  static struct boot_img_t *read_auth_image(struct spl_image_info *spl_image,
 }
 
 static int read_auth_container(struct spl_image_info *spl_image,
-			       struct spl_load_info *info, ulong sector)
+			       struct spl_load_info *info, ulong offset)
 {
 	struct container_hdr *container = NULL;
 	u16 length;
-	u32 sectors;
 	int i, size, ret = 0;
 
 	size = ALIGN(CONTAINER_HDR_ALIGNMENT, info->bl_len);
-	sectors = size / info->bl_len;
 
 	/*
 	 * It will not override the ATF code, so safe to use it here,
@@ -80,9 +75,9 @@  static int read_auth_container(struct spl_image_info *spl_image,
 	if (!container)
 		return -ENOMEM;
 
-	debug("%s: container: %p sector: %lu sectors: %u\n", __func__,
-	      container, sector, sectors);
-	if (info->read(info, sector, sectors, container) != sectors) {
+	debug("%s: container: %p offset: %lu size: %u\n", __func__,
+	      container, offset, size);
+	if (info->read(info, offset, size, container) != size) {
 		ret = -EIO;
 		goto end;
 	}
@@ -104,17 +99,15 @@  static int read_auth_container(struct spl_image_info *spl_image,
 
 	if (length > CONTAINER_HDR_ALIGNMENT) {
 		size = ALIGN(length, info->bl_len);
-		sectors = size / info->bl_len;
 
 		free(container);
 		container = malloc(size);
 		if (!container)
 			return -ENOMEM;
 
-		debug("%s: container: %p sector: %lu sectors: %u\n",
-		      __func__, container, sector, sectors);
-		if (info->read(info, sector, sectors, container) !=
-		    sectors) {
+		debug("%s: container: %p offset: %lu size: %u\n",
+		      __func__, container, offset, size);
+		if (info->read(info, offset, size, container) != size) {
 			ret = -EIO;
 			goto end;
 		}
@@ -129,7 +122,7 @@  static int read_auth_container(struct spl_image_info *spl_image,
 	for (i = 0; i < container->num_images; i++) {
 		struct boot_img_t *image = read_auth_image(spl_image, info,
 							   container, i,
-							   sector);
+							   offset);
 
 		if (!image) {
 			ret = -EINVAL;
@@ -154,7 +147,7 @@  end:
 }
 
 int spl_load_imx_container(struct spl_image_info *spl_image,
-			   struct spl_load_info *info, ulong sector)
+			   struct spl_load_info *info, ulong offset)
 {
-	return read_auth_container(spl_image, info, sector);
+	return read_auth_container(spl_image, info, offset);
 }
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index 3d7551a7dae..9f41ea648ce 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -62,12 +62,14 @@  static int mmc_load_legacy(struct spl_image_info *spl_image,
 	return 0;
 }
 
-static ulong h_spl_load_read(struct spl_load_info *load, ulong sector,
-			     ulong count, void *buf)
+static ulong h_spl_load_read(struct spl_load_info *load, ulong off,
+			     ulong size, void *buf)
 {
-	struct mmc *mmc = load->priv;
+	struct blk_desc *bd = load->priv;
+	lbaint_t sector = off >> bd->log2blksz;
+	lbaint_t count = size >> bd->log2blksz;
 
-	return blk_dread(mmc_get_blk_desc(mmc), sector, count, buf);
+	return blk_dread(bd, sector, count, buf) << bd->log2blksz;
 }
 
 static __maybe_unused unsigned long spl_mmc_raw_uboot_offset(int part)
@@ -105,21 +107,23 @@  int mmc_load_image_raw_sector(struct spl_image_info *spl_image,
 		struct spl_load_info load;
 
 		debug("Found FIT\n");
-		load.priv = mmc;
+		load.priv = bd;
 		load.filename = NULL;
-		load.bl_len = mmc->read_bl_len;
+		load.bl_len = bd->blksz;
 		load.read = h_spl_load_read;
-		ret = spl_load_simple_fit(spl_image, &load, sector, header);
+		ret = spl_load_simple_fit(spl_image, &load,
+					  sector << bd->log2blksz, header);
 	} else if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER) &&
 		   valid_container_hdr((void *)header)) {
 		struct spl_load_info load;
 
-		load.priv = mmc;
+		load.priv = bd;
 		load.filename = NULL;
-		load.bl_len = mmc->read_bl_len;
+		load.bl_len = bd->blksz;
 		load.read = h_spl_load_read;
 
-		ret = spl_load_imx_container(spl_image, &load, sector);
+		ret = spl_load_imx_container(spl_image, &load,
+					     sector << bd->log2blksz);
 	} else {
 		ret = mmc_load_legacy(spl_image, bootdev, mmc, sector, header);
 	}
diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c
index 9a5a5ffa04a..1fcc89fa660 100644
--- a/common/spl/spl_nand.c
+++ b/common/spl/spl_nand.c
@@ -49,14 +49,12 @@  static ulong spl_nand_fit_read(struct spl_load_info *load, ulong offs,
 	ulong sector;
 
 	sector = *(int *)load->priv;
-	offs *= load->bl_len;
-	size *= load->bl_len;
 	offs = sector + nand_spl_adjust_offset(sector, offs - sector);
 	err = nand_spl_load_image(offs, size, dst);
 	if (err)
 		return 0;
 
-	return size / load->bl_len;
+	return size;
 }
 
 static ulong spl_nand_legacy_read(struct spl_load_info *load, ulong offs,
@@ -95,7 +93,7 @@  static int spl_nand_load_element(struct spl_image_info *spl_image,
 		load.filename = NULL;
 		load.bl_len = bl_len;
 		load.read = spl_nand_fit_read;
-		return spl_load_simple_fit(spl_image, &load, offset / bl_len, header);
+		return spl_load_simple_fit(spl_image, &load, offset, header);
 	} else if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER) &&
 		   valid_container_hdr((void *)header)) {
 		struct spl_load_info load;
@@ -104,7 +102,7 @@  static int spl_nand_load_element(struct spl_image_info *spl_image,
 		load.filename = NULL;
 		load.bl_len = bl_len;
 		load.read = spl_nand_fit_read;
-		return spl_load_imx_container(spl_image, &load, offset / bl_len);
+		return spl_load_imx_container(spl_image, &load, offset);
 	} else if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_FORMAT) &&
 		   image_get_magic(header) == IH_MAGIC) {
 		struct spl_load_info load;
diff --git a/include/spl.h b/include/spl.h
index 951e136b9ea..ecfc50e0095 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -297,10 +297,10 @@  struct spl_load_info {
 	 * read() - Read from device
 	 *
 	 * @load: Information about the load state
-	 * @sector: Sector number to read from (each @load->bl_len bytes)
-	 * @count: Number of sectors to read
+	 * @offset: Offset to read from in bytes, in multiples of @load->bl_len
+	 * @count: Number of bytes to read, in multiples of @load->bl_len
 	 * @buf: Buffer to read into
-	 * @return number of sectors read, 0 on error
+	 * @return number of bytes read, 0 on error
 	 */
 	ulong (*read)(struct spl_load_info *load, ulong sector, ulong count,
 		      void *buf);
@@ -368,7 +368,8 @@  void *spl_load_simple_fit_fix_load(const void *fit);
  * spl_load_simple_fit() - Loads a fit image from a device.
  * @spl_image:	Image description to set up
  * @info:	Structure containing the information required to load data.
- * @sector:	Sector number where FIT image is located in the device
+ * @offset:	Offset where FIT image is located in the device. Must be aligned
+ *              to the device's bl_len.
  * @fdt:	Pointer to the copied FIT header.
  *
  * Reads the FIT image @sector in the device. Loads u-boot image to
@@ -376,7 +377,7 @@  void *spl_load_simple_fit_fix_load(const void *fit);
  * Returns 0 on success.
  */
 int spl_load_simple_fit(struct spl_image_info *spl_image,
-			struct spl_load_info *info, ulong sector, void *fdt);
+			struct spl_load_info *info, ulong offset, void *fdt);
 
 #define SPL_COPY_PAYLOAD_ONLY	1
 #define SPL_FIT_FOUND		2
@@ -402,13 +403,14 @@  int spl_load_legacy_img(struct spl_image_info *spl_image,
  * spl_load_imx_container() - Loads a imx container image from a device.
  * @spl_image:	Image description to set up
  * @info:	Structure containing the information required to load data.
- * @sector:	Sector number where container image is located in the device
+ * @sector:	Offset where container image is located in the device. Must be
+ *              aligned to the device block size.
  *
  * Reads the container image @sector in the device. Loads u-boot image to
  * specified load address.
  */
 int spl_load_imx_container(struct spl_image_info *spl_image,
-			   struct spl_load_info *info, ulong sector);
+			   struct spl_load_info *info, ulong offset);
 
 /* SPL common functions */
 void preloader_console_init(void);
diff --git a/test/image/spl_load_os.c b/test/image/spl_load_os.c
index 49edf152d78..794cfad4e70 100644
--- a/test/image/spl_load_os.c
+++ b/test/image/spl_load_os.c
@@ -16,14 +16,13 @@  struct text_ctx {
 	int fd;
 };
 
-static ulong read_fit_image(struct spl_load_info *load, ulong sector,
-			    ulong count, void *buf)
+static ulong read_fit_image(struct spl_load_info *load, ulong offset,
+			    ulong size, void *buf)
 {
 	struct text_ctx *text_ctx = load->priv;
-	off_t offset, ret;
+	off_t ret;
 	ssize_t res;
 
-	offset = sector * load->bl_len;
 	ret = os_lseek(text_ctx->fd, offset, OS_SEEK_SET);
 	if (ret != offset) {
 		printf("Failed to seek to %zx, got %zx (errno=%d)\n", offset,
@@ -31,14 +30,14 @@  static ulong read_fit_image(struct spl_load_info *load, ulong sector,
 		return 0;
 	}
 
-	res = os_read(text_ctx->fd, buf, count * load->bl_len);
+	res = os_read(text_ctx->fd, buf, size);
 	if (res == -1) {
 		printf("Failed to read %lx bytes, got %ld (errno=%d)\n",
-		       count * load->bl_len, res, errno);
+		       size, res, errno);
 		return 0;
 	}
 
-	return count;
+	return size;
 }
 
 static int spl_test_load(struct unit_test_state *uts)