Skip to contentSkip to conversation

SPI: Add i.MX ECSPI driver

PATCH New Dirk Behme 6 participants 17 messages Jan 12, 2012 – Jan 16, 2012

Subsystems (2)
lore
Share & tools
lore

Dirk Behme

author
#0 lore raw

PATCH 1/2 SPI: Add i.MX ECSPI driver

From: Eric Nelson <[email protected]>

CC Jason Liu <[email protected]>
CC Stefano Babic <[email protected]>
---
 drivers/spi/Makefile    |    1 +
 drivers/spi/imx_ecspi.c |  334 +++++++++++++++++++++++++++++++++++++++++++++++
 include/imx_spi.h       |   97 ++++++++++++++
 3 files changed, 432 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/imx_ecspi.c
 create mode 100644 include/imx_spi.h
drivers/spi/Makefile +1 -0
index c967d87..e27ef41 100644
@@ -33,6 +33,7 @@ COBJS-$(CONFIG_ATMEL_SPI) += atmel_spi.o
 COBJS-$(CONFIG_BFIN_SPI) += bfin_spi.o
 COBJS-$(CONFIG_CF_SPI) += cf_spi.o
 COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
+COBJS-$(CONFIG_IMX_ECSPI) += imx_ecspi.o
 COBJS-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
 COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
 COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
drivers/spi/imx_ecspi.c new file +334 -0

334 lines not on this page · raw message

include/imx_spi.h new file +97 -0

97 lines not on this page · raw message

Signature · 1 line
1.7.0.4
author
#1 lore raw

PATCH 2/2 i.mx6q: SabreLite: Add SPI NOR support

From: Eric Nelson <[email protected]>

CC Jason Liu <[email protected]>
CC Stefano Babic <[email protected]>

---
Note: These two patches are against the recent head of u-boot-imx.git including
the SabreLite support:

5b894e4d00ff94a221f8cc23d54d08b889f54190
i.mx: i.mx6q: add the initial support for i.mx6q Sabre Lite board

 board/freescale/mx6qsabrelite/imximage.cfg    |    2 +-
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |   51 +++++++++++++++++++++++++
 include/configs/mx6qsabrelite.h               |   15 +++++++
 3 files changed, 67 insertions(+), 1 deletions(-)
board/freescale/mx6qsabrelite/imximage.cfg +1 -1
index 83dee6f..c389427 100644
@@ -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
board/freescale/mx6qsabrelite/mx6qsabrelite.c +51 -0

75 lines not on this page · raw message

include/configs/mx6qsabrelite.h +15 -0
index 464f0ec..48db42c 100644
@@ -44,6 +44,21 @@
 #define CONFIG_MXC_UART
 #define CONFIG_MXC_UART_BASE           UART2_BASE
 
+#define CONFIG_CMD_SF
+/*
+ * SPI Configs
+ */
+#ifdef CONFIG_CMD_SF
+	#define CONFIG_FSL_SF		1
+	#define CONFIG_SPI_FLASH       1
+	#define CONFIG_SPI_FLASH_SST	1
+	#define CONFIG_SPI_FLASH_CS	1
+	#define CONFIG_IMX_ECSPI
+	#define IMX_CSPI_VER_2_3        1
+
+	#define MAX_SPI_BYTES		(64 * 4)
+#endif
+
 /* MMC Configs */
 #define CONFIG_FSL_ESDHC
 #define CONFIG_FSL_USDHC
Signature · 1 line
1.7.0.4
reviewer
#2 lore raw

PATCH 2/2 i.mx6q: SabreLite: Add SPI NOR support

Quoted 7 lines · Dirk Behme

+#ifdef CONFIG_CMD_SF
+ ? ? ? #define CONFIG_FSL_SF ? ? ? ? ? 1
+ ? ? ? #define CONFIG_SPI_FLASH ? ? ? 1
+ ? ? ? #define CONFIG_SPI_FLASH_SST ? ?1
+ ? ? ? #define CONFIG_SPI_FLASH_CS ? ? 1
+ ? ? ? #define CONFIG_IMX_ECSPI
+ ? ? ? #define IMX_CSPI_VER_2_3 ? ? ? ?1

Please remove all these unneeded "1".

Regards,

Fabio Estevam

#3 lore raw

PATCH 2/2 i.mx6q: SabreLite: Add SPI NOR support

In reply to Dirk Behme
Quoted 59 lines

From: Eric Nelson <[email protected]>

CC Jason Liu <[email protected]>
CC Stefano Babic <[email protected]>

---
Note: These two patches are against the recent head of u-boot-imx.git including the SabreLite support:

5b894e4d00ff94a221f8cc23d54d08b889f54190
i.mx: i.mx6q: add the initial support for i.mx6q Sabre Lite board

 board/freescale/mx6qsabrelite/imximage.cfg    |    2 +-
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |   51
+++++++++++++++++++++++++ include/configs/mx6qsabrelite.h               | 
 15 +++++++
 3 files changed, 67 insertions(+), 1 deletions(-)
(quoted diff) +0 -0

b/board/freescale/mx6qsabrelite/imximage.cfg index 83dee6f..c389427 100644

board/freescale/mx6qsabrelite/imximage.cfg +1 -1
@@ -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
(quoted diff) +0 -0

b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index 4028789..d69adfa 100644

board/freescale/mx6qsabrelite/mx6qsabrelite.c +8 -0
@@ -29,6 +29,10 @@
 #include <asm/gpio.h>
 #include <mmc.h>
 #include <fsl_esdhc.h>
+#ifdef CONFIG_IMX_ECSPI
+#include <spi.h>
+#include <imx_spi.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -40,6 +44,10 @@ DECLARE_GLOBAL_DATA_PTR;
        PAD_CTL_PUS_47K_UP  | PAD_CTL_SPEED_LOW |               \
        PAD_CTL_DSE_80ohm   | PAD_CTL_SRE_FAST  | 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);

board/freescale/mx6qsabrelite/mx6qsabrelite.c int board_mmc_init(bd_t *bis) · line 145

Quoted 10 lines
board/freescale/mx6qsabrelite/mx6qsabrelite.c +7 -0
@@ -128,6 +136,46 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#ifdef CONFIG_IMX_ECSPI
+s32 spi_get_cfg(struct imx_spi_dev_t *dev)
+{
+	int rval = 0 ;
+	if (1 == dev->slave.cs) {
+		dev->base = ECSPI1_BASE_ADDR;
+		dev->ss = 1 ;

" ;" again.

Also, Do you even need the ecspi thing? Doesn't uboot support some kind of imx spi driver already?

M

Quoted 72 lines
board/freescale/mx6qsabrelite/mx6qsabrelite.c +36 -0
+		dev->ss_pol = IMX_SPI_ACTIVE_LOW; /* SPI NOR */
+		dev->freq = 25000000;
+		dev->fifo_sz = 64 * 4;
+		dev->us_delay = 0;
+	} else {
+		printf("%s: invalid chip select %d\n", __func__, dev->slave.cs);
+		rval = -EINVAL ;
+	}
+	return rval;
+}
+
+void spi_io_init(struct imx_spi_dev_t *dev, int active)
+{
+	if (dev->ss == 1)
+		gpio_set_value(83, active ? 0 : 1); /* GPIO 3.19 */
+}
+
+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(83, 1); /* GPIO 3.19 */
+	imx_iomux_v3_setup_multiple_pads(ecspi1_pads,
+					 ARRAY_SIZE(ecspi1_pads));
+}
+#endif
+
 int board_early_init_f(void)
 {
        setup_iomux_uart();
@@ -140,6 +188,9 @@ int board_init(void)
        /* address of boot parameters */
        gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
 
+#ifdef CONFIG_IMX_ECSPI
+	setup_spi();
+#endif
        return 0;
 }
 
(quoted diff) +0 -0

b/include/configs/mx6qsabrelite.h index 464f0ec..48db42c 100644

include/configs/mx6qsabrelite.h +15 -0
@@ -44,6 +44,21 @@
 #define CONFIG_MXC_UART
 #define CONFIG_MXC_UART_BASE           UART2_BASE
 
+#define CONFIG_CMD_SF
+/*
+ * SPI Configs
+ */
+#ifdef CONFIG_CMD_SF
+	#define CONFIG_FSL_SF		1
+	#define CONFIG_SPI_FLASH       1
+	#define CONFIG_SPI_FLASH_SST	1
+	#define CONFIG_SPI_FLASH_CS	1
+	#define CONFIG_IMX_ECSPI
+	#define IMX_CSPI_VER_2_3        1
+
+	#define MAX_SPI_BYTES		(64 * 4)
+#endif
+
 /* MMC Configs */
 #define CONFIG_FSL_ESDHC
 #define CONFIG_FSL_USDHC
reviewer
#4 lore raw

PATCH 2/2 i.mx6q: SabreLite: Add SPI NOR support

Quoted 2 lines · Marek Vasut

Also, Do you even need the ecspi thing? Doesn't uboot support some kind of imx spi driver already?

Yes, this is the same question I have.

Can't drivers/spi/mxc_spi.c be extended to support mx6?

Regards,

Fabio Estevam

author
#5 lore raw

PATCH 2/2 i.mx6q: SabreLite: Add SPI NOR support

Quoted 8 lines · Fabio Estevam
Quoted 2 lines · Marek Vasut

Also, Do you even need the ecspi thing? Doesn't uboot support some kind of imx spi driver already?

Yes, this is the same question I have.

Can't drivers/spi/mxc_spi.c be extended to support mx6?

Eric, what do you think?

Best regards

Dirk

#6 lore raw

PATCH 2/2 i.mx6q: SabreLite: Add SPI NOR support

In reply to Marek Vasut
Quoted 11 lines · On Thursday 12 January 2012 10:38:40 Marek Vasut wrote:

board/freescale/mx6qsabrelite/mx6qsabrelite.c int board_mmc_init(bd_t *bis) · line 145

Quoted 8 lines

From: Eric Nelson <[email protected]>

board/freescale/mx6qsabrelite/mx6qsabrelite.c +7 -0
+#ifdef CONFIG_IMX_ECSPI
+s32 spi_get_cfg(struct imx_spi_dev_t *dev)
+{
+	int rval = 0 ;
+	if (1 == dev->slave.cs) {
+		dev->base = ECSPI1_BASE_ADDR;
+		dev->ss = 1 ;

" ;" again.

Quoted 2 lines

Also, Do you even need the ecspi thing? Doesn't uboot support some kind of imx spi driver already?

when you reply, please delete all unnecessary context. there was like ~170 lines of context when you only needed ~10.
-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/20120114/79a08313/attachment.pgp>

#7 lore raw

PATCH 2/2 i.mx6q: SabreLite: Add SPI NOR support

In reply to Dirk Behme

include/configs/mx6qsabrelite.h line 57

Quoted 7 lines · On Thursday 12 January 2012 10:27:14 Dirk Behme wrote:
include/configs/mx6qsabrelite.h +7 -0
+#ifdef CONFIG_CMD_SF
+	#define CONFIG_FSL_SF		1
+	#define CONFIG_SPI_FLASH       1
+	#define CONFIG_SPI_FLASH_SST	1
+	#define CONFIG_SPI_FLASH_CS	1
+	#define CONFIG_IMX_ECSPI
+	#define IMX_CSPI_VER_2_3        1

don't indent the "#"
-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/20120114/f30c4d55/attachment.pgp>

#8 lore raw

PATCH 1/2 SPI: Add i.MX ECSPI driver

In reply to Dirk Behme
Quoted 93 lines

From: Eric Nelson <[email protected]>

CC Jason Liu <[email protected]>
CC Stefano Babic <[email protected]>
---
 drivers/spi/Makefile    |    1 +
 drivers/spi/imx_ecspi.c |  334
+++++++++++++++++++++++++++++++++++++++++++++++ include/imx_spi.h       | 
 97 ++++++++++++++
 3 files changed, 432 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/imx_ecspi.c
 create mode 100644 include/imx_spi.h
drivers/spi/Makefile +1 -0
index c967d87..e27ef41 100644
@@ -33,6 +33,7 @@ COBJS-$(CONFIG_ATMEL_SPI) += atmel_spi.o
 COBJS-$(CONFIG_BFIN_SPI) += bfin_spi.o
 COBJS-$(CONFIG_CF_SPI) += cf_spi.o
 COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
+COBJS-$(CONFIG_IMX_ECSPI) += imx_ecspi.o
 COBJS-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
 COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
 COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
drivers/spi/imx_ecspi.c new file +33 -0
new file mode 100644
index 0000000..1468208
@@ -0,0 +1,334 @@
+/*
+ * (C) Copyright 2008-2010 Freescale Semiconductor, Inc.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <config.h>
+#include <common.h>
+#include <spi.h>
+#include <asm/errno.h>
+#include <linux/types.h>
+#include <asm/io.h>
+#include <malloc.h>
+#include <asm/arch/clock.h>
+#include <imx_spi.h>
+
+static inline struct imx_spi_dev_t *to_imx_spi_slave(struct spi_slave

*slave) +{

drivers/spi/imx_ecspi.c new file +27 -0
+	return container_of(slave, struct imx_spi_dev_t, slave);
+}
+
+static s32 spi_reset(struct spi_slave *slave)
+{
+	u32 clk_src = mxc_get_clock(MXC_CSPI_CLK);
+	u32 pre_div = 0, post_div = 0, reg_ctrl, reg_config;
+	struct imx_spi_dev_t *dev = to_imx_spi_slave(slave);
+	struct spi_reg_t *reg = &(dev->reg);
+
+	if (dev->freq == 0) {
+		printf("Error: desired clock is 0\n");
+		return 1;
+	}
+
+	reg_ctrl = readl(dev->base + SPI_CON_REG);
+	reg_config = readl(dev->base + SPI_CFG_REG);
+	/* Reset spi */
+	writel(0, dev->base + SPI_CON_REG);
+	writel((reg_ctrl | 0x1), dev->base + SPI_CON_REG);
+
+	/* Control register setup */
+	pre_div = (clk_src + dev->freq - 1) / dev->freq;
+	while (pre_div > 16) {
+		pre_div = (pre_div + 1) >> 1;
+		post_div++;
+	}

drivers/spi/imx_ecspi.c line 71

Quoted 10 lines
drivers/spi/imx_ecspi.c new file +10 -0
+	if (post_div > 0x0f) {
+		printf("Error: no divider can meet the freq: %d\n", dev->freq);
+		return -1;
+	}
+	if (pre_div)
+		pre_div--;
+
+	debug("pre_div = %d, post_div=%d, clk_src=%d, spi_freq=%d\n", pre_div,
+			post_div, clk_src, (clk_src/(pre_div + 1)) >> post_div);
+	reg_ctrl &= ~((3 << 18) | (0xF << 12) | (0xF << 8));

Magic numbers, fix globally

Quoted 46 lines
drivers/spi/imx_ecspi.c new file +46 -0
+	reg_ctrl |= (dev->ss << 18);
+	reg_ctrl |= (pre_div << 12);
+	reg_ctrl |= (post_div << 8);
+	reg_ctrl |= (1 << (dev->ss + 4));	/* always set to master mode */
+	reg_ctrl |= 1;
+
+	/* configuration register setup */
+	reg_config &= ~(0x111111 << dev->ss);
+	reg_config |= (dev->in_sctl << (dev->ss + 20));
+	reg_config |= (dev->in_dctl << (dev->ss + 16));
+	reg_config |= (dev->ss_pol << (dev->ss + 12));
+	reg_config |= (dev->ssctl << (dev->ss + 8));
+	reg_config |= (dev->sclkpol << (dev->ss + 4));
+	reg_config |= (dev->sclkpha << (dev->ss));
+
+	reg_config &= 0x0f << 12 ;
+	reg_config |= (dev->ss_pol)<<(12+dev->ss);
+	debug("ss%x, reg_config = 0x%x\n", dev->ss, reg_config);
+	writel(reg_config, dev->base + SPI_CFG_REG);
+	debug("ss%x, reg_ctrl = 0x%x\n", dev->ss, reg_ctrl);
+	writel(reg_ctrl & ~1, dev->base + SPI_CON_REG);
+
+	/* save config register and control register */
+	reg->cfg_reg  = reg_config;
+	reg->ctrl_reg = reg_ctrl;
+
+	/* clear interrupt reg */
+	writel(0, dev->base + SPI_INT_REG);
+	writel(3 << 6, dev->base + SPI_STAT_REG);
+	return 0;
+}
+
+void spi_init(void)
+{
+}
+
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+		unsigned int max_hz, unsigned int mode)
+{
+	struct imx_spi_dev_t *imx_spi_slave = NULL;
+
+	if (!spi_cs_is_valid(bus, cs))
+		return NULL;
+
+	imx_spi_slave = (struct imx_spi_dev_t *)
+			calloc(sizeof(struct imx_spi_dev_t), 1);

drivers/spi/imx_ecspi.c line 127

Quoted 10 lines
drivers/spi/imx_ecspi.c new file +10 -0
+	if (!imx_spi_slave)
+		return NULL;
+
+	imx_spi_slave->slave.bus = bus;
+	imx_spi_slave->slave.cs = cs;
+
+	spi_get_cfg(imx_spi_slave);
+
+	if (max_hz < imx_spi_slave->freq)
+		imx_spi_slave->freq = max_hz ;

" ;" <-- fix globally ... run tools/checkpatch.pl before submitting

Quoted 121 lines
drivers/spi/imx_ecspi.c new file +121 -0
+	spi_io_init(imx_spi_slave, 0);
+
+	spi_reset(&(imx_spi_slave->slave));
+
+	return &(imx_spi_slave->slave);
+}
+
+void spi_free_slave(struct spi_slave *slave)
+{
+	struct imx_spi_dev_t *imx_spi_slave;
+
+	if (slave) {
+		imx_spi_slave = to_imx_spi_slave(slave);
+		free(imx_spi_slave);
+	}
+}
+
+int spi_claim_bus(struct spi_slave *slave)
+{
+	return 0;
+}
+
+void spi_release_bus(struct spi_slave *slave)
+{
+
+}
+
+/*
+ * SPI xchg
+ */
+
+int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
+	const u8 *dout, u8 *din, unsigned long flags)
+{
+	int nbytes = (bitlen + 7) / 8;
+	struct imx_spi_dev_t *dev = to_imx_spi_slave(slave);
+	struct spi_reg_t *spi_reg = &(dev->reg);
+	u32 loop_cnt ;
+	if (!slave)
+		return -1;
+
+	if (spi_reg->ctrl_reg == 0) {
+		printf("Error: spi(base=0x%x) has not been initialized yet\n",
+				dev->base);
+		return -1;
+	}
+	spi_reg->ctrl_reg = (spi_reg->ctrl_reg & ~0xFFF00000) | \
+			((bitlen - 1) << 20);
+
+	writel(spi_reg->ctrl_reg, dev->base + SPI_CON_REG);
+	writel(spi_reg->cfg_reg, dev->base + SPI_CFG_REG);
+
+	/* move data to the tx fifo */
+	debug("dout=0x%p, bitlen=%x\n", dout, bitlen);
+
+	/*
+	 * The SPI controller works only with words,
+	 * check if less than a word is sent.
+	 * Access to the FIFO is only 32 bit
+	 */
+	if (bitlen % 32) {
+		u32 data = 0;
+		u32 cnt = (bitlen % 32) / 8;
+		if (dout) {
+			int i ;
+			for (i = 0; i < cnt; i++)
+				data = (data << 8) | (*dout++ & 0xFF);
+		}
+		debug("Sending SPI 0x%x\n", data);
+
+		writel(data, dev->base + SPI_TX_DATA);
+		nbytes -= cnt;
+	}
+
+	while (nbytes > 0) {
+		u32 data = 0;
+		if (dout) {
+			/* Buffer is not 32-bit aligned */
+			if ((unsigned long)dout & 0x03) {
+				int i ;
+				data = 0;
+				for (i = 0; i < 4; i++)
+					data = (data << 8) | (*dout++ & 0xFF);
+			} else {
+				data = *(u32 *)dout;
+				data = cpu_to_be32(data);
+			}
+			dout += 4;
+		}
+		debug("Sending SPI 0x%x\n", data);
+		writel(data, dev->base + SPI_TX_DATA);
+		nbytes -= 4;
+	}
+
+	writel(spi_reg->ctrl_reg | (1 << 2), dev->base + SPI_CON_REG);
+
+	loop_cnt = SPI_RETRY_TIMES ;
+	/* poll on the TC bit (transfer complete) */
+	while ((readl(dev->base + SPI_STAT_REG) & (1 << 7)) == 0) {
+		loop_cnt--;
+		if (loop_cnt <= 0) {
+			printf("%s: Error: re-tried %d times\n",
+				__func__, SPI_RETRY_TIMES);
+			bitlen = 0 ;
+			spi_cs_deactivate(slave);
+			return -1;
+		}
+		udelay(100);
+	}
+
+	/* clear the TC bit */
+	writel(3 << 6, dev->base + SPI_STAT_REG);
+
+	nbytes = (bitlen + 7) / 8;
+
+	if (bitlen % 32) {
+		u32 data = readl(dev->base + SPI_RX_DATA);
+		u32 cnt = (bitlen % 32) / 8;
+		data = cpu_to_be32(data) >> ((sizeof(data) - cnt) * 8);
+		debug("SPI Rx unaligned: 0x%x\n", data);
+		if (din) {

drivers/spi/imx_ecspi.c line 258

Quoted 10 lines
drivers/spi/imx_ecspi.c new file +10 -0
+			memcpy(din, &data, cnt);
+			din += cnt;
+		}
+		nbytes -= cnt;
+	}
+
+	while (nbytes > 0) {
+		u32 tmp = readl(dev->base + SPI_RX_DATA);
+		u32 data = cpu_to_be32(tmp);
+		u32 cnt = min(nbytes, sizeof(data));

Don't define vars in the middle of code.

Quoted 143 lines
drivers/spi/imx_ecspi.c new file +15 -0
+		debug("SPI Rx: 0x%x 0x%x\n", tmp, data);
+		if (din) {
+			memcpy(din, &data, cnt);
+			din += cnt;
+		}
+		nbytes -= cnt;
+	}
+
+	return 0;
+}
+
+/*
+ * SPI transfer:
+ *
+ * See include/spi.h and

http://www.altera.com/literature/ds/ds_nios_spi.pdf + * for more informations.
+ */
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void
*dout, + void *din, unsigned long flags)

drivers/spi/imx_ecspi.c new file +57 -0
+{
+	int n_bytes = (bitlen + 7) / 8;
+	int n_bits;
+	int ret;
+	u32 blk_size;
+	u8 *p_outbuf = (u8 *)dout;
+	u8 *p_inbuf = (u8 *)din;
+
+	if (!slave)
+		return -1;
+
+	if (flags & SPI_XFER_BEGIN)
+		spi_cs_activate(slave);
+
+	while (n_bytes > 0) {
+		if (n_bytes < MAX_SPI_BYTES)
+			blk_size = n_bytes;
+		else
+			blk_size = MAX_SPI_BYTES;
+
+		n_bits = blk_size * 8;
+
+		ret = spi_xchg_single(slave, n_bits, p_outbuf, p_inbuf, 0);
+
+		if (ret)
+			return ret;
+		if (dout)
+			p_outbuf += blk_size;
+		if (din)
+			p_inbuf += blk_size;
+		n_bytes -= blk_size;
+	}
+
+	if (flags & SPI_XFER_END)
+		spi_cs_deactivate(slave);
+
+	return 0;
+}
+
+int spi_cs_is_valid(unsigned int bus, unsigned int cs)
+{
+	return 1;
+}
+
+void spi_cs_activate(struct spi_slave *slave)
+{
+	struct imx_spi_dev_t *dev = to_imx_spi_slave(slave);
+
+	spi_io_init(dev, 1);
+}
+
+void spi_cs_deactivate(struct spi_slave *slave)
+{
+	struct imx_spi_dev_t *dev = to_imx_spi_slave(slave);
+	spi_io_init(dev, 0);
+	writel(0, dev->base + SPI_CON_REG);
+}
include/imx_spi.h new file +60 -0
new file mode 100644
index 0000000..32544d9
@@ -0,0 +1,97 @@
+/*
+ * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __IMX_SPI_H__
+#define __IMX_SPI_H__
+
+#include <spi.h>
+
+#undef IMX_SPI_DEBUG
+
+#define IMX_SPI_ACTIVE_HIGH     1
+#define IMX_SPI_ACTIVE_LOW      0
+#define SPI_RETRY_TIMES         100
+
+#if defined(IMX_CSPI_VER_0_7)
+	#define	SPI_RX_DATA		0x0
+	#define SPI_TX_DATA		0x4
+	#define SPI_CON_REG		0x8
+	#define SPI_INT_REG		0xC
+	#define SPI_DMA_REG		0x10
+	#define SPI_STAT_REG		0x14
+	#define SPI_PERIOD_REG		0x18
+
+	#define SPI_CTRL_EN		(1 << 0)
+	#define SPI_CTRL_MODE		(1 << 1)
+	#define SPI_CTRL_REG_XCH_BIT	(1 << 2)
+	#define SPI_CTRL_SSPOL		(1 << 7)
+	#define SPI_CTRL_SSPOL_OFF	(7)
+	#define SPI_CTRL_SSCTL		(1 << 6)
+	#define SPI_CTRL_SSCTL_OFF	(6)
+	#define SPI_CTRL_SCLK_POL	(1 << 4)
+	#define SPI_CTRL_SCLK_POL_OFF	(4)
+	#define SPI_CTRL_SCLK_PHA	(1 << 5)
+	#define SPI_CTRL_SCLK_PHA_OFF	(5)
+	#define SPI_CTRL_SS_OFF		(12)
+	#define SPI_CTRL_SS_MASK	(3 << 12)
+	#define SPI_CTRL_DATA_OFF	(16)
+	#define SPI_CTRL_DATA_MASK	(7 << 16)
+	#define SPI_CTRL_BURST_OFF	(20)
+	#define SPI_CTRL_BURST_MASK	(0xFFF << 20)
+	#define SPI_INT_STAT_TC		(1 << 7)

include/imx_spi.h line 70

Quoted 10 lines
include/imx_spi.h new file +10 -0
+
+#elif defined(IMX_CSPI_VER_2_3)
+	#define	SPI_RX_DATA		0x0
+	#define SPI_TX_DATA		0x4
+	#define SPI_CON_REG		0x8
+	#define SPI_CFG_REG		0xC
+	#define SPI_INT_REG		0x10
+	#define SPI_DMA_REG		0x14
+	#define SPI_STAT_REG		0x18
+	#define SPI_PERIOD_REG		0x1C

Use struct-based access

include/imx_spi.h line 78

Quoted 8 lines
include/imx_spi.h new file +8 -0
+#endif
+
+struct spi_reg_t {
+	u32 ctrl_reg;
+	u32 cfg_reg;
+};
+
+struct imx_spi_dev_t {

Drop the _t suffix

Quoted 19 lines
include/imx_spi.h new file +19 -0
+	struct spi_slave slave;
+	u32 base;      /* base address of SPI module */
+	u32 freq;      /* desired clock freq in Hz for this device */
+	u32 ss_pol;    /* ss polarity: 1=active high; 0=active low */
+	u32 ss;        /* slave select */
+	u32 in_sctl;   /* inactive sclk ctl: 1=stay low; 0=stay high */
+	u32 in_dctl;   /* inactive data ctl: 1=stay low; 0=stay high */
+	u32 ssctl;     /* single burst mode vs multiple: 0=single; 1=multi */
+	u32 sclkpol;   /* sclk polarity: active high=0; active low=1 */
+	u32 sclkpha;   /* sclk phase: 0=phase 0; 1=phase1 */
+	u32 fifo_sz;   /* fifo size in bytes for either tx or rx. */
+	u32 us_delay;  /* us delay in each xfer */
+	struct spi_reg_t reg; /* pointer to a set of SPI registers */
+};
+
+extern void spi_io_init(struct imx_spi_dev_t *dev, int active);
+extern s32 spi_get_cfg(struct imx_spi_dev_t *dev);
+
+#endif /* __IMX_SPI_H__ */
author
#9 lore raw

PATCH 1/2 SPI: Add i.MX ECSPI driver

Quoted 6 lines · Marek Vasut
Quoted 6 lines

From: Eric Nelson <[email protected]>

CC Jason Liu <[email protected]>
CC Stefano Babic <[email protected]>

---

...

Quoted 4 lines

drivers/spi/imx_ecspi.c line 127

Quoted 2 lines
drivers/spi/imx_ecspi.c new file +2 -0
+     if (max_hz < imx_spi_slave->freq)
+             imx_spi_slave->freq = max_hz ;

" ;" <-- fix globally ... run tools/checkpatch.pl before submitting

First, many thanks for the review!

Just one question: Which checkpatch do you use for this?

I ran checkpatch from Linux 3.2 before submitting and got [1] (which I think is ok). Using U-Boot's checkpatch I get [2].

Many thanks again and bet regards

Dirk

[1]

linux-2.6.git/scripts/checkpatch.pl 0001-SPI-Add-i.MX-ECSPI-driver.patch

WARNING: Use #include <linux/errno.h> instead of <asm/errno.h> #60: FILE: drivers/spi/imx_ecspi.c:26:
+#include <asm/errno.h>

WARNING: Use #include <linux/io.h> instead of <asm/io.h> #62: FILE: drivers/spi/imx_ecspi.c:28:
+#include <asm/io.h>

total: 0 errors, 2 warnings, 438 lines checked

0001-SPI-Add-i.MX-ECSPI-driver.patch has style problems, please review.
If any of these errors
are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.

[2]

u-boot/tools/checkpatch.pl 0001-SPI-Add-i.MX-ECSPI-driver.patch

total: 0 errors, 0 warnings, 438 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE

0001-SPI-Add-i.MX-ECSPI-driver.patch has no obvious style problems and is ready for submission.

#10 lore raw

PATCH 1/2 SPI: Add i.MX ECSPI driver

Quoted 15 lines · Marek Vasut
Quoted 6 lines
Quoted 6 lines

From: Eric Nelson <[email protected]>

CC Jason Liu <[email protected]>
CC Stefano Babic <[email protected]>

---

...

Quoted 4 lines

drivers/spi/imx_ecspi.c line 127

Quoted 2 lines
drivers/spi/imx_ecspi.c new file +2 -0
+     if (max_hz < imx_spi_slave->freq)
+             imx_spi_slave->freq = max_hz ;

" ;" <-- fix globally ... run tools/checkpatch.pl before submitting

Quoted 8 lines

First, many thanks for the review!

Just one question: Which checkpatch do you use for this?

I ran checkpatch from Linux 3.2 before submitting and got [1] (which I think is ok). Using U-Boot's checkpatch I get [2].

Many thanks again and bet regards

Ok then, it's probably another issue checpatch doesn't catch.

M

Quoted 34 lines

Dirk

[1]

linux-2.6.git/scripts/checkpatch.pl 0001-SPI-Add-i.MX-ECSPI-driver.patch

WARNING: Use #include <linux/errno.h> instead of <asm/errno.h> #60: FILE: drivers/spi/imx_ecspi.c:26:
+#include <asm/errno.h>

WARNING: Use #include <linux/io.h> instead of <asm/io.h> #62: FILE: drivers/spi/imx_ecspi.c:28:
+#include <asm/io.h>

total: 0 errors, 2 warnings, 438 lines checked

0001-SPI-Add-i.MX-ECSPI-driver.patch has style problems, please review.
If any of these errors
are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.

[2]

u-boot/tools/checkpatch.pl 0001-SPI-Add-i.MX-ECSPI-driver.patch

total: 0 errors, 0 warnings, 438 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE

0001-SPI-Add-i.MX-ECSPI-driver.patch has no obvious style problems and is ready for submission.

reviewer
#11 lore raw

PATCH 1/2 SPI: Add i.MX ECSPI driver

In reply to Dirk Behme

Review

Quoted 10 lines · Dirk Behme
(quoted diff) +10 -0
+static s32 spi_reset(struct spi_slave *slave)
+{
+ ? ? ? u32 clk_src = mxc_get_clock(MXC_CSPI_CLK);
+ ? ? ? u32 pre_div = 0, post_div = 0, reg_ctrl, reg_config;
+ ? ? ? struct imx_spi_dev_t *dev = to_imx_spi_slave(slave);
+ ? ? ? struct spi_reg_t *reg = &(dev->reg);
+
+ ? ? ? if (dev->freq == 0) {
+ ? ? ? ? ? ? ? printf("Error: desired clock is 0\n");
+ ? ? ? ? ? ? ? return 1;

I think that returning a negative error code, such as -EINVAL would be better here.

Regards,

Fabio Estevam

#12 lore raw

PATCH 1/2 SPI: Add i.MX ECSPI driver

In reply to Dirk Behme
Quoted 5 lines · Dirk Behme

From: Eric Nelson <[email protected]>

CC Jason Liu <[email protected]>
CC Stefano Babic <[email protected]>
Quoted 8 lines
---
 drivers/spi/Makefile    |    1 +
 drivers/spi/imx_ecspi.c |  334 +++++++++++++++++++++++++++++++++++++++++++++++
 include/imx_spi.h       |   97 ++++++++++++++
 3 files changed, 432 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/imx_ecspi.c
 create mode 100644 include/imx_spi.h

Hi Dirk,

before digging too deep inside the driver: I do not see any apoparent reason why we must add a separate driver instead of adapting what we currently have. And most part are quite identical to drivers/spi/mxc_spi.c. We should change mxc_spi.c for the imx6 relevant parts, instead of adding a new copy.

Quoted 47 lines

new file mode 100644
index 0000000..1468208

drivers/spi/imx_ecspi.c new file +42 -0
@@ -0,0 +1,334 @@
+/*
+ * (C) Copyright 2008-2010 Freescale Semiconductor, Inc.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <config.h>
+#include <common.h>
+#include <spi.h>
+#include <asm/errno.h>
+#include <linux/types.h>
+#include <asm/io.h>
+#include <malloc.h>
+#include <asm/arch/clock.h>
+#include <imx_spi.h>
+
+static inline struct imx_spi_dev_t *to_imx_spi_slave(struct spi_slave *slave)
+{
+	return container_of(slave, struct imx_spi_dev_t, slave);
+}
+
+static s32 spi_reset(struct spi_slave *slave)
+{
+	u32 clk_src = mxc_get_clock(MXC_CSPI_CLK);
+	u32 pre_div = 0, post_div = 0, reg_ctrl, reg_config;
+	struct imx_spi_dev_t *dev = to_imx_spi_slave(slave);

drivers/spi/imx_ecspi.c line 52

Quoted 10 lines
drivers/spi/imx_ecspi.c new file +10 -0
+	struct spi_reg_t *reg = &(dev->reg);
+
+	if (dev->freq == 0) {
+		printf("Error: desired clock is 0\n");
+		return 1;
+	}
+
+	reg_ctrl = readl(dev->base + SPI_CON_REG);
+	reg_config = readl(dev->base + SPI_CFG_REG);
+	/* Reset spi */

The functiion reassembles spi_cfg_mxc in mxc_spi.c, usinf fixed offset instead of structures.

Quoted 25 lines
drivers/spi/imx_ecspi.c new file +25 -0
+/*
+ * SPI xchg
+ */
+
+int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
+	const u8 *dout, u8 *din, unsigned long flags)
+{
+	int nbytes = (bitlen + 7) / 8;
+	struct imx_spi_dev_t *dev = to_imx_spi_slave(slave);
+	struct spi_reg_t *spi_reg = &(dev->reg);
+	u32 loop_cnt ;
+	if (!slave)
+		return -1;
+
+	if (spi_reg->ctrl_reg == 0) {
+		printf("Error: spi(base=0x%x) has not been initialized yet\n",
+				dev->base);
+		return -1;
+	}
+	spi_reg->ctrl_reg = (spi_reg->ctrl_reg & ~0xFFF00000) | \
+			((bitlen - 1) << 20);
+
+	writel(spi_reg->ctrl_reg, dev->base + SPI_CON_REG);
+	writel(spi_reg->cfg_reg, dev->base + SPI_CFG_REG);
+

drivers/spi/imx_ecspi.c line 189

Quoted 10 lines
drivers/spi/imx_ecspi.c new file +10 -0
+	/* move data to the tx fifo */
+	debug("dout=0x%p, bitlen=%x\n", dout, bitlen);
+
+	/*
+	 * The SPI controller works only with words,
+	 * check if less than a word is sent.
+	 * Access to the FIFO is only 32 bit
+	 */
+	if (bitlen % 32) {
+		u32 data = 0;

I see no specific i.MX6 code here, and the function is not very different as spi_xchg_single(). Really I do not think we can add a copy of the already provided driver, sorry.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
author
#13 lore raw

PATCH 1/2 SPI: Add i.MX ECSPI driver

Quoted 115 lines · Stefano Babic
Quoted 5 lines · Dirk Behme

From: Eric Nelson <[email protected]>

CC Jason Liu <[email protected]>
CC Stefano Babic <[email protected]>
Quoted 8 lines
---
 drivers/spi/Makefile    |    1 +
 drivers/spi/imx_ecspi.c |  334 +++++++++++++++++++++++++++++++++++++++++++++++
 include/imx_spi.h       |   97 ++++++++++++++
 3 files changed, 432 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/imx_ecspi.c
 create mode 100644 include/imx_spi.h

Hi Dirk,

before digging too deep inside the driver: I do not see any apoparent reason why we must add a separate driver instead of adapting what we currently have. And most part are quite identical to drivers/spi/mxc_spi.c. We should change mxc_spi.c for the imx6 relevant parts, instead of adding a new copy.

Quoted 47 lines

new file mode 100644
index 0000000..1468208

drivers/spi/imx_ecspi.c new file +42 -0
@@ -0,0 +1,334 @@
+/*
+ * (C) Copyright 2008-2010 Freescale Semiconductor, Inc.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <config.h>
+#include <common.h>
+#include <spi.h>
+#include <asm/errno.h>
+#include <linux/types.h>
+#include <asm/io.h>
+#include <malloc.h>
+#include <asm/arch/clock.h>
+#include <imx_spi.h>
+
+static inline struct imx_spi_dev_t *to_imx_spi_slave(struct spi_slave *slave)
+{
+	return container_of(slave, struct imx_spi_dev_t, slave);
+}
+
+static s32 spi_reset(struct spi_slave *slave)
+{
+	u32 clk_src = mxc_get_clock(MXC_CSPI_CLK);
+	u32 pre_div = 0, post_div = 0, reg_ctrl, reg_config;
+	struct imx_spi_dev_t *dev = to_imx_spi_slave(slave);

drivers/spi/imx_ecspi.c line 52

Quoted 10 lines
drivers/spi/imx_ecspi.c new file +10 -0
+	struct spi_reg_t *reg = &(dev->reg);
+
+	if (dev->freq == 0) {
+		printf("Error: desired clock is 0\n");
+		return 1;
+	}
+
+	reg_ctrl = readl(dev->base + SPI_CON_REG);
+	reg_config = readl(dev->base + SPI_CFG_REG);
+	/* Reset spi */

The functiion reassembles spi_cfg_mxc in mxc_spi.c, usinf fixed offset instead of structures.

Quoted 31 lines
drivers/spi/imx_ecspi.c new file +31 -0
+/*
+ * SPI xchg
+ */
+
+int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
+	const u8 *dout, u8 *din, unsigned long flags)
+{
+	int nbytes = (bitlen + 7) / 8;
+	struct imx_spi_dev_t *dev = to_imx_spi_slave(slave);
+	struct spi_reg_t *spi_reg = &(dev->reg);
+	u32 loop_cnt ;
+	if (!slave)
+		return -1;
+
+	if (spi_reg->ctrl_reg == 0) {
+		printf("Error: spi(base=0x%x) has not been initialized yet\n",
+				dev->base);
+		return -1;
+	}
+	spi_reg->ctrl_reg = (spi_reg->ctrl_reg & ~0xFFF00000) | \
+			((bitlen - 1) << 20);
+
+	writel(spi_reg->ctrl_reg, dev->base + SPI_CON_REG);
+	writel(spi_reg->cfg_reg, dev->base + SPI_CFG_REG);
+
+	/* move data to the tx fifo */
+	debug("dout=0x%p, bitlen=%x\n", dout, bitlen);
+
+	/*
+	 * The SPI controller works only with words,
+	 * check if less than a word is sent.
Quoted 8 lines

drivers/spi/imx_ecspi.c line 189

Quoted 4 lines
drivers/spi/imx_ecspi.c new file +4 -0
+	 * Access to the FIFO is only 32 bit
+	 */
+	if (bitlen % 32) {
+		u32 data = 0;

I see no specific i.MX6 code here, and the function is not very different as spi_xchg_single(). Really I do not think we can add a copy of the already provided driver, sorry.

Yes, understood, see

http://lists.denx.de/pipermail/u-boot/2012-January/115611.html

Let's have Eric a look to this.

Anyway, many thanks for looking at it,

Dirk

#14 lore raw

PATCH 1/2 SPI: Add i.MX ECSPI driver

Quoted 14 lines · Dirk Behme
Quoted 9 lines · Stefano Babic
Quoted 7 lines · Dirk Behme

From: Eric Nelson <[email protected]>

CC Jason Liu <[email protected]>
CC Stefano Babic <[email protected]>

---
<snip>

Quoted 3 lines

I see no specific i.MX6 code here, and the function is not very different as spi_xchg_single(). Really I do not think we can add a copy of the already provided driver, sorry.

Quoted 6 lines

Yes, understood, see

http://lists.denx.de/pipermail/u-boot/2012-January/115611.html

Let's have Eric a look to this.

Good catch guys.

The mxc_spi driver already has support for the ECSPI on i.MX6, and just needs a small amount of glue to function.

FWIW, it should also be easy to add support for i.MX53.

Regards,

Eric

#15 lore raw

PATCH 1/2 SPI: Add i.MX ECSPI driver

Quoted 5 lines · Eric Nelson
Quoted 1 line

Good catch guys.

The mxc_spi driver already has support for the ECSPI on i.MX6, and just needs a small amount of glue to function.

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.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#16 lore raw

PATCH 1/2 SPI: Add i.MX ECSPI driver

In reply to Dirk Behme

drivers/spi/imx_ecspi.c line 111

Quoted 4 lines · On Thursday 12 January 2012 10:27:13 Dirk Behme wrote:
drivers/spi/imx_ecspi.c new file +4 -0
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+		unsigned int max_hz, unsigned int mode)
+{
+	struct imx_spi_dev_t *imx_spi_slave = NULL;

setting to NULL is kind of pointless when you init it immediately below

drivers/spi/imx_ecspi.c line 117

Quoted 2 lines
drivers/spi/imx_ecspi.c new file +2 -0
+	imx_spi_slave = (struct imx_spi_dev_t *)
+			calloc(sizeof(struct imx_spi_dev_t), 1);

no need for that cast on the return of calloc

drivers/spi/imx_ecspi.c line 128

Quoted 2 lines
drivers/spi/imx_ecspi.c new file +2 -0
+	spi_get_cfg(imx_spi_slave);
+	spi_io_init(imx_spi_slave, 0);

i don't see these funcs defined anywhere in this patch. since they aren't part of the common SPI API, you should namespace them accordingly (like with an SoC prefix or something).

drivers/spi/imx_ecspi.c line 130

Quoted 1 line
drivers/spi/imx_ecspi.c new file +1 -0
+	spi_reset(&(imx_spi_slave->slave));

the inner params are not needed. also, programming of hardware does not happen in the spi_setup_slave() step. that's what the spi_claim_bus() is for.

drivers/spi/imx_ecspi.c line 132

Quoted 1 line
drivers/spi/imx_ecspi.c new file +1 -0
+	return &(imx_spi_slave->slave);

drop the paren

drivers/spi/imx_ecspi.c line 143

Quoted 9 lines
drivers/spi/imx_ecspi.c new file +9 -0
+void spi_free_slave(struct spi_slave *slave)
+{
+	struct imx_spi_dev_t *imx_spi_slave;
+
+	if (slave) {
+		imx_spi_slave = to_imx_spi_slave(slave);
+		free(imx_spi_slave);
+	}
+}

the NULL check on "slave" is not necessary. we assume everywhere else that it is valid ahead of time.

drivers/spi/imx_ecspi.c line 160

Quoted 2 lines
drivers/spi/imx_ecspi.c new file +2 -0
+int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
+	const u8 *dout, u8 *din, unsigned long flags)

static

drivers/spi/imx_ecspi.c line 171

Quoted 3 lines
drivers/spi/imx_ecspi.c new file +3 -0
+	if (spi_reg->ctrl_reg == 0) {
+		printf("Error: spi(base=0x%x) has not been initialized yet\n",
+				dev->base);

not necessary either ... we don't bother supporting broken callers in the bus drivers

drivers/spi/imx_ecspi.c line 167

Quoted 5 lines
drivers/spi/imx_ecspi.c new file +4 -0
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void
+		*dout, void *din, unsigned long flags)
...
+	if (!slave)
+		return -1;

not necessary either

drivers/spi/imx_ecspi.c line 320

Quoted 4 lines
drivers/spi/imx_ecspi.c new file +4 -0
+int spi_cs_is_valid(unsigned int bus, unsigned int cs)
+{
+	return 1;
+}

this can't be right ...
-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/20120114/dd879533/attachment.pgp>