[U-Boot] [PATCH] Add SPI support to mx51evk board

[U-Boot] [PATCH] Add SPI support to mx51evk board

[Stefano Babic]

The patch adds SPI devices to the mx51evk board. Two devices
are supported: Atmel SPI flash and MC13892 power
controller.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 board/freescale/mx51evk/mx51evk.c |  178 +++++++++++++++++++++++++++++++++++++
 include/configs/mx51evk.h         |   17 ++++
 2 files changed, 195 insertions(+), 0 deletions(-)

diff --git a/board/freescale/mx51evk/mx51evk.c b/board/freescale/mx51evk/mx51evk.c
index 8754563..b52dedb 100644
--- a/board/freescale/mx51evk/mx51evk.c
+++ b/board/freescale/mx51evk/mx51evk.c
@@ -27,9 +27,11 @@
 #include <asm/arch/iomux.h>
 #include <asm/errno.h>
 #include <asm/arch/sys_proto.h>
+#include <asm/arch/crm_regs.h>
 #include <i2c.h>
 #include <mmc.h>
 #include <fsl_esdhc.h>
+#include <mc13892.h>
 #include "mx51evk.h"
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -213,6 +215,167 @@ static void setup_iomux_fec(void)
 	mxc_iomux_set_pad(MX51_PIN_NANDF_D11, 0x2180);
 }
 
+#ifdef CONFIG_MXC_SPI
+static void setup_iomux_spi(int cs)
+{
+	/* 000: Select mux mode: ALT0 mux port: MOSI of instance: ecspi1 */
+	mxc_request_iomux(MX51_PIN_CSPI1_MOSI, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX51_PIN_CSPI1_MOSI, 0x105);
+
+	/* 000: Select mux mode: ALT0 mux port: MISO of instance: ecspi1. */
+	mxc_request_iomux(MX51_PIN_CSPI1_MISO, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX51_PIN_CSPI1_MISO, 0x105);
+
+	if (cs == 0) {
+		/* de-select SS1 of instance: ecspi1. */
+		mxc_request_iomux(MX51_PIN_CSPI1_SS1, IOMUX_CONFIG_ALT3);
+		mxc_iomux_set_pad(MX51_PIN_CSPI1_SS1, 0x85);
+		/* 000: Select mux mode: ALT0 mux port: SS0 ecspi1 */
+		mxc_request_iomux(MX51_PIN_CSPI1_SS0, IOMUX_CONFIG_ALT0);
+		mxc_iomux_set_pad(MX51_PIN_CSPI1_SS0, 0x185);
+	} else if (cs == 1) {
+		/* de-select SS0 of instance: ecspi1. */
+		mxc_request_iomux(MX51_PIN_CSPI1_SS0, IOMUX_CONFIG_ALT3);
+		mxc_iomux_set_pad(MX51_PIN_CSPI1_SS0, 0x85);
+		/* 000: Select mux mode: ALT0 mux port: SS1 ecspi1. */
+		mxc_request_iomux(MX51_PIN_CSPI1_SS1, IOMUX_CONFIG_ALT0);
+		mxc_iomux_set_pad(MX51_PIN_CSPI1_SS1, 0x105);
+	}
+
+	/* 000: Select mux mode: ALT0 mux port: RDY of instance: ecspi1. */
+	mxc_request_iomux(MX51_PIN_CSPI1_RDY, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX51_PIN_CSPI1_RDY, 0x180);
+
+	/* 000: Select mux mode: ALT0 mux port: SCLK of instance: ecspi1. */
+	mxc_request_iomux(MX51_PIN_CSPI1_SCLK, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX51_PIN_CSPI1_SCLK, 0x105);
+}
+#endif
+
+static void power_init(void)
+{
+	struct spi_slave *slave;
+	unsigned int val;
+	unsigned int reg;
+	u32 atlas_id;
+	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)MXC_CCM_BASE;
+
+#define REV_ATLAS_LITE_1_0	   0x8
+#define REV_ATLAS_LITE_1_1	   0x9
+#define REV_ATLAS_LITE_2_0	   0x10
+#define REV_ATLAS_LITE_2_1	   0x11
+
+	slave = mc13892_spi_probe();
+
+	/* Write needed to Power Gate 2 register */
+	val = mc13892_reg_read(slave, MC13892_REG_POWER_MISC);
+	val &= ~0x10000;
+	mc13892_reg_write(slave, MC13892_REG_POWER_MISC, val);
+
+	/* Write needed to update Charger 0 */
+	mc13892_reg_write(slave, MC13892_REG_CHARGE, 0x0023807F);
+
+	/* power up the system first */
+	mc13892_reg_write(slave, MC13892_REG_POWER_MISC, 0x00200000);
+
+	if (is_soc_rev(CHIP_REV_2_0) >= 0) {
+		/* Set core voltage to 1.1V */
+		val = mc13892_reg_read(slave, MC13892_REG_SW_0);
+		val = (val & (~0x1F)) | 0x14;
+		mc13892_reg_write(slave, MC13892_REG_SW_0, val);
+
+		/* Setup VCC (SW2) to 1.25 */
+		val = mc13892_reg_read(slave, MC13892_REG_SW_1);
+		val = (val & (~0x1F)) | 0x1A;
+		mc13892_reg_write(slave, MC13892_REG_SW_1, val);
+
+		/* Setup 1V2_DIG1 (SW3) to 1.25 */
+		val = mc13892_reg_read(slave, MC13892_REG_SW_2);
+		val = (val & (~0x1F)) | 0x1A;
+		mc13892_reg_write(slave, MC13892_REG_SW_2, val);
+		udelay(50);
+		/* Raise the core frequency to 800MHz */
+		writel(0x0, &mxc_ccm->cacrr);
+	} else {
+		/* TO 3.0 */
+		/* Setup VCC (SW2) to 1.225 */
+		val = mc13892_reg_read(slave, MC13892_REG_SW_1);
+		val = (val & (~0x1F)) | 0x19;
+		mc13892_reg_write(slave, MC13892_REG_SW_1, val);
+
+		/* Setup 1V2_DIG1 (SW3) to 1.2 */
+		val = mc13892_reg_read(slave, MC13892_REG_SW_2);
+		val = (val & (~0x1F)) | 0x18;
+		mc13892_reg_write(slave, MC13892_REG_SW_2, val);
+	}
+
+	atlas_id = mc13892_reg_read(slave, MC13892_REG_IDENTIFICATION);
+	if (((atlas_id & 0x1F) < REV_ATLAS_LITE_2_0) ||
+		(((atlas_id >> 9) & 0x3) == 0)) {
+		/* Set switchers in PWM mode for Atlas 2.0 and lower */
+		/* Setup the switcher mode for SW1 & SW2*/
+		val = mc13892_reg_read(slave, MC13892_REG_SW_4);
+		val = (val & (~0x3C0F)) | 0x1405;
+		mc13892_reg_write(slave, MC13892_REG_SW_4, val);
+
+		/* Setup the switcher mode for SW3 & SW4 */
+		val = mc13892_reg_read(slave, MC13892_REG_SW_5);
+		val = (val & (~0xF0F)) | 0x505;
+		mc13892_reg_write(slave, MC13892_REG_SW_5, val);
+	} else {
+		/* Set switchers in Auto in NORMAL mode & STANDBY mode */
+		/* Setup the switcher mode for SW1 & SW2*/
+		val = mc13892_reg_read(slave, MC13892_REG_SW_4);
+		val = (val & (~0x3C0F)) | 0x2008;
+		mc13892_reg_write(slave, MC13892_REG_SW_4, val);
+
+		/* Setup the switcher mode for SW3 & SW4 */
+		val = mc13892_reg_read(slave, MC13892_REG_SW_5);
+		val = (val & (~0xF0F)) | 0x808;
+		mc13892_reg_write(slave, MC13892_REG_SW_5, val);
+	}
+
+	/* Set VDIG to 1.65V, VGEN3 to 1.8V, VCAM to 2.5V */
+	val = mc13892_reg_read(slave, MC13892_REG_SETTING_0);
+	val &= ~0x34030;
+	val |= 0x10020;
+	mc13892_reg_write(slave, MC13892_REG_SETTING_0, val);
+
+	/* Set VVIDEO to 2.775V, VAUDIO to 3V, VSD to 3.15V */
+	val = mc13892_reg_read(slave, MC13892_REG_SETTING_1);
+	val &= ~0x1FC;
+	val |= 0x1F4;
+	mc13892_reg_write(slave, MC13892_REG_SETTING_1, val);
+
+	/* Configure VGEN3 and VCAM regulators to use external PNP */
+	val = 0x208;
+	mc13892_reg_write(slave, MC13892_REG_MODE_1, val);
+	udelay(200);
+
+	reg = readl(GPIO2_BASE_ADDR + 0x0);
+	reg &= ~0x4000;  /* Lower reset line */
+	writel(reg, GPIO2_BASE_ADDR + 0x0);
+
+	reg = readl(GPIO2_BASE_ADDR + 0x4);
+	reg |= 0x4000;	/* configure GPIO lines as output */
+	writel(reg, GPIO2_BASE_ADDR + 0x4);
+
+	/* Reset the ethernet controller over GPIO */
+	writel(0x1, IOMUXC_BASE_ADDR + 0x0AC);
+
+	/* Enable VGEN3, VCAM, VAUDIO, VVIDEO, VSD regulators */
+	val = 0x49249;
+	mc13892_reg_write(slave, MC13892_REG_MODE_1, val);
+
+	udelay(500);
+
+	reg = readl(GPIO2_BASE_ADDR + 0x0);
+	reg |= 0x4000;
+	writel(reg, GPIO2_BASE_ADDR + 0x0);
+
+	mc13892_spi_free(slave);
+}
+
 #ifdef CONFIG_FSL_ESDHC
 int board_mmc_getcd(u8 *cd, struct mmc *mmc)
 {
@@ -351,8 +514,23 @@ int board_init(void)
 	setup_iomux_uart();
 	setup_expio();
 	setup_iomux_fec();
+#ifdef CONFIG_MXC_SPI
+	setup_iomux_spi(1);
+#endif
+	return 0;
+}
+
+#ifdef BOARD_LATE_INIT
+int board_late_init(void)
+{
+#ifdef CONFIG_MXC_SPI
+	setup_iomux_spi(0);
+	power_init();
+#endif
+	setup_iomux_spi(1);
 	return 0;
 }
+#endif
 
 int checkboard(void)
 {
diff --git a/include/configs/mx51evk.h b/include/configs/mx51evk.h
index 903fe6d..e2daeca 100644
--- a/include/configs/mx51evk.h
+++ b/include/configs/mx51evk.h
@@ -54,6 +54,8 @@
 /* size in bytes reserved for initial data */
 #define CONFIG_SYS_GBL_DATA_SIZE	128
 
+#define BOARD_LATE_INIT
+
 /*
  * Hardware drivers
  */
@@ -61,6 +63,21 @@
 #define CONFIG_SYS_MX51_UART1
 
 /*
+ * SPI Configs
+ * */
+#define CONFIG_CMD_SPI
+
+#define CONFIG_MXC_SPI
+
+#define CONFIG_MC13892_SPI_PMIC
+#define CONFIG_MC13892_SPI_PMIC_BUS	0
+#define CONFIG_MC13892_SPI_PMIC_CS 	0
+#define CONFIG_MC13892_SPI_PMIC_CLK 	2500000
+#define CONFIG_MC13892_SPI_PMIC_MODE	(SPI_CPOL | SPI_CS_HIGH)
+
+#define MAX_SPI_BYTES		(64 * 4)
+
+/*
  * MMC Configs
  * */
 #define CONFIG_FSL_ESDHC
-- 
1.6.3.3

[U-Boot] [PATCH] Add SPI support to mx51evk board

[Tom]

Stefano Babic wrote:
> The patch adds SPI devices to the mx51evk board. Two devices
> are supported: Atmel SPI flash and MC13892 power
> controller.

Is Atmel SPI flash already supported?
It does not look like anything atmel is setup here

This is dependent on the
Added MC13892VK Power Management driver.
These patches must be combined in a patchset.

> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> ---
>  board/freescale/mx51evk/mx51evk.c |  178 +++++++++++++++++++++++++++++++++++++
>  include/configs/mx51evk.h         |   17 ++++
>  2 files changed, 195 insertions(+), 0 deletions(-)
> 
> diff --git a/board/freescale/mx51evk/mx51evk.c b/board/freescale/mx51evk/mx51evk.c
> index 8754563..b52dedb 100644
> --- a/board/freescale/mx51evk/mx51evk.c
> +++ b/board/freescale/mx51evk/mx51evk.c
> @@ -27,9 +27,11 @@
>  #include <asm/arch/iomux.h>
>  #include <asm/errno.h>
>  #include <asm/arch/sys_proto.h>
> +#include <asm/arch/crm_regs.h>
>  #include <i2c.h>
>  #include <mmc.h>
>  #include <fsl_esdhc.h>
> +#include <mc13892.h>
>  #include "mx51evk.h"
>  
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -213,6 +215,167 @@ static void setup_iomux_fec(void)
>  	mxc_iomux_set_pad(MX51_PIN_NANDF_D11, 0x2180);
>  }
>  
> +#ifdef CONFIG_MXC_SPI
> +static void setup_iomux_spi(int cs)
> +{
> +	/* 000: Select mux mode: ALT0 mux port: MOSI of instance: ecspi1 */
> +	mxc_request_iomux(MX51_PIN_CSPI1_MOSI, IOMUX_CONFIG_ALT0);
> +	mxc_iomux_set_pad(MX51_PIN_CSPI1_MOSI, 0x105);
> +
> +	/* 000: Select mux mode: ALT0 mux port: MISO of instance: ecspi1. */
> +	mxc_request_iomux(MX51_PIN_CSPI1_MISO, IOMUX_CONFIG_ALT0);
> +	mxc_iomux_set_pad(MX51_PIN_CSPI1_MISO, 0x105);
> +
> +	if (cs == 0) {
> +		/* de-select SS1 of instance: ecspi1. */
> +		mxc_request_iomux(MX51_PIN_CSPI1_SS1, IOMUX_CONFIG_ALT3);
> +		mxc_iomux_set_pad(MX51_PIN_CSPI1_SS1, 0x85);
> +		/* 000: Select mux mode: ALT0 mux port: SS0 ecspi1 */
> +		mxc_request_iomux(MX51_PIN_CSPI1_SS0, IOMUX_CONFIG_ALT0);
> +		mxc_iomux_set_pad(MX51_PIN_CSPI1_SS0, 0x185);
> +	} else if (cs == 1) {
> +		/* de-select SS0 of instance: ecspi1. */
> +		mxc_request_iomux(MX51_PIN_CSPI1_SS0, IOMUX_CONFIG_ALT3);
> +		mxc_iomux_set_pad(MX51_PIN_CSPI1_SS0, 0x85);
> +		/* 000: Select mux mode: ALT0 mux port: SS1 ecspi1. */
> +		mxc_request_iomux(MX51_PIN_CSPI1_SS1, IOMUX_CONFIG_ALT0);
> +		mxc_iomux_set_pad(MX51_PIN_CSPI1_SS1, 0x105);
> +	}
> +
> +	/* 000: Select mux mode: ALT0 mux port: RDY of instance: ecspi1. */
> +	mxc_request_iomux(MX51_PIN_CSPI1_RDY, IOMUX_CONFIG_ALT0);
> +	mxc_iomux_set_pad(MX51_PIN_CSPI1_RDY, 0x180);
> +
> +	/* 000: Select mux mode: ALT0 mux port: SCLK of instance: ecspi1. */
> +	mxc_request_iomux(MX51_PIN_CSPI1_SCLK, IOMUX_CONFIG_ALT0);
> +	mxc_iomux_set_pad(MX51_PIN_CSPI1_SCLK, 0x105);
> +}
> +#endif
> +
> +static void power_init(void)
> +{
> +	struct spi_slave *slave;
> +	unsigned int val;
> +	unsigned int reg;
> +	u32 atlas_id;
> +	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)MXC_CCM_BASE;
> +
> +#define REV_ATLAS_LITE_1_0	   0x8
> +#define REV_ATLAS_LITE_1_1	   0x9
> +#define REV_ATLAS_LITE_2_0	   0x10
> +#define REV_ATLAS_LITE_2_1	   0x11

Move these define out of the function.
Preferablly to an h-file
Looks like LITE_2_0 is the only one used

> +
> +	slave = mc13892_spi_probe();

The read / write routine bails if slave is invalid.
It would be better to check earlier here and bail.
As some of the logic of this function will still run and
this would leave the hw in a bad state

> +
> +	/* Write needed to Power Gate 2 register */
> +	val = mc13892_reg_read(slave, MC13892_REG_POWER_MISC);
> +	val &= ~0x10000;
> +	mc13892_reg_write(slave, MC13892_REG_POWER_MISC, val);
> +
> +	/* Write needed to update Charger 0 */
> +	mc13892_reg_write(slave, MC13892_REG_CHARGE, 0x0023807F);

Change magic number 0x0023807F to a logical if-def
If it is an or-ing of bit, show the or-ing as or-ing of if-def's
There are other similar uses of magic that should be changed

> +
> +	/* power up the system first */
> +	mc13892_reg_write(slave, MC13892_REG_POWER_MISC, 0x00200000);
> +
> +	if (is_soc_rev(CHIP_REV_2_0) >= 0) {
> +		/* Set core voltage to 1.1V */
> +		val = mc13892_reg_read(slave, MC13892_REG_SW_0);
> +		val = (val & (~0x1F)) | 0x14;
> +		mc13892_reg_write(slave, MC13892_REG_SW_0, val);
> +
> +		/* Setup VCC (SW2) to 1.25 */
> +		val = mc13892_reg_read(slave, MC13892_REG_SW_1);
> +		val = (val & (~0x1F)) | 0x1A;
> +		mc13892_reg_write(slave, MC13892_REG_SW_1, val);
> +
> +		/* Setup 1V2_DIG1 (SW3) to 1.25 */
> +		val = mc13892_reg_read(slave, MC13892_REG_SW_2);
> +		val = (val & (~0x1F)) | 0x1A;
> +		mc13892_reg_write(slave, MC13892_REG_SW_2, val);
> +		udelay(50);
> +		/* Raise the core frequency to 800MHz */
> +		writel(0x0, &mxc_ccm->cacrr);
> +	} else {
> +		/* TO 3.0 */
> +		/* Setup VCC (SW2) to 1.225 */
> +		val = mc13892_reg_read(slave, MC13892_REG_SW_1);
> +		val = (val & (~0x1F)) | 0x19;
> +		mc13892_reg_write(slave, MC13892_REG_SW_1, val);
> +
> +		/* Setup 1V2_DIG1 (SW3) to 1.2 */
> +		val = mc13892_reg_read(slave, MC13892_REG_SW_2);
> +		val = (val & (~0x1F)) | 0x18;
> +		mc13892_reg_write(slave, MC13892_REG_SW_2, val);
> +	}
> +
> +	atlas_id = mc13892_reg_read(slave, MC13892_REG_IDENTIFICATION);
> +	if (((atlas_id & 0x1F) < REV_ATLAS_LITE_2_0) ||
> +		(((atlas_id >> 9) & 0x3) == 0)) {
> +		/* Set switchers in PWM mode for Atlas 2.0 and lower */
> +		/* Setup the switcher mode for SW1 & SW2*/
> +		val = mc13892_reg_read(slave, MC13892_REG_SW_4);
> +		val = (val & (~0x3C0F)) | 0x1405;
> +		mc13892_reg_write(slave, MC13892_REG_SW_4, val);
> +
> +		/* Setup the switcher mode for SW3 & SW4 */
> +		val = mc13892_reg_read(slave, MC13892_REG_SW_5);
> +		val = (val & (~0xF0F)) | 0x505;
> +		mc13892_reg_write(slave, MC13892_REG_SW_5, val);
> +	} else {
> +		/* Set switchers in Auto in NORMAL mode & STANDBY mode */
> +		/* Setup the switcher mode for SW1 & SW2*/
> +		val = mc13892_reg_read(slave, MC13892_REG_SW_4);
> +		val = (val & (~0x3C0F)) | 0x2008;
> +		mc13892_reg_write(slave, MC13892_REG_SW_4, val);
> +
> +		/* Setup the switcher mode for SW3 & SW4 */
> +		val = mc13892_reg_read(slave, MC13892_REG_SW_5);
> +		val = (val & (~0xF0F)) | 0x808;
> +		mc13892_reg_write(slave, MC13892_REG_SW_5, val);
> +	}
> +
> +	/* Set VDIG to 1.65V, VGEN3 to 1.8V, VCAM to 2.5V */
> +	val = mc13892_reg_read(slave, MC13892_REG_SETTING_0);
> +	val &= ~0x34030;
> +	val |= 0x10020;
> +	mc13892_reg_write(slave, MC13892_REG_SETTING_0, val);
> +
> +	/* Set VVIDEO to 2.775V, VAUDIO to 3V, VSD to 3.15V */
> +	val = mc13892_reg_read(slave, MC13892_REG_SETTING_1);
> +	val &= ~0x1FC;
> +	val |= 0x1F4;
> +	mc13892_reg_write(slave, MC13892_REG_SETTING_1, val);
> +
> +	/* Configure VGEN3 and VCAM regulators to use external PNP */
> +	val = 0x208;
> +	mc13892_reg_write(slave, MC13892_REG_MODE_1, val);
> +	udelay(200);
> +
> +	reg = readl(GPIO2_BASE_ADDR + 0x0);
> +	reg &= ~0x4000;  /* Lower reset line */
> +	writel(reg, GPIO2_BASE_ADDR + 0x0);
> +
> +	reg = readl(GPIO2_BASE_ADDR + 0x4);
> +	reg |= 0x4000;	/* configure GPIO lines as output */
> +	writel(reg, GPIO2_BASE_ADDR + 0x4);
> +
> +	/* Reset the ethernet controller over GPIO */
> +	writel(0x1, IOMUXC_BASE_ADDR + 0x0AC);
> +
> +	/* Enable VGEN3, VCAM, VAUDIO, VVIDEO, VSD regulators */
> +	val = 0x49249;
> +	mc13892_reg_write(slave, MC13892_REG_MODE_1, val);
> +
> +	udelay(500);
> +
> +	reg = readl(GPIO2_BASE_ADDR + 0x0);
> +	reg |= 0x4000;
> +	writel(reg, GPIO2_BASE_ADDR + 0x0);
> +
> +	mc13892_spi_free(slave);
> +}
> +
>  #ifdef CONFIG_FSL_ESDHC
>  int board_mmc_getcd(u8 *cd, struct mmc *mmc)
>  {
> @@ -351,8 +514,23 @@ int board_init(void)
>  	setup_iomux_uart();
>  	setup_expio();
>  	setup_iomux_fec();
> +#ifdef CONFIG_MXC_SPI
> +	setup_iomux_spi(1);
> +#endif

In board_late_init, for CONFIG_MXC_SPI, cs=0, here it is cs=1
Is this correct?
Please use a if-def here instead of the 0,1
Maybe CONFIG_MC13892_SPI_PMIC_CS ?

> +	return 0;
> +}
> +
> +#ifdef BOARD_LATE_INIT
> +int board_late_init(void)
> +{
> +#ifdef CONFIG_MXC_SPI
> +	setup_iomux_spi(0);
> +	power_init();
> +#endif
> +	setup_iomux_spi(1);
>  	return 0;
>  }
> +#endif
>  
>  int checkboard(void)
>  {
> diff --git a/include/configs/mx51evk.h b/include/configs/mx51evk.h
> index 903fe6d..e2daeca 100644
> --- a/include/configs/mx51evk.h
> +++ b/include/configs/mx51evk.h
> @@ -54,6 +54,8 @@
>  /* size in bytes reserved for initial data */
>  #define CONFIG_SYS_GBL_DATA_SIZE	128
>  
> +#define BOARD_LATE_INIT
> +
>  /*
>   * Hardware drivers
>   */
> @@ -61,6 +63,21 @@
>  #define CONFIG_SYS_MX51_UART1
>  
>  /*
> + * SPI Configs
> + * */

Remove the extra '* '

> +#define CONFIG_CMD_SPI
> +
> +#define CONFIG_MXC_SPI

Remove above line space
> +
> +#define CONFIG_MC13892_SPI_PMIC
> +#define CONFIG_MC13892_SPI_PMIC_BUS	0
> +#define CONFIG_MC13892_SPI_PMIC_CS 	0
> +#define CONFIG_MC13892_SPI_PMIC_CLK 	2500000
> +#define CONFIG_MC13892_SPI_PMIC_MODE	(SPI_CPOL | SPI_CS_HIGH)
> +
> +#define MAX_SPI_BYTES		(64 * 4)
> +
The patch
'Added MC13892VK Power Management driver.'
Depends on this
Move this #define's to that patch, or an earlier path in the patch set.
These patches should be bi-sectable

Tom

> +/*
>   * MMC Configs
>   * */
>  #define CONFIG_FSL_ESDHC

[U-Boot] [PATCH] Add SPI support to mx51evk board

[Stefano Babic]

Tom wrote:
> Stefano Babic wrote:
>> The patch adds SPI devices to the mx51evk board. Two devices
>> are supported: Atmel SPI flash and MC13892 power
>> controller.
> 
> Is Atmel SPI flash already supported?
> It does not look like anything atmel is setup here

No, not yes, only in a future patch set. I must change the comment.

> 
> This is dependent on the
> Added MC13892VK Power Management driver.
> These patches must be combined in a patchset.

Yes, they are. And with the changes in the SPI master (mxc_spi.c). I
will combine all three in a patchset.

>> +
>> +#define REV_ATLAS_LITE_1_0       0x8
>> +#define REV_ATLAS_LITE_1_1       0x9
>> +#define REV_ATLAS_LITE_2_0       0x10
>> +#define REV_ATLAS_LITE_2_1       0x11
> 
> Move these define out of the function.
> Preferablly to an h-file

Ok

>> +
>> +    slave = mc13892_spi_probe();
> 
> The read / write routine bails if slave is invalid.
> It would be better to check earlier here and bail.
> As some of the logic of this function will still run and
> this would leave the hw in a bad state

You are right, I will fix it.

>> +#ifdef CONFIG_MXC_SPI
>> +    setup_iomux_spi(1);
>> +#endif
> 
> In board_late_init, for CONFIG_MXC_SPI, cs=0, here it is cs=1
> Is this correct?
> Please use a if-def here instead of the 0,1
> Maybe CONFIG_MC13892_SPI_PMIC_CS ?

Yes, that is correct. I will change it.

>>  /*
>> + * SPI Configs
>> + * */
> 
> Remove the extra '* '

Ok, thanks.

>> +
>> +#define MAX_SPI_BYTES        (64 * 4)
>> +
> The patch
> 'Added MC13892VK Power Management driver.'
> Depends on this
> Move this #define's to that patch, or an earlier path in the patch set.
> These patches should be bi-sectable

Understood, I will move it.

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] Add SPI support to mx51evk board

[Wolfgang Denk]

Dear Stefano Babic,

In message <1268756541-2179-1-git-send-email-sbabic@denx.de> you wrote:
> The patch adds SPI devices to the mx51evk board. Two devices
> are supported: Atmel SPI flash and MC13892 power
> controller.
> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> ---
>  board/freescale/mx51evk/mx51evk.c |  178 +++++++++++++++++++++++++++++++++++++
>  include/configs/mx51evk.h         |   17 ++++
>  2 files changed, 195 insertions(+), 0 deletions(-)

It seems this patch has not been picked up yet - can you please check?

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
Do you suppose the reason the ends of the `Intel Inside'  logo  don't
match up is that it was drawn on a Pentium?

[U-Boot] [PATCH] Add SPI support to mx51evk board

[Stefano Babic]

Wolfgang Denk wrote:
> Dear Stefano Babic,
> 

Hi Wolfgang,

> In message <1268756541-2179-1-git-send-email-sbabic@denx.de> you wrote:
>> The patch adds SPI devices to the mx51evk board. Two devices
>> are supported: Atmel SPI flash and MC13892 power
>> controller.
>>
>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>> ---
>>  board/freescale/mx51evk/mx51evk.c |  178 +++++++++++++++++++++++++++++++++++++
>>  include/configs/mx51evk.h         |   17 ++++
>>  2 files changed, 195 insertions(+), 0 deletions(-)
> 
> It seems this patch has not been picked up yet - can you please check?

This is not correct. The patch was reviewed and after the review I split
 as suggested this patch in several patches, testing for both mx51 and
mx31 targets (the original patch broke mx31 targets). You can consider
this patch as obsolete and substituded by "SPI: added support for MX51
to mxc_spi". I sent today a pull request to Tom with the updated version
of the patch.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================