[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[Stefano Babic]

The patch adds support for setting gpios to the
MX51 processor and change name to the corresponding
functions for MX31. In this way, it is possible to get rid
of nasty #ifdef switches related to the processor type.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 arch/arm/include/asm/arch-mx31/mx31-regs.h |   14 +++-
 arch/arm/include/asm/arch-mx31/mx31.h      |   25 -------
 arch/arm/include/asm/arch-mx51/imx-regs.h  |    6 ++
 board/davedenx/qong/qong.c                 |   29 ++++----
 drivers/gpio/Makefile                      |    2 +-
 drivers/gpio/mx31_gpio.c                   |   88 -----------------------
 drivers/gpio/mxc_gpio.c                    |  107 ++++++++++++++++++++++++++++
 drivers/spi/mxc_spi.c                      |    9 +--
 include/configs/imx31_phycore.h            |    2 +-
 include/configs/qong.h                     |    2 +-
 include/mxc_gpio.h                         |   52 +++++++++++++
 11 files changed, 197 insertions(+), 139 deletions(-)
 delete mode 100644 drivers/gpio/mx31_gpio.c
 create mode 100644 drivers/gpio/mxc_gpio.c
 create mode 100644 include/mxc_gpio.h

diff --git a/arch/arm/include/asm/arch-mx31/mx31-regs.h b/arch/arm/include/asm/arch-mx31/mx31-regs.h
index d72585c..f05e743 100644
--- a/arch/arm/include/asm/arch-mx31/mx31-regs.h
+++ b/arch/arm/include/asm/arch-mx31/mx31-regs.h
@@ -57,6 +57,14 @@ struct clock_control_regs {
 	u32 pdr2;
 };
 
+/* GPIO Registers */
+struct gpio_regs {
+	u32	gpio_dr;
+	u32	gpio_dir;
+	u32	gpio_psr;
+};
+
+
 /* Bit definitions for RCSR register in CCM */
 #define CCM_RCSR_NF16B	(1 << 31)
 #define CCM_RCSR_NFMS	(1 << 30)
@@ -153,9 +161,9 @@ struct clock_control_regs {
 /*
  * GPIO
  */
-#define GPIO1_BASE	0x53FCC000
-#define GPIO2_BASE	0x53FD0000
-#define GPIO3_BASE	0x53FA4000
+#define GPIO1_BASE_ADDR	0x53FCC000
+#define GPIO2_BASE_ADDR	0x53FD0000
+#define GPIO3_BASE_ADDR	0x53FA4000
 #define GPIO_DR		0x00000000	/* data register */
 #define GPIO_GDIR	0x00000004	/* direction register */
 #define GPIO_PSR	0x00000008	/* pad status register */
diff --git a/arch/arm/include/asm/arch-mx31/mx31.h b/arch/arm/include/asm/arch-mx31/mx31.h
index f702d26..5a5aa11 100644
--- a/arch/arm/include/asm/arch-mx31/mx31.h
+++ b/arch/arm/include/asm/arch-mx31/mx31.h
@@ -28,31 +28,6 @@ extern u32 mx31_get_ipg_clk(void);
 #define imx_get_uartclk mx31_get_ipg_clk
 extern void mx31_gpio_mux(unsigned long mode);
 
-enum mx31_gpio_direction {
-	MX31_GPIO_DIRECTION_IN,
-	MX31_GPIO_DIRECTION_OUT,
-};
-
-#ifdef CONFIG_MX31_GPIO
-extern int mx31_gpio_direction(unsigned int gpio,
-			       enum mx31_gpio_direction direction);
-extern void mx31_gpio_set(unsigned int gpio, unsigned int value);
-extern int mx31_gpio_get(unsigned int gpio);
-#else
-static inline int mx31_gpio_direction(unsigned int gpio,
-				      enum mx31_gpio_direction direction)
-{
-	return 1;
-}
-static inline int mx31_gpio_get(unsigned int gpio)
-{
-	return 1;
-}
-static inline void mx31_gpio_set(unsigned int gpio, unsigned int value)
-{
-}
-#endif
-
 void mx31_uart1_hw_init(void);
 void mx31_spi2_hw_init(void);
 
diff --git a/arch/arm/include/asm/arch-mx51/imx-regs.h b/arch/arm/include/asm/arch-mx51/imx-regs.h
index 3887d3c..0e3bc2a 100644
--- a/arch/arm/include/asm/arch-mx51/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx51/imx-regs.h
@@ -256,6 +256,12 @@ struct weim {
 	u32	cswcr2;
 };
 
+/* GPIO Registers */
+struct gpio_regs {
+	u32	gpio_dr;
+	u32	gpio_dir;
+	u32	gpio_psr;
+};
 #endif /* __ASSEMBLER__*/
 
 #endif				/*  __ASM_ARCH_MXC_MX51_H__ */
diff --git a/board/davedenx/qong/qong.c b/board/davedenx/qong/qong.c
index 781333b..59589c2 100644
--- a/board/davedenx/qong/qong.c
+++ b/board/davedenx/qong/qong.c
@@ -27,6 +27,7 @@
 #include <asm/arch/mx31-regs.h>
 #include <nand.h>
 #include <fsl_pmic.h>
+#include <mxc_gpio.h>
 #include "qong_fpga.h"
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -42,9 +43,9 @@ int dram_init (void)
 
 static void qong_fpga_reset(void)
 {
-	mx31_gpio_set(QONG_FPGA_RST_PIN, 0);
+	mxc_gpio_set(QONG_FPGA_RST_PIN, 0);
 	udelay(30);
-	mx31_gpio_set(QONG_FPGA_RST_PIN, 1);
+	mxc_gpio_set(QONG_FPGA_RST_PIN, 1);
 
 	udelay(300);
 }
@@ -115,11 +116,11 @@ int board_init (void)
 
 	/* FPGA reset  Pin */
 	/* rstn = 0 */
-	mx31_gpio_set(QONG_FPGA_RST_PIN, 0);
-	mx31_gpio_direction(QONG_FPGA_RST_PIN, MX31_GPIO_DIRECTION_OUT);
+	mxc_gpio_set(QONG_FPGA_RST_PIN, 0);
+	mxc_gpio_direction(QONG_FPGA_RST_PIN, MXC_GPIO_DIRECTION_OUT);
 
 	/* set interrupt pin as input */
-	mx31_gpio_direction(QONG_FPGA_IRQ_PIN, MX31_GPIO_DIRECTION_IN);
+	mxc_gpio_direction(QONG_FPGA_IRQ_PIN, MXC_GPIO_DIRECTION_IN);
 
 #endif
 
@@ -201,27 +202,27 @@ static void board_nand_setup(void)
 	qong_fpga_reset();
 
 	/* Enable NAND flash */
-	mx31_gpio_set(15, 1);
-	mx31_gpio_set(14, 1);
-	mx31_gpio_direction(15, MX31_GPIO_DIRECTION_OUT);
-	mx31_gpio_direction(16, MX31_GPIO_DIRECTION_IN);
-	mx31_gpio_direction(14, MX31_GPIO_DIRECTION_IN);
-	mx31_gpio_set(15, 0);
+	mxc_gpio_set(15, 1);
+	mxc_gpio_set(14, 1);
+	mxc_gpio_direction(15, MXC_GPIO_DIRECTION_OUT);
+	mxc_gpio_direction(16, MXC_GPIO_DIRECTION_IN);
+	mxc_gpio_direction(14, MXC_GPIO_DIRECTION_IN);
+	mxc_gpio_set(15, 0);
 
 }
 
 int qong_nand_rdy(void *chip)
 {
 	udelay(1);
-	return mx31_gpio_get(16);
+	return mxc_gpio_get(16);
 }
 
 void qong_nand_select_chip(struct mtd_info *mtd, int chip)
 {
 	if (chip >= 0)
-		mx31_gpio_set(15, 0);
+		mxc_gpio_set(15, 0);
 	else
-		mx31_gpio_set(15, 1);
+		mxc_gpio_set(15, 1);
 
 }
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 528ca2e..738396a 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -27,7 +27,7 @@ LIB 	:= $(obj)libgpio.a
 
 COBJS-$(CONFIG_AT91_GPIO)	+= at91_gpio.o
 COBJS-$(CONFIG_KIRKWOOD_GPIO)	+= kw_gpio.o
-COBJS-$(CONFIG_MX31_GPIO)	+= mx31_gpio.o
+COBJS-$(CONFIG_MXC_GPIO)	+= mxc_gpio.o
 COBJS-$(CONFIG_PCA953X)		+= pca953x.o
 COBJS-$(CONFIG_S5PC1XX)		+= s5p_gpio.o
 
diff --git a/drivers/gpio/mx31_gpio.c b/drivers/gpio/mx31_gpio.c
deleted file mode 100644
index b07f038..0000000
--- a/drivers/gpio/mx31_gpio.c
+++ /dev/null
@@ -1,88 +0,0 @@
-/*
- * Copyright (C) 2009
- * Guennadi Liakhovetski, DENX Software Engineering, <lg@denx.de>
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * 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., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-#include <common.h>
-#include <asm/arch/mx31.h>
-#include <asm/arch/mx31-regs.h>
-
-/* GPIO port description */
-static unsigned long gpio_ports[] = {
-	[0] = GPIO1_BASE,
-	[1] = GPIO2_BASE,
-	[2] = GPIO3_BASE,
-};
-
-int mx31_gpio_direction(unsigned int gpio, enum mx31_gpio_direction direction)
-{
-	unsigned int port = gpio >> 5;
-	u32 l;
-
-	if (port >= ARRAY_SIZE(gpio_ports))
-		return 1;
-
-	gpio &= 0x1f;
-
-	l = __REG(gpio_ports[port] + GPIO_GDIR);
-	switch (direction) {
-	case MX31_GPIO_DIRECTION_OUT:
-		l |= 1 << gpio;
-		break;
-	case MX31_GPIO_DIRECTION_IN:
-		l &= ~(1 << gpio);
-	}
-	__REG(gpio_ports[port] + GPIO_GDIR) = l;
-
-	return 0;
-}
-
-void mx31_gpio_set(unsigned int gpio, unsigned int value)
-{
-	unsigned int port = gpio >> 5;
-	u32 l;
-
-	if (port >= ARRAY_SIZE(gpio_ports))
-		return;
-
-	gpio &= 0x1f;
-
-	l = __REG(gpio_ports[port] + GPIO_DR);
-	if (value)
-		l |= 1 << gpio;
-	else
-		l &= ~(1 << gpio);
-	__REG(gpio_ports[port] + GPIO_DR) = l;
-}
-
-int mx31_gpio_get(unsigned int gpio)
-{
-	unsigned int port = gpio >> 5;
-	u32 l;
-
-	if (port >= ARRAY_SIZE(gpio_ports))
-		return -1;
-
-	gpio &= 0x1f;
-
-	l = (__REG(gpio_ports[port] + GPIO_DR) >> gpio) & 0x01;
-
-	return l;
-}
diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
new file mode 100644
index 0000000..663141f
--- /dev/null
+++ b/drivers/gpio/mxc_gpio.c
@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2009
+ * Guennadi Liakhovetski, DENX Software Engineering, <lg@denx.de>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+#include <common.h>
+#ifdef CONFIG_MX31
+#include <asm/arch/mx31-regs.h>
+#endif
+#ifdef CONFIG_MX51
+#include <asm/arch/imx-regs.h>
+#endif
+#include <asm/io.h>
+#include <mxc_gpio.h>
+
+/* GPIO port description */
+static unsigned long gpio_ports[] = {
+	[0] = GPIO1_BASE_ADDR,
+	[1] = GPIO2_BASE_ADDR,
+	[2] = GPIO3_BASE_ADDR,
+#ifdef CONFIG_MX51
+	[3] = GPIO4_BASE_ADDR,
+#endif
+};
+
+int mxc_gpio_direction(unsigned int gpio, enum mxc_gpio_direction direction)
+{
+	unsigned int port = gpio >> 5;
+	struct gpio_regs *regs;
+	u32 l;
+
+	if (port >= ARRAY_SIZE(gpio_ports))
+		return 1;
+
+	gpio &= 0x1f;
+
+	regs = (struct gpio_regs *)gpio_ports[port];
+
+	l = readl(&regs->gpio_dir);
+
+	switch (direction) {
+	case MXC_GPIO_DIRECTION_OUT:
+		l |= 1 << gpio;
+		break;
+	case MXC_GPIO_DIRECTION_IN:
+		l &= ~(1 << gpio);
+	}
+	writel(l, &regs->gpio_dir);
+
+	return 0;
+}
+
+void mxc_gpio_set(unsigned int gpio, unsigned int value)
+{
+	unsigned int port = gpio >> 5;
+	struct gpio_regs *regs;
+	u32 l;
+
+	if (port >= ARRAY_SIZE(gpio_ports))
+		return;
+
+	gpio &= 0x1f;
+
+	regs = (struct gpio_regs *)gpio_ports[port];
+
+	l = readl(&regs->gpio_dr);
+	if (value)
+		l |= 1 << gpio;
+	else
+		l &= ~(1 << gpio);
+	writel(l, &regs->gpio_dr);
+}
+
+int mxc_gpio_get(unsigned int gpio)
+{
+	unsigned int port = gpio >> 5;
+	struct gpio_regs *regs;
+	u32 l;
+
+	if (port >= ARRAY_SIZE(gpio_ports))
+		return -1;
+
+	gpio &= 0x1f;
+
+	regs = (struct gpio_regs *)gpio_ports[port];
+
+	l = (readl(&regs->gpio_dr) >> gpio) & 0x01;
+
+	return l;
+}
diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
index e15a63c..54af2e3 100644
--- a/drivers/spi/mxc_spi.c
+++ b/drivers/spi/mxc_spi.c
@@ -23,6 +23,7 @@
 #include <spi.h>
 #include <asm/errno.h>
 #include <asm/io.h>
+#include <mxc_gpio.h>
 
 #ifdef CONFIG_MX27
 /* i.MX27 has a completely wrong register layout and register definitions in the
@@ -68,9 +69,6 @@ static unsigned long spi_bases[] = {
 	0x53f84000,
 };
 
-#define OUT	MX31_GPIO_DIRECTION_OUT
-#define mxc_gpio_direction	mx31_gpio_direction
-#define mxc_gpio_set		mx31_gpio_set
 #elif defined(CONFIG_MX51)
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
@@ -111,13 +109,12 @@ static unsigned long spi_bases[] = {
 	CSPI2_BASE_ADDR,
 	CSPI3_BASE_ADDR,
 };
-#define mxc_gpio_direction(gpio, dir)	(0)
-#define mxc_gpio_set(gpio, value)	{}
-#define OUT	1
 #else
 #error "Unsupported architecture"
 #endif
 
+#define OUT	MXC_GPIO_DIRECTION_OUT
+
 struct mxc_spi_slave {
 	struct spi_slave slave;
 	unsigned long	base;
diff --git a/include/configs/imx31_phycore.h b/include/configs/imx31_phycore.h
index 1dbafa0..62944a9 100644
--- a/include/configs/imx31_phycore.h
+++ b/include/configs/imx31_phycore.h
@@ -183,7 +183,7 @@
 #ifdef CONFIG_IMX31_PHYCORE_EET
 #define BOARD_LATE_INIT
 
-#define CONFIG_MX31_GPIO			1
+#define CONFIG_MXC_GPIO
 
 #define CONFIG_HARD_SPI				1
 #define CONFIG_MXC_SPI				1
diff --git a/include/configs/qong.h b/include/configs/qong.h
index 100fa3f..2ee1d60 100644
--- a/include/configs/qong.h
+++ b/include/configs/qong.h
@@ -52,7 +52,7 @@
 #define CONFIG_MXC_UART	1
 #define CONFIG_SYS_MX31_UART1	1
 
-#define CONFIG_MX31_GPIO
+#define CONFIG_MXC_GPIO
 
 #define CONFIG_MXC_SPI
 #define CONFIG_DEFAULT_SPI_BUS	1
diff --git a/include/mxc_gpio.h b/include/mxc_gpio.h
new file mode 100644
index 0000000..002ba61
--- /dev/null
+++ b/include/mxc_gpio.h
@@ -0,0 +1,52 @@
+/*
+ *
+ * (c) 2007 Pengutronix, Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __MXC_GPIO_H
+#define __MXC_GPIO_H
+
+enum mxc_gpio_direction {
+	MXC_GPIO_DIRECTION_IN,
+	MXC_GPIO_DIRECTION_OUT,
+};
+
+#ifdef CONFIG_MXC_GPIO
+extern int mxc_gpio_direction(unsigned int gpio,
+			       enum mxc_gpio_direction direction);
+extern void mxc_gpio_set(unsigned int gpio, unsigned int value);
+extern int mxc_gpio_get(unsigned int gpio);
+#else
+static inline int mxc_gpio_direction(unsigned int gpio,
+				      enum mxc_gpio_direction direction)
+{
+	return 1;
+}
+static inline int mxc_gpio_get(unsigned int gpio)
+{
+	return 1;
+}
+static inline void mxc_gpio_set(unsigned int gpio, unsigned int value)
+{
+}
+#endif
+
+#endif
-- 
1.6.3.3

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[David Jander]

On Friday 20 August 2010 10:20:11 am Stefano Babic wrote:
> The patch adds support for setting gpios to the
> MX51 processor and change name to the corresponding
> functions for MX31. In this way, it is possible to get rid
> of nasty #ifdef switches related to the processor type.

Argh! I was just writing the exact same code as in this patch :-(
Thanks a lot anyway.

Best regards,

-- 
David Jander
Protonic Holland.

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[David Jander]

Hi Stefano,

On Friday 20 August 2010 10:20:11 am Stefano Babic wrote:
> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
> index e15a63c..54af2e3 100644
> --- a/drivers/spi/mxc_spi.c
> +++ b/drivers/spi/mxc_spi.c
> @@ -23,6 +23,7 @@
>  #include <spi.h>
>  #include <asm/errno.h>
>  #include <asm/io.h>
> +#include <mxc_gpio.h>
> 
>  #ifdef CONFIG_MX27
>  /* i.MX27 has a completely wrong register layout and register definitions
>  in the @@ -68,9 +69,6 @@ static unsigned long spi_bases[] = {
>  	0x53f84000,
>  };
> 
> -#define OUT	MX31_GPIO_DIRECTION_OUT
> -#define mxc_gpio_direction	mx31_gpio_direction
> -#define mxc_gpio_set		mx31_gpio_set
>  #elif defined(CONFIG_MX51)
>  #include <asm/arch/imx-regs.h>
>  #include <asm/arch/clock.h>
> @@ -111,13 +109,12 @@ static unsigned long spi_bases[] = {
>  	CSPI2_BASE_ADDR,
>  	CSPI3_BASE_ADDR,
>  };
> -#define mxc_gpio_direction(gpio, dir)	(0)
> -#define mxc_gpio_set(gpio, value)	{}
> -#define OUT	1

After this change, it seems something else is missing:
GCC somehow removed the following code for i.MX51 without actually compiling 
the arguments to the functions (???), but now it becomes evident this only 
compiles for i.MX31:

void spi_cs_activate(struct spi_slave *slave)
{
	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
	if (mxcs->gpio > 0)
		mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
}

void spi_cs_deactivate(struct spi_slave *slave)
{
	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
	if (mxcs->gpio > 0)
		mxc_gpio_set(mxcs->gpio,
			      !(mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL));
}

On i.MX51 SSPOL is set in the config register, and per SS individually. 
Therefore, MXC_CSPICTRL_SSPOL isn't defined for i.MX51.

Best regards,

-- 
David Jander
Protonic Holland.

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[Stefano Babic]

David Jander wrote:
> Hi Stefano,
> 
Hi David,

> After this change, it seems something else is missing:
> GCC somehow removed the following code for i.MX51 without actually compiling 
> the arguments to the functions (???), but now it becomes evident this only 
> compiles for i.MX31:

Understood, in SPI driver. Really I had another patch in my private
tree, but I have not yet sent. However, as you note, it makes no sense
to split changes in two patches.

I will rebase my tree and send V2 version of the pacth, inclusive the
changes for mxc_spi.c

> void spi_cs_activate(struct spi_slave *slave)
> {
> 	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> 	if (mxcs->gpio > 0)
> 		mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);

I know, I had already change this code...I store ss_pol in the priivate
structure to be independent from the processor register.

I will send the compound patch soon.

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
=====================================================================

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[David Jander]

Stefano,

On Friday 20 August 2010 12:01:00 pm Stefano Babic wrote:
> > After this change, it seems something else is missing:
> > GCC somehow removed the following code for i.MX51 without actually
> > compiling the arguments to the functions (???), but now it becomes
> > evident this only compiles for i.MX31:
> 
> Understood, in SPI driver. Really I had another patch in my private
> tree, but I have not yet sent. However, as you note, it makes no sense
> to split changes in two patches.
> 
> I will rebase my tree and send V2 version of the pacth, inclusive the
> changes for mxc_spi.c

Ok, thanks!

> > void spi_cs_activate(struct spi_slave *slave)
> > {
> > 	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> > 	if (mxcs->gpio > 0)
> > 		mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
> 
> I know, I had already change this code...I store ss_pol in the priivate
> structure to be independent from the processor register.
> 
> I will send the compound patch soon.

Great. I'll wait.
In the meantime I have just done this to get it working:

#ifdef CONFIG_MX31
void spi_cs_activate(struct spi_slave *slave)
{
	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
	if (mxcs->gpio > 0)
		mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
}

void spi_cs_deactivate(struct spi_slave *slave)
{
	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
	if (mxcs->gpio > 0)
		mxc_gpio_set(mxcs->gpio,
			      !(mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL));
}
#elif defined (CONFIG_MX51)
void spi_cs_activate(struct spi_slave *slave)
{
	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
	unsigned int val = mxcs->cfg_reg & 
			(1 << (slave->cs + MXC_CSPICON_SSPOL));
	if (mxcs->gpio > 0)
		mxc_gpio_set(mxcs->gpio, val);
}

void spi_cs_deactivate(struct spi_slave *slave)
{
	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
	unsigned int val = !(mxcs->cfg_reg & 
			(1 << (slave->cs + MXC_CSPICON_SSPOL)));
	if (mxcs->gpio > 0)
		mxc_gpio_set(mxcs->gpio, val);
}
#endif

Seems to work, but never mind...

Best regards,

-- 
David Jander
Protonic Holland.

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[Stefano Babic]

David Jander wrote:

> Great. I'll wait.
> In the meantime I have just done this to get it working:
> 
> #ifdef CONFIG_MX31
> void spi_cs_activate(struct spi_slave *slave)
> {
> 	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> 	if (mxcs->gpio > 0)
> 		mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
> }
> 

Ok, but one goal I have is to get rid of nasty #ifdef CONFIG_MX*. I
introduce general gpio functions to make code more common, and I do not
want to fall back adding processor switches.

> Seems to work, but never mind...

Ok, I will resend my patch, I hope you can give a chance a test it on
your target.

Best regards,
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] Use common function to set GPIOs for MX3 and MX5

[David Jander]

On Friday 20 August 2010 12:20:25 pm Stefano Babic wrote:
> David Jander wrote:
> > Great. I'll wait.
> > In the meantime I have just done this to get it working:
> >
> > #ifdef CONFIG_MX31
> > void spi_cs_activate(struct spi_slave *slave)
> > {
> > 	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> > 	if (mxcs->gpio > 0)
> > 		mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
> > }
> 
> Ok, but one goal I have is to get rid of nasty #ifdef CONFIG_MX*. I
> introduce general gpio functions to make code more common, and I do not
> want to fall back adding processor switches.

Absolutely right. I just posted it as reference for your patch eventually, not 
because I thought it was good that way.

> > Seems to work, but never mind...
> 
> Ok, I will resend my patch, I hope you can give a chance a test it on
> your target.

Will do.
Btw, do you have any idea why spi_xchg_single() hangs while transmitting the 
second word without claiming the bus again?

Also, I don't know if you already fixed mxc_spi.c, to use the correct byte-
ordering when sending u8 buffers. I have a fix, but it is not yet ready.
I essentially renamed spi_xfer() to spi_xfer_fsl(), to be used in the (broken) 
pmic driver, and wrote a new spi_xfer() function which works correcly for u8 
buffers.

Best regards,

-- 
David Jander
Protonic Holland.

[U-Boot] [PATCH V2] Use common function to set GPIOs for MX3 and MX5

[Stefano Babic]

The patch adds support for setting gpios to the
MX51 processor and change name to the corresponding
functions for MX31. In this way, it is possible to get rid
of nasty #ifdef switches related to the processor type.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 arch/arm/include/asm/arch-mx31/mx31-regs.h |   14 +++-
 arch/arm/include/asm/arch-mx31/mx31.h      |   25 -------
 arch/arm/include/asm/arch-mx51/imx-regs.h  |    6 ++
 board/davedenx/qong/qong.c                 |   29 ++++----
 drivers/gpio/Makefile                      |    2 +-
 drivers/gpio/mx31_gpio.c                   |   88 -----------------------
 drivers/gpio/mxc_gpio.c                    |  107 ++++++++++++++++++++++++++++
 drivers/spi/mxc_spi.c                      |   15 ++--
 include/configs/imx31_phycore.h            |    2 +-
 include/configs/qong.h                     |    2 +-
 include/mxc_gpio.h                         |   52 +++++++++++++
 11 files changed, 201 insertions(+), 141 deletions(-)
 delete mode 100644 drivers/gpio/mx31_gpio.c
 create mode 100644 drivers/gpio/mxc_gpio.c
 create mode 100644 include/mxc_gpio.h

diff --git a/arch/arm/include/asm/arch-mx31/mx31-regs.h b/arch/arm/include/asm/arch-mx31/mx31-regs.h
index d72585c..f05e743 100644
--- a/arch/arm/include/asm/arch-mx31/mx31-regs.h
+++ b/arch/arm/include/asm/arch-mx31/mx31-regs.h
@@ -57,6 +57,14 @@ struct clock_control_regs {
 	u32 pdr2;
 };
 
+/* GPIO Registers */
+struct gpio_regs {
+	u32	gpio_dr;
+	u32	gpio_dir;
+	u32	gpio_psr;
+};
+
+
 /* Bit definitions for RCSR register in CCM */
 #define CCM_RCSR_NF16B	(1 << 31)
 #define CCM_RCSR_NFMS	(1 << 30)
@@ -153,9 +161,9 @@ struct clock_control_regs {
 /*
  * GPIO
  */
-#define GPIO1_BASE	0x53FCC000
-#define GPIO2_BASE	0x53FD0000
-#define GPIO3_BASE	0x53FA4000
+#define GPIO1_BASE_ADDR	0x53FCC000
+#define GPIO2_BASE_ADDR	0x53FD0000
+#define GPIO3_BASE_ADDR	0x53FA4000
 #define GPIO_DR		0x00000000	/* data register */
 #define GPIO_GDIR	0x00000004	/* direction register */
 #define GPIO_PSR	0x00000008	/* pad status register */
diff --git a/arch/arm/include/asm/arch-mx31/mx31.h b/arch/arm/include/asm/arch-mx31/mx31.h
index f702d26..5a5aa11 100644
--- a/arch/arm/include/asm/arch-mx31/mx31.h
+++ b/arch/arm/include/asm/arch-mx31/mx31.h
@@ -28,31 +28,6 @@ extern u32 mx31_get_ipg_clk(void);
 #define imx_get_uartclk mx31_get_ipg_clk
 extern void mx31_gpio_mux(unsigned long mode);
 
-enum mx31_gpio_direction {
-	MX31_GPIO_DIRECTION_IN,
-	MX31_GPIO_DIRECTION_OUT,
-};
-
-#ifdef CONFIG_MX31_GPIO
-extern int mx31_gpio_direction(unsigned int gpio,
-			       enum mx31_gpio_direction direction);
-extern void mx31_gpio_set(unsigned int gpio, unsigned int value);
-extern int mx31_gpio_get(unsigned int gpio);
-#else
-static inline int mx31_gpio_direction(unsigned int gpio,
-				      enum mx31_gpio_direction direction)
-{
-	return 1;
-}
-static inline int mx31_gpio_get(unsigned int gpio)
-{
-	return 1;
-}
-static inline void mx31_gpio_set(unsigned int gpio, unsigned int value)
-{
-}
-#endif
-
 void mx31_uart1_hw_init(void);
 void mx31_spi2_hw_init(void);
 
diff --git a/arch/arm/include/asm/arch-mx51/imx-regs.h b/arch/arm/include/asm/arch-mx51/imx-regs.h
index 3887d3c..0e3bc2a 100644
--- a/arch/arm/include/asm/arch-mx51/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx51/imx-regs.h
@@ -256,6 +256,12 @@ struct weim {
 	u32	cswcr2;
 };
 
+/* GPIO Registers */
+struct gpio_regs {
+	u32	gpio_dr;
+	u32	gpio_dir;
+	u32	gpio_psr;
+};
 #endif /* __ASSEMBLER__*/
 
 #endif				/*  __ASM_ARCH_MXC_MX51_H__ */
diff --git a/board/davedenx/qong/qong.c b/board/davedenx/qong/qong.c
index 781333b..59589c2 100644
--- a/board/davedenx/qong/qong.c
+++ b/board/davedenx/qong/qong.c
@@ -27,6 +27,7 @@
 #include <asm/arch/mx31-regs.h>
 #include <nand.h>
 #include <fsl_pmic.h>
+#include <mxc_gpio.h>
 #include "qong_fpga.h"
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -42,9 +43,9 @@ int dram_init (void)
 
 static void qong_fpga_reset(void)
 {
-	mx31_gpio_set(QONG_FPGA_RST_PIN, 0);
+	mxc_gpio_set(QONG_FPGA_RST_PIN, 0);
 	udelay(30);
-	mx31_gpio_set(QONG_FPGA_RST_PIN, 1);
+	mxc_gpio_set(QONG_FPGA_RST_PIN, 1);
 
 	udelay(300);
 }
@@ -115,11 +116,11 @@ int board_init (void)
 
 	/* FPGA reset  Pin */
 	/* rstn = 0 */
-	mx31_gpio_set(QONG_FPGA_RST_PIN, 0);
-	mx31_gpio_direction(QONG_FPGA_RST_PIN, MX31_GPIO_DIRECTION_OUT);
+	mxc_gpio_set(QONG_FPGA_RST_PIN, 0);
+	mxc_gpio_direction(QONG_FPGA_RST_PIN, MXC_GPIO_DIRECTION_OUT);
 
 	/* set interrupt pin as input */
-	mx31_gpio_direction(QONG_FPGA_IRQ_PIN, MX31_GPIO_DIRECTION_IN);
+	mxc_gpio_direction(QONG_FPGA_IRQ_PIN, MXC_GPIO_DIRECTION_IN);
 
 #endif
 
@@ -201,27 +202,27 @@ static void board_nand_setup(void)
 	qong_fpga_reset();
 
 	/* Enable NAND flash */
-	mx31_gpio_set(15, 1);
-	mx31_gpio_set(14, 1);
-	mx31_gpio_direction(15, MX31_GPIO_DIRECTION_OUT);
-	mx31_gpio_direction(16, MX31_GPIO_DIRECTION_IN);
-	mx31_gpio_direction(14, MX31_GPIO_DIRECTION_IN);
-	mx31_gpio_set(15, 0);
+	mxc_gpio_set(15, 1);
+	mxc_gpio_set(14, 1);
+	mxc_gpio_direction(15, MXC_GPIO_DIRECTION_OUT);
+	mxc_gpio_direction(16, MXC_GPIO_DIRECTION_IN);
+	mxc_gpio_direction(14, MXC_GPIO_DIRECTION_IN);
+	mxc_gpio_set(15, 0);
 
 }
 
 int qong_nand_rdy(void *chip)
 {
 	udelay(1);
-	return mx31_gpio_get(16);
+	return mxc_gpio_get(16);
 }
 
 void qong_nand_select_chip(struct mtd_info *mtd, int chip)
 {
 	if (chip >= 0)
-		mx31_gpio_set(15, 0);
+		mxc_gpio_set(15, 0);
 	else
-		mx31_gpio_set(15, 1);
+		mxc_gpio_set(15, 1);
 
 }
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 528ca2e..738396a 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -27,7 +27,7 @@ LIB 	:= $(obj)libgpio.a
 
 COBJS-$(CONFIG_AT91_GPIO)	+= at91_gpio.o
 COBJS-$(CONFIG_KIRKWOOD_GPIO)	+= kw_gpio.o
-COBJS-$(CONFIG_MX31_GPIO)	+= mx31_gpio.o
+COBJS-$(CONFIG_MXC_GPIO)	+= mxc_gpio.o
 COBJS-$(CONFIG_PCA953X)		+= pca953x.o
 COBJS-$(CONFIG_S5PC1XX)		+= s5p_gpio.o
 
diff --git a/drivers/gpio/mx31_gpio.c b/drivers/gpio/mx31_gpio.c
deleted file mode 100644
index b07f038..0000000
--- a/drivers/gpio/mx31_gpio.c
+++ /dev/null
@@ -1,88 +0,0 @@
-/*
- * Copyright (C) 2009
- * Guennadi Liakhovetski, DENX Software Engineering, <lg@denx.de>
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * 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., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-#include <common.h>
-#include <asm/arch/mx31.h>
-#include <asm/arch/mx31-regs.h>
-
-/* GPIO port description */
-static unsigned long gpio_ports[] = {
-	[0] = GPIO1_BASE,
-	[1] = GPIO2_BASE,
-	[2] = GPIO3_BASE,
-};
-
-int mx31_gpio_direction(unsigned int gpio, enum mx31_gpio_direction direction)
-{
-	unsigned int port = gpio >> 5;
-	u32 l;
-
-	if (port >= ARRAY_SIZE(gpio_ports))
-		return 1;
-
-	gpio &= 0x1f;
-
-	l = __REG(gpio_ports[port] + GPIO_GDIR);
-	switch (direction) {
-	case MX31_GPIO_DIRECTION_OUT:
-		l |= 1 << gpio;
-		break;
-	case MX31_GPIO_DIRECTION_IN:
-		l &= ~(1 << gpio);
-	}
-	__REG(gpio_ports[port] + GPIO_GDIR) = l;
-
-	return 0;
-}
-
-void mx31_gpio_set(unsigned int gpio, unsigned int value)
-{
-	unsigned int port = gpio >> 5;
-	u32 l;
-
-	if (port >= ARRAY_SIZE(gpio_ports))
-		return;
-
-	gpio &= 0x1f;
-
-	l = __REG(gpio_ports[port] + GPIO_DR);
-	if (value)
-		l |= 1 << gpio;
-	else
-		l &= ~(1 << gpio);
-	__REG(gpio_ports[port] + GPIO_DR) = l;
-}
-
-int mx31_gpio_get(unsigned int gpio)
-{
-	unsigned int port = gpio >> 5;
-	u32 l;
-
-	if (port >= ARRAY_SIZE(gpio_ports))
-		return -1;
-
-	gpio &= 0x1f;
-
-	l = (__REG(gpio_ports[port] + GPIO_DR) >> gpio) & 0x01;
-
-	return l;
-}
diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
new file mode 100644
index 0000000..663141f
--- /dev/null
+++ b/drivers/gpio/mxc_gpio.c
@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2009
+ * Guennadi Liakhovetski, DENX Software Engineering, <lg@denx.de>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+#include <common.h>
+#ifdef CONFIG_MX31
+#include <asm/arch/mx31-regs.h>
+#endif
+#ifdef CONFIG_MX51
+#include <asm/arch/imx-regs.h>
+#endif
+#include <asm/io.h>
+#include <mxc_gpio.h>
+
+/* GPIO port description */
+static unsigned long gpio_ports[] = {
+	[0] = GPIO1_BASE_ADDR,
+	[1] = GPIO2_BASE_ADDR,
+	[2] = GPIO3_BASE_ADDR,
+#ifdef CONFIG_MX51
+	[3] = GPIO4_BASE_ADDR,
+#endif
+};
+
+int mxc_gpio_direction(unsigned int gpio, enum mxc_gpio_direction direction)
+{
+	unsigned int port = gpio >> 5;
+	struct gpio_regs *regs;
+	u32 l;
+
+	if (port >= ARRAY_SIZE(gpio_ports))
+		return 1;
+
+	gpio &= 0x1f;
+
+	regs = (struct gpio_regs *)gpio_ports[port];
+
+	l = readl(&regs->gpio_dir);
+
+	switch (direction) {
+	case MXC_GPIO_DIRECTION_OUT:
+		l |= 1 << gpio;
+		break;
+	case MXC_GPIO_DIRECTION_IN:
+		l &= ~(1 << gpio);
+	}
+	writel(l, &regs->gpio_dir);
+
+	return 0;
+}
+
+void mxc_gpio_set(unsigned int gpio, unsigned int value)
+{
+	unsigned int port = gpio >> 5;
+	struct gpio_regs *regs;
+	u32 l;
+
+	if (port >= ARRAY_SIZE(gpio_ports))
+		return;
+
+	gpio &= 0x1f;
+
+	regs = (struct gpio_regs *)gpio_ports[port];
+
+	l = readl(&regs->gpio_dr);
+	if (value)
+		l |= 1 << gpio;
+	else
+		l &= ~(1 << gpio);
+	writel(l, &regs->gpio_dr);
+}
+
+int mxc_gpio_get(unsigned int gpio)
+{
+	unsigned int port = gpio >> 5;
+	struct gpio_regs *regs;
+	u32 l;
+
+	if (port >= ARRAY_SIZE(gpio_ports))
+		return -1;
+
+	gpio &= 0x1f;
+
+	regs = (struct gpio_regs *)gpio_ports[port];
+
+	l = (readl(&regs->gpio_dr) >> gpio) & 0x01;
+
+	return l;
+}
diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
index e15a63c..802cd2e 100644
--- a/drivers/spi/mxc_spi.c
+++ b/drivers/spi/mxc_spi.c
@@ -23,6 +23,7 @@
 #include <spi.h>
 #include <asm/errno.h>
 #include <asm/io.h>
+#include <mxc_gpio.h>
 
 #ifdef CONFIG_MX27
 /* i.MX27 has a completely wrong register layout and register definitions in the
@@ -68,9 +69,6 @@ static unsigned long spi_bases[] = {
 	0x53f84000,
 };
 
-#define OUT	MX31_GPIO_DIRECTION_OUT
-#define mxc_gpio_direction	mx31_gpio_direction
-#define mxc_gpio_set		mx31_gpio_set
 #elif defined(CONFIG_MX51)
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
@@ -111,13 +109,12 @@ static unsigned long spi_bases[] = {
 	CSPI2_BASE_ADDR,
 	CSPI3_BASE_ADDR,
 };
-#define mxc_gpio_direction(gpio, dir)	(0)
-#define mxc_gpio_set(gpio, value)	{}
-#define OUT	1
 #else
 #error "Unsupported architecture"
 #endif
 
+#define OUT	MXC_GPIO_DIRECTION_OUT
+
 struct mxc_spi_slave {
 	struct spi_slave slave;
 	unsigned long	base;
@@ -126,6 +123,7 @@ struct mxc_spi_slave {
 	u32		cfg_reg;
 #endif
 	int		gpio;
+	int		ss_pol;
 };
 
 static inline struct mxc_spi_slave *to_mxc_spi_slave(struct spi_slave *slave)
@@ -147,7 +145,7 @@ void spi_cs_activate(struct spi_slave *slave)
 {
 	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
 	if (mxcs->gpio > 0)
-		mxc_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
+		mxc_gpio_set(mxcs->gpio, mxcs->ss_pol);
 }
 
 void spi_cs_deactivate(struct spi_slave *slave)
@@ -155,7 +153,7 @@ void spi_cs_deactivate(struct spi_slave *slave)
 	struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
 	if (mxcs->gpio > 0)
 		mxc_gpio_set(mxcs->gpio,
-			      !(mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL));
+			      !(mxcs->ss_pol));
 }
 
 #ifdef CONFIG_MX51
@@ -394,6 +392,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 	mxcs->slave.bus = bus;
 	mxcs->slave.cs = cs;
 	mxcs->base = spi_bases[bus];
+	mxcs->ss_pol = (mode & SPI_CS_HIGH) ? 1 : 0;
 
 #ifdef CONFIG_MX51
 	/* Can be used for i.MX31 too ? */
diff --git a/include/configs/imx31_phycore.h b/include/configs/imx31_phycore.h
index 1dbafa0..62944a9 100644
--- a/include/configs/imx31_phycore.h
+++ b/include/configs/imx31_phycore.h
@@ -183,7 +183,7 @@
 #ifdef CONFIG_IMX31_PHYCORE_EET
 #define BOARD_LATE_INIT
 
-#define CONFIG_MX31_GPIO			1
+#define CONFIG_MXC_GPIO
 
 #define CONFIG_HARD_SPI				1
 #define CONFIG_MXC_SPI				1
diff --git a/include/configs/qong.h b/include/configs/qong.h
index 100fa3f..2ee1d60 100644
--- a/include/configs/qong.h
+++ b/include/configs/qong.h
@@ -52,7 +52,7 @@
 #define CONFIG_MXC_UART	1
 #define CONFIG_SYS_MX31_UART1	1
 
-#define CONFIG_MX31_GPIO
+#define CONFIG_MXC_GPIO
 
 #define CONFIG_MXC_SPI
 #define CONFIG_DEFAULT_SPI_BUS	1
diff --git a/include/mxc_gpio.h b/include/mxc_gpio.h
new file mode 100644
index 0000000..002ba61
--- /dev/null
+++ b/include/mxc_gpio.h
@@ -0,0 +1,52 @@
+/*
+ *
+ * (c) 2007 Pengutronix, Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __MXC_GPIO_H
+#define __MXC_GPIO_H
+
+enum mxc_gpio_direction {
+	MXC_GPIO_DIRECTION_IN,
+	MXC_GPIO_DIRECTION_OUT,
+};
+
+#ifdef CONFIG_MXC_GPIO
+extern int mxc_gpio_direction(unsigned int gpio,
+			       enum mxc_gpio_direction direction);
+extern void mxc_gpio_set(unsigned int gpio, unsigned int value);
+extern int mxc_gpio_get(unsigned int gpio);
+#else
+static inline int mxc_gpio_direction(unsigned int gpio,
+				      enum mxc_gpio_direction direction)
+{
+	return 1;
+}
+static inline int mxc_gpio_get(unsigned int gpio)
+{
+	return 1;
+}
+static inline void mxc_gpio_set(unsigned int gpio, unsigned int value)
+{
+}
+#endif
+
+#endif
-- 
1.6.3.3

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[Stefano Babic]

David Jander wrote:
> Absolutely right. I just posted it as reference for your patch eventually, not 
> because I thought it was good that way.

Yes, I know. I want only to point out what we have to reach ;-)

>>> Seems to work, but never mind...
>> Ok, I will resend my patch, I hope you can give a chance a test it on
>> your target.
> 
> Will do.
> Btw, do you have any idea why spi_xchg_single() hangs while transmitting the 
> second word without claiming the bus again?

Can you see where does it hang ? Which device is connected to your SPI
bus ? Does it work with the pmic (I think so, see later...).

> 
> Also, I don't know if you already fixed mxc_spi.c, to use the correct byte-
> ordering when sending u8 buffers.

Well, it seems we are working at the same issues, and probably it is
better if we try some coordination ;-)

I have already fix the byte ordering, but I am fixing now the misaligned
access (this is the reason I have not sent the changes for gpios in the
mxc_spi.c: I am reworking this driver...). In the mainline driver, only
32-bit aligned buffers are allowed, and this is a strong limitation. I
cannot use some other components in u-boot, such as SPI flash, because
the code allocates u8 buffers that can be disaligned. And with other
devices (sensors, eeprom, ...) does not work, because most of them
require to transfer only one or two bytes as command.

Now that I think...is it maybe your case now ? The FIFO can be accessed
only as word, other accesses are not allowed according to the manual.

However, I am currently working on several issues for MX51. It should be
nice to know which are your plans to save both some time ;-)

> I have a fix, but it is not yet ready.
> I essentially renamed spi_xfer() to spi_xfer_fsl(), to be used in the (broken) 
> pmic driver,

As I said, I changed the pmic driver, too. I do not agree we must have
"special" functions, only because something is broken. The pmic works
because it is connected to the FSL SPI controller, and the endianess is
consistent. However, it is common for a SPI driver to allocate a "char"
buffer, and the first byte in the buffer is the first byte sent to the
SPI bus. This is not the case with mxc_spi.c

Regards,
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] Use common function to set GPIOs for MX3 and MX5

[David Jander]

Hi Stefano,

On Friday 20 August 2010 01:19:28 pm Stefano Babic wrote:
> > Will do.
> > Btw, do you have any idea why spi_xchg_single() hangs while transmitting
> > the second word without claiming the bus again?
> 
> Can you see where does it hang ? Which device is connected to your SPI
> bus ? Does it work with the pmic (I think so, see later...).

Just figured out one big mistake. I was debugging spi_flash.c, and had 
CONFIG_ENV_IS_IN_SPI_FLASH set. That means, first SPI access is done before 
malloc is available, and guess what? spi_setup_slave() uses malloc ;-)
So I did something in the way of this:

static struct mxc_spi_slave mxc_spi_slave_pool[4];

struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
			unsigned int max_hz, unsigned int mode)
{
	unsigned int ctrl_reg;
	struct mxc_spi_slave *mxcs;
	int ret;

	if (bus >= ARRAY_SIZE(spi_bases))
		return NULL;

	//mxcs = malloc(sizeof(struct mxc_spi_slave));
	//if (!mxcs)
	//	return NULL;
	mxcs = &mxc_spi_slave_pool[cs];

The C++ comments show original code, the line below is new.

It still hangs though, waiting for MXC_CSPICTRL_TC to become active. Busy 
finding out why...

I see a one byte access followed by a 5 byte access, of which only 8.5 clock 
pulses come out of SCLK, and then nothing more. MXC_CSPICTRL_TC stays low.

> > Also, I don't know if you already fixed mxc_spi.c, to use the correct
> > byte- ordering when sending u8 buffers.
> 
> Well, it seems we are working at the same issues, and probably it is
> better if we try some coordination ;-)

Good idea.

> I have already fix the byte ordering, but I am fixing now the misaligned
> access (this is the reason I have not sent the changes for gpios in the
> mxc_spi.c: I am reworking this driver...). In the mainline driver, only
> 32-bit aligned buffers are allowed, and this is a strong limitation. I
> cannot use some other components in u-boot, such as SPI flash, because
> the code allocates u8 buffers that can be disaligned. And with other
> devices (sensors, eeprom, ...) does not work, because most of them
> require to transfer only one or two bytes as command.

I assumed that u-boot spi drivers use only u8 buffers for simplicity.
So my fix looks as this:

int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
		void *din, unsigned long flags)
{
	int n_blks = (bitlen + 31) / 32;
	u32 out, in;
	u8 *in_b, *out_b;
	int i;
	
	out_b = (u8 *)dout;
	in_b = (u8 *)din;
	
	for(i = 0; i< n_blks; i++, bitlen -= 32) {
		if(dout) {
			out = out_b[0];
			if(bitlen > 8)
				out = (out<<8) | out_b[1];
			if(bitlen > 16)
				out = (out<<8) | out_b[2];
			if(bitlen > 24)
				out = (out<<8) | out_b[3];
			out_b += 4;
		} else {
			out=0;
		}
		in = spi_xchg_single(slave, out, bitlen, flags);
		if(din) {
			if(bitlen > 24) {
				in_b[3] = in & 0xff;
				in >>= 8;
			}
			if(bitlen > 16) {
				in_b[2] = in & 0xff;
				in >>= 8;
			}
			if(bitlen > 8) {
				in_b[1] = in & 0xff;
				in >>= 8;
			}
			in_b[0] = in & 0xff;
			in_b += 4;
		}
	}
	return 0;
}

Does this sound like it could work?

> Now that I think...is it maybe your case now ? The FIFO can be accessed
> only as word, other accesses are not allowed according to the manual.

I am aware of that. That's why I used spi_xchg_single() as is.

> However, I am currently working on several issues for MX51. It should be
> nice to know which are your plans to save both some time ;-)

Well, I am in a bit of a hurry, and essentially what I need is to be able to 
access SPI-nor flash (spansion type) for environment and booting linux.
MMC/SD access would be nice, but is not yet necessary.

> > I have a fix, but it is not yet ready.
> > I essentially renamed spi_xfer() to spi_xfer_fsl(), to be used in the
> > (broken) pmic driver,
> 
> As I said, I changed the pmic driver, too. I do not agree we must have
> "special" functions, only because something is broken. The pmic works
> because it is connected to the FSL SPI controller, and the endianess is
> consistent. However, it is common for a SPI driver to allocate a "char"
> buffer, and the first byte in the buffer is the first byte sent to the
> SPI bus. This is not the case with mxc_spi.c

I know. I thought to do it in two steps: Fix mxc_spi.c with a workaround for 
the pmic driver (which amounts to '#define spi_xfer spi_xfer_fsl' at the 
beginning of this driver basically) and fix the pmic driver later, since it is 
probably not trivial, and needs to be done carefully (you know, one can smoke 
a board by mistake :-)

Best regards,

-- 
David Jander
Protonic Holland.

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[Stefano Babic]

David Jander wrote:
> Hi Stefano,
>

Hi David,

> On Friday 20 August 2010 01:19:28 pm Stefano Babic wrote:
>>> Will do.
>>> Btw, do you have any idea why spi_xchg_single() hangs while transmitting
>>> the second word without claiming the bus again?
>> Can you see where does it hang ? Which device is connected to your SPI
>> bus ? Does it work with the pmic (I think so, see later...).
> 
> Just figured out one big mistake. I was debugging spi_flash.c, and had 
> CONFIG_ENV_IS_IN_SPI_FLASH set. That means, first SPI access is done before 
> malloc is available, and guess what? spi_setup_slave() uses malloc ;-)

I have CONFIG_ENV_IS_IN_SPI_FLASH set, too.
I try to figure out how the functions are called, but I do not get the
same issue. I set with the debugger two breakpoints, one in
mem_malloc_init and the second one in spi_setup_slave. I see that
mem_alloc_init is hit first, and when spi_setup_slave is called, malloc
is available. I get a valid pointer for the private structure. It seems
there something in our config files that makes the things different. I
do not yet know why.

> The C++ comments show original code, the line below is new.

Understood, if malloc is not called, we have to use static or (better)
try to call mem_malloc_init() first

> I see a one byte access followed by a 5 byte access,

That is correct, you see the code in spi_flash.c. First the ID command
is sent (0x97), without reading nothing (that is, din is NULL). Then the
answer is read (dout is NULL), and the id buffer is 5 bytes long.

> I assumed that u-boot spi drivers use only u8 buffers for simplicity.
> So my fix looks as this:
> 
> int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> 		void *din, unsigned long flags)
> {
> 	int n_blks = (bitlen + 31) / 32;
> 	u32 out, in;
> 	u8 *in_b, *out_b;
> 	int i;
> 	
> 	out_b = (u8 *)dout;
> 	in_b = (u8 *)din;
> 	
> 	for(i = 0; i< n_blks; i++, bitlen -= 32) {
> 		if(dout) {
> 			out = out_b[0];
> 			if(bitlen > 8)
> 				out = (out<<8) | out_b[1];
> 			if(bitlen > 16)
> 				out = (out<<8) | out_b[2];
> 			if(bitlen > 24)
> 				out = (out<<8) | out_b[3];
> 			out_b += 4;
> 		} else {
> 			out=0;
> 		}
> 		in = spi_xchg_single(slave, out, bitlen, flags);
> 		if(din) {
> 			if(bitlen > 24) {
> 				in_b[3] = in & 0xff;
> 				in >>= 8;
> 			}
> 			if(bitlen > 16) {
> 				in_b[2] = in & 0xff;
> 				in >>= 8;
> 			}
> 			if(bitlen > 8) {
> 				in_b[1] = in & 0xff;
> 				in >>= 8;
> 			}
> 			in_b[0] = in & 0xff;
> 			in_b += 4;
> 		}
> 	}
> 	return 0;
> }
> 
> Does this sound like it could work?

I am trying another approach. As the MX51 has 32 bytes FIFO, it makes
sense to use it and not send a single word, if we can. This must not
change the behavior for the MX31, because this processor has no FIFO and
a single word can be sent.
So I replaced completely spi_xfer, and the logic you put in spi_xfer I
have (more or less, I have not checked in details) moved inside the
spi_xcgh:single, that now has the meaning for me as single transation:
up to 1 word for i.MX31, up to 32 words (128 bytes) for i.MX51.

Take into account that loading the kernel using a single word takes a
lot of time..

>> However, I am currently working on several issues for MX51. It should be
>> nice to know which are your plans to save both some time ;-)
> 
> Well, I am in a bit of a hurry, and essentially what I need is to be able to 
> access SPI-nor flash (spansion type) for environment and booting linux.
> MMC/SD access would be nice, but is not yet necessary.

Ok, quite the same. I have a ST flash, but we get the same problems, I see.

> I know. I thought to do it in two steps: Fix mxc_spi.c with a workaround for 
> the pmic driver (which amounts to '#define spi_xfer spi_xfer_fsl' at the 
> beginning of this driver basically) and fix the pmic driver later, since it is 
> probably not trivial, and needs to be done carefully (you know, one can smoke 
> a board by mistake :-)

I know, this makes funny setting voltages via software....

Best regards,
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] Use common function to set GPIOs for MX3 and MX5

[David Jander]

Hi again,

On Friday 20 August 2010 03:35:57 pm Stefano Babic wrote:
> > Just figured out one big mistake. I was debugging spi_flash.c, and had
> > CONFIG_ENV_IS_IN_SPI_FLASH set. That means, first SPI access is done
> > before malloc is available, and guess what? spi_setup_slave() uses malloc
> > ;-)
> 
> I have CONFIG_ENV_IS_IN_SPI_FLASH set, too.
> I try to figure out how the functions are called, but I do not get the
> same issue. I set with the debugger two breakpoints, one in
> mem_malloc_init and the second one in spi_setup_slave. I see that
> mem_alloc_init is hit first, and when spi_setup_slave is called, malloc
> is available. I get a valid pointer for the private structure. It seems
> there something in our config files that makes the things different. I
> do not yet know why.

Are you sure? In arch/arm/lib/board.c function start_armboot(), init_sequence 
is processed first, which contains env_init() before dram_init() and just 
after completing init_sequence, mem_malloc_init() is called. How can you have 
working malloc then?

> > The C++ comments show original code, the line below is new.
> 
> Understood, if malloc is not called, we have to use static or (better)
> try to call mem_malloc_init() first

I don't know if that is possible. I know that physically RAM is initialized 
before u-boot even starts (it runs from RAM), but logically, dram_init() is 
called _after_ env_init(), so I'm not sure if you are supposed to call 
mem_malloc_init() in env_init()...

> > I see a one byte access followed by a 5 byte access,
> 
> That is correct, you see the code in spi_flash.c. First the ID command
> is sent (0x97), without reading nothing (that is, din is NULL). Then the
> answer is read (dout is NULL), and the id buffer is 5 bytes long.

I am slowly progressing... now the transfer succeeds, but I only read FF ;-)

> > [...]
> I am trying another approach. As the MX51 has 32 bytes FIFO, it makes
> sense to use it and not send a single word, if we can. This must not
> change the behavior for the MX31, because this processor has no FIFO and
> a single word can be sent.
> So I replaced completely spi_xfer, and the logic you put in spi_xfer I
> have (more or less, I have not checked in details) moved inside the
> spi_xcgh:single, that now has the meaning for me as single transation:
> up to 1 word for i.MX31, up to 32 words (128 bytes) for i.MX51.
> 
> Take into account that loading the kernel using a single word takes a
> lot of time..

A good point. I was following the premise that u-boot drivers should be 
simple, but a little bit of speed for booting is surely not a bad idea ;-)

> >> However, I am currently working on several issues for MX51. It should be
> >> nice to know which are your plans to save both some time ;-)
> >
> > Well, I am in a bit of a hurry, and essentially what I need is to be able
> > to access SPI-nor flash (spansion type) for environment and booting
> > linux. MMC/SD access would be nice, but is not yet necessary.
> 
> Ok, quite the same. I have a ST flash, but we get the same problems, I see.

I am just now picking up where I left last week, so give me a few hours and I  
should have something working, I guess.

> > I know. I thought to do it in two steps: Fix mxc_spi.c with a workaround
> > for the pmic driver (which amounts to '#define spi_xfer spi_xfer_fsl' at
> > the beginning of this driver basically) and fix the pmic driver later,
> > since it is probably not trivial, and needs to be done carefully (you
> > know, one can smoke a board by mistake :-)
> 
> I know, this makes funny setting voltages via software....

I always say: "Electronics work on smoke. If the smoke escapes, it stops 
working" :-)

Best regards,

-- 
David Jander
Protonic Holland.

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[David Jander]

On Monday 23 August 2010 10:50:53 am David Jander wrote:
> I am just now picking up where I left last week, so give me a few hours and
>  I should have something working, I guess.

Ok, I guess I was pessimistic. There is a weird bug in mxc_spi.c: CPOL is 
negated!
I just saw that in the mx51evk.h header file CONFIG_FSL_PMIC_MODE was set to 
low-active clock (CPOL=1), which is not supposed to work. But it did work, and 
on the scope clock-polarity was active-high.

In spi_cfg(), I saw this line:

if (!(mode & SPI_CPOL))
	sclkpol = 1;

AFAIK, this should be:

if (mode & SPI_CPOL)
	sclkpol = 1;

At least for the MX51. Can you confirm that this is different on the MX31?

Now I get a correct flash id.

Best regards,

-- 
David Jander
Protonic Holland.

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[Stefano Babic]

David Jander wrote:
> Hi again,
> 
Hi David,

> Are you sure? In arch/arm/lib/board.c function start_armboot(), init_sequence 
> is processed first, which contains env_init() before dram_init() and just 
> after completing init_sequence, mem_malloc_init() is called. How can you have 
> working malloc then?

Well, I tried to investigate to see if I get the same issue, env_init is
called before mem_malloc_init, but it does nothing (in env_sf.c):


int env_init(void)
{
        /* SPI flash isn't usable before relocation */
        gd->env_addr = (ulong)&default_environment[0];
        gd->env_valid = 1;

        return 0;
}

And really I can see that mem_malloc_init() is called before
spi_setup_slave(). I guess there is a different configuration on our
targets, that makes the difference, but I do not know which one.

> I don't know if that is possible. I know that physically RAM is initialized 
> before u-boot even starts (it runs from RAM), but logically, dram_init() is 
> called _after_ env_init(), so I'm not sure if you are supposed to call 
> mem_malloc_init() in env_init()...

Yes, but as I see env_init does not read from flash and does not call
spi_setup_flash(). This is done by env_relocate_spec(), but this
function is called later.

> I am slowly progressing... now the transfer succeeds, but I only read FF ;-)

I think I have a good status now. I can correctly read/write the whole
flash. I have still an issue with saveenv (the first time does not work,
and it works after I called "sf probe"...crazy, but probably depends on
my target, I am investigating), but I should find a fix for that too.

> A good point. I was following the premise that u-boot drivers should be 
> simple, but a little bit of speed for booting is surely not a bad idea ;-)

Even using the whole FIFO I need about 8 Seconds to read the whole Flash
(4 MB). In normal case, this means I need 3 Seconds for a kernel.

> I am just now picking up where I left last week, so give me a few hours and I  
> should have something working, I guess.

Ok. We can make a point later. As I said, things are not so bad on my
target.

> I always say: "Electronics work on smoke. If the smoke escapes, it stops 
> working" :-)

;-)))

Best regards,
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] Use common function to set GPIOs for MX3 and MX5

[Stefano Babic]

David Jander wrote:
> Ok, I guess I was pessimistic. There is a weird bug in mxc_spi.c: CPOL is 
> negated!
> I just saw that in the mx51evk.h header file CONFIG_FSL_PMIC_MODE was set to 
> low-active clock (CPOL=1), which is not supposed to work. But it did work, and 
> on the scope clock-polarity was active-high.

Ok

> 
> In spi_cfg(), I saw this line:
> 
> if (!(mode & SPI_CPOL))
> 	sclkpol = 1;
> 
> AFAIK, this should be:
> 
> if (mode & SPI_CPOL)
> 	sclkpol = 1;
> 
> At least for the MX51. Can you confirm that this is different on the MX31?

Agree. According to the MX51 Manual, the register must be set with:
	0 Active high polarity (0 = Idle).
	1 Active low polarity (1 = Idle).

So we need to change both CONFIG_FSL_PMIC_MODE in config and in
mxc_spi.c. Do you send a patch or do you prefer I will do this job ? I
will add your signed-off-by if you agree.

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
=====================================================================

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[David Jander]

Hi Stefano,

On Monday 23 August 2010 12:37:16 pm Stefano Babic wrote:
>[...]
> > In spi_cfg(), I saw this line:
> >
> > if (!(mode & SPI_CPOL))
> > 	sclkpol = 1;
> >
> > AFAIK, this should be:
> >
> > if (mode & SPI_CPOL)
> > 	sclkpol = 1;
> >
> > At least for the MX51. Can you confirm that this is different on the
> > MX31?
> 
> Agree. According to the MX51 Manual, the register must be set with:
> 	0 Active high polarity (0 = Idle).
> 	1 Active low polarity (1 = Idle).

I just checked in the reference manual of the i.MX31, and there the meaning of 
this bit has the same polarity as on the i.MX51, so you'll need to fix this 
also at the end of the spi_setup_slave() function, in the #else path of the 
#ifdef CONFIG_MX51 directive.

if (!(mode & SPI_CPOL))
	ctrl_reg |= MXC_CSPICTRL_POL;

should be:

if (mode & SPI_CPOL)
	ctrl_reg |= MXC_CSPICTRL_POL;

Would be nice if someone with a MX31 board could verify this.

> So we need to change both CONFIG_FSL_PMIC_MODE in config and in
> mxc_spi.c. Do you send a patch or do you prefer I will do this job ? I
> will add your signed-off-by if you agree.

I agree, and I guess you can better include it in your patch-set, otherwise 
I'd have to wait for your patches and then provide my own patch on top of 
that.... too complicated :-)

I am also adding support for S25FL032P chips to the spansion driver. Will post 
a patch later.

Right now I have correct detection of the chip, but the environment is not 
saved and read back correctly. Still investigating... maybe some chip 
configuration prolem in the spansion driver?

Best regards,

-- 
David Jander
Protonic Holland.

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[Stefano Babic]

David Jander wrote:
> Hi Stefano,
> 

Hi David,

> I just checked in the reference manual of the i.MX31, and there the meaning of 
> this bit has the same polarity as on the i.MX51, so you'll need to fix this 
> also at the end of the spi_setup_slave() function, in the #else path of the 
> #ifdef CONFIG_MX51 directive.
> 
> if (!(mode & SPI_CPOL))
> 	ctrl_reg |= MXC_CSPICTRL_POL;
> 
> should be:
> 
> if (mode & SPI_CPOL)
> 	ctrl_reg |= MXC_CSPICTRL_POL;
> 
> Would be nice if someone with a MX31 board could verify this.

I can test myself on a qong board - I hope someone else can test on
other MX.31 boards.

> I agree, and I guess you can better include it in your patch-set, otherwise 
> I'd have to wait for your patches and then provide my own patch on top of 
> that.... too complicated :-)

I will do it - I think I could send in a short time the whole patchset
for review :-).

> 
> I am also adding support for S25FL032P chips to the spansion driver. Will post 
> a patch later.

I have seen. However, it should be better if you send the patch also to
the maintainer for the MTD subsystem (Stefan Roese, I set his address in
CC).

> Right now I have correct detection of the chip, but the environment is not 
> saved and read back correctly. Still investigating... maybe some chip 
> configuration prolem in the spansion driver?

Probably. I have got some issues with the ST that do not depend from the
modifications in SPI driver.

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
=====================================================================

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[Stefan Roese]

On Monday 23 August 2010 17:55:44 Stefano Babic wrote:
> > I am also adding support for S25FL032P chips to the spansion driver. Will
> > post a patch later.
> 
> I have seen. However, it should be better if you send the patch also to
> the maintainer for the MTD subsystem (Stefan Roese, I set his address in
> CC).

Small correction:

I'm not the custodian of the "MTD subsystem", but of the CFI flash driver 
(amongst others). And I have never really taken care of the SPI flash patches 
before (and never used one of those drivers before). IIRC, then Mike (added to 
Cc) has the most insight here.

Cheers,
Stefan

--
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] Use common function to set GPIOs for MX3 and MX5

[Detlev Zundel]

Hi Stefan,

> On Monday 23 August 2010 17:55:44 Stefano Babic wrote:
>> > I am also adding support for S25FL032P chips to the spansion driver. Will
>> > post a patch later.
>> 
>> I have seen. However, it should be better if you send the patch also to
>> the maintainer for the MTD subsystem (Stefan Roese, I set his address in
>> CC).
>
> Small correction:
>
> I'm not the custodian of the "MTD subsystem", but of the CFI flash driver 
> (amongst others). And I have never really taken care of the SPI flash patches 
> before (and never used one of those drivers before). IIRC, then Mike (added to 
> Cc) has the most insight here.

In fact I cannot see that you extended the CC list.  Wanna have another
go? ;)

Cheers
  Detlev

-- 
Paul Simon sang "Fifty Ways to Lose Your Lover", because quite
frankly, "A Million Ways to Tell a Developer He Is a D*ckhead"
doesn't scan nearly as well.  But I'm sure he thought about it.
                            -- linux/Documentation/ManagementStyle
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[Mike Frysinger]

On Monday, August 23, 2010 17:03:24 Detlev Zundel wrote:
> Hi Stefan,
> > On Monday 23 August 2010 17:55:44 Stefano Babic wrote:
> >> > I am also adding support for S25FL032P chips to the spansion driver.
> >> > Will post a patch later.
> >> 
> >> I have seen. However, it should be better if you send the patch also to
> >> the maintainer for the MTD subsystem (Stefan Roese, I set his address in
> >> CC).
> > 
> > Small correction:
> > 
> > I'm not the custodian of the "MTD subsystem", but of the CFI flash driver
> > (amongst others). And I have never really taken care of the SPI flash
> > patches before (and never used one of those drivers before). IIRC, then
> > Mike (added to Cc) has the most insight here.
> 
> In fact I cannot see that you extended the CC list.  Wanna have another
> go? ;)

i did since he added me to the cc ...

but as David said, he posted a sep patch, so i'll stick to that thread.  i 
generally delete threads about parts i dont care about (like whatever a "MX3" 
and "MX5" are).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100823/48105c27/attachment.pgp 

[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[Detlev Zundel]

Hi Mike,

> On Monday, August 23, 2010 17:03:24 Detlev Zundel wrote:
>> Hi Stefan,
>> > On Monday 23 August 2010 17:55:44 Stefano Babic wrote:
>> >> > I am also adding support for S25FL032P chips to the spansion driver.
>> >> > Will post a patch later.
>> >> 
>> >> I have seen. However, it should be better if you send the patch also to
>> >> the maintainer for the MTD subsystem (Stefan Roese, I set his address in
>> >> CC).
>> > 
>> > Small correction:
>> > 
>> > I'm not the custodian of the "MTD subsystem", but of the CFI flash driver
>> > (amongst others). And I have never really taken care of the SPI flash
>> > patches before (and never used one of those drivers before). IIRC, then
>> > Mike (added to Cc) has the most insight here.
>> 
>> In fact I cannot see that you extended the CC list.  Wanna have another
>> go? ;)
>
> i did since he added me to the cc ...

Hm, hm, hm.  That strange invisible CC phaenomenon again.  I just
checked again and on the mail that I received you are definitely not on
the CC list....

Anyway, sorry for the noise
  Detlev

-- 
To you I'm an atheist; to God, I'm the Loyal Opposition.
                                        -- Woody Allen
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de