[U-Boot] mxc_spi refactoring (for mx6q)

[U-Boot] mxc_spi refactoring (for mx6q)

[Eric Nelson]

This patch set refactors mxc_spi as described in 
    http://lists.denx.de/pipermail/u-boot/2010-March/068791.html
and requested in 
    http://lists.denx.de/pipermail/u-boot/2012-January/116023.html
in order to add support for the MX6Q in general and the mx6qsabrelite 
specifically.

Patch 1 simply moves the conditional parts of mxc_spi.c into the
respective CPU-specific imx-regs.h files.

Patch 2 adds general support for SPI to the i.MX6.

Patch 3 adds support to the mx6qsabrelite board

Patch 4 modifies the 'sf' command to allow a default chip-select
to be specified by board headers as is done on efika et al. This allows
a bare 'sf' probe command:
     U-Boot> sf probe
instead of the more cumbersome usage when a GPIO is tacked onto
the chip-select. Otherwise, this command-line would be needed  
to specify GP3:19 on SabreLite:
     U-Boot> sf probe 0x5300   

Patch 5 adds default chip-select values for mx6qsabrelite platform.

Patch 6 adds reference macros for use in storing the environment
in serial flash to match the use on Freescale's U-Boot release

This patch set has been compiled against the following configurations,
but only tested on mx6qsabrelite:
	mx6qsabrelite
	mx51evk
	mx31pdk
	mx35pdk

[U-Boot] [PATCH 1/6] mxc_spi: move machine specifics into CPU headers

[Eric Nelson]

Move (E)CSPI register declarations into the imx-regs.h files for each supported CPU

Introduce two new macros to control conditional setup
     MXC_CSPI - Used for processors with the Configurable Serial Peripheral Interface (MX3x)
     MXC_ECSPI - For processors with Enhanced Configurable... (MX5x, MX6x)

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 arch/arm/include/asm/arch-mx31/imx-regs.h |   27 ++++++++
 arch/arm/include/asm/arch-mx35/imx-regs.h |   25 ++++++++
 arch/arm/include/asm/arch-mx5/imx-regs.h  |   30 +++++++++
 drivers/spi/mxc_spi.c                     |   93 ++---------------------------
 4 files changed, 88 insertions(+), 87 deletions(-)

diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h
index 6a517dd..70e3338 100644
--- a/arch/arm/include/asm/arch-mx31/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
@@ -890,4 +890,31 @@ struct esdc_regs {
 #define MXC_EHCI_IPPUE_DOWN		(1 << 8)
 #define MXC_EHCI_IPPUE_UP		(1 << 9)
 
+/*
+ * CSPI register definitions
+ */
+#define MXC_CSPI
+#define MXC_CSPICTRL_EN		(1 << 0)
+#define MXC_CSPICTRL_MODE	(1 << 1)
+#define MXC_CSPICTRL_XCH	(1 << 2)
+#define MXC_CSPICTRL_SMC	(1 << 3)
+#define MXC_CSPICTRL_POL	(1 << 4)
+#define MXC_CSPICTRL_PHA	(1 << 5)
+#define MXC_CSPICTRL_SSCTL	(1 << 6)
+#define MXC_CSPICTRL_SSPOL	(1 << 7)
+#define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 24)
+#define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0x1f) << 8)
+#define MXC_CSPICTRL_DATARATE(x)	(((x) & 0x7) << 16)
+#define MXC_CSPICTRL_TC		(1 << 8)
+#define MXC_CSPICTRL_RXOVF	(1 << 6)
+#define MXC_CSPICTRL_MAXBITS	0x1f
+
+#define MXC_CSPIPERIOD_32KHZ	(1 << 15)
+#define MAX_SPI_BYTES	4
+
+#define MXC_SPI_BASE_ADDRESSES \
+	0x43fa4000, \
+	0x50010000, \
+	0x53f84000,
+
 #endif /* __ASM_ARCH_MX31_IMX_REGS_H */
diff --git a/arch/arm/include/asm/arch-mx35/imx-regs.h b/arch/arm/include/asm/arch-mx35/imx-regs.h
index df74508..e570ad1 100644
--- a/arch/arm/include/asm/arch-mx35/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx35/imx-regs.h
@@ -179,6 +179,31 @@
 #define IPU_CONF_IC_EN		(1<<1)
 #define IPU_CONF_SCI_EN		(1<<0)
 
+/*
+ * CSPI register definitions
+ */
+#define MXC_CSPI
+#define MXC_CSPICTRL_EN		(1 << 0)
+#define MXC_CSPICTRL_MODE	(1 << 1)
+#define MXC_CSPICTRL_XCH	(1 << 2)
+#define MXC_CSPICTRL_SMC	(1 << 3)
+#define MXC_CSPICTRL_POL	(1 << 4)
+#define MXC_CSPICTRL_PHA	(1 << 5)
+#define MXC_CSPICTRL_SSCTL	(1 << 6)
+#define MXC_CSPICTRL_SSPOL	(1 << 7)
+#define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 12)
+#define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0xfff) << 20)
+#define MXC_CSPICTRL_DATARATE(x)	(((x) & 0x7) << 16)
+#define MXC_CSPICTRL_TC		(1 << 7)
+#define MXC_CSPICTRL_RXOVF	(1 << 6)
+#define MXC_CSPICTRL_MAXBITS	0xfff
+#define MXC_CSPIPERIOD_32KHZ	(1 << 15)
+#define MAX_SPI_BYTES	4
+
+#define MXC_SPI_BASE_ADDRESSES \
+	0x43fa4000, \
+	0x50010000,
+
 #define GPIO_PORT_NUM		3
 #define GPIO_NUM_PIN		32
 
diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h b/arch/arm/include/asm/arch-mx5/imx-regs.h
index 0ee88d2..4fa6658 100644
--- a/arch/arm/include/asm/arch-mx5/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx5/imx-regs.h
@@ -223,6 +223,36 @@
 #define CS0_32M_CS1_32M_CS2_32M_CS3_32M		3
 
 /*
+ * CSPI register definitions
+ */
+#define MXC_ECSPI
+#define MXC_CSPICTRL_EN		(1 << 0)
+#define MXC_CSPICTRL_MODE	(1 << 1)
+#define MXC_CSPICTRL_XCH	(1 << 2)
+#define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 12)
+#define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0xfff) << 20)
+#define MXC_CSPICTRL_PREDIV(x)	(((x) & 0xF) << 12)
+#define MXC_CSPICTRL_POSTDIV(x)	(((x) & 0xF) << 8)
+#define MXC_CSPICTRL_SELCHAN(x)	(((x) & 0x3) << 18)
+#define MXC_CSPICTRL_MAXBITS	0xfff
+#define MXC_CSPICTRL_TC		(1 << 7)
+#define MXC_CSPICTRL_RXOVF	(1 << 6)
+#define MXC_CSPIPERIOD_32KHZ	(1 << 15)
+#define MAX_SPI_BYTES	32
+
+/* Bit position inside CTRL register to be associated with SS */
+#define MXC_CSPICTRL_CHAN	18
+
+/* Bit position inside CON register to be associated with SS */
+#define MXC_CSPICON_POL		4
+#define MXC_CSPICON_PHA		0
+#define MXC_CSPICON_SSPOL	12
+#define MXC_SPI_BASE_ADDRESSES \
+	CSPI1_BASE_ADDR, \
+	CSPI2_BASE_ADDR, \
+	CSPI3_BASE_ADDR,
+
+/*
  * Number of GPIO pins per port
  */
 #define GPIO_NUM_PIN            32
diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
index 2fa7486..2e15318 100644
--- a/drivers/spi/mxc_spi.c
+++ b/drivers/spi/mxc_spi.c
@@ -33,93 +33,12 @@
 
 #error "i.MX27 CSPI not supported due to drastic differences in register definitions" \
 "See linux mxc_spi driver from Freescale for details."
-
-#elif defined(CONFIG_MX31)
-
-#define MXC_CSPICTRL_EN		(1 << 0)
-#define MXC_CSPICTRL_MODE	(1 << 1)
-#define MXC_CSPICTRL_XCH	(1 << 2)
-#define MXC_CSPICTRL_SMC	(1 << 3)
-#define MXC_CSPICTRL_POL	(1 << 4)
-#define MXC_CSPICTRL_PHA	(1 << 5)
-#define MXC_CSPICTRL_SSCTL	(1 << 6)
-#define MXC_CSPICTRL_SSPOL	(1 << 7)
-#define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 24)
-#define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0x1f) << 8)
-#define MXC_CSPICTRL_DATARATE(x)	(((x) & 0x7) << 16)
-#define MXC_CSPICTRL_TC		(1 << 8)
-#define MXC_CSPICTRL_RXOVF	(1 << 6)
-#define MXC_CSPICTRL_MAXBITS	0x1f
-
-#define MXC_CSPIPERIOD_32KHZ	(1 << 15)
-#define MAX_SPI_BYTES	4
-
-static unsigned long spi_bases[] = {
-	0x43fa4000,
-	0x50010000,
-	0x53f84000,
-};
-
-#elif defined(CONFIG_MX51)
-
-#define MXC_CSPICTRL_EN		(1 << 0)
-#define MXC_CSPICTRL_MODE	(1 << 1)
-#define MXC_CSPICTRL_XCH	(1 << 2)
-#define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 12)
-#define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0xfff) << 20)
-#define MXC_CSPICTRL_PREDIV(x)	(((x) & 0xF) << 12)
-#define MXC_CSPICTRL_POSTDIV(x)	(((x) & 0xF) << 8)
-#define MXC_CSPICTRL_SELCHAN(x)	(((x) & 0x3) << 18)
-#define MXC_CSPICTRL_MAXBITS	0xfff
-#define MXC_CSPICTRL_TC		(1 << 7)
-#define MXC_CSPICTRL_RXOVF	(1 << 6)
-
-#define MXC_CSPIPERIOD_32KHZ	(1 << 15)
-#define MAX_SPI_BYTES	32
-
-/* Bit position inside CTRL register to be associated with SS */
-#define MXC_CSPICTRL_CHAN	18
-
-/* Bit position inside CON register to be associated with SS */
-#define MXC_CSPICON_POL		4
-#define MXC_CSPICON_PHA		0
-#define MXC_CSPICON_SSPOL	12
-
-static unsigned long spi_bases[] = {
-	CSPI1_BASE_ADDR,
-	CSPI2_BASE_ADDR,
-	CSPI3_BASE_ADDR,
-};
-
-#elif defined(CONFIG_MX35)
-
-#define MXC_CSPICTRL_EN		(1 << 0)
-#define MXC_CSPICTRL_MODE	(1 << 1)
-#define MXC_CSPICTRL_XCH	(1 << 2)
-#define MXC_CSPICTRL_SMC	(1 << 3)
-#define MXC_CSPICTRL_POL	(1 << 4)
-#define MXC_CSPICTRL_PHA	(1 << 5)
-#define MXC_CSPICTRL_SSCTL	(1 << 6)
-#define MXC_CSPICTRL_SSPOL	(1 << 7)
-#define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 12)
-#define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0xfff) << 20)
-#define MXC_CSPICTRL_DATARATE(x)	(((x) & 0x7) << 16)
-#define MXC_CSPICTRL_TC		(1 << 7)
-#define MXC_CSPICTRL_RXOVF	(1 << 6)
-#define MXC_CSPICTRL_MAXBITS	0xfff
-
-#define MXC_CSPIPERIOD_32KHZ	(1 << 15)
-#define MAX_SPI_BYTES	4
+#endif
 
 static unsigned long spi_bases[] = {
-	0x43fa4000,
-	0x50010000,
+	MXC_SPI_BASE_ADDRESSES
 };
 
-#else
-#error "Unsupported architecture"
-#endif
-
 #define OUT	MXC_GPIO_DIRECTION_OUT
 
 #define reg_read readl
@@ -129,7 +48,7 @@ struct mxc_spi_slave {
 	struct spi_slave slave;
 	unsigned long	base;
 	u32		ctrl_reg;
-#if defined(CONFIG_MX51)
+#if defined(MXC_ECSPI)
 	u32		cfg_reg;
 #endif
 	int		gpio;
@@ -167,7 +86,7 @@ u32 get_cspi_div(u32 div)
 	return i;
 }
 
-#if defined(CONFIG_MX31) || defined(CONFIG_MX35)
+#ifdef MXC_CSPI
 static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs,
 		unsigned int max_hz, unsigned int mode)
 {
@@ -204,7 +123,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs,
 }
 #endif
 
-#if defined(CONFIG_MX51)
+#ifdef MXC_ECSPI
 static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs,
 		unsigned int max_hz, unsigned int mode)
 {
@@ -316,7 +235,7 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
 		MXC_CSPICTRL_BITCOUNT(bitlen - 1);
 
 	reg_write(&regs->ctrl, mxcs->ctrl_reg | MXC_CSPICTRL_EN);
-#ifdef CONFIG_MX51
+#ifdef MXC_ECSPI
 	reg_write(&regs->cfg, mxcs->cfg_reg);
 #endif
 
-- 
1.7.1

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 arch/arm/include/asm/arch-mx6/imx-regs.h |   44 ++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h
index 7650cb9..00040c4 100644
--- a/arch/arm/include/asm/arch-mx6/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
@@ -190,6 +190,50 @@ struct src {
 	u32     gpr10;
 };
 
+/* ECSPI registers */
+struct cspi_regs {
+	u32 rxdata;
+	u32 txdata;
+	u32 ctrl;
+	u32 cfg;
+	u32 intr;
+	u32 dma;
+	u32 stat;
+	u32 period;
+};
+
+/*
+ * CSPI register definitions
+ */
+#define MXC_ECSPI
+#define MXC_CSPICTRL_EN		(1 << 0)
+#define MXC_CSPICTRL_MODE	(1 << 1)
+#define MXC_CSPICTRL_XCH	(1 << 2)
+#define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 12)
+#define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0xfff) << 20)
+#define MXC_CSPICTRL_PREDIV(x)	(((x) & 0xF) << 12)
+#define MXC_CSPICTRL_POSTDIV(x)	(((x) & 0xF) << 8)
+#define MXC_CSPICTRL_SELCHAN(x)	(((x) & 0x3) << 18)
+#define MXC_CSPICTRL_MAXBITS	0xfff
+#define MXC_CSPICTRL_TC		(1 << 7)
+#define MXC_CSPICTRL_RXOVF	(1 << 6)
+#define MXC_CSPIPERIOD_32KHZ	(1 << 15)
+#define MAX_SPI_BYTES	32
+
+/* Bit position inside CTRL register to be associated with SS */
+#define MXC_CSPICTRL_CHAN	18
+
+/* Bit position inside CON register to be associated with SS */
+#define MXC_CSPICON_POL		4
+#define MXC_CSPICON_PHA		0
+#define MXC_CSPICON_SSPOL	12
+#define MXC_SPI_BASE_ADDRESSES \
+	ECSPI1_BASE_ADDR, \
+	ECSPI2_BASE_ADDR, \
+	ECSPI3_BASE_ADDR, \
+	ECSPI4_BASE_ADDR, \
+	ECSPI5_BASE_ADDR
+
 struct iim_regs {
 	u32	ctrl;
 	u32	ctrl_set;
-- 
1.7.1

[U-Boot] [PATCH 3/6] mx6q: mx6qsabrelite: Add ECSPI support to the Sabrelite platform

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 board/freescale/mx6qsabrelite/imximage.cfg    |    2 +-
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |   25 +++++++++++++++++++++++++
 include/configs/mx6qsabrelite.h               |    9 +++++++++
 3 files changed, 35 insertions(+), 1 deletions(-)

diff --git a/board/freescale/mx6qsabrelite/imximage.cfg b/board/freescale/mx6qsabrelite/imximage.cfg
index b4ff010..fa40bff 100644
--- a/board/freescale/mx6qsabrelite/imximage.cfg
+++ b/board/freescale/mx6qsabrelite/imximage.cfg
@@ -156,7 +156,7 @@ DATA 4 0x021b0404 0x00011006
 
 # set the default clock gate to save power
 DATA 4 0x020c4068 0x00C03F3F
-DATA 4 0x020c406c 0x0030FC00
+DATA 4 0x020c406c 0x0030FC03
 DATA 4 0x020c4070 0x0FFFC000
 DATA 4 0x020c4074 0x3FF00000
 DATA 4 0x020c4078 0x00FFF300
diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index a0b648f..56c54d6 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -46,6 +46,10 @@ DECLARE_GLOBAL_DATA_PTR;
 	PAD_CTL_PUS_100K_UP | PAD_CTL_SPEED_MED   |		\
 	PAD_CTL_DSE_40ohm   | PAD_CTL_HYS)
 
+#define SPI_PAD_CTRL (PAD_CTL_HYS |				\
+	PAD_CTL_PUS_100K_DOWN | PAD_CTL_SPEED_MED |		\
+	PAD_CTL_DSE_40ohm     | PAD_CTL_SRE_FAST)
+
 int dram_init(void)
 {
 	gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
@@ -193,6 +197,23 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#ifdef CONFIG_MXC_SPI
+iomux_v3_cfg_t ecspi1_pads[] = {
+	/* SS1 */
+	MX6Q_PAD_EIM_D19__GPIO_3_19   | MUX_PAD_CTRL(SPI_PAD_CTRL),
+	MX6Q_PAD_EIM_D17__ECSPI1_MISO | MUX_PAD_CTRL(SPI_PAD_CTRL),
+	MX6Q_PAD_EIM_D18__ECSPI1_MOSI | MUX_PAD_CTRL(SPI_PAD_CTRL),
+	MX6Q_PAD_EIM_D16__ECSPI1_SCLK | MUX_PAD_CTRL(SPI_PAD_CTRL),
+};
+
+void setup_spi(void)
+{
+	gpio_direction_output(IMX_GPIO_NR(3, 19), 1);
+	imx_iomux_v3_setup_multiple_pads(ecspi1_pads,
+					 ARRAY_SIZE(ecspi1_pads));
+}
+#endif
+
 #define MII_1000BASET_CTRL		0x9
 #define MII_EXTENDED_CTRL		0xb
 #define MII_EXTENDED_DATAW		0xc
@@ -250,6 +271,10 @@ int board_early_init_f(void)
 {
 	setup_iomux_uart();
 
+#ifdef CONFIG_MXC_SPI
+	setup_spi();
+#endif
+
 	return 0;
 }
 
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index 034fc40..8dd6e39 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -44,6 +44,15 @@
 #define CONFIG_MXC_UART
 #define CONFIG_MXC_UART_BASE		UART2_BASE
 
+#define CONFIG_CMD_SF
+#ifdef CONFIG_CMD_SF
+#define CONFIG_SPI_FLASH
+#define CONFIG_SPI_FLASH_SST
+#define CONFIG_MXC_SPI
+#define CONFIG_SF_DEFAULT_SPEED 25000000
+#define CONFIG_SF_DEFAULT_MODE (SPI_MODE_0)
+#endif
+
 /* MMC Configs */
 #define CONFIG_FSL_ESDHC
 #define CONFIG_FSL_USDHC
-- 
1.7.1

[U-Boot] [PATCH 4/6] sf command: allow default chip select through CONFIG_SPI_FLASH_CS

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 common/cmd_sf.c |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 7225656..97fce16 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -70,20 +70,28 @@ static int do_spi_flash_probe(int argc, char * const argv[])
 	char *endp;
 	struct spi_flash *new;
 
-	if (argc < 2)
-		return -1;
-
-	cs = simple_strtoul(argv[1], &endp, 0);
-	if (*argv[1] == 0 || (*endp != 0 && *endp != ':'))
+#ifndef CONFIG_SPI_FLASH_CS
+	if (argc < 2) {
+		printf("%s: missing arguments\n", __func__);
 		return -1;
-	if (*endp == ':') {
-		if (endp[1] == 0)
-			return -1;
+	}
+#else
+	cs = CONFIG_SPI_FLASH_CS ;
+#endif
 
-		bus = cs;
-		cs = simple_strtoul(endp + 1, &endp, 0);
-		if (*endp != 0)
+	if (argc >= 2) {
+		cs = simple_strtoul(argv[1], &endp, 0);
+		if (*argv[1] == 0 || (*endp != 0 && *endp != ':'))
 			return -1;
+		if (*endp == ':') {
+			if (endp[1] == 0)
+				return -1;
+
+			bus = cs;
+			cs = simple_strtoul(endp + 1, &endp, 0);
+			if (*endp != 0)
+				return -1;
+		}
 	}
 
 	if (argc >= 3) {
@@ -299,7 +307,11 @@ usage:
 U_BOOT_CMD(
 	sf,	5,	1,	do_spi_flash,
 	"SPI flash sub-system",
+#ifndef CONFIG_SPI_FLASH_CS
 	"probe [bus:]cs [hz] [mode]	- init flash device on given SPI bus\n"
+#else
+	"probe [[bus:]cs] [hz] [mode]	- init flash device on given SPI bus\n"
+#endif
 	"				  and chip select\n"
 	"sf read addr offset len 	- read `len' bytes starting at\n"
 	"				  `offset' to memory at `addr'\n"
-- 
1.7.1

[U-Boot] [PATCH 5/6] mx6q: mx6qsabrelite: Provide default chip-select for serial flash

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 include/configs/mx6qsabrelite.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index 8dd6e39..44b028a 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -46,6 +46,7 @@
 
 #define CONFIG_CMD_SF
 #ifdef CONFIG_CMD_SF
+#define CONFIG_SPI_FLASH_CS 0x5300
 #define CONFIG_SPI_FLASH
 #define CONFIG_SPI_FLASH_SST
 #define CONFIG_MXC_SPI
-- 
1.7.1

[U-Boot] [PATCH 6/6] mx6q: mx6qsabrelite: Provide defaults for placing environment in serial flash

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 include/configs/mx6qsabrelite.h |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index 44b028a..160894c 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -174,10 +174,20 @@
 /* FLASH and environment organization */
 #define CONFIG_SYS_NO_FLASH
 
-#define CONFIG_ENV_OFFSET		(6 * 64 * 1024)
 #define CONFIG_ENV_SIZE			(8 * 1024)
+
 #define CONFIG_ENV_IS_IN_MMC
+/* #define CONFIG_ENV_IS_IN_SPI_FLASH */
+
+#if defined(CONFIG_ENV_IS_IN_MMC)
+#define CONFIG_ENV_OFFSET		(6 * 64 * 1024)
 #define CONFIG_SYS_MMC_ENV_DEV		0
+#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH)
+#define CONFIG_ENV_OFFSET		(768 * 1024)
+#define CONFIG_ENV_SECT_SIZE		(8 * 1024)
+#define CONFIG_ENV_SPI_CS		0x5300
+#define CONFIG_ENV_SPI_MODE		SPI_MODE_0
+#endif
 
 #define CONFIG_OF_LIBFDT
 
-- 
1.7.1

[U-Boot] mxc_spi refactoring (for mx6q)

[Marek Vasut]

> This patch set refactors mxc_spi as described in
>     http://lists.denx.de/pipermail/u-boot/2010-March/068791.html
> and requested in
>     http://lists.denx.de/pipermail/u-boot/2012-January/116023.html
> in order to add support for the MX6Q in general and the mx6qsabrelite
> specifically.
> 
> Patch 1 simply moves the conditional parts of mxc_spi.c into the
> respective CPU-specific imx-regs.h files.
> 
> Patch 2 adds general support for SPI to the i.MX6.
> 
> Patch 3 adds support to the mx6qsabrelite board
> 
> Patch 4 modifies the 'sf' command to allow a default chip-select
> to be specified by board headers as is done on efika et al. This allows
> a bare 'sf' probe command:
>      U-Boot> sf probe
> instead of the more cumbersome usage when a GPIO is tacked onto
> the chip-select. Otherwise, this command-line would be needed
> to specify GP3:19 on SabreLite:
>      U-Boot> sf probe 0x5300
> 
> Patch 5 adds default chip-select values for mx6qsabrelite platform.
> 
> Patch 6 adds reference macros for use in storing the environment
> in serial flash to match the use on Freescale's U-Boot release
> 
> This patch set has been compiled against the following configurations,
> but only tested on mx6qsabrelite:
> 	mx6qsabrelite
> 	mx51evk
> 	mx31pdk
> 	mx35pdk

Muhehehe ... time for a little review-torture :-)

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Marek Vasut]

> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> ---
>  arch/arm/include/asm/arch-mx6/imx-regs.h |   44
> ++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0
> deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h
> b/arch/arm/include/asm/arch-mx6/imx-regs.h index 7650cb9..00040c4 100644
> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
> @@ -190,6 +190,50 @@ struct src {
>  	u32     gpr10;
>  };
> 
> +/* ECSPI registers */
> +struct cspi_regs {
> +	u32 rxdata;
> +	u32 txdata;
> +	u32 ctrl;
> +	u32 cfg;
> +	u32 intr;
> +	u32 dma;
> +	u32 stat;
> +	u32 period;
> +};

Sigh ... it's no fun I can have only one remark :-)

Is this part common for all imx-es ?

> +
> +/*
> + * CSPI register definitions
> + */
> +#define MXC_ECSPI
> +#define MXC_CSPICTRL_EN		(1 << 0)
> +#define MXC_CSPICTRL_MODE	(1 << 1)
> +#define MXC_CSPICTRL_XCH	(1 << 2)
> +#define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 12)
> +#define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0xfff) << 20)
> +#define MXC_CSPICTRL_PREDIV(x)	(((x) & 0xF) << 12)
> +#define MXC_CSPICTRL_POSTDIV(x)	(((x) & 0xF) << 8)
> +#define MXC_CSPICTRL_SELCHAN(x)	(((x) & 0x3) << 18)
> +#define MXC_CSPICTRL_MAXBITS	0xfff
> +#define MXC_CSPICTRL_TC		(1 << 7)
> +#define MXC_CSPICTRL_RXOVF	(1 << 6)
> +#define MXC_CSPIPERIOD_32KHZ	(1 << 15)
> +#define MAX_SPI_BYTES	32
> +
> +/* Bit position inside CTRL register to be associated with SS */
> +#define MXC_CSPICTRL_CHAN	18
> +
> +/* Bit position inside CON register to be associated with SS */
> +#define MXC_CSPICON_POL		4
> +#define MXC_CSPICON_PHA		0
> +#define MXC_CSPICON_SSPOL	12
> +#define MXC_SPI_BASE_ADDRESSES \
> +	ECSPI1_BASE_ADDR, \
> +	ECSPI2_BASE_ADDR, \
> +	ECSPI3_BASE_ADDR, \
> +	ECSPI4_BASE_ADDR, \
> +	ECSPI5_BASE_ADDR
> +
>  struct iim_regs {
>  	u32	ctrl;
>  	u32	ctrl_set;

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Eric Nelson]

On 01/17/2012 04:19 PM, Marek Vasut wrote:
>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>> ---
>>   arch/arm/include/asm/arch-mx6/imx-regs.h |   44
>> ++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0
>> deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h
>> b/arch/arm/include/asm/arch-mx6/imx-regs.h index 7650cb9..00040c4 100644
>> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
>> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
>> @@ -190,6 +190,50 @@ struct src {
>>   	u32     gpr10;
>>   };
>>
>> +/* ECSPI registers */
>> +struct cspi_regs {
>> +	u32 rxdata;
>> +	u32 txdata;
>> +	u32 ctrl;
>> +	u32 cfg;
>> +	u32 intr;
>> +	u32 dma;
>> +	u32 stat;
>> +	u32 period;
>> +};
>
> Sigh ... it's no fun I can have only one remark :-)
>
> Is this part common for all imx-es ?
>

All i.MX6's

This is a cut & paste from MX51.

I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration
for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h'
for i.MX31 and i.MX35 that share the CSPI peripheral.

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Marek Vasut]

> On 01/17/2012 04:19 PM, Marek Vasut wrote:
> >> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> >> ---
> >> 
> >>   arch/arm/include/asm/arch-mx6/imx-regs.h |   44
> >> 
> >> ++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0
> >> deletions(-)
> >> 
> >> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h
> >> b/arch/arm/include/asm/arch-mx6/imx-regs.h index 7650cb9..00040c4 100644
> >> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
> >> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
> >> @@ -190,6 +190,50 @@ struct src {
> >> 
> >>   	u32     gpr10;
> >>   
> >>   };
> >> 
> >> +/* ECSPI registers */
> >> +struct cspi_regs {
> >> +	u32 rxdata;
> >> +	u32 txdata;
> >> +	u32 ctrl;
> >> +	u32 cfg;
> >> +	u32 intr;
> >> +	u32 dma;
> >> +	u32 stat;
> >> +	u32 period;
> >> +};
> > 
> > Sigh ... it's no fun I can have only one remark :-)
> > 
> > Is this part common for all imx-es ?
> 
> All i.MX6's
> 
> This is a cut & paste from MX51.
> 
> I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration
> for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h'
> for i.MX31 and i.MX35 that share the CSPI peripheral.

But you don't even need this outside of the spi driver so just put it into the 
spi driver and be done with it. That'll solve your duplication issue.

M

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Eric Nelson]

On 01/17/2012 06:27 PM, Marek Vasut wrote:
>> On 01/17/2012 04:19 PM, Marek Vasut wrote:
>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>> +/* ECSPI registers */
>>>> +struct cspi_regs {
>>>> +	u32 rxdata;
>>>> +	u32 txdata;
>>>> +	u32 ctrl;
>>>> +	u32 cfg;
>>>> +	u32 intr;
>>>> +	u32 dma;
>>>> +	u32 stat;
>>>> +	u32 period;
>>>> +};
>>>
>>> Sigh ... it's no fun I can have only one remark :-)
>>>
>>> Is this part common for all imx-es ?
>>
>> All i.MX6's
>>
>> This is a cut&  paste from MX51.
>>
>> I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration
>> for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h'
>> for i.MX31 and i.MX35 that share the CSPI peripheral.
>
> But you don't even need this outside of the spi driver so just put it into the
> spi driver and be done with it. That'll solve your duplication issue.
>
> M
>

I'll defer to Stefano on this one, since I did this in response
to his request:

> Right - and we already discussed in the past how to avoid to put
> specific SOC code inside the driver. In fact, the cspi_regs structure
> was already moved into the specific SOC header (imx-regs.h) - but the
> definitions of the single bits of the registers are still inside the
> driver, as well as the base address of the (e)cspi controllers.
>
> They should also be moved - take into acoount by implementing your
> changes for i.mx6

The struct cspi_regs is already present in mx31, mx35, and mx51 headers,
so I'm not breaking new ground here, only in the bitfield declarations.

	http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-mx31/imx-regs.h;h=6a517ddd931ca0d1e598bd7456c4c611741602eb;hb=HEAD#l60
	http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-mx35/imx-regs.h;h=df74508a93ee87ae986f7c2f48f6c5fb36626070;hb=HEAD#l279
	http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-mx5/imx-regs.h;h=0ee88d25b7800ae9e6aed809d02dd19d9cac9c82;hb=HEAD#l423

My interpretation of Stefano's intent is to clean up the driver at the expense
of extra defines in the arch-specific headers.

Regards,


Eric

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Marek Vasut]

> On 01/17/2012 06:27 PM, Marek Vasut wrote:
> >> On 01/17/2012 04:19 PM, Marek Vasut wrote:
> >>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> >>>> +/* ECSPI registers */
> >>>> +struct cspi_regs {
> >>>> +	u32 rxdata;
> >>>> +	u32 txdata;
> >>>> +	u32 ctrl;
> >>>> +	u32 cfg;
> >>>> +	u32 intr;
> >>>> +	u32 dma;
> >>>> +	u32 stat;
> >>>> +	u32 period;
> >>>> +};
> >>> 
> >>> Sigh ... it's no fun I can have only one remark :-)
> >>> 
> >>> Is this part common for all imx-es ?
> >> 
> >> All i.MX6's
> >> 
> >> This is a cut&  paste from MX51.
> >> 
> >> I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration
> >> for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h'
> >> for i.MX31 and i.MX35 that share the CSPI peripheral.
> > 
> > But you don't even need this outside of the spi driver so just put it
> > into the spi driver and be done with it. That'll solve your duplication
> > issue.
> > 
> > M
> 
> I'll defer to Stefano on this one, since I did this in response
> 
> to his request:
> > Right - and we already discussed in the past how to avoid to put
> > specific SOC code inside the driver. In fact, the cspi_regs structure
> > was already moved into the specific SOC header (imx-regs.h) - but the
> > definitions of the single bits of the registers are still inside the
> > driver, as well as the base address of the (e)cspi controllers.
> > 
> > They should also be moved - take into acoount by implementing your
> > changes for i.mx6
> 
> The struct cspi_regs is already present in mx31, mx35, and mx51 headers,
> so I'm not breaking new ground here, only in the bitfield declarations.
> 
> 	http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-
mx31/i
> mx-regs.h;h=6a517ddd931ca0d1e598bd7456c4c611741602eb;hb=HEAD#l60
> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-mx35/i
> mx-regs.h;h=df74508a93ee87ae986f7c2f48f6c5fb36626070;hb=HEAD#l279
> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-mx5/im
> x-regs.h;h=0ee88d25b7800ae9e6aed809d02dd19d9cac9c82;hb=HEAD#l423
> 
> My interpretation of Stefano's intent is to clean up the driver at the
> expense of extra defines in the arch-specific headers.

But they're all the same, right? So we have now the same structure defined 
thrice?

M
> 
> Regards,
> 
> 
> Eric

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Eric Nelson]

On 01/17/2012 06:47 PM, Marek Vasut wrote:
>> On 01/17/2012 06:27 PM, Marek Vasut wrote:
>>
>> I'll defer to Stefano on this one, since I did this in response
>> to his request:
 >>
>>> Right - and we already discussed in the past how to avoid to put
>>> specific SOC code inside the driver. In fact, the cspi_regs structure
>>> was already moved into the specific SOC header (imx-regs.h) - but the
>>> definitions of the single bits of the registers are still inside the
>>> driver, as well as the base address of the (e)cspi controllers.
>>>
>>> They should also be moved - take into acoount by implementing your
>>> changes for i.mx6
>>
>> The struct cspi_regs is already present in mx31, mx35, and mx51 headers,
>> so I'm not breaking new ground here, only in the bitfield declarations.
 >> <snip>
>>
>> My interpretation of Stefano's intent is to clean up the driver at the
>> expense of extra defines in the arch-specific headers.
>
> But they're all the same, right? So we have now the same structure defined
> thrice?
>

Almost, but not quite: mx31 and mx35 both use the CSPI peripheral and have
one layout. mx5 and mx6 have the ECSPI peripheral, which has an extra
register "cfg" to control the polarity and phase of the signals.

Actually, that comment is wrong. The MX51 and MX53 have **both** CSPI and
ECSPI peripherals, but the existing code in mxc_spi only supports ECSPI.

The bitfields that my patches move into the imx-regs.h files are also
almost identical.

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Stefano Babic]

On 18/01/2012 02:44, Eric Nelson wrote:
> On 01/17/2012 06:27 PM, Marek Vasut wrote:
>>> On 01/17/2012 04:19 PM, Marek Vasut wrote:
>>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>>> +/* ECSPI registers */
>>>>> +struct cspi_regs {
>>>>> +    u32 rxdata;
>>>>> +    u32 txdata;
>>>>> +    u32 ctrl;
>>>>> +    u32 cfg;
>>>>> +    u32 intr;
>>>>> +    u32 dma;
>>>>> +    u32 stat;
>>>>> +    u32 period;
>>>>> +};
>>>>
>>>> Sigh ... it's no fun I can have only one remark :-)
>>>>
>>>> Is this part common for all imx-es ?
>>>
>>> All i.MX6's
>>>
>>> This is a cut&  paste from MX51.
>>>
>>> I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration
>>> for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h'
>>> for i.MX31 and i.MX35 that share the CSPI peripheral.
>>
>> But you don't even need this outside of the spi driver so just put it
>> into the
>> spi driver and be done with it. That'll solve your duplication issue.
>>
>> M
>>
> 
> I'll defer to Stefano on this one, since I did this in response
> to his request:

Yes, I admit I am guilty about this !

The layout of the CSPI registers is not exactly the same for all SOCs.
For example, the MXC_CSPICTRL_TC has a different position, as well as
the BITCOUNT (because the MX31 can send less bits in one shot) and
MXC_CSPICTRL_CHIPSELECT.

So they are similar, but not exactly the same.

Then we have the MX5 registers. Even if we check the CSPI (not eCSPI)
controller, the layout of the registers is different compared to the MX3
SOCs.

> The struct cspi_regs is already present in mx31, mx35, and mx51 headers,
> so I'm not breaking new ground here, only in the bitfield declarations.

Right, I see the same. The cspi_regs structure is already defined into
the imx_regs.h, only the bit layout was moved. And as the bit layout is
SOC dependent, I think the right place for it is inside the imx-regs.h
registers.

> My interpretation of Stefano's intent is to clean up the driver at the
> expense
> of extra defines in the arch-specific headers.

Yes, you're right - of course, I am open also to other solutions if they
are proofed to be better ;-).

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] mxc_spi refactoring (for mx6q)

[Dirk Behme]

On 17.01.2012 23:09, Eric Nelson wrote:
> This patch set refactors mxc_spi as described in 
>     http://lists.denx.de/pipermail/u-boot/2010-March/068791.html
> and requested in 
>     http://lists.denx.de/pipermail/u-boot/2012-January/116023.html
> in order to add support for the MX6Q in general and the mx6qsabrelite 
> specifically.

Whole patch series

Acked-by: Dirk Behme <dirk.behme@de.bosch.com>

Many thanks!

Dirk

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Marek Vasut]

> On 18/01/2012 02:44, Eric Nelson wrote:
> > On 01/17/2012 06:27 PM, Marek Vasut wrote:
> >>> On 01/17/2012 04:19 PM, Marek Vasut wrote:
> >>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> >>>>> +/* ECSPI registers */
> >>>>> +struct cspi_regs {
> >>>>> +    u32 rxdata;
> >>>>> +    u32 txdata;
> >>>>> +    u32 ctrl;
> >>>>> +    u32 cfg;
> >>>>> +    u32 intr;
> >>>>> +    u32 dma;
> >>>>> +    u32 stat;
> >>>>> +    u32 period;
> >>>>> +};
> >>>> 
> >>>> Sigh ... it's no fun I can have only one remark :-)
> >>>> 
> >>>> Is this part common for all imx-es ?
> >>> 
> >>> All i.MX6's
> >>> 
> >>> This is a cut&  paste from MX51.
> >>> 
> >>> I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration
> >>> for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h'
> >>> for i.MX31 and i.MX35 that share the CSPI peripheral.
> >> 
> >> But you don't even need this outside of the spi driver so just put it
> >> into the
> >> spi driver and be done with it. That'll solve your duplication issue.
> >> 
> >> M
> > 
> > I'll defer to Stefano on this one, since I did this in response
> 
> > to his request:
> Yes, I admit I am guilty about this !
> 
> The layout of the CSPI registers is not exactly the same for all SOCs.
> For example, the MXC_CSPICTRL_TC has a different position, as well as
> the BITCOUNT (because the MX31 can send less bits in one shot) and
> MXC_CSPICTRL_CHIPSELECT.
> 
> So they are similar, but not exactly the same.
> 
> Then we have the MX5 registers. Even if we check the CSPI (not eCSPI)
> controller, the layout of the registers is different compared to the MX3
> SOCs.
> 
> > The struct cspi_regs is already present in mx31, mx35, and mx51 headers,
> > so I'm not breaking new ground here, only in the bitfield declarations.
> 
> Right, I see the same. The cspi_regs structure is already defined into
> the imx_regs.h, only the bit layout was moved. And as the bit layout is
> SOC dependent, I think the right place for it is inside the imx-regs.h
> registers.
> 
> > My interpretation of Stefano's intent is to clean up the driver at the
> > expense
> > of extra defines in the arch-specific headers.
> 
> Yes, you're right - of course, I am open also to other solutions if they
> are proofed to be better ;-).
> 
> Best regards,
> Stefano

Ok guys, I see ... Stefano, you're ok with putting the reg structures into these 
header files?

M

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Stefano Babic]

On 18/01/2012 17:08, Marek Vasut wrote:

> Ok guys, I see ... Stefano, you're ok with putting the reg structures into these 
> header files?

The reg structures are already into these header files - the patch moves
only the bit meaning inside the imx-regs.h files. We can discuss if we
should move them again into the driver, making the decision on which
structure to be used not on a SOC level (#ifdef CONFIG_MXxx), but on
basis of the controller type (CSPI or ECSPI).

This makes sense if we run (we had until now no use case with the
currently supported boards) a MX5 board using a CSPI instead of a ECSPI.
In this case, we should also transform MXC_CSPI / MXC_ECSPI in a CONFIG_
define to be put in the board configuration file.

However, as usual in u-boot, dead code or code that has no use case and
cannot be tested is not allowed. Until we have not such a board (MX5
board requiring CSPI instead of ECSPI), we should avoid to introduce not
tested code and let those changes for a next patch.

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 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Eric Nelson]

On 01/18/2012 01:39 AM, Stefano Babic wrote:
> On 18/01/2012 02:44, Eric Nelson wrote:
>> I'll defer to Stefano on this one, since I did this in response
>> to his request:
>
> Yes, I admit I am guilty about this !
>
> The layout of the CSPI registers is not exactly the same for all SOCs.
> For example, the MXC_CSPICTRL_TC has a different position, as well as
> the BITCOUNT (because the MX31 can send less bits in one shot) and
> MXC_CSPICTRL_CHIPSELECT.
>
> So they are similar, but not exactly the same.
>
> Then we have the MX5 registers. Even if we check the CSPI (not eCSPI)
> controller, the layout of the registers is different compared to the MX3
> SOCs.
>
>> The struct cspi_regs is already present in mx31, mx35, and mx51 headers,
>> so I'm not breaking new ground here, only in the bitfield declarations.
>
> Right, I see the same. The cspi_regs structure is already defined into
> the imx_regs.h, only the bit layout was moved. And as the bit layout is
> SOC dependent, I think the right place for it is inside the imx-regs.h
> registers.
>
>> My interpretation of Stefano's intent is to clean up the driver at the
>> expense of extra defines in the arch-specific headers.
>
> Yes, you're right - of course, I am open also to other solutions if they
> are proofed to be better ;-).
>

I think this is about as good as things get with the current code base.
I would argue that the driver would be better if it explicitly supported
ECSPI and CSPI at the same time since the mx5x CPUs support it.

Implememting that would likely require a de-structuring (removing the
use of structs to represent the register set). IOW, a re-write.

That's probably not worth the effort unless someone's built hardware
that needs it (I'm not aware of any).

On our boards that use more than one channel of SPI (for PMIC and SF), we're 
using ECSPI on both. I think the same was true on the MX51 EVK.

[U-Boot] [PATCH 2/6] mx6q: Add support for ECSPI through mxc_spi driver

[Stefano Babic]

On 18/01/2012 21:05, Eric Nelson wrote:

>>
>> Yes, you're right - of course, I am open also to other solutions if they
>> are proofed to be better ;-).
>>
> 
> I think this is about as good as things get with the current code base.
> I would argue that the driver would be better if it explicitly supported
> ECSPI and CSPI at the same time since the mx5x CPUs support it.

This means that the driver goes to support multiple interfaces at the
same time, independently if they are CSPI or ECSPI. At the moment, there
is no use case for it.

> 
> Implememting that would likely require a de-structuring (removing the
> use of structs to represent the register set). IOW, a re-write.

Yes, this is also for most drivers in u-boot to support multiple
interface and not only one.

> 
> That's probably not worth the effort unless someone's built hardware
> that needs it (I'm not aware of any).

Agree.

> 
> On our boards that use more than one channel of SPI (for PMIC and SF),
> we're using ECSPI on both. I think the same was true on the MX51 EVK.

Yes, it is the same.

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 6/6] mx6q: mx6qsabrelite: Provide defaults for placing environment in serial flash

[Jason Hui]

On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson
<eric.nelson@boundarydevices.com> wrote:
> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> ---
> ?include/configs/mx6qsabrelite.h | ? 12 +++++++++++-
> ?1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
> index 44b028a..160894c 100644
> --- a/include/configs/mx6qsabrelite.h
> +++ b/include/configs/mx6qsabrelite.h
> @@ -174,10 +174,20 @@
> ?/* FLASH and environment organization */
> ?#define CONFIG_SYS_NO_FLASH
>
> -#define CONFIG_ENV_OFFSET ? ? ? ? ? ? ?(6 * 64 * 1024)
> ?#define CONFIG_ENV_SIZE ? ? ? ? ? ? ? ? ? ? ? ?(8 * 1024)
> +
> ?#define CONFIG_ENV_IS_IN_MMC
> +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */

[U-Boot] [PATCH 6/6] mx6q: mx6qsabrelite: Provide defaults for placing environment in serial flash

[Dirk Behme]

On 20.01.2012 04:27, Jason Hui wrote:
> On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson
> <eric.nelson@boundarydevices.com> wrote:
>> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
>> ---
>>  include/configs/mx6qsabrelite.h |   12 +++++++++++-
>>  1 files changed, 11 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
>> index 44b028a..160894c 100644
>> --- a/include/configs/mx6qsabrelite.h
>> +++ b/include/configs/mx6qsabrelite.h
>> @@ -174,10 +174,20 @@
>>  /* FLASH and environment organization */
>>  #define CONFIG_SYS_NO_FLASH
>>
>> -#define CONFIG_ENV_OFFSET              (6 * 64 * 1024)
>>  #define CONFIG_ENV_SIZE                        (8 * 1024)
>> +
>>  #define CONFIG_ENV_IS_IN_MMC
>> +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */
> 
>>From the commit log, it says the default is in serial flash,

No, the commit log doesn't say this. It doesn't say 'it is'. It says it 
'provides the defaults'. But it doesn't say that it actually uses these 
defaults.

> but
> apparently in the code
> the env is default to MMC, which mismatch. please fix it.

As mentioned above, I understand this differently. While I reviewed it 
some days ago, I found the description and the doing here quite fine.

It enables the MMC env and additionally _provides_ everything needed to 
easily switch to SPI env by uncommenting one switch. This is fine and 
quite helpful, see e.g. [1].

I like this, please keep it as is.

Best regards

Dirk

[1] http://lists.denx.de/pipermail/u-boot/2012-January/116266.html

"you can place the environment in SPI-NOR as well by commenting out 
CONFIG_ENV_IS_IN_MMC, and un-commenting ..._IN_SPI_FLASH in 
include/configs/mx6qsabrelite.h."

>> +
>> +#if defined(CONFIG_ENV_IS_IN_MMC)
>> +#define CONFIG_ENV_OFFSET              (6 * 64 * 1024)
>>  #define CONFIG_SYS_MMC_ENV_DEV         0
>> +#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH)
>> +#define CONFIG_ENV_OFFSET              (768 * 1024)
>> +#define CONFIG_ENV_SECT_SIZE           (8 * 1024)
>> +#define CONFIG_ENV_SPI_CS              0x5300
> 
> I'm wondering how the CONFIG_ENV_SPI_CS  could be 0x5300? Vague?
> 
>> +#define CONFIG_ENV_SPI_MODE            SPI_MODE_0
>> +#endif
>>
>>  #define CONFIG_OF_LIBFDT
>>
>> --
>> 1.7.1
>>
> 


-- 
======================================================================
Dirk Behme                      Robert Bosch Car Multimedia GmbH
                                 CM-AI/PJ-CF32
Phone: +49 5121 49-3274         Dirk Behme
Fax:   +49 711 811 5053274      PO Box 77 77 77
mailto:dirk.behme at de.bosch.com  D-31132 Hildesheim - Germany

Bosch Group, Car Multimedia (CM)
              Automotive Navigation and Infotainment Systems (AI)
              ProJect - CoreFunctions (PJ-CF)

Robert Bosch Car Multimedia GmbH - Ein Unternehmen der Bosch Gruppe
Sitz: Hildesheim
Registergericht: Amtsgericht Hildesheim HRB 201334
Aufsichtsratsvorsitzender: Volkmar Denner
Gesch?ftsf?hrung: Uwe Thomas, Michael Bolle, Robby Drave, Egbert Hellwig
======================================================================

[U-Boot] [PATCH 6/6] mx6q: mx6qsabrelite: Provide defaults for placing environment in serial flash

[Jason Hui]

On Fri, Jan 20, 2012 at 3:06 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 20.01.2012 04:27, Jason Hui wrote:
>>
>> On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson
>> <eric.nelson@boundarydevices.com> wrote:
>>>
>>> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
>>> ---
>>> ?include/configs/mx6qsabrelite.h | ? 12 +++++++++++-
>>> ?1 files changed, 11 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/include/configs/mx6qsabrelite.h
>>> b/include/configs/mx6qsabrelite.h
>>> index 44b028a..160894c 100644
>>> --- a/include/configs/mx6qsabrelite.h
>>> +++ b/include/configs/mx6qsabrelite.h
>>> @@ -174,10 +174,20 @@
>>> ?/* FLASH and environment organization */
>>> ?#define CONFIG_SYS_NO_FLASH
>>>
>>> -#define CONFIG_ENV_OFFSET ? ? ? ? ? ? ?(6 * 64 * 1024)
>>> ?#define CONFIG_ENV_SIZE ? ? ? ? ? ? ? ? ? ? ? ?(8 * 1024)
>>> +
>>> ?#define CONFIG_ENV_IS_IN_MMC
>>> +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */
>>
>>
>>> From the commit log, it says the default is in serial flash,
>
>
> No, the commit log doesn't say this. It doesn't say 'it is'. It says it
> 'provides the defaults'. But it doesn't say that it actually uses these
> defaults.
>
>
>> but
>> apparently in the code
>> the env is default to MMC, which mismatch. please fix it.
>
>
> As mentioned above, I understand this differently. While I reviewed it some
> days ago, I found the description and the doing here quite fine.

OK, I get it, then I'm fine with it too.

>
> It enables the MMC env and additionally _provides_ everything needed to
> easily switch to SPI env by uncommenting one switch. This is fine and quite
> helpful, see e.g. [1].
>
> I like this, please keep it as is.
>
[...]

>>> +#define CONFIG_ENV_SPI_CS ? ? ? ? ? ? ?0x5300
>>
>>
>> I'm wondering how the CONFIG_ENV_SPI_CS ?could be 0x5300? Vague?

Then the left open question is only above one.

Jason

[U-Boot] [PATCH 6/6] mx6q: mx6qsabrelite: Provide defaults for placing environment in serial flash

[Stefano Babic]

On 20/01/2012 08:48, Jason Hui wrote:

>>>
>>> I'm wondering how the CONFIG_ENV_SPI_CS  could be 0x5300? Vague?
> 
> Then the left open question is only above one.

The SPI driver can take as chip select the controller's chip selects as
well as an external GPIO. The LSB byte has the value of the internal
chip select, the highest (thought as 16-bit value) contains the GPIO
number. Reading this configuration, the GPIO used on this board should
be the number 83 (0x53).
	
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 6/6] mx6q: mx6qsabrelite: Provide defaults for placing environment in serial flash

[Eric Nelson]

On 01/19/2012 08:27 PM, Jason Hui wrote:
> On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson
> <eric.nelson@boundarydevices.com>  wrote:
>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>> ---
>>   include/configs/mx6qsabrelite.h |   12 +++++++++++-
>>   1 files changed, 11 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
>> index 44b028a..160894c 100644
>> --- a/include/configs/mx6qsabrelite.h
>> +++ b/include/configs/mx6qsabrelite.h
>> @@ -174,10 +174,20 @@
>>   /* FLASH and environment organization */
>>   #define CONFIG_SYS_NO_FLASH
>>
>> -#define CONFIG_ENV_OFFSET              (6 * 64 * 1024)
>>   #define CONFIG_ENV_SIZE                        (8 * 1024)
>> +
>>   #define CONFIG_ENV_IS_IN_MMC
>> +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */
>
> From the commit log, it says the default is in serial flash, but
> apparently in the code the env is default to MMC, which mismatch.
 > please fix it.
>

You're asking that I change the comment not the default, right?

>> +
>> +#if defined(CONFIG_ENV_IS_IN_MMC)
>> +#define CONFIG_ENV_OFFSET              (6 * 64 * 1024)
>>   #define CONFIG_SYS_MMC_ENV_DEV         0
>> +#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH)
>> +#define CONFIG_ENV_OFFSET              (768 * 1024)
>> +#define CONFIG_ENV_SECT_SIZE           (8 * 1024)
>> +#define CONFIG_ENV_SPI_CS              0x5300
>
> I'm wondering how the CONFIG_ENV_SPI_CS  could be 0x5300? Vague?
>

Please review the updated patch below and see whether the
expanded commit message fixes things.

Regards,


Eric
commit 0443433bf80c5203a8ce67fb4faaf4032e398e1d
Author: Eric Nelson <eric.nelson@boundarydevices.com>
Date:   Tue Jan 17 14:11:54 2012 -0700

     mx6q: mx6qsabrelite: Provide macros for environment in serial flash

     The default settings store the persistent environment on SD card
     and not serial flash (SPI NOR).

     To use SPI NOR to save the environment instead of SD card, edit
     include/configs/mx6qsabrelite.h and

         - undefine CONFIG_ENV_IS_IN_MMC
         - define   CONFIG_ENV_IS_IN_SPI_FLASH

     Note that the mxc_spi driver (drivers/spi/mxc_spi.c) uses the
     Chip-Select variable (CONFIG_ENV_SPI_CS) to allow the use of
     a GPIO if the chip-select is greater than 3. The low 8-bits
     set the chip select number and bits 8-15 set the GPIO number.

     The GPIO used on Sabre Lite is GP3:19 == 83.

     Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>

diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index e34f108..024a94c 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -174,10 +174,20 @@
  /* FLASH and environment organization */
  #define CONFIG_SYS_NO_FLASH

-#define CONFIG_ENV_OFFSET		(6 * 64 * 1024)
  #define CONFIG_ENV_SIZE			(8 * 1024)
+
  #define CONFIG_ENV_IS_IN_MMC
+/* #define CONFIG_ENV_IS_IN_SPI_FLASH */
+
+#if defined(CONFIG_ENV_IS_IN_MMC)
+#define CONFIG_ENV_OFFSET		(6 * 64 * 1024)
  #define CONFIG_SYS_MMC_ENV_DEV		0
+#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH)
+#define CONFIG_ENV_OFFSET		(768 * 1024)
+#define CONFIG_ENV_SECT_SIZE		(8 * 1024)
+#define CONFIG_ENV_SPI_CS		(0|(83<<8))
+#define CONFIG_ENV_SPI_MODE		SPI_MODE_0
+#endif

  #define CONFIG_OF_LIBFDT

[U-Boot] [PATCH 6/6] mx6q: mx6qsabrelite: Provide defaults for placing environment in serial flash

[Eric Nelson]

On 01/20/2012 01:47 AM, Stefano Babic wrote:
> On 20/01/2012 08:48, Jason Hui wrote:
>
>>>>
>>>> I'm wondering how the CONFIG_ENV_SPI_CS  could be 0x5300? Vague?
>>
>> Then the left open question is only above one.
>
> The SPI driver can take as chip select the controller's chip selects as
> well as an external GPIO. The LSB byte has the value of the internal
> chip select, the highest (thought as 16-bit value) contains the GPIO
> number. Reading this configuration, the GPIO used on this board should
> be the number 83 (0x53).
> 	
> Stefano
>
Thanks Stefano,

I like your description better than the one I just wrote... I should
have scanned all of my e-mails before drafting my earlier commit msg ;)