[U-Boot] mxc_spi refactoring (for mx6q)
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
Move (E)CSPI register declarations into the imx-regs.h files for each supported CPU
Introduce two new macros to control conditional setup MXC_CSPI - Used for processors with the Configurable Serial Peripheral Interface (MX3x) MXC_ECSPI - For processors with Enhanced Configurable... (MX5x, MX6x)
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com --- arch/arm/include/asm/arch-mx31/imx-regs.h | 27 ++++++++ arch/arm/include/asm/arch-mx35/imx-regs.h | 25 ++++++++ arch/arm/include/asm/arch-mx5/imx-regs.h | 30 +++++++++ drivers/spi/mxc_spi.c | 93 ++--------------------------- 4 files changed, 88 insertions(+), 87 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h index 6a517dd..70e3338 100644 --- a/arch/arm/include/asm/arch-mx31/imx-regs.h +++ b/arch/arm/include/asm/arch-mx31/imx-regs.h @@ -890,4 +890,31 @@ struct esdc_regs { #define MXC_EHCI_IPPUE_DOWN (1 << 8) #define MXC_EHCI_IPPUE_UP (1 << 9)
+/* + * CSPI register definitions + */ +#define MXC_CSPI +#define MXC_CSPICTRL_EN (1 << 0) +#define MXC_CSPICTRL_MODE (1 << 1) +#define MXC_CSPICTRL_XCH (1 << 2) +#define MXC_CSPICTRL_SMC (1 << 3) +#define MXC_CSPICTRL_POL (1 << 4) +#define MXC_CSPICTRL_PHA (1 << 5) +#define MXC_CSPICTRL_SSCTL (1 << 6) +#define MXC_CSPICTRL_SSPOL (1 << 7) +#define MXC_CSPICTRL_CHIPSELECT(x) (((x) & 0x3) << 24) +#define MXC_CSPICTRL_BITCOUNT(x) (((x) & 0x1f) << 8) +#define MXC_CSPICTRL_DATARATE(x) (((x) & 0x7) << 16) +#define MXC_CSPICTRL_TC (1 << 8) +#define MXC_CSPICTRL_RXOVF (1 << 6) +#define MXC_CSPICTRL_MAXBITS 0x1f + +#define MXC_CSPIPERIOD_32KHZ (1 << 15) +#define MAX_SPI_BYTES 4 + +#define MXC_SPI_BASE_ADDRESSES \ + 0x43fa4000, \ + 0x50010000, \ + 0x53f84000, + #endif /* __ASM_ARCH_MX31_IMX_REGS_H */ diff --git a/arch/arm/include/asm/arch-mx35/imx-regs.h b/arch/arm/include/asm/arch-mx35/imx-regs.h index df74508..e570ad1 100644 --- a/arch/arm/include/asm/arch-mx35/imx-regs.h +++ b/arch/arm/include/asm/arch-mx35/imx-regs.h @@ -179,6 +179,31 @@ #define IPU_CONF_IC_EN (1<<1) #define IPU_CONF_SCI_EN (1<<0)
+/* + * CSPI register definitions + */ +#define MXC_CSPI +#define MXC_CSPICTRL_EN (1 << 0) +#define MXC_CSPICTRL_MODE (1 << 1) +#define MXC_CSPICTRL_XCH (1 << 2) +#define MXC_CSPICTRL_SMC (1 << 3) +#define MXC_CSPICTRL_POL (1 << 4) +#define MXC_CSPICTRL_PHA (1 << 5) +#define MXC_CSPICTRL_SSCTL (1 << 6) +#define MXC_CSPICTRL_SSPOL (1 << 7) +#define MXC_CSPICTRL_CHIPSELECT(x) (((x) & 0x3) << 12) +#define MXC_CSPICTRL_BITCOUNT(x) (((x) & 0xfff) << 20) +#define MXC_CSPICTRL_DATARATE(x) (((x) & 0x7) << 16) +#define MXC_CSPICTRL_TC (1 << 7) +#define MXC_CSPICTRL_RXOVF (1 << 6) +#define MXC_CSPICTRL_MAXBITS 0xfff +#define MXC_CSPIPERIOD_32KHZ (1 << 15) +#define MAX_SPI_BYTES 4 + +#define MXC_SPI_BASE_ADDRESSES \ + 0x43fa4000, \ + 0x50010000, + #define GPIO_PORT_NUM 3 #define GPIO_NUM_PIN 32
diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h b/arch/arm/include/asm/arch-mx5/imx-regs.h index 0ee88d2..4fa6658 100644 --- a/arch/arm/include/asm/arch-mx5/imx-regs.h +++ b/arch/arm/include/asm/arch-mx5/imx-regs.h @@ -223,6 +223,36 @@ #define CS0_32M_CS1_32M_CS2_32M_CS3_32M 3
/* + * CSPI register definitions + */ +#define MXC_ECSPI +#define MXC_CSPICTRL_EN (1 << 0) +#define MXC_CSPICTRL_MODE (1 << 1) +#define MXC_CSPICTRL_XCH (1 << 2) +#define MXC_CSPICTRL_CHIPSELECT(x) (((x) & 0x3) << 12) +#define MXC_CSPICTRL_BITCOUNT(x) (((x) & 0xfff) << 20) +#define MXC_CSPICTRL_PREDIV(x) (((x) & 0xF) << 12) +#define MXC_CSPICTRL_POSTDIV(x) (((x) & 0xF) << 8) +#define MXC_CSPICTRL_SELCHAN(x) (((x) & 0x3) << 18) +#define MXC_CSPICTRL_MAXBITS 0xfff +#define MXC_CSPICTRL_TC (1 << 7) +#define MXC_CSPICTRL_RXOVF (1 << 6) +#define MXC_CSPIPERIOD_32KHZ (1 << 15) +#define MAX_SPI_BYTES 32 + +/* Bit position inside CTRL register to be associated with SS */ +#define MXC_CSPICTRL_CHAN 18 + +/* Bit position inside CON register to be associated with SS */ +#define MXC_CSPICON_POL 4 +#define MXC_CSPICON_PHA 0 +#define MXC_CSPICON_SSPOL 12 +#define MXC_SPI_BASE_ADDRESSES \ + CSPI1_BASE_ADDR, \ + CSPI2_BASE_ADDR, \ + CSPI3_BASE_ADDR, + +/* * Number of GPIO pins per port */ #define GPIO_NUM_PIN 32 diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 2fa7486..2e15318 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -33,93 +33,12 @@
#error "i.MX27 CSPI not supported due to drastic differences in register definitions" \ "See linux mxc_spi driver from Freescale for details." - -#elif defined(CONFIG_MX31) - -#define MXC_CSPICTRL_EN (1 << 0) -#define MXC_CSPICTRL_MODE (1 << 1) -#define MXC_CSPICTRL_XCH (1 << 2) -#define MXC_CSPICTRL_SMC (1 << 3) -#define MXC_CSPICTRL_POL (1 << 4) -#define MXC_CSPICTRL_PHA (1 << 5) -#define MXC_CSPICTRL_SSCTL (1 << 6) -#define MXC_CSPICTRL_SSPOL (1 << 7) -#define MXC_CSPICTRL_CHIPSELECT(x) (((x) & 0x3) << 24) -#define MXC_CSPICTRL_BITCOUNT(x) (((x) & 0x1f) << 8) -#define MXC_CSPICTRL_DATARATE(x) (((x) & 0x7) << 16) -#define MXC_CSPICTRL_TC (1 << 8) -#define MXC_CSPICTRL_RXOVF (1 << 6) -#define MXC_CSPICTRL_MAXBITS 0x1f - -#define MXC_CSPIPERIOD_32KHZ (1 << 15) -#define MAX_SPI_BYTES 4 - -static unsigned long spi_bases[] = { - 0x43fa4000, - 0x50010000, - 0x53f84000, -}; - -#elif defined(CONFIG_MX51) - -#define MXC_CSPICTRL_EN (1 << 0) -#define MXC_CSPICTRL_MODE (1 << 1) -#define MXC_CSPICTRL_XCH (1 << 2) -#define MXC_CSPICTRL_CHIPSELECT(x) (((x) & 0x3) << 12) -#define MXC_CSPICTRL_BITCOUNT(x) (((x) & 0xfff) << 20) -#define MXC_CSPICTRL_PREDIV(x) (((x) & 0xF) << 12) -#define MXC_CSPICTRL_POSTDIV(x) (((x) & 0xF) << 8) -#define MXC_CSPICTRL_SELCHAN(x) (((x) & 0x3) << 18) -#define MXC_CSPICTRL_MAXBITS 0xfff -#define MXC_CSPICTRL_TC (1 << 7) -#define MXC_CSPICTRL_RXOVF (1 << 6) - -#define MXC_CSPIPERIOD_32KHZ (1 << 15) -#define MAX_SPI_BYTES 32 - -/* Bit position inside CTRL register to be associated with SS */ -#define MXC_CSPICTRL_CHAN 18 - -/* Bit position inside CON register to be associated with SS */ -#define MXC_CSPICON_POL 4 -#define MXC_CSPICON_PHA 0 -#define MXC_CSPICON_SSPOL 12 - -static unsigned long spi_bases[] = { - CSPI1_BASE_ADDR, - CSPI2_BASE_ADDR, - CSPI3_BASE_ADDR, -}; - -#elif defined(CONFIG_MX35) - -#define MXC_CSPICTRL_EN (1 << 0) -#define MXC_CSPICTRL_MODE (1 << 1) -#define MXC_CSPICTRL_XCH (1 << 2) -#define MXC_CSPICTRL_SMC (1 << 3) -#define MXC_CSPICTRL_POL (1 << 4) -#define MXC_CSPICTRL_PHA (1 << 5) -#define MXC_CSPICTRL_SSCTL (1 << 6) -#define MXC_CSPICTRL_SSPOL (1 << 7) -#define MXC_CSPICTRL_CHIPSELECT(x) (((x) & 0x3) << 12) -#define MXC_CSPICTRL_BITCOUNT(x) (((x) & 0xfff) << 20) -#define MXC_CSPICTRL_DATARATE(x) (((x) & 0x7) << 16) -#define MXC_CSPICTRL_TC (1 << 7) -#define MXC_CSPICTRL_RXOVF (1 << 6) -#define MXC_CSPICTRL_MAXBITS 0xfff - -#define MXC_CSPIPERIOD_32KHZ (1 << 15) -#define MAX_SPI_BYTES 4 +#endif
static unsigned long spi_bases[] = { - 0x43fa4000, - 0x50010000, + MXC_SPI_BASE_ADDRESSES };
-#else -#error "Unsupported architecture" -#endif - #define OUT MXC_GPIO_DIRECTION_OUT
#define reg_read readl @@ -129,7 +48,7 @@ struct mxc_spi_slave { struct spi_slave slave; unsigned long base; u32 ctrl_reg; -#if defined(CONFIG_MX51) +#if defined(MXC_ECSPI) u32 cfg_reg; #endif int gpio; @@ -167,7 +86,7 @@ u32 get_cspi_div(u32 div) return i; }
-#if defined(CONFIG_MX31) || defined(CONFIG_MX35) +#ifdef MXC_CSPI static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { @@ -204,7 +123,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, } #endif
-#if defined(CONFIG_MX51) +#ifdef MXC_ECSPI static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { @@ -316,7 +235,7 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen, MXC_CSPICTRL_BITCOUNT(bitlen - 1);
reg_write(®s->ctrl, mxcs->ctrl_reg | MXC_CSPICTRL_EN); -#ifdef CONFIG_MX51 +#ifdef MXC_ECSPI reg_write(®s->cfg, mxcs->cfg_reg); #endif
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com --- arch/arm/include/asm/arch-mx6/imx-regs.h | 44 ++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h index 7650cb9..00040c4 100644 --- a/arch/arm/include/asm/arch-mx6/imx-regs.h +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h @@ -190,6 +190,50 @@ struct src { u32 gpr10; };
+/* ECSPI registers */ +struct cspi_regs { + u32 rxdata; + u32 txdata; + u32 ctrl; + u32 cfg; + u32 intr; + u32 dma; + u32 stat; + u32 period; +}; + +/* + * CSPI register definitions + */ +#define MXC_ECSPI +#define MXC_CSPICTRL_EN (1 << 0) +#define MXC_CSPICTRL_MODE (1 << 1) +#define MXC_CSPICTRL_XCH (1 << 2) +#define MXC_CSPICTRL_CHIPSELECT(x) (((x) & 0x3) << 12) +#define MXC_CSPICTRL_BITCOUNT(x) (((x) & 0xfff) << 20) +#define MXC_CSPICTRL_PREDIV(x) (((x) & 0xF) << 12) +#define MXC_CSPICTRL_POSTDIV(x) (((x) & 0xF) << 8) +#define MXC_CSPICTRL_SELCHAN(x) (((x) & 0x3) << 18) +#define MXC_CSPICTRL_MAXBITS 0xfff +#define MXC_CSPICTRL_TC (1 << 7) +#define MXC_CSPICTRL_RXOVF (1 << 6) +#define MXC_CSPIPERIOD_32KHZ (1 << 15) +#define MAX_SPI_BYTES 32 + +/* Bit position inside CTRL register to be associated with SS */ +#define MXC_CSPICTRL_CHAN 18 + +/* Bit position inside CON register to be associated with SS */ +#define MXC_CSPICON_POL 4 +#define MXC_CSPICON_PHA 0 +#define MXC_CSPICON_SSPOL 12 +#define MXC_SPI_BASE_ADDRESSES \ + ECSPI1_BASE_ADDR, \ + ECSPI2_BASE_ADDR, \ + ECSPI3_BASE_ADDR, \ + ECSPI4_BASE_ADDR, \ + ECSPI5_BASE_ADDR + struct iim_regs { u32 ctrl; u32 ctrl_set;
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com
arch/arm/include/asm/arch-mx6/imx-regs.h | 44 ++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h index 7650cb9..00040c4 100644 --- a/arch/arm/include/asm/arch-mx6/imx-regs.h +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h @@ -190,6 +190,50 @@ struct src { u32 gpr10; };
+/* ECSPI registers */ +struct cspi_regs {
- u32 rxdata;
- u32 txdata;
- u32 ctrl;
- u32 cfg;
- u32 intr;
- u32 dma;
- u32 stat;
- u32 period;
+};
Sigh ... it's no fun I can have only one remark :-)
Is this part common for all imx-es ?
+/*
- CSPI register definitions
- */
+#define MXC_ECSPI +#define MXC_CSPICTRL_EN (1 << 0) +#define MXC_CSPICTRL_MODE (1 << 1) +#define MXC_CSPICTRL_XCH (1 << 2) +#define MXC_CSPICTRL_CHIPSELECT(x) (((x) & 0x3) << 12) +#define MXC_CSPICTRL_BITCOUNT(x) (((x) & 0xfff) << 20) +#define MXC_CSPICTRL_PREDIV(x) (((x) & 0xF) << 12) +#define MXC_CSPICTRL_POSTDIV(x) (((x) & 0xF) << 8) +#define MXC_CSPICTRL_SELCHAN(x) (((x) & 0x3) << 18) +#define MXC_CSPICTRL_MAXBITS 0xfff +#define MXC_CSPICTRL_TC (1 << 7) +#define MXC_CSPICTRL_RXOVF (1 << 6) +#define MXC_CSPIPERIOD_32KHZ (1 << 15) +#define MAX_SPI_BYTES 32
+/* Bit position inside CTRL register to be associated with SS */ +#define MXC_CSPICTRL_CHAN 18
+/* Bit position inside CON register to be associated with SS */ +#define MXC_CSPICON_POL 4 +#define MXC_CSPICON_PHA 0 +#define MXC_CSPICON_SSPOL 12 +#define MXC_SPI_BASE_ADDRESSES \
- ECSPI1_BASE_ADDR, \
- ECSPI2_BASE_ADDR, \
- ECSPI3_BASE_ADDR, \
- ECSPI4_BASE_ADDR, \
- ECSPI5_BASE_ADDR
struct iim_regs { u32 ctrl; u32 ctrl_set;
On 01/17/2012 04:19 PM, Marek Vasut wrote:
Signed-off-by: Eric Nelsoneric.nelson@boundarydevices.com
arch/arm/include/asm/arch-mx6/imx-regs.h | 44 ++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h index 7650cb9..00040c4 100644 --- a/arch/arm/include/asm/arch-mx6/imx-regs.h +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h @@ -190,6 +190,50 @@ struct src { u32 gpr10; };
+/* ECSPI registers */ +struct cspi_regs {
- u32 rxdata;
- u32 txdata;
- u32 ctrl;
- u32 cfg;
- u32 intr;
- u32 dma;
- u32 stat;
- u32 period;
+};
Sigh ... it's no fun I can have only one remark :-)
Is this part common for all imx-es ?
All i.MX6's
This is a cut & paste from MX51.
I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h' for i.MX31 and i.MX35 that share the CSPI peripheral.
On 01/17/2012 04:19 PM, Marek Vasut wrote:
Signed-off-by: Eric Nelsoneric.nelson@boundarydevices.com
arch/arm/include/asm/arch-mx6/imx-regs.h | 44
++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h index 7650cb9..00040c4 100644 --- a/arch/arm/include/asm/arch-mx6/imx-regs.h +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h @@ -190,6 +190,50 @@ struct src {
u32 gpr10;
};
+/* ECSPI registers */ +struct cspi_regs {
- u32 rxdata;
- u32 txdata;
- u32 ctrl;
- u32 cfg;
- u32 intr;
- u32 dma;
- u32 stat;
- u32 period;
+};
Sigh ... it's no fun I can have only one remark :-)
Is this part common for all imx-es ?
All i.MX6's
This is a cut & paste from MX51.
I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h' for i.MX31 and i.MX35 that share the CSPI peripheral.
But you don't even need this outside of the spi driver so just put it into the spi driver and be done with it. That'll solve your duplication issue.
M
On 01/17/2012 06:27 PM, Marek Vasut wrote:
On 01/17/2012 04:19 PM, Marek Vasut wrote:
Signed-off-by: Eric Nelsoneric.nelson@boundarydevices.com +/* ECSPI registers */ +struct cspi_regs {
- u32 rxdata;
- u32 txdata;
- u32 ctrl;
- u32 cfg;
- u32 intr;
- u32 dma;
- u32 stat;
- u32 period;
+};
Sigh ... it's no fun I can have only one remark :-)
Is this part common for all imx-es ?
All i.MX6's
This is a cut& paste from MX51.
I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h' for i.MX31 and i.MX35 that share the CSPI peripheral.
But you don't even need this outside of the spi driver so just put it into the spi driver and be done with it. That'll solve your duplication issue.
M
I'll defer to Stefano on this one, since I did this in response to his request:
Right - and we already discussed in the past how to avoid to put specific SOC code inside the driver. In fact, the cspi_regs structure was already moved into the specific SOC header (imx-regs.h) - but the definitions of the single bits of the registers are still inside the driver, as well as the base address of the (e)cspi controllers.
They should also be moved - take into acoount by implementing your changes for i.mx6
The struct cspi_regs is already present in mx31, mx35, and mx51 headers, so I'm not breaking new ground here, only in the bitfield declarations.
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-mx31/imx... http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-mx35/imx... http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-mx5/imx-...
My interpretation of Stefano's intent is to clean up the driver at the expense of extra defines in the arch-specific headers.
Regards,
Eric
On 01/17/2012 06:27 PM, Marek Vasut wrote:
On 01/17/2012 04:19 PM, Marek Vasut wrote:
Signed-off-by: Eric Nelsoneric.nelson@boundarydevices.com +/* ECSPI registers */ +struct cspi_regs {
- u32 rxdata;
- u32 txdata;
- u32 ctrl;
- u32 cfg;
- u32 intr;
- u32 dma;
- u32 stat;
- u32 period;
+};
Sigh ... it's no fun I can have only one remark :-)
Is this part common for all imx-es ?
All i.MX6's
This is a cut& paste from MX51.
I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h' for i.MX31 and i.MX35 that share the CSPI peripheral.
But you don't even need this outside of the spi driver so just put it into the spi driver and be done with it. That'll solve your duplication issue.
M
I'll defer to Stefano on this one, since I did this in response
to his request:
Right - and we already discussed in the past how to avoid to put specific SOC code inside the driver. In fact, the cspi_regs structure was already moved into the specific SOC header (imx-regs.h) - but the definitions of the single bits of the registers are still inside the driver, as well as the base address of the (e)cspi controllers.
They should also be moved - take into acoount by implementing your changes for i.mx6
The struct cspi_regs is already present in mx31, mx35, and mx51 headers, so I'm not breaking new ground here, only in the bitfield declarations.
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-
mx31/i
mx-regs.h;h=6a517ddd931ca0d1e598bd7456c4c611741602eb;hb=HEAD#l60 http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-mx35/i mx-regs.h;h=df74508a93ee87ae986f7c2f48f6c5fb36626070;hb=HEAD#l279 http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/include/asm/arch-mx5/im x-regs.h;h=0ee88d25b7800ae9e6aed809d02dd19d9cac9c82;hb=HEAD#l423
My interpretation of Stefano's intent is to clean up the driver at the expense of extra defines in the arch-specific headers.
But they're all the same, right? So we have now the same structure defined thrice?
M
Regards,
Eric
On 01/17/2012 06:47 PM, Marek Vasut wrote:
On 01/17/2012 06:27 PM, Marek Vasut wrote:
I'll defer to Stefano on this one, since I did this in response to his request:
Right - and we already discussed in the past how to avoid to put specific SOC code inside the driver. In fact, the cspi_regs structure was already moved into the specific SOC header (imx-regs.h) - but the definitions of the single bits of the registers are still inside the driver, as well as the base address of the (e)cspi controllers.
They should also be moved - take into acoount by implementing your changes for i.mx6
The struct cspi_regs is already present in mx31, mx35, and mx51 headers, so I'm not breaking new ground here, only in the bitfield declarations.
<snip>
My interpretation of Stefano's intent is to clean up the driver at the expense of extra defines in the arch-specific headers.
But they're all the same, right? So we have now the same structure defined thrice?
Almost, but not quite: mx31 and mx35 both use the CSPI peripheral and have one layout. mx5 and mx6 have the ECSPI peripheral, which has an extra register "cfg" to control the polarity and phase of the signals.
Actually, that comment is wrong. The MX51 and MX53 have **both** CSPI and ECSPI peripherals, but the existing code in mxc_spi only supports ECSPI.
The bitfields that my patches move into the imx-regs.h files are also almost identical.
On 18/01/2012 02:44, Eric Nelson wrote:
On 01/17/2012 06:27 PM, Marek Vasut wrote:
On 01/17/2012 04:19 PM, Marek Vasut wrote:
Signed-off-by: Eric Nelsoneric.nelson@boundarydevices.com +/* ECSPI registers */ +struct cspi_regs {
- u32 rxdata;
- u32 txdata;
- u32 ctrl;
- u32 cfg;
- u32 intr;
- u32 dma;
- u32 stat;
- u32 period;
+};
Sigh ... it's no fun I can have only one remark :-)
Is this part common for all imx-es ?
All i.MX6's
This is a cut& paste from MX51.
I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h' for i.MX31 and i.MX35 that share the CSPI peripheral.
But you don't even need this outside of the spi driver so just put it into the spi driver and be done with it. That'll solve your duplication issue.
M
I'll defer to Stefano on this one, since I did this in response to his request:
Yes, I admit I am guilty about this !
The layout of the CSPI registers is not exactly the same for all SOCs. For example, the MXC_CSPICTRL_TC has a different position, as well as the BITCOUNT (because the MX31 can send less bits in one shot) and MXC_CSPICTRL_CHIPSELECT.
So they are similar, but not exactly the same.
Then we have the MX5 registers. Even if we check the CSPI (not eCSPI) controller, the layout of the registers is different compared to the MX3 SOCs.
The struct cspi_regs is already present in mx31, mx35, and mx51 headers, so I'm not breaking new ground here, only in the bitfield declarations.
Right, I see the same. The cspi_regs structure is already defined into the imx_regs.h, only the bit layout was moved. And as the bit layout is SOC dependent, I think the right place for it is inside the imx-regs.h registers.
My interpretation of Stefano's intent is to clean up the driver at the expense of extra defines in the arch-specific headers.
Yes, you're right - of course, I am open also to other solutions if they are proofed to be better ;-).
Best regards, Stefano
On 18/01/2012 02:44, Eric Nelson wrote:
On 01/17/2012 06:27 PM, Marek Vasut wrote:
On 01/17/2012 04:19 PM, Marek Vasut wrote:
Signed-off-by: Eric Nelsoneric.nelson@boundarydevices.com +/* ECSPI registers */ +struct cspi_regs {
- u32 rxdata;
- u32 txdata;
- u32 ctrl;
- u32 cfg;
- u32 intr;
- u32 dma;
- u32 stat;
- u32 period;
+};
Sigh ... it's no fun I can have only one remark :-)
Is this part common for all imx-es ?
All i.MX6's
This is a cut& paste from MX51.
I was tempted to introduce an 'mxc_ecspi.h' to merge the declaration for i.MX5x and i.MX6 which share the ECSPI peripheral and 'mxc_cspi.h' for i.MX31 and i.MX35 that share the CSPI peripheral.
But you don't even need this outside of the spi driver so just put it into the spi driver and be done with it. That'll solve your duplication issue.
M
I'll defer to Stefano on this one, since I did this in response
to his request:
Yes, I admit I am guilty about this !
The layout of the CSPI registers is not exactly the same for all SOCs. For example, the MXC_CSPICTRL_TC has a different position, as well as the BITCOUNT (because the MX31 can send less bits in one shot) and MXC_CSPICTRL_CHIPSELECT.
So they are similar, but not exactly the same.
Then we have the MX5 registers. Even if we check the CSPI (not eCSPI) controller, the layout of the registers is different compared to the MX3 SOCs.
The struct cspi_regs is already present in mx31, mx35, and mx51 headers, so I'm not breaking new ground here, only in the bitfield declarations.
Right, I see the same. The cspi_regs structure is already defined into the imx_regs.h, only the bit layout was moved. And as the bit layout is SOC dependent, I think the right place for it is inside the imx-regs.h registers.
My interpretation of Stefano's intent is to clean up the driver at the expense of extra defines in the arch-specific headers.
Yes, you're right - of course, I am open also to other solutions if they are proofed to be better ;-).
Best regards, Stefano
Ok guys, I see ... Stefano, you're ok with putting the reg structures into these header files?
M
On 18/01/2012 17:08, Marek Vasut wrote:
Ok guys, I see ... Stefano, you're ok with putting the reg structures into these header files?
The reg structures are already into these header files - the patch moves only the bit meaning inside the imx-regs.h files. We can discuss if we should move them again into the driver, making the decision on which structure to be used not on a SOC level (#ifdef CONFIG_MXxx), but on basis of the controller type (CSPI or ECSPI).
This makes sense if we run (we had until now no use case with the currently supported boards) a MX5 board using a CSPI instead of a ECSPI. In this case, we should also transform MXC_CSPI / MXC_ECSPI in a CONFIG_ define to be put in the board configuration file.
However, as usual in u-boot, dead code or code that has no use case and cannot be tested is not allowed. Until we have not such a board (MX5 board requiring CSPI instead of ECSPI), we should avoid to introduce not tested code and let those changes for a next patch.
Stefano
On 01/18/2012 01:39 AM, Stefano Babic wrote:
On 18/01/2012 02:44, Eric Nelson wrote:
I'll defer to Stefano on this one, since I did this in response to his request:
Yes, I admit I am guilty about this !
The layout of the CSPI registers is not exactly the same for all SOCs. For example, the MXC_CSPICTRL_TC has a different position, as well as the BITCOUNT (because the MX31 can send less bits in one shot) and MXC_CSPICTRL_CHIPSELECT.
So they are similar, but not exactly the same.
Then we have the MX5 registers. Even if we check the CSPI (not eCSPI) controller, the layout of the registers is different compared to the MX3 SOCs.
The struct cspi_regs is already present in mx31, mx35, and mx51 headers, so I'm not breaking new ground here, only in the bitfield declarations.
Right, I see the same. The cspi_regs structure is already defined into the imx_regs.h, only the bit layout was moved. And as the bit layout is SOC dependent, I think the right place for it is inside the imx-regs.h registers.
My interpretation of Stefano's intent is to clean up the driver at the expense of extra defines in the arch-specific headers.
Yes, you're right - of course, I am open also to other solutions if they are proofed to be better ;-).
I think this is about as good as things get with the current code base. I would argue that the driver would be better if it explicitly supported ECSPI and CSPI at the same time since the mx5x CPUs support it.
Implememting that would likely require a de-structuring (removing the use of structs to represent the register set). IOW, a re-write.
That's probably not worth the effort unless someone's built hardware that needs it (I'm not aware of any).
On our boards that use more than one channel of SPI (for PMIC and SF), we're using ECSPI on both. I think the same was true on the MX51 EVK.
On 18/01/2012 21:05, Eric Nelson wrote:
Yes, you're right - of course, I am open also to other solutions if they are proofed to be better ;-).
I think this is about as good as things get with the current code base. I would argue that the driver would be better if it explicitly supported ECSPI and CSPI at the same time since the mx5x CPUs support it.
This means that the driver goes to support multiple interfaces at the same time, independently if they are CSPI or ECSPI. At the moment, there is no use case for it.
Implememting that would likely require a de-structuring (removing the use of structs to represent the register set). IOW, a re-write.
Yes, this is also for most drivers in u-boot to support multiple interface and not only one.
That's probably not worth the effort unless someone's built hardware that needs it (I'm not aware of any).
Agree.
On our boards that use more than one channel of SPI (for PMIC and SF), we're using ECSPI on both. I think the same was true on the MX51 EVK.
Yes, it is the same.
Best regards, Stefano Babic
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com --- board/freescale/mx6qsabrelite/imximage.cfg | 2 +- board/freescale/mx6qsabrelite/mx6qsabrelite.c | 25 +++++++++++++++++++++++++ include/configs/mx6qsabrelite.h | 9 +++++++++ 3 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/board/freescale/mx6qsabrelite/imximage.cfg b/board/freescale/mx6qsabrelite/imximage.cfg index b4ff010..fa40bff 100644 --- a/board/freescale/mx6qsabrelite/imximage.cfg +++ b/board/freescale/mx6qsabrelite/imximage.cfg @@ -156,7 +156,7 @@ DATA 4 0x021b0404 0x00011006
# set the default clock gate to save power DATA 4 0x020c4068 0x00C03F3F -DATA 4 0x020c406c 0x0030FC00 +DATA 4 0x020c406c 0x0030FC03 DATA 4 0x020c4070 0x0FFFC000 DATA 4 0x020c4074 0x3FF00000 DATA 4 0x020c4078 0x00FFF300 diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index a0b648f..56c54d6 100644 --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c @@ -46,6 +46,10 @@ DECLARE_GLOBAL_DATA_PTR; PAD_CTL_PUS_100K_UP | PAD_CTL_SPEED_MED | \ PAD_CTL_DSE_40ohm | PAD_CTL_HYS)
+#define SPI_PAD_CTRL (PAD_CTL_HYS | \ + PAD_CTL_PUS_100K_DOWN | PAD_CTL_SPEED_MED | \ + PAD_CTL_DSE_40ohm | PAD_CTL_SRE_FAST) + int dram_init(void) { gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE); @@ -193,6 +197,23 @@ int board_mmc_init(bd_t *bis) } #endif
+#ifdef CONFIG_MXC_SPI +iomux_v3_cfg_t ecspi1_pads[] = { + /* SS1 */ + MX6Q_PAD_EIM_D19__GPIO_3_19 | MUX_PAD_CTRL(SPI_PAD_CTRL), + MX6Q_PAD_EIM_D17__ECSPI1_MISO | MUX_PAD_CTRL(SPI_PAD_CTRL), + MX6Q_PAD_EIM_D18__ECSPI1_MOSI | MUX_PAD_CTRL(SPI_PAD_CTRL), + MX6Q_PAD_EIM_D16__ECSPI1_SCLK | MUX_PAD_CTRL(SPI_PAD_CTRL), +}; + +void setup_spi(void) +{ + gpio_direction_output(IMX_GPIO_NR(3, 19), 1); + imx_iomux_v3_setup_multiple_pads(ecspi1_pads, + ARRAY_SIZE(ecspi1_pads)); +} +#endif + #define MII_1000BASET_CTRL 0x9 #define MII_EXTENDED_CTRL 0xb #define MII_EXTENDED_DATAW 0xc @@ -250,6 +271,10 @@ int board_early_init_f(void) { setup_iomux_uart();
+#ifdef CONFIG_MXC_SPI + setup_spi(); +#endif + return 0; }
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index 034fc40..8dd6e39 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -44,6 +44,15 @@ #define CONFIG_MXC_UART #define CONFIG_MXC_UART_BASE UART2_BASE
+#define CONFIG_CMD_SF +#ifdef CONFIG_CMD_SF +#define CONFIG_SPI_FLASH +#define CONFIG_SPI_FLASH_SST +#define CONFIG_MXC_SPI +#define CONFIG_SF_DEFAULT_SPEED 25000000 +#define CONFIG_SF_DEFAULT_MODE (SPI_MODE_0) +#endif + /* MMC Configs */ #define CONFIG_FSL_ESDHC #define CONFIG_FSL_USDHC
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com --- common/cmd_sf.c | 34 +++++++++++++++++++++++----------- 1 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/common/cmd_sf.c b/common/cmd_sf.c index 7225656..97fce16 100644 --- a/common/cmd_sf.c +++ b/common/cmd_sf.c @@ -70,20 +70,28 @@ static int do_spi_flash_probe(int argc, char * const argv[]) char *endp; struct spi_flash *new;
- if (argc < 2) - return -1; - - cs = simple_strtoul(argv[1], &endp, 0); - if (*argv[1] == 0 || (*endp != 0 && *endp != ':')) +#ifndef CONFIG_SPI_FLASH_CS + if (argc < 2) { + printf("%s: missing arguments\n", __func__); return -1; - if (*endp == ':') { - if (endp[1] == 0) - return -1; + } +#else + cs = CONFIG_SPI_FLASH_CS ; +#endif
- bus = cs; - cs = simple_strtoul(endp + 1, &endp, 0); - if (*endp != 0) + if (argc >= 2) { + cs = simple_strtoul(argv[1], &endp, 0); + if (*argv[1] == 0 || (*endp != 0 && *endp != ':')) return -1; + if (*endp == ':') { + if (endp[1] == 0) + return -1; + + bus = cs; + cs = simple_strtoul(endp + 1, &endp, 0); + if (*endp != 0) + return -1; + } }
if (argc >= 3) { @@ -299,7 +307,11 @@ usage: U_BOOT_CMD( sf, 5, 1, do_spi_flash, "SPI flash sub-system", +#ifndef CONFIG_SPI_FLASH_CS "probe [bus:]cs [hz] [mode] - init flash device on given SPI bus\n" +#else + "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n" +#endif " and chip select\n" "sf read addr offset len - read `len' bytes starting at\n" " `offset' to memory at `addr'\n"
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com --- include/configs/mx6qsabrelite.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index 8dd6e39..44b028a 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -46,6 +46,7 @@
#define CONFIG_CMD_SF #ifdef CONFIG_CMD_SF +#define CONFIG_SPI_FLASH_CS 0x5300 #define CONFIG_SPI_FLASH #define CONFIG_SPI_FLASH_SST #define CONFIG_MXC_SPI
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com --- include/configs/mx6qsabrelite.h | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index 44b028a..160894c 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -174,10 +174,20 @@ /* FLASH and environment organization */ #define CONFIG_SYS_NO_FLASH
-#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_ENV_SIZE (8 * 1024) + #define CONFIG_ENV_IS_IN_MMC +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */ + +#if defined(CONFIG_ENV_IS_IN_MMC) +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_SYS_MMC_ENV_DEV 0 +#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH) +#define CONFIG_ENV_OFFSET (768 * 1024) +#define CONFIG_ENV_SECT_SIZE (8 * 1024) +#define CONFIG_ENV_SPI_CS 0x5300 +#define CONFIG_ENV_SPI_MODE SPI_MODE_0 +#endif
#define CONFIG_OF_LIBFDT
On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson eric.nelson@boundarydevices.com wrote:
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com
include/configs/mx6qsabrelite.h | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index 44b028a..160894c 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -174,10 +174,20 @@ /* FLASH and environment organization */ #define CONFIG_SYS_NO_FLASH
-#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_ENV_SIZE (8 * 1024)
#define CONFIG_ENV_IS_IN_MMC +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */
From the commit log, it says the default is in serial flash, but
apparently in the code the env is default to MMC, which mismatch. please fix it.
+#if defined(CONFIG_ENV_IS_IN_MMC) +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_SYS_MMC_ENV_DEV 0 +#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH) +#define CONFIG_ENV_OFFSET (768 * 1024) +#define CONFIG_ENV_SECT_SIZE (8 * 1024) +#define CONFIG_ENV_SPI_CS 0x5300
I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague?
+#define CONFIG_ENV_SPI_MODE SPI_MODE_0 +#endif
#define CONFIG_OF_LIBFDT
-- 1.7.1
On 20.01.2012 04:27, Jason Hui wrote:
On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson eric.nelson@boundarydevices.com wrote:
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com
include/configs/mx6qsabrelite.h | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index 44b028a..160894c 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -174,10 +174,20 @@ /* FLASH and environment organization */ #define CONFIG_SYS_NO_FLASH
-#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_ENV_SIZE (8 * 1024)
#define CONFIG_ENV_IS_IN_MMC +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */
From the commit log, it says the default is in serial flash,
No, the commit log doesn't say this. It doesn't say 'it is'. It says it 'provides the defaults'. But it doesn't say that it actually uses these defaults.
but apparently in the code the env is default to MMC, which mismatch. please fix it.
As mentioned above, I understand this differently. While I reviewed it some days ago, I found the description and the doing here quite fine.
It enables the MMC env and additionally _provides_ everything needed to easily switch to SPI env by uncommenting one switch. This is fine and quite helpful, see e.g. [1].
I like this, please keep it as is.
Best regards
Dirk
[1] http://lists.denx.de/pipermail/u-boot/2012-January/116266.html
"you can place the environment in SPI-NOR as well by commenting out CONFIG_ENV_IS_IN_MMC, and un-commenting ..._IN_SPI_FLASH in include/configs/mx6qsabrelite.h."
+#if defined(CONFIG_ENV_IS_IN_MMC) +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_SYS_MMC_ENV_DEV 0 +#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH) +#define CONFIG_ENV_OFFSET (768 * 1024) +#define CONFIG_ENV_SECT_SIZE (8 * 1024) +#define CONFIG_ENV_SPI_CS 0x5300
I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague?
+#define CONFIG_ENV_SPI_MODE SPI_MODE_0 +#endif
#define CONFIG_OF_LIBFDT
-- 1.7.1
On Fri, Jan 20, 2012 at 3:06 PM, Dirk Behme dirk.behme@de.bosch.com wrote:
On 20.01.2012 04:27, Jason Hui wrote:
On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson eric.nelson@boundarydevices.com wrote:
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com
include/configs/mx6qsabrelite.h | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index 44b028a..160894c 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -174,10 +174,20 @@ /* FLASH and environment organization */ #define CONFIG_SYS_NO_FLASH
-#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_ENV_SIZE (8 * 1024)
#define CONFIG_ENV_IS_IN_MMC +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */
From the commit log, it says the default is in serial flash,
No, the commit log doesn't say this. It doesn't say 'it is'. It says it 'provides the defaults'. But it doesn't say that it actually uses these defaults.
but apparently in the code the env is default to MMC, which mismatch. please fix it.
As mentioned above, I understand this differently. While I reviewed it some days ago, I found the description and the doing here quite fine.
OK, I get it, then I'm fine with it too.
It enables the MMC env and additionally _provides_ everything needed to easily switch to SPI env by uncommenting one switch. This is fine and quite helpful, see e.g. [1].
I like this, please keep it as is.
[...]
+#define CONFIG_ENV_SPI_CS 0x5300
I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague?
Then the left open question is only above one.
Jason
On 20/01/2012 08:48, Jason Hui wrote:
I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague?
Then the left open question is only above one.
The SPI driver can take as chip select the controller's chip selects as well as an external GPIO. The LSB byte has the value of the internal chip select, the highest (thought as 16-bit value) contains the GPIO number. Reading this configuration, the GPIO used on this board should be the number 83 (0x53). Stefano
On 01/20/2012 01:47 AM, Stefano Babic wrote:
On 20/01/2012 08:48, Jason Hui wrote:
I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague?
Then the left open question is only above one.
The SPI driver can take as chip select the controller's chip selects as well as an external GPIO. The LSB byte has the value of the internal chip select, the highest (thought as 16-bit value) contains the GPIO number. Reading this configuration, the GPIO used on this board should be the number 83 (0x53).
Stefano
Thanks Stefano,
I like your description better than the one I just wrote... I should have scanned all of my e-mails before drafting my earlier commit msg ;)
On 01/19/2012 08:27 PM, Jason Hui wrote:
On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson eric.nelson@boundarydevices.com wrote:
Signed-off-by: Eric Nelsoneric.nelson@boundarydevices.com
include/configs/mx6qsabrelite.h | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index 44b028a..160894c 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -174,10 +174,20 @@ /* FLASH and environment organization */ #define CONFIG_SYS_NO_FLASH
-#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_ENV_SIZE (8 * 1024)
- #define CONFIG_ENV_IS_IN_MMC
+/* #define CONFIG_ENV_IS_IN_SPI_FLASH */
From the commit log, it says the default is in serial flash, but apparently in the code the env is default to MMC, which mismatch. please fix it.
You're asking that I change the comment not the default, right?
+#if defined(CONFIG_ENV_IS_IN_MMC) +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_SYS_MMC_ENV_DEV 0 +#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH) +#define CONFIG_ENV_OFFSET (768 * 1024) +#define CONFIG_ENV_SECT_SIZE (8 * 1024) +#define CONFIG_ENV_SPI_CS 0x5300
I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague?
Please review the updated patch below and see whether the expanded commit message fixes things.
Regards,
Eric commit 0443433bf80c5203a8ce67fb4faaf4032e398e1d Author: Eric Nelson eric.nelson@boundarydevices.com Date: Tue Jan 17 14:11:54 2012 -0700
mx6q: mx6qsabrelite: Provide macros for environment in serial flash
The default settings store the persistent environment on SD card and not serial flash (SPI NOR).
To use SPI NOR to save the environment instead of SD card, edit include/configs/mx6qsabrelite.h and
- undefine CONFIG_ENV_IS_IN_MMC - define CONFIG_ENV_IS_IN_SPI_FLASH
Note that the mxc_spi driver (drivers/spi/mxc_spi.c) uses the Chip-Select variable (CONFIG_ENV_SPI_CS) to allow the use of a GPIO if the chip-select is greater than 3. The low 8-bits set the chip select number and bits 8-15 set the GPIO number.
The GPIO used on Sabre Lite is GP3:19 == 83.
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index e34f108..024a94c 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -174,10 +174,20 @@ /* FLASH and environment organization */ #define CONFIG_SYS_NO_FLASH
-#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_ENV_SIZE (8 * 1024) + #define CONFIG_ENV_IS_IN_MMC +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */ + +#if defined(CONFIG_ENV_IS_IN_MMC) +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_SYS_MMC_ENV_DEV 0 +#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH) +#define CONFIG_ENV_OFFSET (768 * 1024) +#define CONFIG_ENV_SECT_SIZE (8 * 1024) +#define CONFIG_ENV_SPI_CS (0|(83<<8)) +#define CONFIG_ENV_SPI_MODE SPI_MODE_0 +#endif
#define CONFIG_OF_LIBFDT
This patch set refactors mxc_spi as described in http://lists.denx.de/pipermail/u-boot/2010-March/068791.html and requested in http://lists.denx.de/pipermail/u-boot/2012-January/116023.html in order to add support for the MX6Q in general and the mx6qsabrelite specifically.
Patch 1 simply moves the conditional parts of mxc_spi.c into the respective CPU-specific imx-regs.h files.
Patch 2 adds general support for SPI to the i.MX6.
Patch 3 adds support to the mx6qsabrelite board
Patch 4 modifies the 'sf' command to allow a default chip-select to be specified by board headers as is done on efika et al. This allows a bare 'sf' probe command: U-Boot> sf probe instead of the more cumbersome usage when a GPIO is tacked onto the chip-select. Otherwise, this command-line would be needed to specify GP3:19 on SabreLite: U-Boot> sf probe 0x5300
Patch 5 adds default chip-select values for mx6qsabrelite platform.
Patch 6 adds reference macros for use in storing the environment in serial flash to match the use on Freescale's U-Boot release
This patch set has been compiled against the following configurations, but only tested on mx6qsabrelite: mx6qsabrelite mx51evk mx31pdk mx35pdk
Muhehehe ... time for a little review-torture :-)
On 17.01.2012 23:09, Eric Nelson wrote:
This patch set refactors mxc_spi as described in http://lists.denx.de/pipermail/u-boot/2010-March/068791.html and requested in http://lists.denx.de/pipermail/u-boot/2012-January/116023.html in order to add support for the MX6Q in general and the mx6qsabrelite specifically.
Whole patch series
Acked-by: Dirk Behme dirk.behme@de.bosch.com
Many thanks!
Dirk
participants (5)
-
Dirk Behme -
Eric Nelson -
Jason Hui -
Marek Vasut -
Stefano Babic