Hi,

Few comments, please get back your inputs.

Use commit header as "spi: ftssp010_spi: "

On 07-05-2013 12:04, Kuo-Jung Su wrote:
From: Kuo-Jung Su <dant...@faraday-tech.com>

The Faraday FTSSP010 is a multi-function controller
which supports I2S/SPI/SSP/AC97/SPDIF. However This
patch implements only the SPI mode.

NOTE:
The DMA and CS/Clock control logic has been altered
since hardware revision 1.19.0. So this patch
would first detects the revision id of the underlying
chip, and then switch to the corresponding software
control routines.

Signed-off-by: Kuo-Jung Su <dant...@faraday-tech.com>
CC: Tom Rini <tr...@ti.com>

---
Changes for v4:
    - Coding Style cleanup.
    - Make it a separate patch, rather then a part of
      Faraday A36x patch series
    - Use macro constants for timeout control

Changes for v3:
    - Coding Style cleanup.
    - Drop macros for wirtel()/readl(), call them directly.
    - Always insert a blank line between declarations and code.
    - Replace all the infinite wait loop with a timeout.
    - Add '__iomem' to all the declaration of HW register pointers.

Changes for v2:
    - Coding Style cleanup.
    - Use readl(), writel(), clrsetbits_le32() to replace REG() macros.
    - Use structure based hardware registers to replace the macro constants.
    - Replace BIT() with BIT_MASK().

  drivers/spi/Makefile        |    1 +
  drivers/spi/ftssp010_spi.c  |  385 +++++++++++++++++++++++++++++++++++++++++++
  drivers/spi/ftssp010_spi.h  |   86 ++++++++++
  include/faraday/ftgpio010.h |   25 +++
  4 files changed, 497 insertions(+)
  create mode 100644 drivers/spi/ftssp010_spi.c
  create mode 100644 drivers/spi/ftssp010_spi.h
  create mode 100644 include/faraday/ftgpio010.h

--
1.7.9.5

diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index d08609e..947d60e 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -38,6 +38,7 @@ COBJS-$(CONFIG_BFIN_SPI6XX) += bfin_spi6xx.o
  COBJS-$(CONFIG_CF_SPI) += cf_spi.o
  COBJS-$(CONFIG_CF_QSPI) += cf_qspi.o
  COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
+COBJS-$(CONFIG_FTSSP010_SPI) += ftssp010_spi.o

Place into in alphabetic order, to make sure some kind of coding style.

  COBJS-$(CONFIG_EXYNOS_SPI) += exynos_spi.o
  COBJS-$(CONFIG_ICH_SPI) +=  ich.o
  COBJS-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
diff --git a/drivers/spi/ftssp010_spi.c b/drivers/spi/ftssp010_spi.c
new file mode 100644
index 0000000..d401ecc
--- /dev/null
+++ b/drivers/spi/ftssp010_spi.c
@@ -0,0 +1,385 @@
+/*
+ * Faraday Multi-function Controller - SPI Mode
+ *
+ * (C) Copyright 2010 Faraday Technology
+ * Dante Su <dant...@faraday-tech.com>
+ *
+ * This file is released under the terms of GPL v2 and any later version.
+ * See the file COPYING in the root directory of the source tree for details.
+ */
Little bit uneasy with the above license note, any reason for this GPL w.r.t your company style. refer license note on drivers/spi/exynos_spi.c

+
+#include <common.h>
+#include <asm/io.h>
+#include <spi.h>
+#include <malloc.h>
+#include <faraday/ftgpio010.h>
Does this include file is an arch' specific, means struct ftgpio010_regs
is used by some other drivers on ur board?

+#include "ftssp010_spi.h"
Please don't use extra include file, use the same structure's in driver it self, IMHO.

+
+#define CFG_PIO_TIMEOUT (CONFIG_SYS_HZ >> 3) /* 125 ms */
+#define CFG_CS_TIMEOUT  (CONFIG_SYS_HZ >> 2) /* 250 ms */
+

----- TAG+
+struct ftssp010_chip {
+       void __iomem *regs;
+       uint32_t fifo;
+       uint32_t rev;
+       uint32_t div;
+       uint32_t mode;
+
+       struct {
+               void __iomem *regs;
+               uint32_t      pin;
+       } gpio;
+};
+
+static struct ftssp010_chip chip_list[] = {
+#ifdef CONFIG_FTSSP010_BASE
+       {
+               .regs = (void __iomem *)CONFIG_FTSSP010_BASE,
+# ifdef CONFIG_FTSSP010_GPIO_BASE
+               .gpio = {
+                       (void __iomem *)CONFIG_FTSSP010_GPIO_BASE,
+                       CONFIG_FTSSP010_GPIO_PIN
+               },
+# endif
+       },
+#endif /* #ifdef CONFIG_FTSSP010_BASE */
+#ifdef CONFIG_FTSSP010_BASE1
+       { .regs = (void __iomem *)CONFIG_FTSSP010_BASE1, },
+# ifdef CONFIG_FTSSP010_GPIO_BASE1
+               .gpio = {
+                       (void __iomem *)CONFIG_FTSSP010_GPIO_BASE1,
+                       CONFIG_FTSSP010_GPIO_PIN1
+               },
+# endif
+#endif
+#ifdef CONFIG_FTSSP010_BASE2
+       { .regs = (void __iomem *)CONFIG_FTSSP010_BASE2, },
+# ifdef CONFIG_FTSSP010_GPIO_BASE2
+               .gpio = {
+                       (void __iomem *)CONFIG_FTSSP010_GPIO_BASE2,
+                       CONFIG_FTSSP010_GPIO_PIN2
+               },
+# endif
+#endif
+#ifdef CONFIG_FTSSP010_BASE3
+       { .regs = (void __iomem *)CONFIG_FTSSP010_BASE3, },
+# ifdef CONFIG_FTSSP010_GPIO_BASE3
+               .gpio = {
+                       (void __iomem *)CONFIG_FTSSP010_GPIO_BASE3,
+                       CONFIG_FTSSP010_GPIO_PIN3
+               },
+# endif
+#endif
+};
---- TAG-

TAG+ ... TAG- this type of assignment is old way.
please use the structure macro assignment instead of void __iomem *

ex:
#define CONFIG_MY_REG_BASE      0xF1000000

struct my_reg {
        u32 r1;
        u32 r2;
        u32 r3;
        u32 r4;
}

#define my_reg_base ((struct my_reg *) CONFIG_MY_REG_BASE)

then use readl(&my_reg_base->r3);

see the example usage on
http://patchwork.ozlabs.org/patch/246468/
https://github.com/Xilinx/u-boot-xlnx/blob/master/drivers/spi/zynq_qspips.c

+
+static inline int ftssp010_wait_tx(struct ftssp010_chip *chip)
+{
+       struct ftssp010_regs __iomem *regs = chip->regs;
+       int ret = -1;
+       ulong ts;
+
+       for (ts = get_timer(0); get_timer(ts) < CFG_PIO_TIMEOUT; ) {
+               if (!(readl(&regs->sr) & SR_TFNF))
+                       continue;
+               ret = 0;
+               break;
+       }
+
+       if (ret)
+               printf("ftssp010: tx timeout\n");
+
+       return ret;
+}
+
+static inline int ftssp010_wait_rx(struct ftssp010_chip *chip)
+{
+       struct ftssp010_regs __iomem *regs = chip->regs;
+       int ret = -1;
+       ulong ts;
+
+       for (ts = get_timer(0); get_timer(ts) < CFG_PIO_TIMEOUT; ) {
+               if (!SR_RFVE(readl(&regs->sr)))
+                       continue;
+               ret = 0;
+               break;
+       }
+
+       if (ret)
+               printf("ftssp010: rx timeout\n");
+
+       return ret;
+}
+
+static int ftssp010_spi_work_transfer_v1_19(struct ftssp010_chip *chip,
+       const void *tx_buf, void *rx_buf, int len, uint flags)
+{
+       struct ftssp010_regs __iomem *regs = chip->regs;
+       const uint8_t *txb = tx_buf;
+       uint8_t       *rxb = rx_buf;
+
+       while (len > 0) {
+               int i, depth = min(chip->fifo >> 2, len);
+               uint32_t xmsk = 0;
+
+               if (tx_buf) {
+                       for (i = 0; i < depth; ++i) {
+                               ftssp010_wait_tx(chip);
+                               writel(*txb++, &regs->dr);
+                       }
+                       xmsk |= CR2_TXEN | CR2_TXDOE;
+                       if ((readl(&regs->cr[2]) & xmsk) != xmsk)
+                               setbits_le32(&regs->cr[2], xmsk);
+               }
+               if (rx_buf) {
+                       xmsk |= CR2_RXEN;
+                       if ((readl(&regs->cr[2]) & xmsk) != xmsk)
+                               setbits_le32(&regs->cr[2], xmsk);
+                       for (i = 0; i < depth; ++i) {
+                               ftssp010_wait_rx(chip);
+                               *rxb++ = (uint8_t)readl(&regs->dr);
+                       }
+               }
+
+               len -= depth;
+       }
+
+       return 0;
+}
+
+static int ftssp010_spi_work_transfer(struct ftssp010_chip *chip,
+       const void *tx_buf, void *rx_buf, int len, uint flags)
+{
+       struct ftssp010_regs __iomem *regs = chip->regs;
+       const uint8_t *txb = tx_buf;
+       uint8_t       *rxb = rx_buf;
+
+       while (len > 0) {
+               int i, depth = min(chip->fifo >> 2, len);
+               uint32_t tmp;
+
+               for (i = 0; i < depth; ++i) {
+                       ftssp010_wait_tx(chip);
+                       writel(txb ? (*txb++) : 0, &regs->dr);
+               }
+               for (i = 0; i < depth; ++i) {
+                       ftssp010_wait_rx(chip);
+                       tmp = readl(&regs->dr);
+                       if (rxb)
+                               *rxb++ = (uint8_t)tmp;
+               }
+
+               len -= depth;
+       }
+
+       return 0;
+}
+
+/*
+ * Determine if a SPI chipselect is valid.
+ * This function is provided by the board if the low-level SPI driver
+ * needs it to determine if a given chipselect is actually valid.
+ *
+ * Returns: 1 if bus:cs identifies a valid chip on this board, 0
+ * otherwise.
+ */
+int spi_cs_is_valid(unsigned int bus, unsigned int cs)
+{
+       int ret = 0;
+       struct ftssp010_chip *chip;
+       struct ftssp010_regs __iomem *regs;
+       uint32_t txfifo, rxfifo;
+
+       if (bus >= ARRAY_SIZE(chip_list))
+               return ret;
+
+       chip = chip_list + bus;
+       regs = chip->regs;
+       chip->rev = readl(&regs->revr);
+       txfifo = FEAR_TXFIFO(readl(&regs->fear));
+       rxfifo = FEAR_RXFIFO(readl(&regs->fear));
+       chip->fifo = min(txfifo, rxfifo);
+
+       debug("ftssp010: rev=0x%08X, fifo=%d\n", chip->rev, chip->fifo);
+
+       if (chip->rev >= 0x00011900) {
+               if (cs < 4)
+                       ret = 1;
+       } else if (!cs) {
+               struct ftgpio010_regs *gpio = chip->gpio.regs;
+               uint32_t mask = BIT_MASK(chip->gpio.pin);
+
+               if (gpio) {
+                       /* setup gpio pin as an output pin */
+                       setbits_le32(&gpio->dir, mask);
+                       ret = 1;
+               }
+       }
+
+       return ret;
+}
+
+/*
+ * Activate a SPI chipselect.
+ * This function is provided by the board code when using a driver
+ * that can't control its chipselects automatically (e.g.
+ * common/soft_spi.c). When called, it should activate the chip select
+ * to the device identified by "slave".
+ */
+void spi_cs_activate(struct spi_slave *slave)
+{
+       struct ftssp010_chip *chip = chip_list + slave->bus;
+       struct ftssp010_regs __iomem *regs = chip->regs;
+
+       /* cs pull low */
+       if (chip->rev >= 0x00011900) {
+               writel((slave->cs << 10) | CR2_SSPEN | CR2_TXFCLR
+                       | CR2_RXFCLR, &regs->cr[2]);
+       } else {
+               struct ftgpio010_regs *gpio = chip->gpio.regs;
+               uint32_t mask = BIT_MASK(chip->gpio.pin);
+
+               if (gpio)
+                       setbits_le32(&gpio->clr, mask);
+       }
+       udelay_masked(1);
+}
+
+/*
+ * Deactivate a SPI chipselect.
+ * This function is provided by the board code when using a driver
+ * that can't control its chipselects automatically (e.g.
+ * common/soft_spi.c). When called, it should deactivate the chip
+ * select to the device identified by "slave".
+ */
+void spi_cs_deactivate(struct spi_slave *slave)
+{
+       struct ftssp010_chip *chip = chip_list + slave->bus;
+       struct ftssp010_regs __iomem *regs = chip->regs;
+       ulong ts;
+
+       /* wait until device idle */
+       for (ts = get_timer(0); get_timer(ts) < CFG_CS_TIMEOUT; ) {
+               if (readl(&regs->sr) & SR_BUSY)
+                       continue;
+               break;
+       }
+       if (readl(&regs->sr) & SR_BUSY)
+               printf("ftspi010: busy timeout at cs deactivate\n");
+
+       /* cs pull high */
+       if (chip->rev >= 0x00011900) {
+               writel((slave->cs << 10) | CR2_FS, &regs->cr[2]);
+       } else {
+               struct ftgpio010_regs *gpio = chip->gpio.regs;
+               uint32_t mask = BIT_MASK(chip->gpio.pin);
+
+               if (gpio)
+                       setbits_le32(&gpio->set, mask);
+       }
+       udelay_masked(1);
+}
+
+void spi_init(void)
+{
+}
+
+struct spi_slave *spi_setup_slave(uint bus, uint cs, uint max_hz, uint mode)
+{
+       uint32_t clk, div;
+       struct spi_slave *ss;
+       struct ftssp010_chip *chip;
+
+       if (!spi_cs_is_valid(bus, cs))
+               return NULL;
+
+       if (mode != SPI_MODE_0) {
+               printf("ftssp010: MODE%d is not supported\n", mode);
+               return NULL;
+       }
+

----- TAG+
+       ss = malloc(sizeof(struct spi_slave));
+       if (!ss)
+               return NULL;
+
+       ss->bus = bus;
+       ss->cs  = cs;
----- TAG-
Pleas use spi_alloc_slave() instead of malloc()
see the sample code on "drivers/spi/exynos_spi.c"

+
+#ifdef CONFIG_FTSSP010_SCLK
+       clk = CONFIG_FTSSP010_SCLK;
+#else
+       clk = clk_get_rate("SSP");
+#endif
+       if (max_hz > 0) {
+               for (div = 0; div < 0xFFFF; ++div) {
+                       if ((clk / (2 * (div + 1))) <= max_hz)
+                               break;
+               }
+       } else {
+               div = 7;
+       }
+
+       chip = chip_list + bus;
+       chip->div  = div;
+       chip->mode = mode;
+
+       debug("ftssp010: bus=%d, hz=%u\n", bus, (clk / (2 * (div + 1))));
+
+       return ss;
+}
+
+void spi_free_slave(struct spi_slave *slave)
+{
+       free(slave);
+}
+
+int spi_claim_bus(struct spi_slave *slave)
+{
+       struct ftssp010_chip *chip = chip_list + slave->bus;
+       struct ftssp010_regs __iomem *regs = chip->regs;
+
+       writel(CR1_SDL(8) | CR1_CLKDIV(chip->div), &regs->cr[1]);
+
+       if (chip->rev >= 0x00011900) {
+               writel(CR0_OPM_MASTER | CR0_FFMT_SPI | CR0_FSPO | CR0_FLASH,
+                       &regs->cr[0]);
+               writel(CR2_TXFCLR | CR2_RXFCLR,
+                       &regs->cr[2]);
+       } else {
+               writel(CR0_OPM_MASTER | CR0_FFMT_SPI | CR0_FSPO,
+                       &regs->cr[0]);
+               writel(CR2_TXFCLR | CR2_RXFCLR | CR2_SSPEN | CR2_TXDOE,
+                       &regs->cr[2]);
+       }
+
+       spi_cs_deactivate(slave);
+
+       return 0;
+}
+
+void spi_release_bus(struct spi_slave *slave)
+{
+       struct ftssp010_chip *chip = chip_list + slave->bus;
+       struct ftssp010_regs __iomem *regs = chip->regs;
+
+       writel(0, &regs->cr[2]);
+}
+
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
+                        const void *dout, void *din, unsigned long flags)
+{
+       struct ftssp010_chip *chip = chip_list + slave->bus;
+       uint32_t len = bitlen >> 3;
+
+       if (flags & SPI_XFER_BEGIN)
+               spi_cs_activate(slave);
+
+       if (chip->rev >= 0x00011900)
+               ftssp010_spi_work_transfer_v1_19(chip, dout, din, len, flags);
+       else
+               ftssp010_spi_work_transfer(chip, dout, din, len, flags);
+
+       if (flags & SPI_XFER_END)
+               spi_cs_deactivate(slave);
+
+       return 0;
+}

---- TAG+
diff --git a/drivers/spi/ftssp010_spi.h b/drivers/spi/ftssp010_spi.h
new file mode 100644
index 0000000..5258edc
--- /dev/null
+++ b/drivers/spi/ftssp010_spi.h
@@ -0,0 +1,86 @@
+/*
+ * Faraday Multi-function Controller - SPI Mode
+ *
+ * (C) Copyright 2010 Faraday Technology
+ * Dante Su <dant...@faraday-tech.com>
+ *
+ * This file is released under the terms of GPL v2 and any later version.
+ * See the file COPYING in the root directory of the source tree for details.
+ */
+
+#ifndef _FTSSP010_H
+#define _FTSSP010_H
+
+/* FTSSP010 HW Registers */
+struct ftssp010_regs {
+       uint32_t cr[3];/* control register */
+       uint32_t sr;   /* status register */
+       uint32_t icr;  /* interrupt control register */
+       uint32_t isr;  /* interrupt status register */
+       uint32_t dr;   /* data register */
+       uint32_t rsvd[17];
+       uint32_t revr; /* revision register */
+       uint32_t fear; /* feature register */
+};
+
+/* Control Register 0  */
+#define CR0_FFMT_MASK       (7 << 12)
+#define CR0_FFMT_SSP        (0 << 12)
+#define CR0_FFMT_SPI        (1 << 12)
+#define CR0_FFMT_MICROWIRE  (2 << 12)
+#define CR0_FFMT_I2S        (3 << 12)
+#define CR0_FFMT_AC97       (4 << 12)
+#define CR0_FLASH           (1 << 11)
+#define CR0_FSDIST(x)       (((x) & 0x03) << 8)
+#define CR0_LBM             (1 << 7)  /* Loopback mode */
+#define CR0_LSB             (1 << 6)  /* LSB first */
+#define CR0_FSPO            (1 << 5)  /* Frame sync atcive low */
+#define CR0_FSJUSTIFY       (1 << 4)
+#define CR0_OPM_SLAVE       (0 << 2)
+#define CR0_OPM_MASTER      (3 << 2)
+#define CR0_OPM_I2S_MSST    (3 << 2)  /* Master stereo mode */
+#define CR0_OPM_I2S_MSMO    (2 << 2)  /* Master mono mode */
+#define CR0_OPM_I2S_SLST    (1 << 2)  /* Slave stereo mode */
+#define CR0_OPM_I2S_SLMO    (0 << 2)  /* Slave mono mode */
+#define CR0_SCLKPO          (1 << 1)  /* SCLK Remain HIGH */
+#define CR0_SCLKPH          (1 << 0)  /* Half CLK cycle */
+
+/* Control Register 1 */
+
+#define CR1_PDL(x)          (((x) & 0xff) << 24) /* padding length */
+#define CR1_SDL(x)          ((((x) - 1) & 0x1f) << 16) /* data length */
+#define CR1_CLKDIV(x)       ((x) & 0xffff) /*  clk divider */
+
+/* Control Register 2 */
+#define CR2_FSOS(x)         (((x) & 0x03) << 10)     /* FS/CS Select */
+#define CR2_FS              (1 << 9)     /* FS/CS Signal Level */
+#define CR2_TXEN            (1 << 8)     /* Tx Enable */
+#define CR2_RXEN            (1 << 7)     /* Rx Enable */
+#define CR2_SSPRST          (1 << 6)     /* SSP reset */
+#define CR2_TXFCLR          (1 << 3)     /* TX FIFO Clear */
+#define CR2_RXFCLR          (1 << 2)     /* RX FIFO Clear */
+#define CR2_TXDOE           (1 << 1)     /* TX Data Output Enable */
+#define CR2_SSPEN           (1 << 0)     /* SSP Enable */
+
+/*
+ * Status Register
+ */
+#define SR_RFF       (1 << 0) /* receive FIFO full */
+#define SR_TFNF      (1 << 1) /* transmit FIFO not full */
+#define SR_BUSY      (1 << 2) /* bus is busy */
+#define SR_RFVE(reg) (((reg) >> 4) & 0x1f)  /* receive  FIFO valid entries */
+#define SR_TFVE(reg) (((reg) >> 12) & 0x1f) /* transmit FIFO valid entries */
+
+/*
+ * Feature Register
+ */
+#define FEAR_WIDTH(reg)  ((((reg) >>  0) & 0xff) + 1)
+#define FEAR_RXFIFO(reg) ((((reg) >>  8) & 0xff) + 1)
+#define FEAR_TXFIFO(reg) ((((reg) >> 16) & 0xff) + 1)
+#define FEAR_AC97        (1 << 24)
+#define FEAR_I2S         (1 << 25)
+#define FEAR_SPI_MWR     (1 << 26)
+#define FEAR_SSP         (1 << 27)
+#define FEAR_SPDIF       (1 << 28)
+
+#endif /* EOF */
---- TAG-

Please use the same defination on driver code itself.

Thanks,
Jagan.


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

Reply via email to