public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH V4 0/7] mxc_spi refactoring (for mx6q and mx6qsabrelite)
@ 2012-01-29 18:59 Eric Nelson
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 1/7] mxc_spi: move machine specifics into CPU headers Eric Nelson
                   ` (6 more replies)
  0 siblings, 7 replies; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 18:59 UTC (permalink / raw)
  To: u-boot

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 bus and chip-select
to be specified by board headers. 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 provides a description of usage and configuration of CONFIG_CMD_SF.

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

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 1/7] mxc_spi: move machine specifics into CPU headers
  2012-01-29 18:59 [U-Boot] [PATCH V4 0/7] mxc_spi refactoring (for mx6q and mx6qsabrelite) Eric Nelson
@ 2012-01-29 18:59 ` Eric Nelson
  2012-01-29 19:16   ` Marek Vasut
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 2/7] mx6q: Add support for ECSPI through mxc_spi driver Eric Nelson
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 18:59 UTC (permalink / raw)
  To: u-boot

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>
Acked-by: Dirk Behme <dirk.behme@de.bosch.com>
Acked-by: Stefano Babic <sbabic@denx.de>
---
 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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 2/7] mx6q: Add support for ECSPI through mxc_spi driver
  2012-01-29 18:59 [U-Boot] [PATCH V4 0/7] mxc_spi refactoring (for mx6q and mx6qsabrelite) Eric Nelson
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 1/7] mxc_spi: move machine specifics into CPU headers Eric Nelson
@ 2012-01-29 18:59 ` Eric Nelson
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 3/7] mx6q: mx6qsabrelite: Add ECSPI support to the Sabrelite platform Eric Nelson
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 18:59 UTC (permalink / raw)
  To: u-boot

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
Acked-by: Dirk Behme <dirk.behme@de.bosch.com>
Acked-by: Stefano Babic <sbabic@denx.de>
---
 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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 3/7] mx6q: mx6qsabrelite: Add ECSPI support to the Sabrelite platform
  2012-01-29 18:59 [U-Boot] [PATCH V4 0/7] mxc_spi refactoring (for mx6q and mx6qsabrelite) Eric Nelson
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 1/7] mxc_spi: move machine specifics into CPU headers Eric Nelson
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 2/7] mx6q: Add support for ECSPI through mxc_spi driver Eric Nelson
@ 2012-01-29 18:59 ` Eric Nelson
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 4/7] sf command: allow default bus and chip selects Eric Nelson
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 18:59 UTC (permalink / raw)
  To: u-boot

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
Acked-by: Dirk Behme <dirk.behme@de.bosch.com>
Acked-by: Stefano Babic <sbabic@denx.de>
---
 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..2ba6b0c 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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 4/7] sf command: allow default bus and chip selects
  2012-01-29 18:59 [U-Boot] [PATCH V4 0/7] mxc_spi refactoring (for mx6q and mx6qsabrelite) Eric Nelson
                   ` (2 preceding siblings ...)
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 3/7] mx6q: mx6qsabrelite: Add ECSPI support to the Sabrelite platform Eric Nelson
@ 2012-01-29 18:59 ` Eric Nelson
  2012-01-30  5:13   ` Mike Frysinger
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 5/7] README: Add description of SPI Flash (SF) command configuration Eric Nelson
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 18:59 UTC (permalink / raw)
  To: u-boot

This patch allows a board configuration file to provide default bus
and chip-selects for SPI flash so that first argument to the 'sf' command
is optional.

On boards that use the mxc_spi driver and a GPIO for chip select, this allows
a much simpler command line:
	U-Boot> sf probe
instead of
	U-Boot> sf probe 0x5300
---
 common/cmd_sf.c |   37 +++++++++++++++++++++----------------
 1 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 7225656..04a3258 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -17,6 +17,12 @@
 #ifndef CONFIG_SF_DEFAULT_MODE
 # define CONFIG_SF_DEFAULT_MODE		SPI_MODE_3
 #endif
+#ifndef CONFIG_SF_DEFAULT_CS
+# define CONFIG_SF_DEFAULT_CS		0
+#endif
+#ifndef CONFIG_SF_DEFAULT_BUS
+# define CONFIG_SF_DEFAULT_BUS		0
+#endif
 
 static struct spi_flash *flash;
 
@@ -63,27 +69,26 @@ static int sf_parse_len_arg(char *arg, ulong *len)
 
 static int do_spi_flash_probe(int argc, char * const argv[])
 {
-	unsigned int bus = 0;
-	unsigned int cs;
+	unsigned int bus = CONFIG_SF_DEFAULT_BUS;
+	unsigned int cs = CONFIG_SF_DEFAULT_CS;
 	unsigned int speed = CONFIG_SF_DEFAULT_SPEED;
 	unsigned int mode = CONFIG_SF_DEFAULT_MODE;
 	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 != ':'))
-		return -1;
-	if (*endp == ':') {
-		if (endp[1] == 0)
-			return -1;
-
-		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 +304,7 @@ usage:
 U_BOOT_CMD(
 	sf,	5,	1,	do_spi_flash,
 	"SPI flash sub-system",
-	"probe [bus:]cs [hz] [mode]	- init flash device on given SPI bus\n"
+	"probe [[bus:]cs] [hz] [mode]	- init flash device on given SPI bus\n"
 	"				  and chip select\n"
 	"sf read addr offset len 	- read `len' bytes starting at\n"
 	"				  `offset' to memory at `addr'\n"
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 5/7] README: Add description of SPI Flash (SF) command configuration
  2012-01-29 18:59 [U-Boot] [PATCH V4 0/7] mxc_spi refactoring (for mx6q and mx6qsabrelite) Eric Nelson
                   ` (3 preceding siblings ...)
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 4/7] sf command: allow default bus and chip selects Eric Nelson
@ 2012-01-29 18:59 ` Eric Nelson
  2012-01-29 19:17   ` Marek Vasut
  2012-01-30  5:12   ` Mike Frysinger
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select Eric Nelson
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 7/7] mx6q: mx6qsabrelite: Conditionally define macros for environment in serial flash Eric Nelson
  6 siblings, 2 replies; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 18:59 UTC (permalink / raw)
  To: u-boot

---
 README |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/README b/README
index f6ab85c..1a98915 100644
--- a/README
+++ b/README
@@ -800,6 +800,7 @@ The following options need to be configured:
 					  (requires CONFIG_CMD_I2C)
 		CONFIG_CMD_SETGETDCR	  Support for DCR Register access
 					  (4xx only)
+		CONFIG_CMD_SF		* Read/write/erase SPI NOR flash
 		CONFIG_CMD_SHA1SUM	  print sha1 memory digest
 					  (requires CONFIG_CMD_MEMORY)
 		CONFIG_CMD_SOURCE	  "source" command Support
@@ -2179,6 +2180,25 @@ The following options need to be configured:
 		allows to read/write in Dataflash via the standard
 		commands cp, md...
 
+- Serial Flash support
+		CONFIG_CMD_SF
+
+		Defining this option enables SPI flash commands
+		'sf probe/read/write/erase/update'.
+
+		Usage requires an initial 'probe' to define the serial
+		flash parameters, followed by read/write/erase/update
+		commands.
+
+		The following defaults may be provided by the platform
+		to handle the common case when only a single serial
+		flash is present on the system.
+
+		CONFIG_SF_DEFAULT_BUS		Bus identifier
+		CONFIG_SF_DEFAULT_CS		Chip-select
+		CONFIG_SF_DEFAULT_MODE 		(see include/spi.h)
+		CONFIG_SF_DEFAULT_SPEED		in Hz
+
 - SystemACE Support:
 		CONFIG_SYSTEMACE
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-29 18:59 [U-Boot] [PATCH V4 0/7] mxc_spi refactoring (for mx6q and mx6qsabrelite) Eric Nelson
                   ` (4 preceding siblings ...)
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 5/7] README: Add description of SPI Flash (SF) command configuration Eric Nelson
@ 2012-01-29 18:59 ` Eric Nelson
  2012-01-29 19:18   ` Marek Vasut
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 7/7] mx6q: mx6qsabrelite: Conditionally define macros for environment in serial flash Eric Nelson
  6 siblings, 1 reply; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 18:59 UTC (permalink / raw)
  To: u-boot

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
Acked-by: Dirk Behme <dirk.behme@de.bosch.com>
Acked-by: Stefano Babic <sbabic@denx.de>
---
 include/configs/mx6qsabrelite.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index 8dd6e39..cc770e2 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -46,9 +46,12 @@
 
 #define CONFIG_CMD_SF
 #ifdef CONFIG_CMD_SF
+#define GPIO_3_19 ((2*32)+19)
 #define CONFIG_SPI_FLASH
 #define CONFIG_SPI_FLASH_SST
 #define CONFIG_MXC_SPI
+#define CONFIG_SF_DEFAULT_BUS  0
+#define CONFIG_SF_DEFAULT_CS   (0|(GPIO_3_19<<8))
 #define CONFIG_SF_DEFAULT_SPEED 25000000
 #define CONFIG_SF_DEFAULT_MODE (SPI_MODE_0)
 #endif
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 7/7] mx6q: mx6qsabrelite: Conditionally define macros for environment in serial flash
  2012-01-29 18:59 [U-Boot] [PATCH V4 0/7] mxc_spi refactoring (for mx6q and mx6qsabrelite) Eric Nelson
                   ` (5 preceding siblings ...)
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select Eric Nelson
@ 2012-01-29 18:59 ` Eric Nelson
  6 siblings, 0 replies; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 18:59 UTC (permalink / raw)
  To: u-boot

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

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.

The GPIO used on Sabre Lite is GP3:19 == 83.
---
 include/configs/mx6qsabrelite.h |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index cc770e2..77d30c9 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -176,10 +176,22 @@
 /* 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_BUS		CONFIG_SF_DEFAULT_BUS
+#define CONFIG_ENV_SPI_CS		CONFIG_SF_DEFAULT_CS
+#define CONFIG_ENV_SPI_MODE		CONFIG_SF_DEFAULT_MODE
+#define CONFIG_ENV_SPI_MAX_HZ		CONFIG_SF_DEFAULT_SPEED
+#endif
 
 #define CONFIG_OF_LIBFDT
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 1/7] mxc_spi: move machine specifics into CPU headers
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 1/7] mxc_spi: move machine specifics into CPU headers Eric Nelson
@ 2012-01-29 19:16   ` Marek Vasut
  2012-01-29 20:01     ` Eric Nelson
  0 siblings, 1 reply; 26+ messages in thread
From: Marek Vasut @ 2012-01-29 19:16 UTC (permalink / raw)
  To: u-boot

> 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>
> Acked-by: Dirk Behme <dirk.behme@de.bosch.com>
> Acked-by: Stefano Babic <sbabic@denx.de>
> ---
>  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

Good, but what about the boards that use these? Won't they be broken?

M

>  	reg_write(&regs->cfg, mxcs->cfg_reg);
>  #endif

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 5/7] README: Add description of SPI Flash (SF) command configuration
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 5/7] README: Add description of SPI Flash (SF) command configuration Eric Nelson
@ 2012-01-29 19:17   ` Marek Vasut
  2012-01-29 19:57     ` Eric Nelson
  2012-01-30  5:11     ` Mike Frysinger
  2012-01-30  5:12   ` Mike Frysinger
  1 sibling, 2 replies; 26+ messages in thread
From: Marek Vasut @ 2012-01-29 19:17 UTC (permalink / raw)
  To: u-boot

> ---
>  README |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/README b/README
> index f6ab85c..1a98915 100644
> --- a/README
> +++ b/README
> @@ -800,6 +800,7 @@ The following options need to be configured:
>  					  (requires CONFIG_CMD_I2C)
>  		CONFIG_CMD_SETGETDCR	  Support for DCR Register access
>  					  (4xx only)
> +		CONFIG_CMD_SF		* Read/write/erase SPI NOR flash

Why did you add the asterisk here ?

>  		CONFIG_CMD_SHA1SUM	  print sha1 memory digest
>  					  (requires CONFIG_CMD_MEMORY)
>  		CONFIG_CMD_SOURCE	  "source" command Support
> @@ -2179,6 +2180,25 @@ The following options need to be configured:
>  		allows to read/write in Dataflash via the standard
>  		commands cp, md...
> 
> +- Serial Flash support
> +		CONFIG_CMD_SF
> +
> +		Defining this option enables SPI flash commands
> +		'sf probe/read/write/erase/update'.
> +
> +		Usage requires an initial 'probe' to define the serial
> +		flash parameters, followed by read/write/erase/update
> +		commands.
> +
> +		The following defaults may be provided by the platform
> +		to handle the common case when only a single serial
> +		flash is present on the system.
> +
> +		CONFIG_SF_DEFAULT_BUS		Bus identifier
> +		CONFIG_SF_DEFAULT_CS		Chip-select
> +		CONFIG_SF_DEFAULT_MODE 		(see include/spi.h)
> +		CONFIG_SF_DEFAULT_SPEED		in Hz
> +
>  - SystemACE Support:
>  		CONFIG_SYSTEMACE

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select Eric Nelson
@ 2012-01-29 19:18   ` Marek Vasut
  2012-01-29 20:02     ` Eric Nelson
  0 siblings, 1 reply; 26+ messages in thread
From: Marek Vasut @ 2012-01-29 19:18 UTC (permalink / raw)
  To: u-boot

> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> Acked-by: Dirk Behme <dirk.behme@de.bosch.com>
> Acked-by: Stefano Babic <sbabic@denx.de>
> ---
>  include/configs/mx6qsabrelite.h |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/include/configs/mx6qsabrelite.h
> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
> --- a/include/configs/mx6qsabrelite.h
> +++ b/include/configs/mx6qsabrelite.h
> @@ -46,9 +46,12 @@
> 
>  #define CONFIG_CMD_SF
>  #ifdef CONFIG_CMD_SF
> +#define GPIO_3_19 ((2*32)+19)

I'd expect this to be in platform headers?

>  #define CONFIG_SPI_FLASH
>  #define CONFIG_SPI_FLASH_SST
>  #define CONFIG_MXC_SPI
> +#define CONFIG_SF_DEFAULT_BUS  0
> +#define CONFIG_SF_DEFAULT_CS   (0|(GPIO_3_19<<8))
>  #define CONFIG_SF_DEFAULT_SPEED 25000000
>  #define CONFIG_SF_DEFAULT_MODE (SPI_MODE_0)
>  #endif

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 5/7] README: Add description of SPI Flash (SF) command configuration
  2012-01-29 19:17   ` Marek Vasut
@ 2012-01-29 19:57     ` Eric Nelson
  2012-01-30  5:11     ` Mike Frysinger
  1 sibling, 0 replies; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 19:57 UTC (permalink / raw)
  To: u-boot

On 01/29/2012 12:17 PM, Marek Vasut wrote:
>> ---
>>   README |   20 ++++++++++++++++++++
>>   1 files changed, 20 insertions(+), 0 deletions(-)
>>
>> diff --git a/README b/README
>> index f6ab85c..1a98915 100644
>> --- a/README
>> +++ b/README
>> @@ -800,6 +800,7 @@ The following options need to be configured:
>>   					  (requires CONFIG_CMD_I2C)
>>   		CONFIG_CMD_SETGETDCR	  Support for DCR Register access
>>   					  (4xx only)
>> +		CONFIG_CMD_SF		* Read/write/erase SPI NOR flash
>
> Why did you add the asterisk here ?
>
Because CONFIG_CMD_SF isn't included in the default build.

	http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=README;h=9d713e8f4398b9af70844ab7910c67e400817bf6;hb=HEAD#l740

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 1/7] mxc_spi: move machine specifics into CPU headers
  2012-01-29 19:16   ` Marek Vasut
@ 2012-01-29 20:01     ` Eric Nelson
  0 siblings, 0 replies; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 20:01 UTC (permalink / raw)
  To: u-boot

On 01/29/2012 12:16 PM, Marek Vasut wrote:
>> 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>
>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
>> Acked-by: Stefano Babic<sbabic@denx.de>
>> ---
>>   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
>
> Good, but what about the boards that use these? Won't they be broken?
>
No. If you look above, you'll see that MXC_ECSPI is unconditionally
defined in arch/arm/include/asm/arch-mx5/imx-regs.h.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-29 19:18   ` Marek Vasut
@ 2012-01-29 20:02     ` Eric Nelson
  2012-01-29 20:11       ` Marek Vasut
  0 siblings, 1 reply; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 20:02 UTC (permalink / raw)
  To: u-boot

On 01/29/2012 12:18 PM, Marek Vasut wrote:
>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
>> Acked-by: Stefano Babic<sbabic@denx.de>
>> ---
>>   include/configs/mx6qsabrelite.h |    3 +++
>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/configs/mx6qsabrelite.h
>> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
>> --- a/include/configs/mx6qsabrelite.h
>> +++ b/include/configs/mx6qsabrelite.h
>> @@ -46,9 +46,12 @@
>>
>>   #define CONFIG_CMD_SF
>>   #ifdef CONFIG_CMD_SF
>> +#define GPIO_3_19 ((2*32)+19)
>
> I'd expect this to be in platform headers?
>

This is a choice made in the SabreLite design. I don't think
the same choice has been made for other i.MX6 boards.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-29 20:02     ` Eric Nelson
@ 2012-01-29 20:11       ` Marek Vasut
  2012-01-29 21:35         ` Eric Nelson
  0 siblings, 1 reply; 26+ messages in thread
From: Marek Vasut @ 2012-01-29 20:11 UTC (permalink / raw)
  To: u-boot

> On 01/29/2012 12:18 PM, Marek Vasut wrote:
> >> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> >> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
> >> Acked-by: Stefano Babic<sbabic@denx.de>
> >> ---
> >> 
> >>   include/configs/mx6qsabrelite.h |    3 +++
> >>   1 files changed, 3 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/include/configs/mx6qsabrelite.h
> >> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
> >> --- a/include/configs/mx6qsabrelite.h
> >> +++ b/include/configs/mx6qsabrelite.h
> >> @@ -46,9 +46,12 @@
> >> 
> >>   #define CONFIG_CMD_SF
> >>   #ifdef CONFIG_CMD_SF
> >> 
> >> +#define GPIO_3_19 ((2*32)+19)
> > 
> > I'd expect this to be in platform headers?
> 
> This is a choice made in the SabreLite design. I don't think
> the same choice has been made for other i.MX6 boards.

I mean the definition of the GPIO_3_19 ...

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-29 20:11       ` Marek Vasut
@ 2012-01-29 21:35         ` Eric Nelson
  2012-01-29 22:16           ` Marek Vasut
  0 siblings, 1 reply; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 21:35 UTC (permalink / raw)
  To: u-boot

On 01/29/2012 01:11 PM, Marek Vasut wrote:
>> On 01/29/2012 12:18 PM, Marek Vasut wrote:
>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
>>>> Acked-by: Stefano Babic<sbabic@denx.de>
>>>> ---
>>>>
>>>>    include/configs/mx6qsabrelite.h |    3 +++
>>>>    1 files changed, 3 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/include/configs/mx6qsabrelite.h
>>>> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
>>>> --- a/include/configs/mx6qsabrelite.h
>>>> +++ b/include/configs/mx6qsabrelite.h
>>>> @@ -46,9 +46,12 @@
>>>>
>>>>    #define CONFIG_CMD_SF
>>>>    #ifdef CONFIG_CMD_SF
>>>>
>>>> +#define GPIO_3_19 ((2*32)+19)
>>>
>>> I'd expect this to be in platform headers?
>>
>> This is a choice made in the SabreLite design. I don't think
>> the same choice has been made for other i.MX6 boards.
>
> I mean the definition of the GPIO_3_19 ...
>

I don't think we want to set precedent for defining
constants for the 100s of GPIO numbers.

That said, I could achieve my objective of clarifying
what the number meant (that the constant refers to a GP) by
changing this:

	#define CONFIG_SF_DEFAULT_CS   (0|(GPIO_3_19<<8))

to this

	#define CONFIG_SF_DEFAULT_CS   (0|(IMX_GPIO_NR(3,19)<<8))

There's a bit of an issue with this. The IMX_GPIO_NR() macro
is defined in arch-mx6/gpio.h which is normally included after
mx6qsabrelite.h because the latter defines the machine.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-29 21:35         ` Eric Nelson
@ 2012-01-29 22:16           ` Marek Vasut
  2012-01-29 23:04             ` Eric Nelson
  0 siblings, 1 reply; 26+ messages in thread
From: Marek Vasut @ 2012-01-29 22:16 UTC (permalink / raw)
  To: u-boot

> On 01/29/2012 01:11 PM, Marek Vasut wrote:
> >> On 01/29/2012 12:18 PM, Marek Vasut wrote:
> >>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> >>>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
> >>>> Acked-by: Stefano Babic<sbabic@denx.de>
> >>>> ---
> >>>> 
> >>>>    include/configs/mx6qsabrelite.h |    3 +++
> >>>>    1 files changed, 3 insertions(+), 0 deletions(-)
> >>>> 
> >>>> diff --git a/include/configs/mx6qsabrelite.h
> >>>> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
> >>>> --- a/include/configs/mx6qsabrelite.h
> >>>> +++ b/include/configs/mx6qsabrelite.h
> >>>> @@ -46,9 +46,12 @@
> >>>> 
> >>>>    #define CONFIG_CMD_SF
> >>>>    #ifdef CONFIG_CMD_SF
> >>>> 
> >>>> +#define GPIO_3_19 ((2*32)+19)
> >>> 
> >>> I'd expect this to be in platform headers?
> >> 
> >> This is a choice made in the SabreLite design. I don't think
> >> the same choice has been made for other i.MX6 boards.
> > 
> > I mean the definition of the GPIO_3_19 ...
> 
> I don't think we want to set precedent for defining
> constants for the 100s of GPIO numbers.
> 
> That said, I could achieve my objective of clarifying
> what the number meant (that the constant refers to a GP) by
> changing this:
> 
> 	#define CONFIG_SF_DEFAULT_CS   (0|(GPIO_3_19<<8))
> 
> to this
> 
> 	#define CONFIG_SF_DEFAULT_CS   (0|(IMX_GPIO_NR(3,19)<<8))

Why the (0| part ? Anyway, that indeed looks better, more standard even.

And I think for MX5, there was even stuff defining the GPIO numbers imported 
from Linux.

M

> 
> There's a bit of an issue with this. The IMX_GPIO_NR() macro
> is defined in arch-mx6/gpio.h which is normally included after
> mx6qsabrelite.h because the latter defines the machine.

And the CPP will choke on that ?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-29 22:16           ` Marek Vasut
@ 2012-01-29 23:04             ` Eric Nelson
  2012-01-30  2:36               ` Marek Vasut
  0 siblings, 1 reply; 26+ messages in thread
From: Eric Nelson @ 2012-01-29 23:04 UTC (permalink / raw)
  To: u-boot

On 01/29/2012 03:16 PM, Marek Vasut wrote:
>> On 01/29/2012 01:11 PM, Marek Vasut wrote:
>>>> On 01/29/2012 12:18 PM, Marek Vasut wrote:
>>>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>>>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
>>>>>> Acked-by: Stefano Babic<sbabic@denx.de>
>>>>>> ---
>>>>>>
>>>>>>     include/configs/mx6qsabrelite.h |    3 +++
>>>>>>     1 files changed, 3 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/include/configs/mx6qsabrelite.h
>>>>>> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
>>>>>> --- a/include/configs/mx6qsabrelite.h
>>>>>> +++ b/include/configs/mx6qsabrelite.h
>>>>>> @@ -46,9 +46,12 @@
>>>>>>
>>>>>>     #define CONFIG_CMD_SF
>>>>>>     #ifdef CONFIG_CMD_SF
>>>>>>
>>>>>> +#define GPIO_3_19 ((2*32)+19)
>>>>>
>>>>> I'd expect this to be in platform headers?
>>>>
>>>> This is a choice made in the SabreLite design. I don't think
>>>> the same choice has been made for other i.MX6 boards.
>>>
>>> I mean the definition of the GPIO_3_19 ...
>>
>> I don't think we want to set precedent for defining
>> constants for the 100s of GPIO numbers.
>>
>> That said, I could achieve my objective of clarifying
>> what the number meant (that the constant refers to a GP) by
>> changing this:
>>
>> 	#define CONFIG_SF_DEFAULT_CS   (0|(GPIO_3_19<<8))
>>
>> to this
>>
>> 	#define CONFIG_SF_DEFAULT_CS   (0|(IMX_GPIO_NR(3,19)<<8))
>
> Why the (0| part ? Anyway, that indeed looks better, more standard even.
>
> And I think for MX5, there was even stuff defining the GPIO numbers imported
> from Linux.
>
> M
>
>>
>> There's a bit of an issue with this. The IMX_GPIO_NR() macro
>> is defined in arch-mx6/gpio.h which is normally included after
>> mx6qsabrelite.h because the latter defines the machine.
>
> And the CPP will choke on that ?
>

Assembler or linker. IMX_GPIO_NR will be undefined and treated as an
external unless we add this nested include into include/configs/mx6qsabrelite.h:

#ifndef __ASSEMBLY__
#include <asm/arch/gpio.h>
#endif

arch-mx6/gpio.h isn't directly ASM-friendly.

This seems like a lot of #include-foo for giving a name to a constant, don't
you think?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-29 23:04             ` Eric Nelson
@ 2012-01-30  2:36               ` Marek Vasut
  2012-01-30 18:10                 ` Eric Nelson
  0 siblings, 1 reply; 26+ messages in thread
From: Marek Vasut @ 2012-01-30  2:36 UTC (permalink / raw)
  To: u-boot

> On 01/29/2012 03:16 PM, Marek Vasut wrote:
> >> On 01/29/2012 01:11 PM, Marek Vasut wrote:
> >>>> On 01/29/2012 12:18 PM, Marek Vasut wrote:
> >>>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> >>>>>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
> >>>>>> Acked-by: Stefano Babic<sbabic@denx.de>
> >>>>>> ---
> >>>>>> 
> >>>>>>     include/configs/mx6qsabrelite.h |    3 +++
> >>>>>>     1 files changed, 3 insertions(+), 0 deletions(-)
> >>>>>> 
> >>>>>> diff --git a/include/configs/mx6qsabrelite.h
> >>>>>> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
> >>>>>> --- a/include/configs/mx6qsabrelite.h
> >>>>>> +++ b/include/configs/mx6qsabrelite.h
> >>>>>> @@ -46,9 +46,12 @@
> >>>>>> 
> >>>>>>     #define CONFIG_CMD_SF
> >>>>>>     #ifdef CONFIG_CMD_SF
> >>>>>> 
> >>>>>> +#define GPIO_3_19 ((2*32)+19)
> >>>>> 
> >>>>> I'd expect this to be in platform headers?
> >>>> 
> >>>> This is a choice made in the SabreLite design. I don't think
> >>>> the same choice has been made for other i.MX6 boards.
> >>> 
> >>> I mean the definition of the GPIO_3_19 ...
> >> 
> >> I don't think we want to set precedent for defining
> >> constants for the 100s of GPIO numbers.
> >> 
> >> That said, I could achieve my objective of clarifying
> >> what the number meant (that the constant refers to a GP) by
> >> 
> >> changing this:
> >> 	#define CONFIG_SF_DEFAULT_CS   (0|(GPIO_3_19<<8))
> >> 
> >> to this
> >> 
> >> 	#define CONFIG_SF_DEFAULT_CS   (0|(IMX_GPIO_NR(3,19)<<8))
> > 
> > Why the (0| part ? Anyway, that indeed looks better, more standard even.
> > 
> > And I think for MX5, there was even stuff defining the GPIO numbers
> > imported from Linux.
> > 
> > M
> > 
> >> There's a bit of an issue with this. The IMX_GPIO_NR() macro
> >> is defined in arch-mx6/gpio.h which is normally included after
> >> mx6qsabrelite.h because the latter defines the machine.
> > 
> > And the CPP will choke on that ?
> 
> Assembler or linker. IMX_GPIO_NR will be undefined and treated as an
> external unless we add this nested include into
> include/configs/mx6qsabrelite.h:
> 
> #ifndef __ASSEMBLY__
> #include <asm/arch/gpio.h>
> #endif
> 
> arch-mx6/gpio.h isn't directly ASM-friendly.
> 
> This seems like a lot of #include-foo for giving a name to a constant,
> don't you think?

Better than redefining stuff, which will eventually lead to errors and breakage.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 5/7] README: Add description of SPI Flash (SF) command configuration
  2012-01-29 19:17   ` Marek Vasut
  2012-01-29 19:57     ` Eric Nelson
@ 2012-01-30  5:11     ` Mike Frysinger
  1 sibling, 0 replies; 26+ messages in thread
From: Mike Frysinger @ 2012-01-30  5:11 UTC (permalink / raw)
  To: u-boot

On Sunday 29 January 2012 14:17:50 Marek Vasut wrote:
> > --- a/README
> > +++ b/README
> > 
> > +		CONFIG_CMD_SF		* Read/write/erase SPI NOR flash
> 
> Why did you add the asterisk here ?

from the README:
        The default command configuration includes all commands
        except those marked below with a "*".
-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/20120130/f87e0ecc/attachment.pgp>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 5/7] README: Add description of SPI Flash (SF) command configuration
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 5/7] README: Add description of SPI Flash (SF) command configuration Eric Nelson
  2012-01-29 19:17   ` Marek Vasut
@ 2012-01-30  5:12   ` Mike Frysinger
  1 sibling, 0 replies; 26+ messages in thread
From: Mike Frysinger @ 2012-01-30  5:12 UTC (permalink / raw)
  To: u-boot

this patch is missing your s-o-b tag ...

otherwise, looks good.  thanks!
-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/20120130/e748c199/attachment.pgp>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 4/7] sf command: allow default bus and chip selects
  2012-01-29 18:59 ` [U-Boot] [PATCH V4 4/7] sf command: allow default bus and chip selects Eric Nelson
@ 2012-01-30  5:13   ` Mike Frysinger
  0 siblings, 0 replies; 26+ messages in thread
From: Mike Frysinger @ 2012-01-30  5:13 UTC (permalink / raw)
  To: u-boot

On Sunday 29 January 2012 13:59:51 Eric Nelson wrote:
> This patch allows a board configuration file to provide default bus
> and chip-selects for SPI flash so that first argument to the 'sf' command
> is optional.
> 
> On boards that use the mxc_spi driver and a GPIO for chip select, this
> allows a much simpler command line:
> 	U-Boot> sf probe
> instead of
> 	U-Boot> sf probe 0x5300

missing your s-o-b tag ...

otherwise, looks good to me ...
-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/20120130/a8309145/attachment.pgp>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-30  2:36               ` Marek Vasut
@ 2012-01-30 18:10                 ` Eric Nelson
  2012-01-30 18:28                   ` Marek Vasut
  2012-01-30 18:35                   ` Dirk Behme
  0 siblings, 2 replies; 26+ messages in thread
From: Eric Nelson @ 2012-01-30 18:10 UTC (permalink / raw)
  To: u-boot

On 01/29/2012 07:36 PM, Marek Vasut wrote:
>> On 01/29/2012 03:16 PM, Marek Vasut wrote:
>>>> On 01/29/2012 01:11 PM, Marek Vasut wrote:
>>>>>> On 01/29/2012 12:18 PM, Marek Vasut wrote:
>>>>>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>>>>>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
>>>>>>>> Acked-by: Stefano Babic<sbabic@denx.de>
>>>>>>>> ---
>>>>>>>>
>>>>>>>>      include/configs/mx6qsabrelite.h |    3 +++
>>>>>>>>      1 files changed, 3 insertions(+), 0 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/include/configs/mx6qsabrelite.h
>>>>>>>> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
>>>>>>>> --- a/include/configs/mx6qsabrelite.h
>>>>>>>> +++ b/include/configs/mx6qsabrelite.h
>>>>>>>> @@ -46,9 +46,12 @@
>>>>>>>>
>>>>>>>>      #define CONFIG_CMD_SF
>>>>>>>>      #ifdef CONFIG_CMD_SF
>>>>>>>>
>>>>>>>> +#define GPIO_3_19 ((2*32)+19)
>>>>>>>
>>>>>>> I'd expect this to be in platform headers?
>>>>>>
>>>>>> This is a choice made in the SabreLite design. I don't think
>>>>>> the same choice has been made for other i.MX6 boards.
>>>>>
>>>>> I mean the definition of the GPIO_3_19 ...
>>>>
>>>> I don't think we want to set precedent for defining
>>>> constants for the 100s of GPIO numbers.
>>>>
>>>> That said, I could achieve my objective of clarifying
>>>> what the number meant (that the constant refers to a GP) by
>>>>
>>>> changing this:
>>>> 	#define CONFIG_SF_DEFAULT_CS   (0|(GPIO_3_19<<8))
>>>>
>>>> to this
>>>>
>>>> 	#define CONFIG_SF_DEFAULT_CS   (0|(IMX_GPIO_NR(3,19)<<8))
>>>
>>> Why the (0| part ? Anyway, that indeed looks better, more standard even.
>>>
>>> And I think for MX5, there was even stuff defining the GPIO numbers
>>> imported from Linux.
>>>
>>> M
>>>
>>>> There's a bit of an issue with this. The IMX_GPIO_NR() macro
>>>> is defined in arch-mx6/gpio.h which is normally included after
>>>> mx6qsabrelite.h because the latter defines the machine.
>>>
>>> And the CPP will choke on that ?
>>
>> Assembler or linker. IMX_GPIO_NR will be undefined and treated as an
>> external unless we add this nested include into
>> include/configs/mx6qsabrelite.h:
>>
>> #ifndef __ASSEMBLY__
>> #include<asm/arch/gpio.h>
>> #endif
>>
>> arch-mx6/gpio.h isn't directly ASM-friendly.
>>
>> This seems like a lot of #include-foo for giving a name to a constant,
>> don't you think?
>
> Better than redefining stuff, which will eventually lead to errors and breakage.
>

Hmmm. I just noticed that the IMX_GPIO_NR() macro isn't in Stefano's tree.
My previous patches were against Dirk's	tree on GitHub, which has a patch
from Troy:
	https://github.com/dirkbehme/u-boot-imx6/commit/c8b2870efd041f820a91eebcb888c84a4f79f1f6

If we move this macro into arch-mx6/imx-regs.h, we avoid the #if.

Looking at the remaining content of gpio.h, it seems that it's driver-specific
anyway (only the driver should be worried about the register layout of the GPIO
controller).

If there are no objections, I'll forward a separate patch to define the macro.

Should this be based on Stefano's tree?

Please advise,


Eric

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-30 18:10                 ` Eric Nelson
@ 2012-01-30 18:28                   ` Marek Vasut
  2012-01-30 18:35                   ` Dirk Behme
  1 sibling, 0 replies; 26+ messages in thread
From: Marek Vasut @ 2012-01-30 18:28 UTC (permalink / raw)
  To: u-boot

> On 01/29/2012 07:36 PM, Marek Vasut wrote:
> >> On 01/29/2012 03:16 PM, Marek Vasut wrote:
> >>>> On 01/29/2012 01:11 PM, Marek Vasut wrote:
> >>>>>> On 01/29/2012 12:18 PM, Marek Vasut wrote:
> >>>>>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> >>>>>>>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
> >>>>>>>> Acked-by: Stefano Babic<sbabic@denx.de>
> >>>>>>>> ---
> >>>>>>>> 
> >>>>>>>>      include/configs/mx6qsabrelite.h |    3 +++
> >>>>>>>>      1 files changed, 3 insertions(+), 0 deletions(-)
> >>>>>>>> 
> >>>>>>>> diff --git a/include/configs/mx6qsabrelite.h
> >>>>>>>> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
> >>>>>>>> --- a/include/configs/mx6qsabrelite.h
> >>>>>>>> +++ b/include/configs/mx6qsabrelite.h
> >>>>>>>> @@ -46,9 +46,12 @@
> >>>>>>>> 
> >>>>>>>>      #define CONFIG_CMD_SF
> >>>>>>>>      #ifdef CONFIG_CMD_SF
> >>>>>>>> 
> >>>>>>>> +#define GPIO_3_19 ((2*32)+19)
> >>>>>>> 
> >>>>>>> I'd expect this to be in platform headers?
> >>>>>> 
> >>>>>> This is a choice made in the SabreLite design. I don't think
> >>>>>> the same choice has been made for other i.MX6 boards.
> >>>>> 
> >>>>> I mean the definition of the GPIO_3_19 ...
> >>>> 
> >>>> I don't think we want to set precedent for defining
> >>>> constants for the 100s of GPIO numbers.
> >>>> 
> >>>> That said, I could achieve my objective of clarifying
> >>>> what the number meant (that the constant refers to a GP) by
> >>>> 
> >>>> changing this:
> >>>> 	#define CONFIG_SF_DEFAULT_CS   (0|(GPIO_3_19<<8))
> >>>> 
> >>>> to this
> >>>> 
> >>>> 	#define CONFIG_SF_DEFAULT_CS   (0|(IMX_GPIO_NR(3,19)<<8))
> >>> 
> >>> Why the (0| part ? Anyway, that indeed looks better, more standard
> >>> even.
> >>> 
> >>> And I think for MX5, there was even stuff defining the GPIO numbers
> >>> imported from Linux.
> >>> 
> >>> M
> >>> 
> >>>> There's a bit of an issue with this. The IMX_GPIO_NR() macro
> >>>> is defined in arch-mx6/gpio.h which is normally included after
> >>>> mx6qsabrelite.h because the latter defines the machine.
> >>> 
> >>> And the CPP will choke on that ?
> >> 
> >> Assembler or linker. IMX_GPIO_NR will be undefined and treated as an
> >> external unless we add this nested include into
> >> include/configs/mx6qsabrelite.h:
> >> 
> >> #ifndef __ASSEMBLY__
> >> #include<asm/arch/gpio.h>
> >> #endif
> >> 
> >> arch-mx6/gpio.h isn't directly ASM-friendly.
> >> 
> >> This seems like a lot of #include-foo for giving a name to a constant,
> >> don't you think?
> > 
> > Better than redefining stuff, which will eventually lead to errors and
> > breakage.
> 
> Hmmm. I just noticed that the IMX_GPIO_NR() macro isn't in Stefano's tree.
> My previous patches were against Dirk's	tree on GitHub, which has a 
patch
> from Troy:
> 	https://github.com/dirkbehme/u-boot-
imx6/commit/c8b2870efd041f820a91eebcb8
> 88c84a4f79f1f6
> 
> If we move this macro into arch-mx6/imx-regs.h, we avoid the #if.
> 
> Looking at the remaining content of gpio.h, it seems that it's
> driver-specific anyway (only the driver should be worried about the
> register layout of the GPIO controller).
> 
> If there are no objections, I'll forward a separate patch to define the
> macro.
> 
> Should this be based on Stefano's tree?

Definitelly. Please rebase to u-boot-imx.

Thanks for the patches and good work so far!

M
> 
> Please advise,
> 
> 
> Eric

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-30 18:10                 ` Eric Nelson
  2012-01-30 18:28                   ` Marek Vasut
@ 2012-01-30 18:35                   ` Dirk Behme
  2012-01-30 19:36                     ` Eric Nelson
  1 sibling, 1 reply; 26+ messages in thread
From: Dirk Behme @ 2012-01-30 18:35 UTC (permalink / raw)
  To: u-boot

On 30.01.2012 19:10, Eric Nelson wrote:
> On 01/29/2012 07:36 PM, Marek Vasut wrote:
>>> On 01/29/2012 03:16 PM, Marek Vasut wrote:
>>>>> On 01/29/2012 01:11 PM, Marek Vasut wrote:
>>>>>>> On 01/29/2012 12:18 PM, Marek Vasut wrote:
>>>>>>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>>>>>>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
>>>>>>>>> Acked-by: Stefano Babic<sbabic@denx.de>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> include/configs/mx6qsabrelite.h | 3 +++
>>>>>>>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/include/configs/mx6qsabrelite.h
>>>>>>>>> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
>>>>>>>>> --- a/include/configs/mx6qsabrelite.h
>>>>>>>>> +++ b/include/configs/mx6qsabrelite.h
>>>>>>>>> @@ -46,9 +46,12 @@
>>>>>>>>>
>>>>>>>>> #define CONFIG_CMD_SF
>>>>>>>>> #ifdef CONFIG_CMD_SF
>>>>>>>>>
>>>>>>>>> +#define GPIO_3_19 ((2*32)+19)
>>>>>>>>
>>>>>>>> I'd expect this to be in platform headers?
>>>>>>>
>>>>>>> This is a choice made in the SabreLite design. I don't think
>>>>>>> the same choice has been made for other i.MX6 boards.
>>>>>>
>>>>>> I mean the definition of the GPIO_3_19 ...
>>>>>
>>>>> I don't think we want to set precedent for defining
>>>>> constants for the 100s of GPIO numbers.
>>>>>
>>>>> That said, I could achieve my objective of clarifying
>>>>> what the number meant (that the constant refers to a GP) by
>>>>>
>>>>> changing this:
>>>>> #define CONFIG_SF_DEFAULT_CS (0|(GPIO_3_19<<8))
>>>>>
>>>>> to this
>>>>>
>>>>> #define CONFIG_SF_DEFAULT_CS (0|(IMX_GPIO_NR(3,19)<<8))
>>>>
>>>> Why the (0| part ? Anyway, that indeed looks better, more standard
>>>> even.
>>>>
>>>> And I think for MX5, there was even stuff defining the GPIO numbers
>>>> imported from Linux.
>>>>
>>>> M
>>>>
>>>>> There's a bit of an issue with this. The IMX_GPIO_NR() macro
>>>>> is defined in arch-mx6/gpio.h which is normally included after
>>>>> mx6qsabrelite.h because the latter defines the machine.
>>>>
>>>> And the CPP will choke on that ?
>>>
>>> Assembler or linker. IMX_GPIO_NR will be undefined and treated as an
>>> external unless we add this nested include into
>>> include/configs/mx6qsabrelite.h:
>>>
>>> #ifndef __ASSEMBLY__
>>> #include<asm/arch/gpio.h>
>>> #endif
>>>
>>> arch-mx6/gpio.h isn't directly ASM-friendly.
>>>
>>> This seems like a lot of #include-foo for giving a name to a constant,
>>> don't you think?
>>
>> Better than redefining stuff, which will eventually lead to errors
>> and breakage.
>>
>
> Hmmm. I just noticed that the IMX_GPIO_NR() macro isn't in Stefano's
> tree.
> My previous patches were against Dirk's tree on GitHub, which has a patch
> from Troy:
> https://github.com/dirkbehme/u-boot-imx6/commit/c8b2870efd041f820a91eebcb888c84a4f79f1f6
>
>
> If we move this macro into arch-mx6/imx-regs.h, we avoid the #if.
>
> Looking at the remaining content of gpio.h, it seems that it's
> driver-specific
> anyway (only the driver should be worried about the register layout of
> the GPIO
> controller).
>
> If there are no objections, I'll forward a separate patch to define
> the macro.

Yes, please. I have this on my todo list, too. But I haven't found the 
time to look at the consequences trying to mainline this patch. I.e. 
if we try to mainline this, we have to touch all gpio_xxx() functions 
to use this new macro? At least for i.MX6? Or does this macro apply 
for more i.MX SoCs? If yes, do we have to find out for which it will 
work? And move it to a i.MX common gpio header? And then touch all 
i.MX code it applies to?

Anyway, many thanks for your good work!

Best regards

Dirk

Btw.: It looks to me that the patch series to introduce the i.MX6 SPI 
driver increases from each version of the patch series to the next 
one. I'm not sure if this is ok? Or if we should try to split it to 
smaller chunks which would be easier to get them merged?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select
  2012-01-30 18:35                   ` Dirk Behme
@ 2012-01-30 19:36                     ` Eric Nelson
  0 siblings, 0 replies; 26+ messages in thread
From: Eric Nelson @ 2012-01-30 19:36 UTC (permalink / raw)
  To: u-boot

On 01/30/2012 11:35 AM, Dirk Behme wrote:
> On 30.01.2012 19:10, Eric Nelson wrote:
>> On 01/29/2012 07:36 PM, Marek Vasut wrote:
>>>> On 01/29/2012 03:16 PM, Marek Vasut wrote:
>>>>>> On 01/29/2012 01:11 PM, Marek Vasut wrote:
>>>>>>>> On 01/29/2012 12:18 PM, Marek Vasut wrote:
>>>>>>>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>>>>>>>> Acked-by: Dirk Behme<dirk.behme@de.bosch.com>
>>>>>>>>>> Acked-by: Stefano Babic<sbabic@denx.de>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> include/configs/mx6qsabrelite.h | 3 +++
>>>>>>>>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/include/configs/mx6qsabrelite.h
>>>>>>>>>> b/include/configs/mx6qsabrelite.h index 8dd6e39..cc770e2 100644
>>>>>>>>>> --- a/include/configs/mx6qsabrelite.h
>>>>>>>>>> +++ b/include/configs/mx6qsabrelite.h
>>>>>>>>>> @@ -46,9 +46,12 @@
>>>>>>>>>>
>>>>>>>>>> #define CONFIG_CMD_SF
>>>>>>>>>> #ifdef CONFIG_CMD_SF
>>>>>>>>>>
>>>>>>>>>> +#define GPIO_3_19 ((2*32)+19)
>>>>>>>>>
>>>>>>>>> I'd expect this to be in platform headers?
>>>>>>>>
>>>>>>>> This is a choice made in the SabreLite design. I don't think
>>>>>>>> the same choice has been made for other i.MX6 boards.
>>>>>>>
>>>>>>> I mean the definition of the GPIO_3_19 ...
>>>>>>
>>>>>> I don't think we want to set precedent for defining
>>>>>> constants for the 100s of GPIO numbers.
>>>>>>
>>>>>> That said, I could achieve my objective of clarifying
>>>>>> what the number meant (that the constant refers to a GP) by
>>>>>>
>>>>>> changing this:
>>>>>> #define CONFIG_SF_DEFAULT_CS (0|(GPIO_3_19<<8))
>>>>>>
>>>>>> to this
>>>>>>
>>>>>> #define CONFIG_SF_DEFAULT_CS (0|(IMX_GPIO_NR(3,19)<<8))
>>>>>
>>>>> Why the (0| part ? Anyway, that indeed looks better, more standard
>>>>> even.
>>>>>
>>>>> And I think for MX5, there was even stuff defining the GPIO numbers
>>>>> imported from Linux.
>>>>>
>>>>> M
>>>>>
>>>>>> There's a bit of an issue with this. The IMX_GPIO_NR() macro
>>>>>> is defined in arch-mx6/gpio.h which is normally included after
>>>>>> mx6qsabrelite.h because the latter defines the machine.
>>>>>
>>>>> And the CPP will choke on that ?
>>>>
>>>> Assembler or linker. IMX_GPIO_NR will be undefined and treated as an
>>>> external unless we add this nested include into
>>>> include/configs/mx6qsabrelite.h:
>>>>
>>>> #ifndef __ASSEMBLY__
>>>> #include<asm/arch/gpio.h>
>>>> #endif
>>>>
>>>> arch-mx6/gpio.h isn't directly ASM-friendly.
>>>>
>>>> This seems like a lot of #include-foo for giving a name to a constant,
>>>> don't you think?
>>>
>>> Better than redefining stuff, which will eventually lead to errors
>>> and breakage.
>>>
>>
>> Hmmm. I just noticed that the IMX_GPIO_NR() macro isn't in Stefano's
>> tree.
>> My previous patches were against Dirk's tree on GitHub, which has a patch
>> from Troy:
>> https://github.com/dirkbehme/u-boot-imx6/commit/c8b2870efd041f820a91eebcb888c84a4f79f1f6
>>
>> If we move this macro into arch-mx6/imx-regs.h, we avoid the #if.
>>
>> Looking at the remaining content of gpio.h, it seems that it's
>> driver-specific
>> anyway (only the driver should be worried about the register layout of
>> the GPIO
>> controller).
>>
>> If there are no objections, I'll forward a separate patch to define
>> the macro.
>
> Yes, please. I have this on my todo list, too. But I haven't found the time to
> look at the consequences trying to mainline this patch. I.e. if we try to
> mainline this, we have to touch all gpio_xxx() functions to use this new macro?
> At least for i.MX6? Or does this macro apply for more i.MX SoCs? If yes, do we
> have to find out for which it will work? And move it to a i.MX common gpio
> header? And then touch all i.MX code it applies to?
>

At the moment, the only code which uses IMX_GPIO_NR() is specific to
MX6Q and Sabre-Lite.

I looked for some commonality, but didn't find any in the i.MX trees.

arch-mx35/mx35-pins.h defines GPIO_TO_PORT() and GPIO_TO_INDEX()
which are the opposite of IMX_GPIO_NR().

arch-mx5/mx5x_pins.h does the same.

arch-davinci and arch-tegra2 both define GPIO_BANK() and GPIO_PORT()
for the same purpose

mx28 defines PAD_BANK() and PAD_PIN() but use an input of iomux_cfg_t
and not an integer.

The Linux model allows the platform to define the mapping from GPIO
numbers <-> drivers but doesn't use the concept of banks except within
a driver.

IOW, it doesn't seem like there's an obvious right thing to do, so
I'd like to suggest that either:

	- We define GPIO_NUMBER(bank,offset) inside imx-regs.h for
	use in mapping to GPIO numbers

	- We follow the convention of arch-mx5/ and arch-mx35 and
	define macros GPIO_TO_PORT() and GPIO_TO_INDEX() to go the
	other direction

	- We update drivers/gpio/mxc_gpio.c to use the GPIO_TO_PORT()
	and GPIO_TO_INDEX() macros instead of code like this:

		unsigned int port = gpio >> 5;

Or we just use the constant 0x53 for this value (as is done for the
efikamx and vision2 boards).

> Anyway, many thanks for your good work!
>
> Best regards
>
> Dirk
>
> Btw.: It looks to me that the patch series to introduce the i.MX6 SPI driver
> increases from each version of the patch series to the next one. I'm not sure if
> this is ok? Or if we should try to split it to smaller chunks which would be
> easier to get them merged?
>

Point taken. I've been wondering whether there was a way to steer clear
of the rabbit hole...

At the moment, I think patches 1-3 have been acked and really comprise
the 'refactoring' part.

I think I've addressed all of the concerns about patch 4 and got an ack about
patch 5 from Mike (still need Marek and Matthias to give feedback). Since these
patches define a new feature (default bus and chip-select), they should be
pulled out of the bundle.

I think we also have sign-off on patch 6, which is pretty minor and specific
to Sabre-Lite.

Patch 7 seems to be the only laggard.

To recap, I'll re-submit in four parts.

Regards,


Eric

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2012-01-30 19:36 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-29 18:59 [U-Boot] [PATCH V4 0/7] mxc_spi refactoring (for mx6q and mx6qsabrelite) Eric Nelson
2012-01-29 18:59 ` [U-Boot] [PATCH V4 1/7] mxc_spi: move machine specifics into CPU headers Eric Nelson
2012-01-29 19:16   ` Marek Vasut
2012-01-29 20:01     ` Eric Nelson
2012-01-29 18:59 ` [U-Boot] [PATCH V4 2/7] mx6q: Add support for ECSPI through mxc_spi driver Eric Nelson
2012-01-29 18:59 ` [U-Boot] [PATCH V4 3/7] mx6q: mx6qsabrelite: Add ECSPI support to the Sabrelite platform Eric Nelson
2012-01-29 18:59 ` [U-Boot] [PATCH V4 4/7] sf command: allow default bus and chip selects Eric Nelson
2012-01-30  5:13   ` Mike Frysinger
2012-01-29 18:59 ` [U-Boot] [PATCH V4 5/7] README: Add description of SPI Flash (SF) command configuration Eric Nelson
2012-01-29 19:17   ` Marek Vasut
2012-01-29 19:57     ` Eric Nelson
2012-01-30  5:11     ` Mike Frysinger
2012-01-30  5:12   ` Mike Frysinger
2012-01-29 18:59 ` [U-Boot] [PATCH V4 6/7] mx6q: mx6qsabrelite: Provide default serial flash bus and chip-select Eric Nelson
2012-01-29 19:18   ` Marek Vasut
2012-01-29 20:02     ` Eric Nelson
2012-01-29 20:11       ` Marek Vasut
2012-01-29 21:35         ` Eric Nelson
2012-01-29 22:16           ` Marek Vasut
2012-01-29 23:04             ` Eric Nelson
2012-01-30  2:36               ` Marek Vasut
2012-01-30 18:10                 ` Eric Nelson
2012-01-30 18:28                   ` Marek Vasut
2012-01-30 18:35                   ` Dirk Behme
2012-01-30 19:36                     ` Eric Nelson
2012-01-29 18:59 ` [U-Boot] [PATCH V4 7/7] mx6q: mx6qsabrelite: Conditionally define macros for environment in serial flash Eric Nelson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox