From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lokesh Vutla Date: Tue, 3 May 2016 19:42:28 +0530 Subject: [U-Boot] [PATCH v4 2/3] spl: Allow to load a FIT containing U-Boot from FS In-Reply-To: <5728A0BD.2030500@xilinx.com> References: <1462266311-27515-1-git-send-email-lokeshvutla@ti.com> <1462266311-27515-3-git-send-email-lokeshvutla@ti.com> <5728A0BD.2030500@xilinx.com> Message-ID: <5728B1CC.9010705@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Tuesday 03 May 2016 06:29 PM, Michal Simek wrote: > On 3.5.2016 11:05, Lokesh Vutla wrote: >> This provides a way to load a FIT containing U-Boot and a selection of device >> tree files from a File system. Making sure that all the reads and writes >> are aligned to their respective needs. >> >> Signed-off-by: Lokesh Vutla >> --- >> - Assuming info->priv is used for passing filename for now and using this for >> detecting if it a fs read. If this is not preferred I can create a new field >> for filename. >> >> common/spl/spl_fit.c | 76 +++++++++++++++++++++++++++++++++++++++++----------- >> include/spl.h | 10 +++++++ >> 2 files changed, 71 insertions(+), 15 deletions(-) >> >> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c >> index b1cfa97..d696354 100644 >> --- a/common/spl/spl_fit.c >> +++ b/common/spl/spl_fit.c >> @@ -82,6 +82,42 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp) >> return -ENOENT; >> } >> >> +static int get_aligned_image_offset(struct spl_load_info *info, int offset) >> +{ >> + /* >> + * If it is a FS read, get the first address before offset which is >> + * aligned to ARCH_DMA_MINALIGN. If it is raw read return the >> + * block number to which offset belongs. >> + */ >> + if (info->priv) >> + return offset & ~(ARCH_DMA_MINALIGN - 1); >> + else > > remove this else. Sure. > >> + return offset / info->bl_len; >> +} >> + >> +static int get_aligned_image_overhead(struct spl_load_info *info, int offset) >> +{ >> + /* >> + * If it is a FS read, get the difference between the offset and >> + * the first address before offset which is aligned to >> + * ARCH_DMA_MINALIGN. If it is raw read return the offset within the >> + * block. >> + */ >> + if (info->priv) >> + return offset & (ARCH_DMA_MINALIGN - 1); >> + else > > remove this else. Sure. > >> + return offset % info->bl_len; >> +} >> + >> +static int get_aligned_image_size(struct spl_load_info *info, int data_size, >> + int offset) >> +{ >> + if (info->priv) >> + return data_size + get_aligned_image_overhead(info, offset); >> + else > > remove this else. Sure. > > >> + return (data_size + info->bl_len - 1) / info->bl_len; >> +} >> + >> int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit) >> { >> int sectors; >> @@ -91,7 +127,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit) >> void *load_ptr; >> int fdt_offset, fdt_len; >> int data_offset, data_size; >> - int base_offset; >> + int base_offset, align_len; >> int src_sector; >> void *dst; >> >> @@ -117,8 +153,10 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit) >> * In fact the FIT has its own load address, but we assume it cannot >> * be before CONFIG_SYS_TEXT_BASE. >> */ >> - fit = (void *)(CONFIG_SYS_TEXT_BASE - size - info->bl_len); >> - sectors = (size + info->bl_len - 1) / info->bl_len; >> + align_len = ARCH_DMA_MINALIGN - 1; >> + fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len - >> + align_len) & ~align_len); > > Don't you want to use ALIGN macro here? ALIGN() will return the next aligned address which is equal to: fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len + align_len) & ~align_len); But here we need the previous aligned address so that we do not loose any data. Thanks and regards, Lokesh > >> + sectors = get_aligned_image_size(info, size, 0); >> count = info->read(info, sector, sectors, fit); >> debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu\n", >> sector, sectors, fit, count); >> @@ -151,19 +189,23 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit) >> * byte will be at 'load'. This may mean we need to load it starting >> * before then, since we can only read whole blocks. >> */ >> - sectors = (data_size + info->bl_len - 1) / info->bl_len; >> + sectors = get_aligned_image_size(info, data_size, data_offset); >> data_offset += base_offset; >> load_ptr = (void *)load; >> debug("U-Boot size %x, data %p\n", data_size, load_ptr); >> - dst = load_ptr - (data_offset % info->bl_len); >> + dst = load_ptr; >> >> /* Read the image */ >> - src_sector = sector + data_offset / info->bl_len; >> - debug("image: data_offset=%x, dst=%p, src_sector=%x, sectors=%x\n", >> - data_offset, dst, src_sector, sectors); >> + src_sector = sector + get_aligned_image_offset(info, data_offset); >> + debug("Aligned image read: dst=%p, src_sector=%x, sectors=%x\n", >> + dst, src_sector, sectors); >> count = info->read(info, src_sector, sectors, dst); >> if (count != sectors) >> return -EIO; >> + debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset, >> + data_size); >> + memcpy(dst, dst + get_aligned_image_overhead(info, data_offset), >> + data_size); >> >> /* Figure out which device tree the board wants to use */ >> fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset); >> @@ -173,14 +215,15 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit) >> /* >> * Read the device tree and place it after the image. There may be >> * some extra data before it since we can only read entire blocks. >> + * And also align the destination address to ARCH_DMA_MINALIGN. >> */ >> - dst = load_ptr + data_size; >> + dst = (void *)((load + data_size + align_len) & ~align_len); > > Same here. > > Thanks, > Michal >