On 9/23/2012 8:38 AM, Stefano Babic wrote:
On 22/09/2012 04:39, Troy Kisky wrote:
Add commands
plugin address filename
iomux_entry addr, data1 [, data2, [, data3]]
write_entry addr, data1 [, data2, [, data3]]
Why do we need explicitely an IOMUX command ? As far as I can see, the
program image defined in V2 defines a plugin, but not an iomux.
I am expecting that the imximage generates a iMX header only, without
moving some code from the initialization code directly here. In the
manula there is a "Write Data" (what we have always had), a "Check data"
and an "Unlock" commands.
The table built by iomux_entry and write_entry are not used by the ROM.
The plugin.S file that I add will interpret these entries.
I could have repurposed the "DATA" command if I didn't mind bloating
the table.
Having separate commands made it easy to generate small tables.
If we start to add special commands, maybe we are staring again to
reimplement U-Boot. We could have some SET_CLK, SET_CPU_FREQ, and so on.
What I am really mising in this series is why you are moving a lot of
things from U-Boot into the iMX header.
The cfg file after this patch series does no more setup/initialization
than before
this series. I don't know what "moving" you are referring to.
It seems to me we want to put much more code in the iMX header as what
it is really required to boot the device.
Adding / modifying the syntax requires to update doc/README.imximage, too.
Yes, if leaning toward acceptance I will add this.
Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com>
---
tools/imximage.c | 334 ++++++++++++++++++++++++++++++++++++++++++++----------
tools/imximage.h | 11 +-
2 files changed, 283 insertions(+), 62 deletions(-)
diff --git a/tools/imximage.c b/tools/imximage.c
index 2c5a622..fae786a 100644
--- a/tools/imximage.c
+++ b/tools/imximage.c
@@ -31,7 +31,6 @@
#include "mkimage.h"
#include <image.h>
#include "imximage.h"
-
/*
* Supported commands for configuration file
*/
@@ -39,6 +38,9 @@ static table_entry_t imximage_cmds[] = {
{CMD_BOOT_FROM, "BOOT_FROM", "boot command", },
{CMD_DATA, "DATA", "Reg Write Data", },
{CMD_IMAGE_VERSION, "IMAGE_VERSION", "image version", },
+ {CMD_PLUGIN, "plugin", "plugin addr,file", },
+ {CMD_IOMUX_ENTRY, "iomux_entry", "Write iomux
reg", },
+ {CMD_WRITE_ENTRY, "write_entry", "Write register",
},
{-1, "", "", },
};
@@ -69,8 +71,8 @@ static uint32_t imximage_version;
static set_dcd_val_t set_dcd_val;
static set_imx_hdr_t set_imx_hdr;
-static set_imx_size_t set_imx_size;
static uint32_t *p_max_dcd;
+static uint32_t *header_size_ptr;
static uint32_t g_flash_offset;
static struct image_type_params imximage_params;
@@ -88,8 +90,7 @@ static uint32_t detect_imximage_version(struct imx_header
*imx_hdr)
return IMXIMAGE_V1;
/* Try to detect V2 */
- if ((fhdr_v2->header.tag == IVT_HEADER_TAG) &&
- (hdr_v2->dcd_table.header.tag == DCD_HEADER_TAG))
+ if ((fhdr_v2->header.tag == IVT_HEADER_TAG))
return IMXIMAGE_V2;
Help me to understand. I am reading i.MX6 manual and, even if the number
of DCD entries could be variable, I do not see why the header tag of DCD
is moving. At least, this is what I can see on picture 7-19, Image
Vector table.
If the DCD table is missing, there is no DCD_HEADER_TAG.
DCD table is not required, so there is no need to check for it.
return IMXIMAGE_VER_INVALID;
@@ -160,7 +161,7 @@ static int set_dcd_val_v2(struct imx_header *imxhdr,
uint32_t *data)
}
static int set_imx_hdr_v1(struct imx_header *imxhdr,
- uint32_t entry_point, uint32_t flash_offset)
+ uint32_t entry_point, uint32_t flash_offset, int plugin)
{
imx_header_v1_t *hdr_v1 = &imxhdr->header.hdr_v1;
flash_header_v1_t *fhdr_v1 = &hdr_v1->fhdr;
@@ -180,22 +181,12 @@ static int set_imx_hdr_v1(struct imx_header *imxhdr,
/* Security feature are not supported */
fhdr_v1->app_code_csf = 0;
fhdr_v1->super_root_key = 0;
+ header_size_ptr = (uint32_t *)(((char *)imxhdr) + header_length - 4);
return header_length;
}
-static void set_imx_size_v1(struct imx_header *imxhdr, uint32_t file_size,
- uint32_t flash_offset)
-{
- uint32_t *p = (uint32_t *)(((char *)imxhdr)
- + imximage_params.header_size);
-
- /* The external flash header must be at the end of the DCD table */
- /* file_size includes header */
- p[-1] = file_size + flash_offset;
-}
I think you have to squash some of your patches or to defines them in
another way. You added this code previously, and you drop now. This
makes more difficult to review your patches.
I though this is what you were referring to in patch 4/21 response. So my
agreement there, should have been here. Now I don't know what in 4/21
you were referring to, but I'll reexamine before next version.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
