[U-Boot] [PATCH V3 0/6] mxc_spi refactoring (for mx6q and mx6qsabrelite)

[U-Boot] [PATCH V3 0/6] mxc_spi refactoring (for mx6q and mx6qsabrelite)

[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 V3 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>
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

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

[Eric Nelson]

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

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

[Eric Nelson]

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

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

[Eric Nelson]

This patch allows a board configuration file to provide a default 
chip-select for serial 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

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>
---
 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..4b32171 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 V3 5/6] mx6q: mx6qsabrelite: Provide default chip-select for serial flash

[Eric Nelson]

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 |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index 8dd6e39..e34f108 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 V3 6/6] mx6q: mx6qsabrelite: Conditionally define macros for environment in serial flash

[Eric Nelson]

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.

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
Acked-by: Dirk Behme <dirk.behme@de.bosch.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 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		0x5300
+#define CONFIG_ENV_SPI_MODE		SPI_MODE_0
+#endif
 
 #define CONFIG_OF_LIBFDT
 
-- 
1.7.1

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

[Fabio Estevam]

On 1/24/12, Eric Nelson <eric.nelson@boundarydevices.com> wrote:
> This patch allows a board configuration file to provide a default
> chip-select for serial 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
>
> 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>

Acked-by: Fabio Estevam <fabio.estevam@freescale.com>

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

[Mike Frysinger]

On Tuesday 24 January 2012 11:18:22 Eric Nelson wrote:
> This patch allows a board configuration file to provide a default
> chip-select for serial 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

NAK (to this version of the patch): missing README update, and other issues 
below

> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
>
> +#ifndef CONFIG_SPI_FLASH_CS
> +	if (argc < 2) {
> +		printf("%s: missing arguments\n", __func__);
>  		return -1;

	return cmd_usage(cmdtp);

> -	if (*endp == ':') {
> -		if (endp[1] == 0)
> -			return -1;
> +	}
> +#else
> +	cs = CONFIG_SPI_FLASH_CS ;
> +#endif

you're setting the default CS, not locking it in.  so a better config knob name 
would be something like:
	CONFIG_SF_DEFAULT_CS
this matches the existing CONFIG_SF_XXX defines

also, you have a spurious space before the semicolon there

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

this is ugly.  i'd rather just omit it and not worry about the syntax being 
perfect.
-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/20120124/0eb55ae8/attachment.pgp>

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

[Matthias Fuchs]

Hi Eric,

please see my comments below.

On 24.01.2012 17:18, Eric Nelson wrote:
> This patch allows a board configuration file to provide a default 
> chip-select for serial 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
> 
> 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>
> ---
>  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..4b32171 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__);
I think this format for the error message is a little bit untypical for
u-boot. We do not show up the internal C function name. Better would be
to show the command usage, right?

>  		return -1;
> -	if (*endp == ':') {
> -		if (endp[1] == 0)
> -			return -1;
> +	}
> +#else
> +	cs = CONFIG_SPI_FLASH_CS ;
Other options for the spi flash subsystem are called
CONFIG_SF_DEFAULT_MODE|SPEED. It think it make sense to call
this CONFIG_SF_DEFAULT_CS and CONFIG_SF_DEFAULT_BUS (see below).

> +#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"
Can you also add a config option for the SPI bus number? I think these
two need to handled in the same patch.

So you could add this stuff:

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 9e97c8e..fa4312a 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -63,7 +63,11 @@ static int sf_parse_len_arg(char *arg, ulong *len)

 static int do_spi_flash_probe(int argc, char * const argv[])
 {
+#ifdef CONFIG_SF_DEFAULT_BUS
+       unsigned int bus = CONFIG_SF_DEFAULT_BUS;
+#else
        unsigned int bus = 0;
+#endif
        unsigned int cs;
        unsigned int speed = CONFIG_SF_DEFAULT_SPEED;
        unsigned int mode = CONFIG_SF_DEFAULT_MODE;

Matthias

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

[Mike Frysinger]

On Wednesday 25 January 2012 10:10:18 Matthias Fuchs wrote:
> Can you also add a config option for the SPI bus number? I think these
> two need to handled in the same patch.
> 
> So you could add this stuff:
> 
>  static int do_spi_flash_probe(int argc, char * const argv[])
>  {
> +#ifdef CONFIG_SF_DEFAULT_BUS
> +       unsigned int bus = CONFIG_SF_DEFAULT_BUS;
> +#else
>         unsigned int bus = 0;
> +#endif

move the default logic to the top of the file like the existing 
CONFIG_SF_DEFAULT_SPEED, then the code later on only needs to do:
	unsigned int bus = CONFIG_SF_DEFAULT_BUS;

otherwise, this suggestion sounds good too
-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/20120125/4e46854f/attachment.pgp>

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

[Eric Nelson]

On 01/24/2012 11:08 AM, Mike Frysinger wrote:
> On Tuesday 24 January 2012 11:18:22 Eric Nelson wrote:
>> This patch allows a board configuration file to provide a default
>> chip-select for serial 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
>
> NAK (to this version of the patch): missing README update, and other issues
> below
>

Which README? The only references I find to serial flash support
are in board-specific README files.

~/u-boot$ find . -iname \*readme\* | xargs grep -w sf
./doc/README.p2041rdb:	=> sf erase 0 100000
./doc/README.p2041rdb:	=> sf write 1000000 0 $filesize
./doc/README.p2041rdb:	=> sf erase 110000 10000
./doc/README.p2041rdb:	=> sf write 1000000 110000 $filesize
./doc/README.sh7757lcr:   => sf probe 0
./doc/README.sh7757lcr:   => sf erase 0 80000
./doc/README.sh7757lcr:   => sf write 0x89000000 0 80000

I can start one of those for the SabreLite board, but that's un-related
to this patch.

>> --- a/common/cmd_sf.c
>> +++ b/common/cmd_sf.c
>>
>> +#ifndef CONFIG_SPI_FLASH_CS
>> +	if (argc<  2) {
>> +		printf("%s: missing arguments\n", __func__);
>>   		return -1;
>
> 	return cmd_usage(cmdtp);
>
>> -	if (*endp == ':') {
>> -		if (endp[1] == 0)
>> -			return -1;
>> +	}
>> +#else
>> +	cs = CONFIG_SPI_FLASH_CS ;
>> +#endif
>
> you're setting the default CS, not locking it in.  so a better config knob name
> would be something like:
> 	CONFIG_SF_DEFAULT_CS
> this matches the existing CONFIG_SF_XXX defines
>
> also, you have a spurious space before the semicolon there
>
Thanks Mike,

FWIW, I chose this name on purpose to make life easier on a couple of
other boards immediately (efika and vision2):

~/u-boot$ grep CONFIG_SPI_FLASH_CS include/configs/*
include/configs/efikamx.h:#define CONFIG_SPI_FLASH_CS		(1 | 121 << 8)
include/configs/m28evk.h:#define	CONFIG_SPI_FLASH_CS		2
include/configs/mx6qsabrelite.h.rej: 	#define CONFIG_SPI_FLASH_CS	1
include/configs/vision2.h:#define CONFIG_SPI_FLASH_CS	(1 | (121 << 8))

>>   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"
>
> this is ugly.  i'd rather just omit it and not worry about the syntax being
> perfect.

Works for me.

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

[Eric Nelson]

On 01/25/2012 08:10 AM, Matthias Fuchs wrote:
> Hi Eric,
>
> please see my comments below.
>
> On 24.01.2012 17:18, Eric Nelson wrote:
>> This patch allows a board configuration file to provide a default
>> chip-select for serial 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
>>
>> 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>
>> ---
>>   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..4b32171 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__);
> I think this format for the error message is a little bit untypical for
> u-boot. We do not show up the internal C function name. Better would be
> to show the command usage, right?
>

Looking at this area of the code in more detail, there are quite
a few cases where improper usage silently return -1.

I'm inclined to either follow that lead or toss them together
with a "goto usage" as done in do_spi_flash().

Any preferences?

>>   		return -1;
>> -	if (*endp == ':') {
>> -		if (endp[1] == 0)
>> -			return -1;
>> +	}
>> +#else
>> +	cs = CONFIG_SPI_FLASH_CS ;
> Other options for the spi flash subsystem are called
> CONFIG_SF_DEFAULT_MODE|SPEED. It think it make sense to call
> this CONFIG_SF_DEFAULT_CS and CONFIG_SF_DEFAULT_BUS (see below).
>

See include/configs/efikamx.h (reference to unused symbol 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"
> Can you also add a config option for the SPI bus number? I think these
> two need to handled in the same patch.
>
> So you could add this stuff:
>
> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
> index 9e97c8e..fa4312a 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -63,7 +63,11 @@ static int sf_parse_len_arg(char *arg, ulong *len)
>
>   static int do_spi_flash_probe(int argc, char * const argv[])
>   {
> +#ifdef CONFIG_SF_DEFAULT_BUS
> +       unsigned int bus = CONFIG_SF_DEFAULT_BUS;
> +#else
>          unsigned int bus = 0;
> +#endif
>          unsigned int cs;
>          unsigned int speed = CONFIG_SF_DEFAULT_SPEED;
>          unsigned int mode = CONFIG_SF_DEFAULT_MODE;
>

Can do.

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

[Mike Frysinger]

On Thursday 26 January 2012 20:22:22 Eric Nelson wrote:
> On 01/24/2012 11:08 AM, Mike Frysinger wrote:
> > On Tuesday 24 January 2012 11:18:22 Eric Nelson wrote:
> >> This patch allows a board configuration file to provide a default
> >> chip-select for serial 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
> > 
> > NAK (to this version of the patch): missing README update, and other
> > issues below
> 
> Which README? The only references I find to serial flash support
> are in board-specific README files.

all new CONFIG_xxx defines should be documented in the top level README.  
granted, the existin SF ones have slipped in without being documented, but 
that's bad for them ;).

> >> --- a/common/cmd_sf.c
> >> +++ b/common/cmd_sf.c
> >> 
> >> -	if (*endp == ':') {
> >> -		if (endp[1] == 0)
> >> -			return -1;
> >> +	}
> >> +#else
> >> +	cs = CONFIG_SPI_FLASH_CS ;
> >> +#endif
> > 
> > you're setting the default CS, not locking it in.  so a better config
> > knob name
> > 
> > would be something like:
> > 	CONFIG_SF_DEFAULT_CS
> > 
> > this matches the existing CONFIG_SF_XXX defines
> > 
> > also, you have a spurious space before the semicolon there
> 
> Thanks Mike,
> 
> FWIW, I chose this name on purpose to make life easier on a couple of
> other boards immediately (efika and vision2):

those boards are dumb -- that define isn't used anywhere, so that's just dead 
code.  cmd_sf has a standard already, so new defines specific to cmd_sf should 
follow that.
-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/20120126/18da5739/attachment.pgp>

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

[Mike Frysinger]

On Thursday 26 January 2012 20:36:22 Eric Nelson wrote:
> On 01/25/2012 08:10 AM, Matthias Fuchs wrote:
> > On 24.01.2012 17:18, Eric Nelson wrote:
> >> --- a/common/cmd_sf.c
> >> +++ b/common/cmd_sf.c
> >> 
> >>   	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__);
> > 
> > I think this format for the error message is a little bit untypical for
> > u-boot. We do not show up the internal C function name. Better would be
> > to show the command usage, right?
> 
> Looking at this area of the code in more detail, there are quite
> a few cases where improper usage silently return -1.
> 
> I'm inclined to either follow that lead or toss them together
> with a "goto usage" as done in do_spi_flash().
> 
> Any preferences?

invalid syntax should return cmd_usage().  errors should return non-zero, not 
usage information.
-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/20120126/a40c1afd/attachment.pgp>

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

[Eric Nelson]

On 01/26/2012 07:50 PM, Mike Frysinger wrote:
> On Thursday 26 January 2012 20:22:22 Eric Nelson wrote:
>> On 01/24/2012 11:08 AM, Mike Frysinger wrote:
>>> On Tuesday 24 January 2012 11:18:22 Eric Nelson wrote:
>>>> This patch allows a board configuration file to provide a default
>>>> chip-select for serial 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
>>>
>>> NAK (to this version of the patch): missing README update, and other
>>> issues below
>>
>> Which README? The only references I find to serial flash support
>> are in board-specific README files.
>
> all new CONFIG_xxx defines should be documented in the top level README.
> granted, the existin SF ones have slipped in without being documented, but
> that's bad for them ;).
>

Ok. I'll re-send with README updates.

>>>> --- a/common/cmd_sf.c
>>>> +++ b/common/cmd_sf.c
>>>>
>>>> -	if (*endp == ':') {
>>>> -		if (endp[1] == 0)
>>>> -			return -1;
>>>> +	}
>>>> +#else
>>>> +	cs = CONFIG_SPI_FLASH_CS ;
>>>> +#endif
>>>
>>> you're setting the default CS, not locking it in.  so a better config
>>> knob name
>>>
>>> would be something like:
>>> 	CONFIG_SF_DEFAULT_CS
>>>
>>> this matches the existing CONFIG_SF_XXX defines
>>>
>>> also, you have a spurious space before the semicolon there
>>
>> Thanks Mike,
>>
>> FWIW, I chose this name on purpose to make life easier on a couple of
>> other boards immediately (efika and vision2):
>
> those boards are dumb -- that define isn't used anywhere, so that's just dead
> code.  cmd_sf has a standard already, so new defines specific to cmd_sf should
> follow that.
> -mike

Ok. I'll let their maintainers update appropriately.