Re: [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

2012-01-16 Thread Eric Nelson

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.nel...@boundarydevices.com

Signed-off-by: Eric Nelson eric.nel...@boundarydevices.com
CC: Jason Liu jason@linaro.org
CC: Stefano Babic sba...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

2012-01-16 Thread Stefano Babic
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: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

2012-01-14 Thread Mike Frysinger
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


signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

2012-01-13 Thread Stefano Babic
On 12/01/2012 16:27, Dirk Behme wrote:
 From: Eric Nelson eric.nel...@boundarydevices.com
 
 Signed-off-by: Eric Nelson eric.nel...@boundarydevices.com
 CC: Jason Liu jason@linaro.org
 CC: Stefano Babic sba...@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 000..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  ~0xFFF0) | \
 + ((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: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

2012-01-13 Thread Dirk Behme

On 13.01.2012 11:48, Stefano Babic wrote:

On 12/01/2012 16:27, Dirk Behme wrote:

From: Eric Nelson eric.nel...@boundarydevices.com

Signed-off-by: Eric Nelson eric.nel...@boundarydevices.com
CC: Jason Liu jason@linaro.org
CC: Stefano Babic sba...@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 000..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  ~0xFFF0) | \
+   ((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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

2012-01-13 Thread Marek Vasut
 On 12.01.2012 16:37, Marek Vasut wrote:
  From: Eric Nelson eric.nel...@boundarydevices.com
  
  Signed-off-by: Eric Nelson eric.nel...@boundarydevices.com
  CC: Jason Liu jason@linaro.org
  CC: Stefano Babic sba...@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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

2012-01-12 Thread Dirk Behme
From: Eric Nelson eric.nel...@boundarydevices.com

Signed-off-by: Eric Nelson eric.nel...@boundarydevices.com
CC: Jason Liu jason@linaro.org
CC: Stefano Babic sba...@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 000..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 = ~(0x11  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 

Re: [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

2012-01-12 Thread Marek Vasut
 From: Eric Nelson eric.nel...@boundarydevices.com
 
 Signed-off-by: Eric Nelson eric.nel...@boundarydevices.com
 CC: Jason Liu jason@linaro.org
 CC: Stefano Babic sba...@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 000..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 = ~(0x11  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)
 +{
 + 

Re: [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

2012-01-12 Thread Fabio Estevam
On Thu, Jan 12, 2012 at 1:27 PM, Dirk Behme dirk.be...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/2] SPI: Add i.MX ECSPI driver

2012-01-12 Thread Dirk Behme

On 12.01.2012 16:37, Marek Vasut wrote:

From: Eric Nelson eric.nel...@boundarydevices.com

Signed-off-by: Eric Nelson eric.nel...@boundarydevices.com
CC: Jason Liu jason@linaro.org
CC: Stefano Babic sba...@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.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot