MX5: Add initial support for MX53 processor

This patch add FEC support for Freescale MX53 processor

Signed-off-by Jason Liu <[email protected]>

---
 drivers/net/fec_mxc.c |    2 +-
 drivers/net/fec_mxc.h |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

This patch add UART support for Freescale MX53 processor

Signed-off-by Jason Liu <[email protected]>

---
 drivers/serial/serial_mxc.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

This patch add mxc_gpio support for Freescale MX53 processor

Signed-off-by Jason Liu <[email protected]>

---
Changes for v2
- put this patch into the same patchset with MX53 support as Stefano comments,

---
 drivers/gpio/mxc_gpio.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

This patch add I2C support for Freescale MX53 processor

Signed-off-by Jason Liu <[email protected]>

---
Changes for v2:
-address the comments of Heiko, add #if defined(CONFIG_MX31)
to avoid break MX31 build. Move CONFIG_HARD_I2C to the top of the file and fix the error message from:
#error "define CONFIG_SYS_I2C_PORT to use the I2C driver"
to #error "define CONFIG_SYS_I2C_MXxx_PORTx to use the I2C driver"

Changes for v3:
-address the comments from Albert, change #ifdef to #if defined()
for the sake of homogeneity

---
 drivers/i2c/mxc_i2c.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

Quoted 12 lines · Jason Liu

This patch add I2C support for Freescale MX53 processor

Signed-off-by Jason Liu <[email protected]>

---
Changes for v2:
-address the comments of Heiko, add #if defined(CONFIG_MX31)
to avoid break MX31 build. Move CONFIG_HARD_I2C to the top of the file and fix the error message from:
#error "define CONFIG_SYS_I2C_PORT to use the I2C driver"
to #error "define CONFIG_SYS_I2C_MXxx_PORTx to use the I2C driver"

Quoted 6 lines

Changes for v3:
-address the comments from Albert, change #ifdef to #if defined()
for the sake of homogeneity

---
 drivers/i2c/mxc_i2c.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

Applied to u-boot-imx, thanks.

Quoted 13 lines · In message <[email protected]> you wrote:

Quoted 12 lines · Jason Liu

This patch add I2C support for Freescale MX53 processor

Signed-off-by Jason Liu <[email protected]>

---
Changes for v2:
-address the comments of Heiko, add #if defined(CONFIG_MX31)
to avoid break MX31 build. Move CONFIG_HARD_I2C to the top of the file and fix the error message from:
#error "define CONFIG_SYS_I2C_PORT to use the I2C driver"
to #error "define CONFIG_SYS_I2C_MXxx_PORTx to use the I2C driver"

Quoted 7 lines

Quoted 6 lines

Changes for v3:
-address the comments from Albert, change #ifdef to #if defined()
for the sake of homogeneity

---
 drivers/i2c/mxc_i2c.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

Applied to u-boot-imx, thanks.

Please wait for the ACK from the I2C custodian first.

This patch add I2C support for Freescale MX53 processor

Signed-off-by Jason Liu <[email protected]>

drivers/i2c/mxc_i2c.c +1 -0

+#endif
 
 	for (i = 0; i < 0x1f; i++)
 		if (freq / div[i] <= speed)

This patch add I2C interface for fsl_pmic driver support

Signed-off-by Jason Liu <[email protected]>

---
Changes for v2:
- Address the comments from Stefano,
- factor out the param_check in pmic_reg for both spi/i2c
- Address the comments from Stefano,
- Remvove call i2c_init again
- Fix one logic path for pmic_read issue. Here the pmic dump

    command output:
	MX53EVK U-Boot > pmic dump 4
	PMIC ID: 0x000045d0 [Rev: 2.0]

	0x00: 00015088 00ffffff 00395208 00004081
---
 drivers/misc/fsl_pmic.c |   45 ++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 40 insertions(+), 5 deletions(-)

This patch add the MX53 boot image support.

This patch has been tested on Freescale MX53EVK board and MX51EVK board.

Signed-off-by Jason Liu <[email protected]>

This patch add the MX53 boot image support.

This patch has been tested on Freescale MX53EVK board and MX51EVK board.

Signed-off-by Jason Liu <[email protected]>

---
Changes for v2:
- Address the following comments from Stefano,
- Get rid of #ifdef in the imximage.h and .c file and use the runtime check for imximage version
- Document the IMXIMAGE_VERSION definiton in doc/README.imximage
- Move mx53evk/config.mk and mx53evk/imximage.cfg to MX53EVK board support patch.

Changes for v3:
- Address the following comments from Stefano,
- Not change the mx51evk file. The code should take VERSION=1 as default, and we do not need to change the actual boards.
- add a note in the documentation and raise an error in code if the VERSION command is read after any other suitable commands.
- Change command IMXIMAGE_VERSION simply to IMAGE_VERSION
- Need recognize the version directly from its structure and not storing the version into the header when do header verify and print.
- Use function pointer to simpliy the code when the version of header is recognized

---
 doc/README.imximage |   12 ++-
 tools/imximage.c    |  421 ++++++++++++++++++++++++++++++++++++++++-----------
 tools/imximage.h    |  110 +++++++++++---
 3 files changed, 433 insertions(+), 110 deletions(-)

This patch add the MX53 boot image support.

This patch has been tested on Freescale MX53EVK board and MX51EVK board.

Signed-off-by Jason Liu <[email protected]>

tools/imximage.c +13 -0

 	{-1,			"",		"Invalid",	},
 };
 
+/*
+ * IMXIMAGE version definition for i.MX chips
+ */
+static table_entry_t imximage_versions[] = {
+	{IMXIMAGE_V1,	"",	" (i.MX25/35/51 compatible)", },
+	{IMXIMAGE_V2,	"",	" (i.MX53 compatible)",       },
+	{-1,	        "",     " (Invalid)",                 },
+};
 
 static struct imx_header imximage_header;
+static uint32_t imximage_version;
+
+static set_dcd_val_t set_dcd_val;
+static set_dcd_rst_t set_dcd_rst;
+static set_imx_hdr_t set_imx_hdr;
 
 static uint32_t get_cfg_value(char *token, char *name,  int linenr)
 {

tools/imximage.c +4 -0

+				case CMD_IMAGE_VERSION:
+					imximage_version =
+						get_cfg_value(token,
+							name, lineno);

Stefano, Albert - I apologize for the late review. Please don't pull this into mainline as is.

it is too late, it was already pulled. We need now a patch on top of this. Jason, can you check Wolfgang's comments and send a patch to fix the issues he reported ?

It is not too late. I will not pull this into mainline as is. You can rebase your tree.

Quoted 3 lines

It is not too late. I will not pull this into mainline as is. You can rebase your tree.

No problem on my side. Albert should also drop this patch, because it is already merged into u-boot-arm. I will wait for updates from Jason.

Best regards,
Stefano Babic

Of course I was late to the dance... Stefano, did you rebase your tree? I still see this patch in your current master.

Dear Stefano Babic,

Quoted 4 lines · In message <[email protected]> you wrote:

it is too late, ?it was already pulled. We need now a patch on top of this. Jason, can you check Wolfgang's comments and send a patch to fix the issues he reported ?

It is not too late. ?I will not pull this into mainline as is. ?You can rebase your tree.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "...one of the main causes of the fall of the Roman Empire was ?that, lacking ?zero, ?they had no way to indicate successful termination of their C programs." ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? - Robert Firth
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Do I need send out the new version patch for imximage to address Wolfgang's comments
or send out the whole patchset(8patches) include the new version patch?

What's the process for it?

Dear Jason Liu,

Quoted 1 line · In message <[email protected]> you…

Quoted 5 lines

Do I need send out the new version patch for imximage to address Wolfgang's comments
or send out the whole patchset(8patches) ?include the new version patch?

What's the process for it?

If the changes affect only this one patch, then you can resubmit only this single patch.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de How many hardware guys does it take to change a light bulb? "Well the diagnostics say it's fine buddy, so it's a software problem."

This is the only one patch from the whole pathset(8patches) need get updated So, I just resubmit it with one newer version for only this patch, is it ok?

Dear Jason Liu,

Quoted 6 lines · In message <[email protected]> you wrote:

This patch add the MX53 boot image support.

This patch has been tested on Freescale MX53EVK board and MX51EVK board.

Signed-off-by Jason Liu <[email protected]>

...

Quoted 5 lines

?static table_entry_t imximage_cmds[] = {
+ ? ? {CMD_IMAGE_VERSION, ? ? "IMAGE_VERSION", ? ? ? ?"image version", }, ? ? ? {CMD_BOOT_FROM, ? ? ? ? "BOOT_FROM", ? ? ? ? ? ?"boot command", }, ? ? ? {CMD_DATA, ? ? ? ? ? ? ?"DATA", ? ? ? ? ? ? ? ? "Reg Write Data", }, ? ? ? {-1, ? ? ? ? ? ?"", ? ? ? ? ? ? ? ? ? ? "", ? ? },

Can we please keep the table sorted?

Quoted 23 lines

? ? ? {-1, ? ? ? ? ? ? ? ? ? ?"", ? ? ? ? ? ? "Invalid", ? ? ?}, ?};

tools/imximage.c +8 -0

+/*
+ * IMXIMAGE version definition for i.MX chips
+ */
+static table_entry_t imximage_versions[] = {
+ ? ? {IMXIMAGE_V1, ? "", ? ? " (i.MX25/35/51 compatible)", },
+ ? ? {IMXIMAGE_V2, ? "", ? ? " (i.MX53 compatible)", ? ? ? },
+ ? ? {-1, ? ? ? ? ? ?"", ? ? " (Invalid)", ? ? ? ? ? ? ? ? },
+};

?static struct imx_header imximage_header;

tools/imximage.c +5 -0

+static uint32_t imximage_version;
+
+static set_dcd_val_t set_dcd_val;
+static set_dcd_rst_t set_dcd_rst;
+static set_imx_hdr_t set_imx_hdr;

?static uint32_t get_cfg_value(char *token, char *name, ?int linenr) ?{

Review static uint32_t get_cfg_value(char *token, char *name, ?int linenr) · line 90

Quoted 6 lines

(quoted diff) +5 -0

@@ -71,58 +85,264 @@ static uint32_t get_cfg_value(char *token, char *name, ?int linenr)
 ? ? ? return value;
+ ? ? /* Try to detect V1 */
+ ? ? if (fhdr_v1->app_code_barker == APP_CODE_BARKER &&
+ ? ? ? ? ? ? imx_hdr->header.hdr_v1.dcd_table.preamble.barker == DCD_BARKER)
+
+ ? ? ? ? ? ? return IMXIMAGE_V1;

This needs braces. ?Please fix globally.

Review static uint32_t get_cfg_value(char *token, char *name, ?int linenr) · line 92

Quoted 2 lines

(quoted diff) +2 -0

+static void set_dcd_rst_v1(struct imx_header *imxhdr, uint32_t dcd_len,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? char *name, int lineno)

Could you please add a comment what the somewhat cryptic name "set_dcd_rst_v1" is supposed to mean? ?Similar for the other functions like set_dcd_rst_v2() etc.?

Review static uint32_t get_cfg_value(char *token, char *name, ?int linenr) · line 101

Quoted 10 lines

(quoted diff) +9 -0

+ ? ? if (dcd_len > MAX_HW_CFG_SIZE_V1) {
+ ? ? ? ? ? ? fprintf(stderr, "Error: %s[%d] -"
+ ? ? ? ? ? ? ? ? ? ? "DCD table exceeds maximum size(%d)\n",
+ ? ? ? ? ? ? ? ? ? ? name, lineno, MAX_HW_CFG_SIZE_V1);
+ ? ? }
+
+ ? ? dcd_v1->preamble.barker = DCD_BARKER;
+ ? ? dcd_v1->preamble.length = dcd_len * sizeof(dcd_type_addr_data_t);
+}

It seems these functions can run into error situations - yet they are of type void, i. e. they don't return any information about such errors to their callers. ?In the end, you cannot test the result oif running the command to find out if it had worked or not.

This must be fixed (globally).

Quoted 6 lines

(quoted diff) +4 -0

+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? case CMD_IMAGE_VERSION:
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? imximage_version =
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? get_cfg_value(token,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? name, lineno);

Review static uint32_t get_cfg_value(char *token, char *name, ?int linenr) · line 115

Quoted 10 lines

(quoted diff) +10 -0

+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (cmd_ver_first == 0) {
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? fprintf(stderr,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Error: %s[%d] - ?"
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "IMAGE_VERSION command "
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "need appear the first "
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "before other valid "
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "command in the file\n",
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? name, lineno);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? exit(EXIT_FAILURE);
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? }

Your nesting is too deep. Consider reorganizing the code.

Stefano, Albert - I apologize for the late review. ?Please don't pull this into mainline as is.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 8 lines

...

Quoted 5 lines

static table_entry_t imximage_cmds[] = {

+     {CMD_IMAGE_VERSION,     "IMAGE_VERSION",        "i> mage version", },
      {CMD_BOOT_FROM,         "BOOT_FROM",        >     "boot command", },
      {CMD_DATA,              "DATA",       >           "Reg Write Data", },

{-1, "", > "", },

Can we please keep the table sorted?

what does the "keep the table sorted" mean?

I think this function does not need return something, I will fix as following, is it OK,

static void set_dcd_rst_v1(struct imx_header *imxhdr, uint32_t dcd_len,
char *name, int lineno)

{
        dcd_v1_t *dcd_v1 = &imxhdr->header.hdr_v1.dcd_table;

        if (dcd_len > MAX_HW_CFG_SIZE_V1) {
                fprintf(stderr, "Error: %s[%d] -"
                        "DCD table exceeds maximum size(%d)\n",
                        name, lineno, MAX_HW_CFG_SIZE_V1);
               + exit(EXIT_FAILURE);
        }

Add initial support for MX53EVK board support.
FEC, SD/MMC, UART, I2C, have been supported.

Signed-off-by Jason Liu <[email protected]>

---
Changes for v2:

(quoted diff) +0 -3

-Address the comments from Stefano, Albert and Wolfgang,
        -remove the ivt.S file and use imximage.cfg instead,
        -remove the ivt link to u-boot
-Address the comments from Stefano,
        -Correct the copyright issue,
        -Use mxc_get_gpio() and mxc_set_gpio() accessors.
        -Get rid of system_rev,
        -use i2c enumeration value (0,1,..N) instead of BASE address,
        -use fsl_pmic i2c interface,
        -Add comments for why manage the pmic in a different way for TO2
        -Remove the comments from configs/mx53evk.h,
         - /*
         -  * Disabled for now due to build problems under Debian and a significant
         -  * increase in the final file size: 144260 vs. 109536 Bytes.
         -  */
-Address the comments from Wolfgang,
        -Move CONFIG_SYS_TEXT_BASE  to board config file,
        -Remove the comments from configs/mx53evk.h
         - /* size in bytes reserved for initial data */
         #define BOARD_LATE_INIT
        -Change the comments,  * Hardware drivers to * UART drivers

Changes for v3:
- Address the comments from Stefano,
- put uart and fec iomux setting to board_early_init_f so that uart can print out the early information such as uboot banner and cpuinfo etc.
- Put maintainer name to ARM list,
- Use Macro instead of constant value for PMIC,
- Remove CONFIG_I2C_MXC and BOARD_LATE_INIT,
- Add config.mk and imximage.cfg files to this patch,

Changes for v4:
- Address the comments from Stefano,
- Maintain the list *alfabetically* sorted of MAINTAINERS
- Multiline comment issue in configs/mx53evk.h file.

---
 MAINTAINERS                          |    4 +
 board/freescale/mx53evk/Makefile     |   48 ++++
 board/freescale/mx53evk/config.mk    |   25 +++
 board/freescale/mx53evk/imximage.cfg |  112 ++++++++++
 board/freescale/mx53evk/mx53evk.c    |  397 ++++++++++++++++++++++++++++++++++
 boards.cfg                           |    1 +
 include/configs/mx53evk.h            |  193 +++++++++++++++++
 include/mc13892.h                    |    5 +
 8 files changed, 785 insertions(+), 0 deletions(-)

Add initial support for Freescale MX53 processor,

- Add the iomux support and the pin definition,
- Add the regs definition, clean up some unused def from mx51,
- Add the low level init support, make use the freq input of setup_pll macro

arch/arm/cpu/armv7/mx5/lowlevel_init.S +1 -0

+	ldr r0, =\pll

Dear Jason Liu,

Quoted 5 lines · In message <[email protected]> you wrote:

Add initial support for Freescale MX53 processor,

- Add the iomux support and the pin definition,
- Add the regs definition, clean up some unused def from mx51,
- Add the low level init support, make use the freq input of setup_pll macro

...

Quoted 1 line

+ ? ? ldr r0, =\pll

...

Quoted 1 line

+ ? ? ldr r1, W_DP_OP_\freq

Please see previous post. ?What do these escape sequences mean?

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de The only solution is ... a balance of power. We ?arm ?our ?side ?with exactly ?that ?much ?more. ?A balance of power -- the trickiest, most difficult, dirtiest game of them all. But the only one that preserves both sides.
? ? ? ?-- Kirk, "A Private Little War", stardate 4211.8
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Add initial support for Freescale MX53 processor,

- Add the iomux support and the pin definition,
- Add the regs definition, clean up some unused def from mx51,
- Add the low level init support, make use the freq input of setup_pll macro

---