[U-Boot] [PATCH 0/5] preparation to intriduce keystone support
The Texas Instruments Keystone SOC u-boot support requires some common code modifications to be done before introducing the support itself.
This series of patches makes these modifications.
Signed-off-by: Vitaly Andrianov vitalya@ti.com Signed-off-by: Murali Karicheri m-karicheri2@ti.com
Murali Karicheri (5): tools: mkimage: add support for gpimage format ubifs: fix checkpatch warning ubifs: return filesize from ubifs load operations arm: add support for arch timer NAND: DaVinci: allow forced disable of subpage writes
arch/arm/lib/Makefile | 1 + arch/arm/lib/arch_timer.c | 69 ++++++++++++++++++++++++++ common/cmd_ubifs.c | 16 ++---- common/image.c | 1 + drivers/mtd/nand/davinci_nand.c | 3 ++ fs/ubifs/ubifs.c | 14 +++--- fs/ubifs/ubifs.h | 7 +++ include/image.h | 1 + tools/Makefile | 6 +++ tools/gpheader.h | 41 ++++++++++++++++ tools/gpimage-common.c | 103 +++++++++++++++++++++++++++++++++++++++ tools/gpimage.c | 78 +++++++++++++++++++++++++++++ tools/imagetool.c | 2 + tools/imagetool.h | 1 + tools/omapimage.c | 97 ++++++------------------------------ tools/omapimage.h | 5 -- 16 files changed, 340 insertions(+), 105 deletions(-) create mode 100644 arch/arm/lib/arch_timer.c create mode 100644 tools/gpheader.h create mode 100644 tools/gpimage-common.c create mode 100644 tools/gpimage.c
This patch add support for gpimage format as a preparatory patch for porting u-boot for keystone2 devices and is based on omapimage format. It re-uses gph header to store the size and loadaddr as done in omapimage.c
Signed-off-by: Vitaly Andrianov vitalya@ti.com Signed-off-by: Murali Karicheri m-karicheri2@ti.com --- - Tested only on keystone. Need help to test on OMAP since this is impacted.
common/image.c | 1 + include/image.h | 1 + tools/Makefile | 6 +++ tools/gpheader.h | 41 +++++++++++++++++++ tools/gpimage-common.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++ tools/gpimage.c | 78 ++++++++++++++++++++++++++++++++++++ tools/imagetool.c | 2 + tools/imagetool.h | 1 + tools/omapimage.c | 97 +++++++-------------------------------------- tools/omapimage.h | 5 --- 10 files changed, 248 insertions(+), 87 deletions(-) create mode 100644 tools/gpheader.h create mode 100644 tools/gpimage-common.c create mode 100644 tools/gpimage.c
diff --git a/common/image.c b/common/image.c index ae95c3f..52ee840 100644 --- a/common/image.c +++ b/common/image.c @@ -137,6 +137,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_STANDALONE, "standalone", "Standalone Program", }, { IH_TYPE_UBLIMAGE, "ublimage", "Davinci UBL image",}, { IH_TYPE_MXSIMAGE, "mxsimage", "Freescale MXS Boot Image",}, + { IH_TYPE_GPIMAGE, "gpimage", "TI KeyStone SPL Image",}, { -1, "", "", }, };
diff --git a/include/image.h b/include/image.h index 7de2bb2..0a3d346 100644 --- a/include/image.h +++ b/include/image.h @@ -214,6 +214,7 @@ struct lmb; #define IH_TYPE_KERNEL_NOLOAD 14 /* OS Kernel Image, can run from any load address */ #define IH_TYPE_PBLIMAGE 15 /* Freescale PBL Boot Image */ #define IH_TYPE_MXSIMAGE 16 /* Freescale MXSBoot Image */ +#define IH_TYPE_GPIMAGE 17 /* TI Keystone GPHeader Image */
/* * Compression Types diff --git a/tools/Makefile b/tools/Makefile index 328cea3..cbbe479 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -84,7 +84,9 @@ NOPED_OBJ_FILES-y += imagetool.o NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o NOPED_OBJ_FILES-y += mxsimage.o +NOPED_OBJ_FILES-y += gpimage-common.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += gpimage.o NOPED_OBJ_FILES-y += os_support.o NOPED_OBJ_FILES-y += pblimage.o NOPED_OBJ_FILES-y += proftool.o @@ -219,7 +221,9 @@ $(obj)dumpimage$(SFX): $(obj)aisimage.o \ $(obj)dumpimage.o \ $(obj)md5.o \ $(obj)mxsimage.o \ + $(obj)gpimage-common.o \ $(obj)omapimage.o \ + $(obj)gpimage.o \ $(obj)os_support.o \ $(obj)pblimage.o \ $(obj)sha1.o \ @@ -248,7 +252,9 @@ $(obj)mkimage$(SFX): $(obj)aisimage.o \ $(obj)md5.o \ $(obj)mkimage.o \ $(obj)mxsimage.o \ + $(obj)gpimage-common.o \ $(obj)omapimage.o \ + $(obj)gpimage.o \ $(obj)os_support.o \ $(obj)pblimage.o \ $(obj)sha1.o \ diff --git a/tools/gpheader.h b/tools/gpheader.h new file mode 100644 index 0000000..0bfc0c2 --- /dev/null +++ b/tools/gpheader.h @@ -0,0 +1,41 @@ +/* + * (C) Copyright 2014 + * Texas Instruments Incorporated + * Refactored common functions in to gpimage-common.c. Include this common + * header file + * + * (C) Copyright 2010 + * Linaro LTD, www.linaro.org + * Author: John Rigby john.rigby@linaro.org + * Based on TI's signGP.c + * + * (C) Copyright 2009 + * Stefano Babic, DENX Software Engineering, sbabic@denx.de. + * + * (C) Copyright 2008 + * Marvell Semiconductor <www.marvell.com> + * Written-by: Prafulla Wadaskar prafulla@marvell.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _GPIMAGE_H_ +#define _GPIMAGE_H_ + +/* common headers for gpimage and omapimage formats */ +struct gp_header { + uint32_t size; + uint32_t load_addr; +}; +#define GPIMAGE_HDR_SIZE (sizeof(struct gp_header)) + +/* common functions across gpimage and omapimage handlers */ +uint32_t gpimage_swap32(uint32_t data); +int valid_gph_size(uint32_t size); +int valid_gph_load_addr(uint32_t load_addr); +int gph_verify_header(struct gp_header *gph, int do_swap32); +void gph_print_header(const struct gp_header *gph, int do_swap32); +void gph_set_header(struct gp_header *gph, uint32_t size, uint32_t load_addr, + int do_swap32); +int gpimage_check_params(struct image_tool_params *params); +#endif diff --git a/tools/gpimage-common.c b/tools/gpimage-common.c new file mode 100644 index 0000000..85b9819 --- /dev/null +++ b/tools/gpimage-common.c @@ -0,0 +1,103 @@ +/* + * (C) Copyright 2014 + * Texas Instruments Incorporated + * Refactored common functions in to gpimage-common.c. + * + * (C) Copyright 2010 + * Linaro LTD, www.linaro.org + * Author: John Rigby john.rigby@linaro.org + * Based on TI's signGP.c + * + * (C) Copyright 2009 + * Stefano Babic, DENX Software Engineering, sbabic@denx.de. + * + * (C) Copyright 2008 + * Marvell Semiconductor <www.marvell.com> + * Written-by: Prafulla Wadaskar prafulla@marvell.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include "imagetool.h" +#include <compiler.h> +#include <image.h> +#include "gpheader.h" + +uint32_t gpimage_swap32(uint32_t data) +{ + return cpu_to_be32(data); +} + +/* TODO: do we need the below 2 functions?? */ +int valid_gph_size(uint32_t size) +{ + return size; +} + +int valid_gph_load_addr(uint32_t load_addr) +{ + return load_addr; +} + +int gph_verify_header(struct gp_header *gph, int do_swap32) +{ + uint32_t gph_size, gph_load_addr; + + if (do_swap32) { + gph_size = gpimage_swap32(gph->size); + gph_load_addr = gpimage_swap32(gph->load_addr); + } else { + gph_size = gph->size; + gph_load_addr = gph->load_addr; + } + + if (!valid_gph_size(gph_size)) + return -1; + if (!valid_gph_load_addr(gph_load_addr)) + return -1; + return 0; +} + +void gph_print_header(const struct gp_header *gph, int do_swap32) +{ + uint32_t gph_size, gph_load_addr; + + if (do_swap32) { + gph_size = gpimage_swap32(gph->size); + gph_load_addr = gpimage_swap32(gph->load_addr); + } else { + gph_size = gph->size; + gph_load_addr = gph->load_addr; + } + + if (!valid_gph_size(gph_size)) { + fprintf(stderr, "Error: invalid image size %x\n", gph_size); + exit(EXIT_FAILURE); + } + + if (!valid_gph_load_addr(gph_load_addr)) { + fprintf(stderr, "Error: invalid image load address %x\n", + gph_load_addr); + exit(EXIT_FAILURE); + } + printf("GP Header: Size %x LoadAddr %x\n", gph_size, gph_load_addr); +} + +void gph_set_header(struct gp_header *gph, uint32_t size, uint32_t load_addr, + int do_swap32) +{ + if (do_swap32) { + gph->size = gpimage_swap32(size); + gph->load_addr = gpimage_swap32(load_addr); + } else { + gph->size = size; + gph->load_addr = load_addr; + } +} + +int gpimage_check_params(struct image_tool_params *params) +{ + return (params->dflag && (params->fflag || params->lflag)) || + (params->fflag && (params->dflag || params->lflag)) || + (params->lflag && (params->dflag || params->fflag)); +} diff --git a/tools/gpimage.c b/tools/gpimage.c new file mode 100644 index 0000000..c316bfb --- /dev/null +++ b/tools/gpimage.c @@ -0,0 +1,78 @@ +/* + * (C) Copyright 2014 + * Texas Instruments Incorporated + * Add gpimage format for keystone devices to format spl image. This is + * Based on omapimage.c + * + * (C) Copyright 2010 + * Linaro LTD, www.linaro.org + * Author: John Rigby john.rigby@linaro.org + * Based on TI's signGP.c + * + * (C) Copyright 2009 + * Stefano Babic, DENX Software Engineering, sbabic@denx.de. + * + * (C) Copyright 2008 + * Marvell Semiconductor <www.marvell.com> + * Written-by: Prafulla Wadaskar prafulla@marvell.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include "imagetool.h" +#include <compiler.h> +#include <image.h> +#include "gpheader.h" + +static uint8_t gpimage_header[GPIMAGE_HDR_SIZE]; + +/* to be in keystone gpimage */ +static int gpimage_check_image_types(uint8_t type) +{ + if (type == IH_TYPE_GPIMAGE) + return EXIT_SUCCESS; + else + return EXIT_FAILURE; +} + +static int gpimage_verify_header(unsigned char *ptr, int image_size, + struct image_tool_params *params) +{ + struct gp_header *gph = (struct gp_header *)ptr; + + return gph_verify_header(gph, 1); +} + +static void gpimage_print_header(const void *ptr) +{ + const struct gp_header *gph = (struct gp_header *)ptr; + + gph_print_header(gph, 1); +} + +static void gpimage_set_header(void *ptr, struct stat *sbuf, int ifd, + struct image_tool_params *params) +{ + struct gp_header *gph = (struct gp_header *)ptr; + + gph_set_header(gph, sbuf->st_size - GPIMAGE_HDR_SIZE, params->addr, 1); +} + +/* + * gpimage parameters + */ +static struct image_type_params gpimage_params = { + .name = "TI KeyStone GP Image support", + .header_size = GPIMAGE_HDR_SIZE, + .hdr = (void *)&gpimage_header, + .check_image_type = gpimage_check_image_types, + .verify_header = gpimage_verify_header, + .print_header = gpimage_print_header, + .set_header = gpimage_set_header, + .check_params = gpimage_check_params, +}; + +void init_gpimage_type(void) +{ + register_image_type(&gpimage_params); +} diff --git a/tools/imagetool.c b/tools/imagetool.c index 29d2189..da72115 100644 --- a/tools/imagetool.c +++ b/tools/imagetool.c @@ -45,6 +45,8 @@ void register_image_tool(imagetool_register_t image_register) init_ubl_image_type(); /* Init Davinci AIS support */ init_ais_image_type(); + /* Init TI Keystone boot image generation/list support */ + init_gpimage_type(); }
/* diff --git a/tools/imagetool.h b/tools/imagetool.h index c2c9aea..a3e9d30 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -167,6 +167,7 @@ void init_mxs_image_type(void); void init_fit_image_type(void); void init_ubl_image_type(void); void init_omap_image_type(void); +void init_gpimage_type(void);
void pbl_load_uboot(int fd, struct image_tool_params *mparams);
diff --git a/tools/omapimage.c b/tools/omapimage.c index d59bc4d..de5a50a 100644 --- a/tools/omapimage.c +++ b/tools/omapimage.c @@ -15,26 +15,17 @@ */
#include "imagetool.h" +#include <compiler.h> #include <image.h> +#include "gpheader.h" #include "omapimage.h"
/* Header size is CH header rounded up to 512 bytes plus GP header */ #define OMAP_CH_HDR_SIZE 512 -#define OMAP_GP_HDR_SIZE (sizeof(struct gp_header)) -#define OMAP_FILE_HDR_SIZE (OMAP_CH_HDR_SIZE+OMAP_GP_HDR_SIZE) +#define OMAP_FILE_HDR_SIZE (OMAP_CH_HDR_SIZE + GPIMAGE_HDR_SIZE)
static int do_swap32 = 0;
-static uint32_t omapimage_swap32(uint32_t data) -{ - uint32_t result = 0; - result = (data & 0xFF000000) >> 24; - result |= (data & 0x00FF0000) >> 8; - result |= (data & 0x0000FF00) << 8; - result |= (data & 0x000000FF) << 24; - return result; -} - static uint8_t omapimage_header[OMAP_FILE_HDR_SIZE];
static int omapimage_check_image_types(uint8_t type) @@ -46,40 +37,18 @@ static int omapimage_check_image_types(uint8_t type) } }
-/* - * Only the simplest image type is currently supported: - * TOC pointing to CHSETTINGS - * TOC terminator - * CHSETTINGS - * - * padding to OMAP_CH_HDR_SIZE bytes - * - * gp header - * size - * load_addr - */ -static int valid_gph_size(uint32_t size) -{ - return size; -} - -static int valid_gph_load_addr(uint32_t load_addr) -{ - return load_addr; -} - static int omapimage_verify_header(unsigned char *ptr, int image_size, struct image_tool_params *params) { struct ch_toc *toc = (struct ch_toc *)ptr; struct gp_header *gph = (struct gp_header *)(ptr+OMAP_CH_HDR_SIZE); - uint32_t offset, size, gph_size, gph_load_addr; + uint32_t offset, size;
while (toc->section_offset != 0xffffffff && toc->section_size != 0xffffffff) { if (do_swap32) { - offset = omapimage_swap32(toc->section_offset); - size = omapimage_swap32(toc->section_size); + offset = gpimage_swap32(toc->section_offset); + size = gpimage_swap32(toc->section_size); } else { offset = toc->section_offset; size = toc->section_size; @@ -92,17 +61,7 @@ static int omapimage_verify_header(unsigned char *ptr, int image_size, toc++; }
- if (do_swap32) { - gph_size = omapimage_swap32(gph->size); - gph_load_addr = omapimage_swap32(gph->load_addr); - } else { - gph_size = gph->size; - gph_load_addr = gph->load_addr; - } - - if (!valid_gph_size(gph_size)) - return -1; - if (!valid_gph_load_addr(gph_load_addr)) + if (gph_verify_header(gph, do_swap32) < 0) return -1;
return 0; @@ -135,13 +94,13 @@ static void omapimage_print_header(const void *ptr) const struct ch_toc *toc = (struct ch_toc *)ptr; const struct gp_header *gph = (struct gp_header *)(ptr+OMAP_CH_HDR_SIZE); - uint32_t offset, size, gph_size, gph_load_addr; + uint32_t offset, size;
while (toc->section_offset != 0xffffffff && toc->section_size != 0xffffffff) { if (do_swap32) { - offset = omapimage_swap32(toc->section_offset); - size = omapimage_swap32(toc->section_size); + offset = gpimage_swap32(toc->section_offset); + size = gpimage_swap32(toc->section_size); } else { offset = toc->section_offset; size = toc->section_size; @@ -160,26 +119,7 @@ static void omapimage_print_header(const void *ptr) toc++; }
- if (do_swap32) { - gph_size = omapimage_swap32(gph->size); - gph_load_addr = omapimage_swap32(gph->load_addr); - } else { - gph_size = gph->size; - gph_load_addr = gph->load_addr; - } - - if (!valid_gph_size(gph_size)) { - fprintf(stderr, "Error: invalid image size %x\n", gph_size); - exit(EXIT_FAILURE); - } - - if (!valid_gph_load_addr(gph_load_addr)) { - fprintf(stderr, "Error: invalid image load address %x\n", - gph_load_addr); - exit(EXIT_FAILURE); - } - - printf("GP Header: Size %x LoadAddr %x\n", gph_size, gph_load_addr); + gph_print_header(gph, do_swap32); }
static int toc_offset(void *hdr, void *member) @@ -208,8 +148,8 @@ static void omapimage_set_header(void *ptr, struct stat *sbuf, int ifd, toc++; memset(toc, 0xff, sizeof(*toc));
- gph->size = sbuf->st_size - OMAP_FILE_HDR_SIZE; - gph->load_addr = params->addr; + gph_set_header(gph, sbuf->st_size - OMAP_FILE_HDR_SIZE, + params->addr, 0);
if (strncmp(params->imagename, "byteswap", 8) == 0) { do_swap32 = 1; @@ -217,20 +157,13 @@ static void omapimage_set_header(void *ptr, struct stat *sbuf, int ifd, uint32_t *data = (uint32_t *)ptr;
while (swapped <= (sbuf->st_size / sizeof(uint32_t))) { - *data = omapimage_swap32(*data); + *data = gpimage_swap32(*data); swapped++; data++; } } }
-int omapimage_check_params(struct image_tool_params *params) -{ - return (params->dflag && (params->fflag || params->lflag)) || - (params->fflag && (params->dflag || params->lflag)) || - (params->lflag && (params->dflag || params->fflag)); -} - /* * omapimage parameters */ @@ -242,7 +175,7 @@ static struct image_type_params omapimage_params = { .verify_header = omapimage_verify_header, .print_header = omapimage_print_header, .set_header = omapimage_set_header, - .check_params = omapimage_check_params, + .check_params = gpimage_check_params, };
void init_omap_image_type(void) diff --git a/tools/omapimage.h b/tools/omapimage.h index 45d14ea..8744ae7 100644 --- a/tools/omapimage.h +++ b/tools/omapimage.h @@ -25,10 +25,5 @@ struct ch_settings { uint32_t flags; };
-struct gp_header { - uint32_t size; - uint32_t load_addr; -}; - #define KEY_CHSETTINGS 0xC0C0C0C1 #endif /* _OMAPIMAGE_H_ */
Dear Murali Karicheri,
In message 1390255810-19486-2-git-send-email-m-karicheri2@ti.com you wrote:
This patch add support for gpimage format as a preparatory patch for porting u-boot for keystone2 devices and is based on omapimage format. It re-uses gph header to store the size and loadaddr as done in omapimage.c
...
--- a/common/image.c +++ b/common/image.c @@ -137,6 +137,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_STANDALONE, "standalone", "Standalone Program", }, { IH_TYPE_UBLIMAGE, "ublimage", "Davinci UBL image",}, { IH_TYPE_MXSIMAGE, "mxsimage", "Freescale MXS Boot Image",},
- { IH_TYPE_GPIMAGE, "gpimage", "TI KeyStone SPL Image",}, { -1, "", "", },
Please keep the list sorted. While you are at it, please also fi the incorrect placement of the IH_TYPE_MXSIMAGE entry.
--- a/tools/Makefile +++ b/tools/Makefile @@ -84,7 +84,9 @@ NOPED_OBJ_FILES-y += imagetool.o NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o NOPED_OBJ_FILES-y += mxsimage.o +NOPED_OBJ_FILES-y += gpimage-common.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += gpimage.o NOPED_OBJ_FILES-y += os_support.o
Please keep the list sorted.
@@ -219,7 +221,9 @@ $(obj)dumpimage$(SFX): $(obj)aisimage.o \ $(obj)dumpimage.o \ $(obj)md5.o \ $(obj)mxsimage.o \
$(obj)gpimage-common.o \ $(obj)omapimage.o \$(obj)gpimage.o \
Ditto. Please fix this issue globally.
+uint32_t gpimage_swap32(uint32_t data) +{
- return cpu_to_be32(data);
+}
The function name is misleading. Also it is not clear why you need this function at all. You can use cpu_to_be32() directly - that will make the code much easier to read.
+/* TODO: do we need the below 2 functions?? */ +int valid_gph_size(uint32_t size) +{
- return size;
+}
+int valid_gph_load_addr(uint32_t load_addr) +{
- return load_addr;
+}
These functions are obviously bogus. Please dump them.
+int gph_verify_header(struct gp_header *gph, int do_swap32) +{
- uint32_t gph_size, gph_load_addr;
- if (do_swap32) {
gph_size = gpimage_swap32(gph->size);gph_load_addr = gpimage_swap32(gph->load_addr);- } else {
gph_size = gph->size;gph_load_addr = gph->load_addr;- }
I think it should be possible top write this code in such a way that you can avoid both the if-else and passing the do_swap32 parameter. It is my impression that the whole endianess handling needs some refinemant.
Actually I cannot see a place where do_swap32=0 is used..
+void gph_print_header(const struct gp_header *gph, int do_swap32) +{
- uint32_t gph_size, gph_load_addr;
- if (do_swap32) {
gph_size = gpimage_swap32(gph->size);gph_load_addr = gpimage_swap32(gph->load_addr);- } else {
gph_size = gph->size;gph_load_addr = gph->load_addr;- }
This is repeasted code. Get rid of it.
+void gph_set_header(struct gp_header *gph, uint32_t size, uint32_t load_addr,
- int do_swap32)
+{
- if (do_swap32) {
gph->size = gpimage_swap32(size);gph->load_addr = gpimage_swap32(load_addr);- } else {
gph->size = size;gph->load_addr = load_addr;- }
And again.
Best regards,
Wolfgang Denk
On Mon, Jan 20, 2014 at 11:46:49PM +0100, Wolfgang Denk wrote:
Dear Murali Karicheri,
In message 1390255810-19486-2-git-send-email-m-karicheri2@ti.com you wrote:
This patch add support for gpimage format as a preparatory patch for porting u-boot for keystone2 devices and is based on omapimage format. It re-uses gph header to store the size and loadaddr as done in omapimage.c
...
--- a/common/image.c +++ b/common/image.c @@ -137,6 +137,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_STANDALONE, "standalone", "Standalone Program", }, { IH_TYPE_UBLIMAGE, "ublimage", "Davinci UBL image",}, { IH_TYPE_MXSIMAGE, "mxsimage", "Freescale MXS Boot Image",},
- { IH_TYPE_GPIMAGE, "gpimage", "TI KeyStone SPL Image",}, { -1, "", "", },
Please keep the list sorted. While you are at it, please also fi the incorrect placement of the IH_TYPE_MXSIMAGE entry.
--- a/tools/Makefile +++ b/tools/Makefile @@ -84,7 +84,9 @@ NOPED_OBJ_FILES-y += imagetool.o NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o NOPED_OBJ_FILES-y += mxsimage.o +NOPED_OBJ_FILES-y += gpimage-common.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += gpimage.o NOPED_OBJ_FILES-y += os_support.o
Please keep the list sorted.
@@ -219,7 +221,9 @@ $(obj)dumpimage$(SFX): $(obj)aisimage.o \ $(obj)dumpimage.o \ $(obj)md5.o \ $(obj)mxsimage.o \
$(obj)gpimage-common.o \ $(obj)omapimage.o \$(obj)gpimage.o \Ditto. Please fix this issue globally.
+uint32_t gpimage_swap32(uint32_t data) +{
- return cpu_to_be32(data);
+}
The function name is misleading. Also it is not clear why you need this function at all. You can use cpu_to_be32() directly - that will make the code much easier to read.
+/* TODO: do we need the below 2 functions?? */ +int valid_gph_size(uint32_t size) +{
- return size;
+}
+int valid_gph_load_addr(uint32_t load_addr) +{
- return load_addr;
+}
These functions are obviously bogus. Please dump them.
+int gph_verify_header(struct gp_header *gph, int do_swap32) +{
- uint32_t gph_size, gph_load_addr;
- if (do_swap32) {
gph_size = gpimage_swap32(gph->size);gph_load_addr = gpimage_swap32(gph->load_addr);- } else {
gph_size = gph->size;gph_load_addr = gph->load_addr;- }
I think it should be possible top write this code in such a way that you can avoid both the if-else and passing the do_swap32 parameter. It is my impression that the whole endianess handling needs some refinemant.
Actually I cannot see a place where do_swap32=0 is used..
This appears to be a blind re-use of the omap code where we do have a case where we have to do this kind of swap.
Fix the following checkpatch warning:-
WARNING: externs should be avoided in .c files
Signed-off-by: Murali Karicheri m-karicheri2@ti.com --- common/cmd_ubifs.c | 9 --------- fs/ubifs/ubifs.h | 7 +++++++ 2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c index d9af023..fdc8bfe 100644 --- a/common/cmd_ubifs.c +++ b/common/cmd_ubifs.c @@ -21,15 +21,6 @@ static int ubifs_initialized; static int ubifs_mounted;
-extern struct super_block *ubifs_sb; - -/* Prototypes */ -int ubifs_init(void); -int ubifs_mount(char *vol_name); -void ubifs_umount(struct ubifs_info *c); -int ubifs_ls(char *dir_name); -int ubifs_load(char *filename, u32 addr, u32 size); - int do_ubifs_mount(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { char *vol_name; diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 633631e..2213201 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -2137,6 +2137,13 @@ void ubifs_compress(const void *in_buf, int in_len, void *out_buf, int *out_len, int ubifs_decompress(const void *buf, int len, void *out, int *out_len, int compr_type);
+/* these are used in cmd_ubifs.c */ +int ubifs_init(void); +int ubifs_mount(char *vol_name); +void ubifs_umount(struct ubifs_info *c); +int ubifs_ls(char *dir_name); +int ubifs_load(char *filename, u32 addr, u32 size); + #include "debug.h" #include "misc.h" #include "key.h"
On Mon, Jan 20, 2014 at 05:10:07PM -0500, Karicheri, Muralidharan wrote:
Fix the following checkpatch warning:-
WARNING: externs should be avoided in .c files
Signed-off-by: Murali Karicheri m-karicheri2@ti.com
Applied to u-boot/master, thanks!
This patch extends the ubifs load implementation to return the size of the file loaded from the filesystem. The ubifs command implementation has also been modified to set the filesize environment variable accordingly.
Signed-off-by: Vitaly Andrianov vitalya@ti.com Signed-off-by: Murali Karicheri m-karicheri2@ti.com --- common/cmd_ubifs.c | 7 +++++-- fs/ubifs/ubifs.c | 14 +++++++------- fs/ubifs/ubifs.h | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c index fdc8bfe..a2926e0 100644 --- a/common/cmd_ubifs.c +++ b/common/cmd_ubifs.c @@ -110,6 +110,7 @@ int do_ubifs_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) int ret; u32 addr; u32 size = 0; + char buf[32];
if (!ubifs_mounted) { printf("UBIFS not mounted, use ubifs mount to mount volume first!\n"); @@ -132,12 +133,14 @@ int do_ubifs_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } debug("Loading file '%s' to address 0x%08x (size %d)\n", filename, addr, size);
- ret = ubifs_load(filename, addr, size); + ret = ubifs_load(filename, addr, &size); if (ret) { printf("** File not found %s **\n", filename); ret = CMD_RET_FAILURE; + } else { + sprintf(buf, "%X", size); + setenv("filesize", buf); } - return ret; }
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 273c0a9..4f1d5c7 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -677,7 +677,7 @@ error: return err; }
-int ubifs_load(char *filename, u32 addr, u32 size) +int ubifs_load(char *filename, u32 addr, u32 *size) { struct ubifs_info *c = ubifs_sb->s_fs_info; unsigned long inum; @@ -711,12 +711,12 @@ int ubifs_load(char *filename, u32 addr, u32 size) * If no size was specified or if size bigger than filesize * set size to filesize */ - if ((size == 0) || (size > inode->i_size)) - size = inode->i_size; + if ((*size == 0) || (*size > inode->i_size)) + *size = inode->i_size;
- count = (size + UBIFS_BLOCK_SIZE - 1) >> UBIFS_BLOCK_SHIFT; + count = (*size + UBIFS_BLOCK_SIZE - 1) >> UBIFS_BLOCK_SHIFT; printf("Loading file '%s' to addr 0x%08x with size %d (0x%08x)...\n", - filename, addr, size, size); + filename, addr, *size, *size);
page.addr = (void *)addr; page.index = 0; @@ -725,8 +725,8 @@ int ubifs_load(char *filename, u32 addr, u32 size) /* * Make sure to not read beyond the requested size */ - if (((i + 1) == count) && (size < inode->i_size)) - last_block_size = size - (i * PAGE_SIZE); + if (((i + 1) == count) && (*size < inode->i_size)) + last_block_size = *size - (i * PAGE_SIZE);
err = do_readpage(c, inode, &page, last_block_size); if (err) diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 2213201..00485b3 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -2142,7 +2142,7 @@ int ubifs_init(void); int ubifs_mount(char *vol_name); void ubifs_umount(struct ubifs_info *c); int ubifs_ls(char *dir_name); -int ubifs_load(char *filename, u32 addr, u32 size); +int ubifs_load(char *filename, u32 addr, u32 *size);
#include "debug.h" #include "misc.h"
On Mon, Jan 20, 2014 at 05:10:08PM -0500, Murali Karicheri wrote:
This patch extends the ubifs load implementation to return the size of the file loaded from the filesystem. The ubifs command implementation has also been modified to set the filesize environment variable accordingly.
Signed-off-by: Vitaly Andrianov vitalya@ti.com Signed-off-by: Murali Karicheri m-karicheri2@ti.com
But we already have code to do this, which now produces warnings when this patch is applied. Can you confirm that we are in fact not setting filesize already when loading from ubifs? Thanks!
This patch add basic support for the architecture timer found on recent ARMv7 based SoCs.
Signed-off-by: Vitaly Andrianov vitalya@ti.com Signed-off-by: Murali Karicheri m-karicheri2@ti.com --- arch/arm/lib/Makefile | 1 + arch/arm/lib/arch_timer.c | 69 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 arch/arm/lib/arch_timer.c
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 321997c..58d58f8 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -42,6 +42,7 @@ obj-$(CONFIG_USE_ARCH_MEMCPY) += memcpy.o else obj-$(CONFIG_SPL_FRAMEWORK) += spl.o endif +COBJS-$(CONFIG_SYS_ARCH_TIMER) += arch_timer.o
ifdef CONFIG_ARM64 obj-y += interrupts_64.o diff --git a/arch/arm/lib/arch_timer.c b/arch/arm/lib/arch_timer.c new file mode 100644 index 0000000..6ffa650 --- /dev/null +++ b/arch/arm/lib/arch_timer.c @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2012, Texas Instruments <www.ti.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include <common.h> +#include <asm/io.h> +#include <div64.h> + +DECLARE_GLOBAL_DATA_PTR; + +int timer_init(void) +{ + gd->arch.tbl = 0; + gd->arch.tbu = 0; + + gd->arch.timer_rate_hz = CONFIG_SYS_HZ_CLOCK / CONFIG_SYS_HZ; + + return 0; +} + +unsigned long long get_ticks(void) +{ + ulong nowl, nowu; + + asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (nowl), "=r" (nowu)); + + gd->arch.tbl = nowl; + gd->arch.tbu = nowu; + + return (((unsigned long long)gd->arch.tbu) << 32) | gd->arch.tbl; +} + + +ulong get_timer(ulong base) +{ + return lldiv(get_ticks(), gd->arch.timer_rate_hz) - base; +} + +void __udelay(unsigned long usec) +{ + unsigned long long endtime; + + endtime = lldiv((unsigned long long)usec * gd->arch.timer_rate_hz, + 1000UL); + + endtime += get_ticks(); + + while (get_ticks() < endtime) + ; +} + +ulong get_tbclk(void) +{ + return gd->arch.timer_rate_hz; +}
Dear Murali Karicheri,
In message 1390255810-19486-5-git-send-email-m-karicheri2@ti.com you wrote:
This patch add basic support for the architecture timer found on recent ARMv7 based SoCs.
Are we adding dead code here? Or where exactly is this code being used?
--- /dev/null +++ b/arch/arm/lib/arch_timer.c @@ -0,0 +1,69 @@ +/*
- Copyright (c) 2012, Texas Instruments <www.ti.com>
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- */
Please use SPDX License tag instead.
Best regards,
Wolfgang Denk
This patch introduces a configurable mechanism to disable subpage writes in the DaVinci NAND driver.
Signed-off-by: Vitaly Andrianov vitalya@ti.com Signed-off-by: Murali Karicheri m-karicheri2@ti.com --- drivers/mtd/nand/davinci_nand.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c index 5b17d7b..75b03a7 100644 --- a/drivers/mtd/nand/davinci_nand.c +++ b/drivers/mtd/nand/davinci_nand.c @@ -609,6 +609,9 @@ void davinci_nand_init(struct nand_chip *nand) #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT nand->bbt_options |= NAND_BBT_USE_FLASH; #endif +#ifdef CONFIG_SYS_NAND_NO_SUBPAGE_WRITE + nand->options |= NAND_NO_SUBPAGE_WRITE; +#endif #ifdef CONFIG_SYS_NAND_HW_ECC nand->ecc.mode = NAND_ECC_HW; nand->ecc.size = 512;
Dear Murali Karicheri,
In message 1390255810-19486-6-git-send-email-m-karicheri2@ti.com you wrote:
This patch introduces a configurable mechanism to disable subpage writes in the DaVinci NAND driver.
Signed-off-by: Vitaly Andrianov vitalya@ti.com Signed-off-by: Murali Karicheri m-karicheri2@ti.com
drivers/mtd/nand/davinci_nand.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c index 5b17d7b..75b03a7 100644 --- a/drivers/mtd/nand/davinci_nand.c +++ b/drivers/mtd/nand/davinci_nand.c @@ -609,6 +609,9 @@ void davinci_nand_init(struct nand_chip *nand) #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT nand->bbt_options |= NAND_BBT_USE_FLASH; #endif +#ifdef CONFIG_SYS_NAND_NO_SUBPAGE_WRITE
- nand->options |= NAND_NO_SUBPAGE_WRITE;
+#endif #ifdef CONFIG_SYS_NAND_HW_ECC nand->ecc.mode = NAND_ECC_HW; nand->ecc.size = 512;
New CONFIG_ options MUSt be documented in the README.
Also, you are just adding dead code. There are no users for this option.
Best regards,
Wolfgang Denk
On Mon, 2014-01-20 at 17:10 -0500, Murali Karicheri wrote:
This patch introduces a configurable mechanism to disable subpage writes in the DaVinci NAND driver.
Signed-off-by: Vitaly Andrianov vitalya@ti.com Signed-off-by: Murali Karicheri m-karicheri2@ti.com
drivers/mtd/nand/davinci_nand.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c index 5b17d7b..75b03a7 100644 --- a/drivers/mtd/nand/davinci_nand.c +++ b/drivers/mtd/nand/davinci_nand.c @@ -609,6 +609,9 @@ void davinci_nand_init(struct nand_chip *nand) #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT nand->bbt_options |= NAND_BBT_USE_FLASH; #endif +#ifdef CONFIG_SYS_NAND_NO_SUBPAGE_WRITE
- nand->options |= NAND_NO_SUBPAGE_WRITE;
+#endif #ifdef CONFIG_SYS_NAND_HW_ECC nand->ecc.mode = NAND_ECC_HW; nand->ecc.size = 512;
Why does it need to be configurable?
-Scott
participants (4)
-
Murali Karicheri -
Scott Wood -
Tom Rini -
Wolfgang Denk