All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver
@ 2012-01-12 15:27 Dirk Behme
  2012-01-12 15:27 ` [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support Dirk Behme
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Dirk Behme @ 2012-01-12 15:27 UTC (permalink / raw)
  To: u-boot

From: Eric Nelson <eric.nelson@boundarydevices.com>

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
CC: Jason Liu <jason.hui@linaro.org>
CC: Stefano Babic <sbabic@denx.de>
---
 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

diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index c967d87..e27ef41 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -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
diff --git a/drivers/spi/imx_ecspi.c b/drivers/spi/imx_ecspi.c
new file mode 100644
index 0000000..1468208
--- /dev/null
+++ b/drivers/spi/imx_ecspi.c
@@ -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);
+	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++;
+	}
+	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));
+	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);
+	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 ;
+	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) {
+			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));
+		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)
+{
+	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);
+}
diff --git a/include/imx_spi.h b/include/imx_spi.h
new file mode 100644
index 0000000..32544d9
--- /dev/null
+++ b/include/imx_spi.h
@@ -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)
+
+#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
+#endif
+
+struct spi_reg_t {
+	u32 ctrl_reg;
+	u32 cfg_reg;
+};
+
+struct imx_spi_dev_t {
+	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__ */
-- 
1.7.0.4

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

* [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support
  2012-01-12 15:27 [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver Dirk Behme
@ 2012-01-12 15:27 ` Dirk Behme
  2012-01-12 15:32   ` Fabio Estevam
                     ` (2 more replies)
  2012-01-12 15:37 ` [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver Marek Vasut
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 17+ messages in thread
From: Dirk Behme @ 2012-01-12 15:27 UTC (permalink / raw)
  To: u-boot

From: Eric Nelson <eric.nelson@boundarydevices.com>

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
CC: Jason Liu <jason.hui@linaro.org>
CC: Stefano Babic <sbabic@denx.de>
---
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(-)

diff --git a/board/freescale/mx6qsabrelite/imximage.cfg b/board/freescale/mx6qsabrelite/imximage.cfg
index 83dee6f..c389427 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 4028789..d69adfa 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -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);
@@ -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 ;
+		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;
 }
 
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index 464f0ec..48db42c 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -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
-- 
1.7.0.4

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

* [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support
  2012-01-12 15:27 ` [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support Dirk Behme
@ 2012-01-12 15:32   ` Fabio Estevam
  2012-01-12 15:38   ` Marek Vasut
  2012-01-15  1:04   ` Mike Frysinger
  2 siblings, 0 replies; 17+ messages in thread
From: Fabio Estevam @ 2012-01-12 15:32 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 12, 2012 at 1:27 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:

> +#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

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

* [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver
  2012-01-12 15:27 [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver Dirk Behme
  2012-01-12 15:27 ` [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support Dirk Behme
@ 2012-01-12 15:37 ` Marek Vasut
  2012-01-13  7:32   ` Dirk Behme
  2012-01-12 15:40 ` Fabio Estevam
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2012-01-12 15:37 UTC (permalink / raw)
  To: u-boot

> From: Eric Nelson <eric.nelson@boundarydevices.com>
> 
> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> CC: Jason Liu <jason.hui@linaro.org>
> CC: Stefano Babic <sbabic@denx.de>
> ---
>  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
> 
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index c967d87..e27ef41 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -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
> diff --git a/drivers/spi/imx_ecspi.c b/drivers/spi/imx_ecspi.c
> new file mode 100644
> index 0000000..1468208
> --- /dev/null
> +++ b/drivers/spi/imx_ecspi.c
> @@ -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);
> +	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++;
> +	}
> +	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

> +	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);
> +	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

> +	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) {
> +			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.

> +		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)
> +{
> +	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);
> +}
> diff --git a/include/imx_spi.h b/include/imx_spi.h
> new file mode 100644
> index 0000000..32544d9
> --- /dev/null
> +++ b/include/imx_spi.h
> @@ -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)
> +
> +#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
> +#endif
> +
> +struct spi_reg_t {
> +	u32 ctrl_reg;
> +	u32 cfg_reg;
> +};
> +
> +struct imx_spi_dev_t {

Drop the _t suffix

> +	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__ */

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

* [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support
  2012-01-12 15:27 ` [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support Dirk Behme
  2012-01-12 15:32   ` Fabio Estevam
@ 2012-01-12 15:38   ` Marek Vasut
  2012-01-12 15:48     ` Fabio Estevam
  2012-01-15  1:05     ` Mike Frysinger
  2012-01-15  1:04   ` Mike Frysinger
  2 siblings, 2 replies; 17+ messages in thread
From: Marek Vasut @ 2012-01-12 15:38 UTC (permalink / raw)
  To: u-boot

> From: Eric Nelson <eric.nelson@boundarydevices.com>
> 
> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> CC: Jason Liu <jason.hui@linaro.org>
> CC: Stefano Babic <sbabic@denx.de>
> ---
> 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(-)
> 
> diff --git a/board/freescale/mx6qsabrelite/imximage.cfg
> b/board/freescale/mx6qsabrelite/imximage.cfg index 83dee6f..c389427 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 4028789..d69adfa
> 100644
> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> @@ -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);
> @@ -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
> +		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;
>  }
> 
> diff --git a/include/configs/mx6qsabrelite.h
> b/include/configs/mx6qsabrelite.h index 464f0ec..48db42c 100644
> --- a/include/configs/mx6qsabrelite.h
> +++ b/include/configs/mx6qsabrelite.h
> @@ -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

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

* [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver
  2012-01-12 15:27 [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver Dirk Behme
  2012-01-12 15:27 ` [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support Dirk Behme
  2012-01-12 15:37 ` [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver Marek Vasut
@ 2012-01-12 15:40 ` Fabio Estevam
  2012-01-13 10:48 ` Stefano Babic
  2012-01-15  1:04 ` Mike Frysinger
  4 siblings, 0 replies; 17+ messages in thread
From: Fabio Estevam @ 2012-01-12 15:40 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 12, 2012 at 1:27 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:

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

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

* [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support
  2012-01-12 15:38   ` Marek Vasut
@ 2012-01-12 15:48     ` Fabio Estevam
  2012-01-13  7:19       ` Dirk Behme
  2012-01-15  1:05     ` Mike Frysinger
  1 sibling, 1 reply; 17+ messages in thread
From: Fabio Estevam @ 2012-01-12 15:48 UTC (permalink / raw)
  To: u-boot

On Thu, Jan 12, 2012 at 1:38 PM, Marek Vasut <marek.vasut@gmail.com> wrote:

> 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

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

* [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support
  2012-01-12 15:48     ` Fabio Estevam
@ 2012-01-13  7:19       ` Dirk Behme
  0 siblings, 0 replies; 17+ messages in thread
From: Dirk Behme @ 2012-01-13  7:19 UTC (permalink / raw)
  To: u-boot

On 12.01.2012 16:48, Fabio Estevam wrote:
> On Thu, Jan 12, 2012 at 1:38 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> 
>> 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

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

* [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver
  2012-01-12 15:37 ` [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver Marek Vasut
@ 2012-01-13  7:32   ` Dirk Behme
  2012-01-13 12:17     ` Marek Vasut
  0 siblings, 1 reply; 17+ messages in thread
From: Dirk Behme @ 2012-01-13  7:32 UTC (permalink / raw)
  To: u-boot

On 12.01.2012 16:37, Marek Vasut wrote:
>> From: Eric Nelson <eric.nelson@boundarydevices.com>
>>
>> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
>> CC: Jason Liu <jason.hui@linaro.org>
>> CC: Stefano Babic <sbabic@denx.de>
>> ---
...
>> +     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.

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

* [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver
  2012-01-12 15:27 [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver Dirk Behme
                   ` (2 preceding siblings ...)
  2012-01-12 15:40 ` Fabio Estevam
@ 2012-01-13 10:48 ` Stefano Babic
  2012-01-13 11:45   ` Dirk Behme
  2012-01-15  1:04 ` Mike Frysinger
  4 siblings, 1 reply; 17+ messages in thread
From: Stefano Babic @ 2012-01-13 10:48 UTC (permalink / raw)
  To: u-boot

On 12/01/2012 16:27, Dirk Behme wrote:
> From: Eric Nelson <eric.nelson@boundarydevices.com>
> 
> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> CC: Jason Liu <jason.hui@linaro.org>
> CC: Stefano Babic <sbabic@denx.de>
> ---
>  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.

> new file mode 100644
> index 0000000..1468208
> --- /dev/null
> +++ b/drivers/spi/imx_ecspi.c
> @@ -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);
> +	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.

> +/*
> + * 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;

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

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

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

* [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver
  2012-01-13 10:48 ` Stefano Babic
@ 2012-01-13 11:45   ` Dirk Behme
  2012-01-16 17:29     ` Eric Nelson
  0 siblings, 1 reply; 17+ messages in thread
From: Dirk Behme @ 2012-01-13 11:45 UTC (permalink / raw)
  To: u-boot

On 13.01.2012 11:48, Stefano Babic wrote:
> On 12/01/2012 16:27, Dirk Behme wrote:
>> From: Eric Nelson <eric.nelson@boundarydevices.com>
>>
>> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
>> CC: Jason Liu <jason.hui@linaro.org>
>> CC: Stefano Babic <sbabic@denx.de>
>> ---
>>  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.
> 
>> new file mode 100644
>> index 0000000..1468208
>> --- /dev/null
>> +++ b/drivers/spi/imx_ecspi.c
>> @@ -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);
>> +	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.
> 
>> +/*
>> + * 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;
> 
> 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

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

* [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver
  2012-01-13  7:32   ` Dirk Behme
@ 2012-01-13 12:17     ` Marek Vasut
  0 siblings, 0 replies; 17+ messages in thread
From: Marek Vasut @ 2012-01-13 12:17 UTC (permalink / raw)
  To: u-boot

> On 12.01.2012 16:37, Marek Vasut wrote:
> >> From: Eric Nelson <eric.nelson@boundarydevices.com>
> >> 
> >> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> >> CC: Jason Liu <jason.hui@linaro.org>
> >> CC: Stefano Babic <sbabic@denx.de>
> >> ---
> 
> ...
> 
> >> +     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

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

M
> 
> 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.

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

* [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver
  2012-01-12 15:27 [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver Dirk Behme
                   ` (3 preceding siblings ...)
  2012-01-13 10:48 ` Stefano Babic
@ 2012-01-15  1:04 ` Mike Frysinger
  4 siblings, 0 replies; 17+ messages in thread
From: Mike Frysinger @ 2012-01-15  1:04 UTC (permalink / raw)
  To: u-boot

On Thursday 12 January 2012 10:27:13 Dirk Behme wrote:
> +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

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

> +	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).

> +	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.

> +	return &(imx_spi_slave->slave);

drop the paren

> +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.

> +int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
> +	const u8 *dout, u8 *din, unsigned long flags)

static

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

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

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

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

* [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support
  2012-01-12 15:27 ` [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support Dirk Behme
  2012-01-12 15:32   ` Fabio Estevam
  2012-01-12 15:38   ` Marek Vasut
@ 2012-01-15  1:04   ` Mike Frysinger
  2 siblings, 0 replies; 17+ messages in thread
From: Mike Frysinger @ 2012-01-15  1:04 UTC (permalink / raw)
  To: u-boot

On Thursday 12 January 2012 10:27:14 Dirk Behme wrote:
> +#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>

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

* [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support
  2012-01-12 15:38   ` Marek Vasut
  2012-01-12 15:48     ` Fabio Estevam
@ 2012-01-15  1:05     ` Mike Frysinger
  1 sibling, 0 replies; 17+ messages in thread
From: Mike Frysinger @ 2012-01-15  1:05 UTC (permalink / raw)
  To: u-boot

On Thursday 12 January 2012 10:38:40 Marek Vasut wrote:
> > From: Eric Nelson <eric.nelson@boundarydevices.com>
> > +#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?

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>

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

* [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver
  2012-01-13 11:45   ` Dirk Behme
@ 2012-01-16 17:29     ` Eric Nelson
  2012-01-16 17:38       ` Stefano Babic
  0 siblings, 1 reply; 17+ messages in thread
From: Eric Nelson @ 2012-01-16 17:29 UTC (permalink / raw)
  To: u-boot

On 01/13/2012 04:45 AM, Dirk Behme wrote:
> On 13.01.2012 11:48, Stefano Babic wrote:
>> On 12/01/2012 16:27, Dirk Behme wrote:
>>> From: Eric Nelson <eric.nelson@boundarydevices.com>
>>>
>>> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
>>> CC: Jason Liu <jason.hui@linaro.org>
>>> CC: Stefano Babic <sbabic@denx.de>
>>> ---
>>> <snip>
>>
>> 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.
>
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

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

* [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver
  2012-01-16 17:29     ` Eric Nelson
@ 2012-01-16 17:38       ` Stefano Babic
  0 siblings, 0 replies; 17+ messages in thread
From: Stefano Babic @ 2012-01-16 17:38 UTC (permalink / raw)
  To: u-boot

On 16/01/2012 18:29, Eric Nelson wrote:

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

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

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

end of thread, other threads:[~2012-01-16 17:38 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-12 15:27 [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver Dirk Behme
2012-01-12 15:27 ` [U-Boot] [PATCH 2/2] i.mx6q: SabreLite: Add SPI NOR support Dirk Behme
2012-01-12 15:32   ` Fabio Estevam
2012-01-12 15:38   ` Marek Vasut
2012-01-12 15:48     ` Fabio Estevam
2012-01-13  7:19       ` Dirk Behme
2012-01-15  1:05     ` Mike Frysinger
2012-01-15  1:04   ` Mike Frysinger
2012-01-12 15:37 ` [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver Marek Vasut
2012-01-13  7:32   ` Dirk Behme
2012-01-13 12:17     ` Marek Vasut
2012-01-12 15:40 ` Fabio Estevam
2012-01-13 10:48 ` Stefano Babic
2012-01-13 11:45   ` Dirk Behme
2012-01-16 17:29     ` Eric Nelson
2012-01-16 17:38       ` Stefano Babic
2012-01-15  1:04 ` Mike Frysinger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.