Stefano Babic wrote:
> This patch add support for MX51 processor and
> supports transfer of multiple word in a single
> transation.
> 
> Signed-off-by: Stefano Babic <sba...@denx.de>
> ---
> 
> The patch adds support for the MX51 and wants to remove some
> limitation on the old driver. Actually, the buffer passed
> to the transfer function must be word-aligne, even if it is 
> required to send a single byte.

It may be better to split this patch into
1. fixing the old driver
2. adding mx51

> 
>  drivers/spi/mxc_spi.c |  357 
> +++++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 301 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
> index 3a45200..b04fadc 100644
> --- a/drivers/spi/mxc_spi.c
> +++ b/drivers/spi/mxc_spi.c
> @@ -24,6 +24,11 @@
>  #include <asm/errno.h>
>  #include <asm/io.h>
>  
> +#define MXC_CSPIRXDATA               0x00
> +#define MXC_CSPITXDATA               0x04
> +#define MXC_CSPICTRL         0x08
> +#define MXC_CSPIPERIOD_32KHZ (1 << 15)
> +

Pulling these out and making them common may not be the best thing to do.
Located here, it hides the

#ifdef CONFIG_MX27
#elif defined (CONFIG_MX31)
#elif defined (CONFIG_MX51)
#else
#endif

I would prefer if you just kept the copies in the mx31, mx51 locations

>  #ifdef CONFIG_MX27
>  /* i.MX27 has a completely wrong register layout and register definitions in 
> the
>   * datasheet, the correct one is in the Freescale's Linux driver */
> @@ -31,13 +36,9 @@
>  #error "i.MX27 CSPI not supported due to drastic differences in register 
> definisions" \
>  "See linux mxc_spi driver from Freescale for details."
>  
> -#else
> -
> +#elif defined(CONFIG_MX31)
>  #include <asm/arch/mx31.h>
>  
> -#define MXC_CSPIRXDATA               0x00
> -#define MXC_CSPITXDATA               0x04
> -#define MXC_CSPICTRL         0x08
>  #define MXC_CSPIINT          0x0C
>  #define MXC_CSPIDMA          0x10
>  #define MXC_CSPISTAT         0x14
> @@ -56,21 +57,63 @@
>  #define MXC_CSPICTRL_CHIPSELECT(x)   (((x) & 0x3) << 24)
>  #define MXC_CSPICTRL_BITCOUNT(x)     (((x) & 0x1f) << 8)
>  #define MXC_CSPICTRL_DATARATE(x)     (((x) & 0x7) << 16)
> +#define MXC_CSPICTRL_MAXBITS 0x1f
> +#define MXC_CSPICTRL_TC              (1 << 8)
> +#define MXC_CSPICTRL_RXOVF   (1 << 6)
>  
>  #define MXC_CSPIPERIOD_32KHZ (1 << 15)
> +#define MAX_SPI_BYTES        4

In 'Add SPI support to mx51evk board'
The MAX_SPI_BYTES was defined in the config file.
Here it is defined for mx31 generally.
You should be consistent.
These would be a better place for the mx51 values as you only have to do it 
once.

>  
>  static unsigned long spi_bases[] = {
>       0x43fa4000,
>       0x50010000,
>       0x53f84000,
>  };
> +#elif defined(CONFIG_MX51)
> +
> +#define MXC_CSPICON          0x0C
> +#define MXC_CSPIINT          0x10
> +#define MXC_CSPIDMA          0x14
> +#define MXC_CSPISTAT         0x18
> +#define MXC_CSPIPERIOD               0x1C
> +#define MXC_CSPIRESET                0x00
>  
> +#include <asm/arch/imx-regs.h>
> +#include <asm/arch/clock.h>

Be constistent with mx31
Move the #includes to the first lines after the #elif
The other #defines to follow.

Tom

> +#define MXC_CSPICTRL_EN              (1 << 0)
> +#define MXC_CSPICTRL_MODE    (1 << 1)
> +#define MXC_CSPICTRL_XCH     (1 << 2)
> +#define MXC_CSPICTRL_CHIPSELECT(x)   (((x) & 0x3) << 12)
> +#define MXC_CSPICTRL_BITCOUNT(x)     (((x) & 0xfff) << 20)
> +#define MXC_CSPICTRL_PREDIV(x)       (((x) & 0xF) << 12)
> +#define MXC_CSPICTRL_POSTDIV(x)      (((x) & 0xF) << 8)
> +#define MXC_CSPICTRL_SELCHAN(x)      (((x) & 0x3) << 18)
> +#define MXC_CSPICTRL_MAXBITS 0xfff
> +#define MXC_CSPICTRL_TC              (1 << 7)
> +#define MXC_CSPICTRL_RXOVF   (1 << 6)
> +
> +/* Bit position inside CTRL register to be associated with SS */
> +#define MXC_CSPICTRL_CHAN    18
> +
> +/* Bit position inside CON register to be associated with SS */
> +#define MXC_CSPICON_POL              4
> +#define MXC_CSPICON_PHA              0
> +#define MXC_CSPICON_SSPOL    12
> +
> +static unsigned long spi_bases[] = {
> +     CSPI1_BASE_ADDR,
> +     CSPI2_BASE_ADDR,
> +     CSPI3_BASE_ADDR,
> +};
> +#else
> +#error "Unsupported architecture"
>  #endif
>  
>  struct mxc_spi_slave {
>       struct spi_slave slave;
>       unsigned long   base;
>       u32             ctrl_reg;
> +     u32             cfg_reg;
>       int             gpio;
>  };
>  
> @@ -89,71 +132,262 @@ static inline void reg_write(unsigned long addr, u32 
> val)
>       *(volatile unsigned long*)addr = val;
>  }
>  
> -static u32 spi_xchg_single(struct spi_slave *slave, u32 data, int bitlen,
> -                        unsigned long flags)
> +void spi_cs_activate(struct spi_slave *slave)
>  {
> +#ifdef CONFIG_MX31
>       struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> -     unsigned int cfg_reg = reg_read(mxcs->base + MXC_CSPICTRL);
> -
> -     mxcs->ctrl_reg = (mxcs->ctrl_reg & ~MXC_CSPICTRL_BITCOUNT(31)) |
> -             MXC_CSPICTRL_BITCOUNT(bitlen - 1);
> +     if (mxcs->gpio > 0)
> +             mx31_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
> +#endif
> +}
>  
> -     if (cfg_reg != mxcs->ctrl_reg)
> -             reg_write(mxcs->base + MXC_CSPICTRL, mxcs->ctrl_reg);
> +void spi_cs_deactivate(struct spi_slave *slave)
> +{
> +     struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> +#ifdef CONFIG_MX31
> +     if (mxcs->gpio > 0)
> +             mx31_gpio_set(mxcs->gpio,
> +                           !(mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL));
> +#endif
> +     reg_write(mxcs->base + MXC_CSPICTRL, 0);
> +}
>  
> -     if (mxcs->gpio > 0 && (flags & SPI_XFER_BEGIN))
> -             mx31_gpio_set(mxcs->gpio, mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL);
> +static s32 spi_cfg(struct mxc_spi_slave *mxcs, unsigned int cs,
> +             unsigned int max_hz, unsigned int mode)
> +{
> +     u32 clk_src = mxc_get_clock(MXC_CSPI_CLK);
> +     s32 pre_div = 0, post_div = 0, i, reg_ctrl, reg_config;
> +     u32 ss_pol = 0, sclkpol = 0, sclkpha = 0;
>  
> -     reg_write(mxcs->base + MXC_CSPITXDATA, data);
> +     if (max_hz == 0) {
> +             printf("Error: desired clock is 0\n");
> +             return -1;
> +     }
>  
> -     reg_write(mxcs->base + MXC_CSPICTRL, mxcs->ctrl_reg | MXC_CSPICTRL_XCH);
> +     reg_ctrl = reg_read(mxcs->base + MXC_CSPICTRL);
>  
> -     while (reg_read(mxcs->base + MXC_CSPICTRL) & MXC_CSPICTRL_XCH)
> -             ;
> +     /* Reset spi */
> +     reg_write(mxcs->base + MXC_CSPICTRL, 0);
> +     reg_write(mxcs->base + MXC_CSPICTRL, (reg_ctrl | 0x1));
>  
> -     if (mxcs->gpio > 0 && (flags & SPI_XFER_END)) {
> -             mx31_gpio_set(mxcs->gpio,
> -                           !(mxcs->ctrl_reg & MXC_CSPICTRL_SSPOL));
> +     /*
> +      * The following computation is taken directly from Freescale's code.
> +      */
> +     if (clk_src > max_hz) {
> +             pre_div = clk_src / max_hz;
> +             if (pre_div > 16) {
> +                     post_div = pre_div / 16;
> +                     pre_div = 15;
> +             }
> +             if (post_div != 0) {
> +                     for (i = 0; i < 16; i++) {
> +                             if ((1 << i) >= post_div)
> +                                     break;
> +                     }
> +                     if (i == 16) {
> +                             printf("Error: no divider for the freq: %d\n",
> +                                     max_hz);
> +                             return -1;
> +                     }
> +                     post_div = i;
> +             }
>       }
>  
> -     return reg_read(mxcs->base + MXC_CSPIRXDATA);
> +     debug("pre_div = %d, post_div=%d\n", pre_div, post_div);
> +     reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_SELCHAN(3)) |
> +             MXC_CSPICTRL_SELCHAN(cs);
> +     reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_PREDIV(0x0F)) |
> +             MXC_CSPICTRL_PREDIV(pre_div);
> +     reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) |
> +             MXC_CSPICTRL_POSTDIV(post_div);
> +
> +     /* always set to master mode */
> +     reg_ctrl |= 1 << (cs + 4);
> +
> +     /* We need to disable SPI before changing registers */
> +     reg_ctrl &= ~MXC_CSPICTRL_EN;
> +
> +     if (mode & SPI_CS_HIGH)
> +             ss_pol = 1;
> +
> +     if (!(mode & SPI_CPOL))
> +             sclkpol = 1;
> +
> +     if (mode & SPI_CPHA)
> +             sclkpha = 1;
> +
> +     reg_config = reg_read(mxcs->base + MXC_CSPICON);
> +
> +     /*
> +      * Configuration register setup
> +      * The MX51 has support different setup for each SS
> +      */
> +     reg_config = (reg_config & ~(1 << (cs + MXC_CSPICON_SSPOL))) |
> +             (ss_pol << (cs + MXC_CSPICON_SSPOL));
> +     reg_config = (reg_config & ~(1 << (cs + MXC_CSPICON_POL))) |
> +             (sclkpol << (cs + MXC_CSPICON_POL));
> +     reg_config = (reg_config & ~(1 << (cs + MXC_CSPICON_PHA))) |
> +             (sclkpha << (cs + MXC_CSPICON_PHA));
> +
> +     debug("reg_ctrl = 0x%x\n", reg_ctrl);
> +     reg_write(mxcs->base + MXC_CSPICTRL, reg_ctrl);
> +     debug("reg_config = 0x%x\n", reg_config);
> +     reg_write(mxcs->base + MXC_CSPICON, reg_config);
> +
> +     /* save config register and control register */
> +     mxcs->ctrl_reg = reg_ctrl;
> +     mxcs->cfg_reg = reg_config;
> +
> +     /* clear interrupt reg */
> +     reg_write(mxcs->base + MXC_CSPIINT, 0);
> +     reg_write(mxcs->base + MXC_CSPISTAT,
> +             MXC_CSPICTRL_TC | MXC_CSPICTRL_RXOVF);
> +
> +     return 0;
>  }
>  
> -int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> -             void *din, unsigned long flags)
> +static int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
> +     const void *dout, void *din, unsigned long flags)
>  {
> -     int n_blks = (bitlen + 31) / 32;
> -     u32 *out_l, *in_l;
> -     int i;
> +     struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> +     int n_bytes = (bitlen + 7) / 8;
> +     int n_blks = (n_bytes + 3) / 4;
> +     int cnt_blk;
> +     u32 tmpdata;
> +     u32 spare_bytes = 0;
> +     int j;
> +     char *pbuf;
> +
> +     debug("spi_xchg: %d %d %d\n", bitlen, n_bytes, n_blks);
> +
> +     if (flags & SPI_XFER_BEGIN)
> +             spi_cs_activate(slave);
> +
> +     mxcs->ctrl_reg = (mxcs->ctrl_reg &
> +             ~MXC_CSPICTRL_BITCOUNT(MXC_CSPICTRL_MAXBITS)) |
> +             MXC_CSPICTRL_BITCOUNT(n_bytes * 8 - 1);
> +
> +     /* Enable SPI controller */
> +     reg_write(mxcs->base + MXC_CSPICTRL, mxcs->ctrl_reg | MXC_CSPICTRL_EN);
> +#ifdef CONFIG_MX51
> +     reg_write(mxcs->base + MXC_CSPICON, mxcs->cfg_reg);
> +#endif
>  
> -     if ((int)dout & 3 || (int)din & 3) {
> -             printf("Error: unaligned buffers in: %p, out: %p\n", din, dout);
> -             return 1;
> +     /* Clear interrupt register */
> +     reg_write(mxcs->base + MXC_CSPISTAT,
> +             MXC_CSPICTRL_TC | MXC_CSPICTRL_RXOVF);
> +
> +     for (cnt_blk = 0, pbuf = (char *)dout; cnt_blk < n_blks; cnt_blk++) {
> +             tmpdata = 0;
> +             for (j = 0; j < 4; j++) {
> +                     /*
> +                      * According to the User Manual, only the n
> +                      * least-significant bits in the first word
> +                      * will be shifted out. The remaining bits
> +                      * in first word will be ignored
> +                      */
> +                     if (n_bytes > 0) {
> +                             /*
> +                              * Check for the first word. If the number of
> +                              * bytes is not multiple of a word, only a
> +                              * part of the first word is shifted out
> +                              * and must be written in TX FIFO.
> +                              * For example, sending 5 bytes means to write
> +                              * a single byte first, followed by 4 words.
> +                              */
> +                             if ((cnt_blk == 0) && (bitlen % 32) &&
> +                                     (j >= ((bitlen % 32) / 8))) {
> +                                     continue;
> +                             }
> +                             if (pbuf)
> +                                     tmpdata = *pbuf++ | (tmpdata <<  8);
> +                             n_bytes--;
> +                     }
> +             }
> +             debug("writing TX FIFO 0x%x\n", tmpdata);
> +             reg_write(mxcs->base + MXC_CSPITXDATA, tmpdata);
>       }
>  
> -     /* This driver is currently partly broken, alert the user */
> -     if (bitlen > 16 && (bitlen % 32)) {
> -             printf("Error: SPI transfer with bitlen=%d is broken.\n",
> -                    bitlen);
> -             return 1;
> -     }
> +     /* FIFO is written, now starts the transfer setting the XCH bit */
> +     reg_write(mxcs->base + MXC_CSPICTRL, mxcs->ctrl_reg |
> +             MXC_CSPICTRL_EN | MXC_CSPICTRL_XCH);
> +
> +     /* Wait until the TC (Transfer completed) bit is set */
> +     while ((reg_read(mxcs->base + MXC_CSPISTAT) & MXC_CSPICTRL_TC) == 0)
> +             ;
> +
> +     /* Transfer completed, clear any pending request */
> +     reg_write(mxcs->base + MXC_CSPISTAT,
> +             MXC_CSPICTRL_TC | MXC_CSPICTRL_RXOVF);
>  
> -     for (i = 0, in_l = (u32 *)din, out_l = (u32 *)dout;
> -          i < n_blks;
> -          i++, in_l++, out_l++, bitlen -= 32) {
> -             u32 data = spi_xchg_single(slave, *out_l, bitlen, flags);
> -
> -             /* Check if we're only transfering 8 or 16 bits */
> -             if (!i) {
> -                     if (bitlen < 9)
> -                             *(u8 *)din = data;
> -                     else if (bitlen < 17)
> -                             *(u16 *)din = data;
> -                     else
> -                             *in_l = data;
> +     /*
> +      * Now do the opposite: copy only part of the first word
> +      * if the number of bytes is not multiple of a word size.
> +      */
> +     n_bytes = (bitlen + 7) / 8;
> +     for (cnt_blk = 0, pbuf = (char *)din; n_bytes > 0; cnt_blk++) {
> +             tmpdata = reg_read(mxcs->base + MXC_CSPIRXDATA);
> +             debug("value_read 0x%x\n", tmpdata);
> +             for (j = 0; j < 4; j++) {
> +                     if (n_bytes > 0) {
> +                             /*
> +                              * If it is the first word, compute how many
> +                              * bytes must be copied
> +                              */
> +                             if (cnt_blk == 0)
> +                                     spare_bytes = (bitlen % 32) / 8;
> +
> +                             if ((cnt_blk == 0) && (bitlen % 32) &&
> +                                     (j >= spare_bytes))
> +                                             continue;
> +
> +                             if (din)
> +                                     *pbuf++ = (tmpdata >>
> +                                             ((3 - spare_bytes - j) * 8))
> +                                             & 0xFF;
> +
> +                             n_bytes--;
> +                     }
>               }
>       }
>  
> +     if (flags & SPI_XFER_END)
> +             spi_cs_deactivate(slave);
> +
> +     return 0;
> +}
> +
> +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;
> +     char *p_outbuf = (char *)dout;
> +     char *p_inbuf = (char *)din;
> +
> +     if (!slave)
> +             return -1;
> +
> +     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, dout, din, flags);
> +             if (ret)
> +                     return ret;
> +             if (dout)
> +                     p_outbuf += blk_size;
> +             if (din)
> +                     p_inbuf += blk_size;
> +             n_bytes -= blk_size;
> +     }
> +
>       return 0;
>  }
>  
> @@ -163,8 +397,7 @@ void spi_init(void)
>  
>  static int decode_cs(struct mxc_spi_slave *mxcs, unsigned int cs)
>  {
> -     int ret;
> -
> +     int ret = 0;
>       /*
>        * Some SPI devices require active chip-select over multiple
>        * transactions, we achieve this using a GPIO. Still, the SPI
> @@ -176,11 +409,13 @@ static int decode_cs(struct mxc_spi_slave *mxcs, 
> unsigned int cs)
>       if (cs > 3) {
>               mxcs->gpio = cs >> 8;
>               cs &= 3;
> +#ifdef CONFIG_MX31
>               ret = mx31_gpio_direction(mxcs->gpio, MX31_GPIO_DIRECTION_OUT);
>               if (ret) {
>                       printf("mxc_spi: cannot setup gpio %d\n", mxcs->gpio);
>                       return -EINVAL;
>               }
> +#endif
>       } else {
>               mxcs->gpio = -1;
>       }
> @@ -191,7 +426,6 @@ static int decode_cs(struct mxc_spi_slave *mxcs, unsigned 
> int cs)
>  struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>                       unsigned int max_hz, unsigned int mode)
>  {
> -     unsigned int ctrl_reg;
>       struct mxc_spi_slave *mxcs;
>       int ret;
>  
> @@ -210,6 +444,19 @@ struct spi_slave *spi_setup_slave(unsigned int bus, 
> unsigned int cs,
>  
>       cs = ret;
>  
> +     mxcs->slave.bus = bus;
> +     mxcs->slave.cs = cs;
> +     mxcs->base = spi_bases[bus];
> +
> +#ifdef CONFIG_MX51
> +     /* Can be used for i.MX31 too ? */
> +     ret = spi_cfg(mxcs, cs, max_hz, mode);
> +     if (ret) {
> +             printf("mxc_spi: cannot setup SPI controller\n");
> +             free(mxcs);
> +             return NULL;
> +     }
> +#else
>       ctrl_reg = MXC_CSPICTRL_CHIPSELECT(cs) |
>               MXC_CSPICTRL_BITCOUNT(31) |
>               MXC_CSPICTRL_DATARATE(7) | /* FIXME: calculate data rate */
> @@ -223,10 +470,8 @@ struct spi_slave *spi_setup_slave(unsigned int bus, 
> unsigned int cs,
>       if (mode & SPI_CS_HIGH)
>               ctrl_reg |= MXC_CSPICTRL_SSPOL;
>  
> -     mxcs->slave.bus = bus;
> -     mxcs->slave.cs = cs;
> -     mxcs->base = spi_bases[bus];
>       mxcs->ctrl_reg = ctrl_reg;
> +#endif
>  
>       return &mxcs->slave;
>  }

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

Reply via email to