[U-Boot] [PATCH v1]dm : spi: Convert Freescale DSPI to driver model

Simon Glass sjg at chromium.org
Mon Mar 2 19:56:08 CET 2015


Hi,

On 28 February 2015 at 03:54, Haikun.Wang at freescale.com
<Haikun.Wang at freescale.com> wrote:
> Move the Freescale DSPI driver over to driver model.
>
> Signed-off-by: Haikun Wang <B53464 at freescale.com>
> ---
>
> This patch adds two new files drivers/spi/fsl_dspi.c and include/fsl_dspi.h.
> They will replace files drivers/spi/cf_spi.c and
> arch/m68k/include/asm/coldfire/dspi.h.
> I need submit patch to remove them later.
> Board dts files are also needed to make this change work.

Apart from one thing (the chip selects) this all looks correct to me.
Some style comments below. Also the patch seems to be in Courier font!

I'm interested in your thoughts on what is missing from the SPI uclass too.

>
> Changes in v1: None
>
> drivers/spi/Makefile   |   1 +
> drivers/spi/fsl_dspi.c | 461
> +++++++++++++++++++++++++++++++++++++++++++++++++
> include/fsl_dspi.h     | 156 +++++++++++++++++
> 3 files changed, 618 insertions(+)
> create mode 100644 drivers/spi/fsl_dspi.c
> create mode 100644 include/fsl_dspi.h
>
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index edbd520..9c2b8de 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -49,3 +49,4 @@ obj-$(CONFIG_TI_QSPI) += ti_qspi.o
> obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
> obj-$(CONFIG_ZYNQ_SPI) += zynq_spi.o
> obj-$(CONFIG_FSL_QSPI) += fsl_qspi.o
> +obj-$(CONFIG_FSL_DSPI) += fsl_dspi.o
> diff --git a/drivers/spi/fsl_dspi.c b/drivers/spi/fsl_dspi.c
> new file mode 100644
> index 0000000..69c037b
> --- /dev/null
> +++ b/drivers/spi/fsl_dspi.c
> @@ -0,0 +1,461 @@
> +/*
> + * (C) Copyright 2000-2003
> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> + *
> + * Copyright (C) 2004-2009, 2015 Freescale Semiconductor, Inc.
> + * TsiChung Liew (Tsi-Chung.Liew at freescale.com)
> + * Chao Fu (B44548 at freescale.com)
> + * Haikun Wang (B53464 at freescale.com)
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +#include <dm.h>
> +#include <errno.h>
> +#include <common.h>
> +#include <spi.h>
> +#include <malloc.h>
> +#include <asm/io.h>
> +#include <fdtdec.h>
> +#include <asm/arch/clock.h>
> +#include <fsl_dspi.h>
> +
> +

Remove extract blank line

> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/* fsl_dspi_platdata flag */
> +#define DSPI_FLAG_REGMAP_ENDIAN_BIG    (1 << 0)
> +
> +/* idle data value */
> +#define DSPI_IDLE_VAL                 (0x0)

Please no brackets around simple constants (I understand if they are
-ve, but this serves no purpose).

> +
> +/* max chipselect signals number */
> +#define FSL_DSPI_MAX_CHIPSELECT        (6)
> +
> +/* CTAR register pre-configure value */
> +#define DSPI_CTAR_DEFAULT_VALUE        (DSPI_CTAR_TRSZ(7) | \
> +                                       DSPI_CTAR_PCSSCK_1CLK | \
> +                                       DSPI_CTAR_PASC(0) | \
> +                                       DSPI_CTAR_PDT(0) | \
> +                                       DSPI_CTAR_CSSCK(0) | \
> +                                       DSPI_CTAR_ASC(0) | \
> +                                       DSPI_CTAR_DT(0))
> +
> +/* CTAR register pre-configure mask */
> +#define DSPI_CTAR_SET_MODE_MASK        (DSPI_CTAR_TRSZ(15) | \
> +                                       DSPI_CTAR_PCSSCK(3) | \
> +                                       DSPI_CTAR_PASC(3) | \
> +                                       DSPI_CTAR_PDT(3) | \
> +                                       DSPI_CTAR_CSSCK(15) | \
> +                                       DSPI_CTAR_ASC(15) | \
> +                                       DSPI_CTAR_DT(15))
> +

Please comment these things below:

/**
 * struct fsl_dspi_platdata - platform data for ...
 *
 * @flag:    some sort of flag and you can find the enum here...
...

> +struct fsl_dspi_platdata {
> +       uint flag;
> +       uint baudrate;
> +       uint num_chipselect;
> +       uint regs;
> +};
> +
> +struct fsl_dspi_priv {
> +       uint mode;
> +       uint mcr_val;
> +       uint bus_clk;
> +       uint baudrate;
> +       uint charbit;
> +       uint num_chipselect;
> +       uint ctar_val[FSL_DSPI_MAX_CHIPSELECT];
> +       uint regs;
> +       struct dm_spi_slave_platdata *cur_slave_plat;

What is that for?

> +};
> +
> +static uint dspi_read32(struct udevice *bus, uint offset)
> +{
> +       struct fsl_dspi_platdata *plat = dev_get_platdata(bus);

blank line between declarations and code (throughout)

> +       return plat->flag & DSPI_FLAG_REGMAP_ENDIAN_BIG ?
> +               in_be32(plat->regs + offset) : in_le32(plat->regs + offset);

I think it is better to put regs in your private data rather than
platform data. Platform data should be used in your probe function
mostly. It's OK to use it sparing for uncommon things, but this feels
like something that should be in private data.

I have tended to put an fdt_addr_t in platform data and a struct
reg_definition * in private data.

Also where is your C structure for the registers? We normally use that
sort of thing in U-Boot.

> +}
> +
> +static void dspi_write32(struct udevice *bus, uint offset, uint val)
> +{
> +       struct fsl_dspi_platdata *plat = dev_get_platdata(bus);
> +       plat->flag & DSPI_FLAG_REGMAP_ENDIAN_BIG ?
> +               out_be32(plat->regs + offset, val) :
> +               out_le32(plat->regs + offset, val);
> +       return;
> +}
> +
> +static void dspi_halt(struct udevice *bus, u8 halt)
> +{
> +       uint mcr_val;
> +
> +       mcr_val = dspi_read32(bus, DSPI_MCR);
> +
> +       if (halt)
> +               mcr_val |= DSPI_MCR_HALT;
> +       else
> +               mcr_val &= ~DSPI_MCR_HALT;
> +
> +       dspi_write32(bus, DSPI_MCR, mcr_val);
> +
> +       return;
> +}
> +
> +static int fsl_dspi_child_pre_probe(struct udevice *dev)
> +{
> +       struct dm_spi_slave_platdata *slave_plat =
> dev_get_parent_platdata(dev);
> +       struct fsl_dspi_priv *priv = dev_get_priv(dev->parent);
> +
> +       if (slave_plat->cs >= priv->num_chipselect) {
> +               printf("DSPI invalid chipselect number %d(max %d)!\n",
> +                      slave_plat->cs, priv->num_chipselect - 1);

Maybe you want debug() here, up to you.

> +               return -EINVAL;
> +       }
> +
> +       priv->ctar_val[slave_plat->cs] = DSPI_CTAR_DEFAULT_VALUE;
> +
> +       priv->cur_slave_plat = slave_plat;

Not sure what you are doing here?

> +
> +       debug("DSPI pre_probe slave device on CS %u, max_hz %u, mode
> 0x%x.\n",
> +             slave_plat->cs, slave_plat->max_hz, slave_plat->mode);
> +
> +       return 0;
> +}
> +
> +static int fsl_dspi_child_post_remove(struct udevice *dev)
> +{
> +       struct fsl_dspi_priv *priv = dev_get_priv(dev->parent);
> +       priv->cur_slave_plat = NULL;

or here?

> +       return 0;
> +}
> +
> +static int fsl_dspi_probe(struct udevice *bus)
> +{
> +       struct fsl_dspi_platdata *plat = dev_get_platdata(bus);
> +       struct fsl_dspi_priv *priv = dev_get_priv(bus);
> +       struct dm_spi_bus *dm_spi_bus;
> +       uint mcr_val = 0;
> +
> +       dm_spi_bus = bus->uclass_priv;
> +
> +       /* get input clk frequency */
> +       priv->bus_clk = mxc_get_clock(MXC_DSPI_CLK);
> +       priv->regs = plat->regs;
> +       priv->num_chipselect = plat->num_chipselect;
> +       priv->baudrate = plat->baudrate;
> +       /* frame data length in bits, default 8bits */
> +       priv->charbit = 8;
> +
> +       dm_spi_bus->max_hz = plat->baudrate;
> +
> +       /* halt DSPI module */
> +       dspi_halt(bus, 1);
> +
> +       /* default: all CS inactive state is high */
> +       mcr_val |= DSPI_MCR_MSTR | DSPI_MCR_PCSIS_MASK |
> +               DSPI_MCR_CRXF | DSPI_MCR_CTXF;
> +       dspi_write32(bus, DSPI_MCR, mcr_val);
> +
> +       /* resume module */
> +       dspi_halt(bus, 0);
> +
> +       priv->mcr_val = mcr_val;
> +
> +       debug("%s probe done, bus-num %d.\n", bus->name, bus->seq);
> +
> +       return 0;
> +}
> +
> +static int fsl_dspi_claim_bus(struct udevice *bus)
> +{
> +       uint mcr_val;
> +
> +       dspi_halt(bus, 1);
> +       mcr_val = dspi_read32(bus, DSPI_MCR);
> +       /* flush RX and TX FIFO */
> +       mcr_val |= (DSPI_MCR_CTXF | DSPI_MCR_CRXF);
> +       dspi_write32(bus, DSPI_MCR, mcr_val);
> +
> +       dspi_halt(bus, 0);
> +
> +       return 0;
> +}
> +
> +static int fsl_dspi_release_bus(struct udevice *bus)
> +{
> +       /* halt module */
> +       dspi_halt(bus, 1);
> +       return 0;
> +}
> +
> +static void dspi_tx(struct udevice *bus, u32 ctrl, u16 data)
> +{
> +       while (((dspi_read32(bus, DSPI_SR) & 0x0000F000) >> 12) >= 4)
> +               ;

Can this ever timeout/hang? Also why are you using u16 for the data?
It doesn't seem necessary here or in the next function. You could use
u32 I think?

> +
> +       dspi_write32(bus, DSPI_TFR, (ctrl | data));
> +
> +       return;
> +}
> +
> +static u16 dspi_rx(struct udevice *bus)
> +{
> +       while ((dspi_read32(bus, DSPI_SR) & 0x000000F0) == 0)
> +               ;

or this?

> +
> +       return (u16)(dspi_read32(bus, DSPI_RFR) & 0xFFFF);
> +}
> +
> +static int fsl_dspi_xfer(struct udevice *dev, unsigned int bitlen,
> +               const void *dout, void *din, unsigned long flags)
> +{
> +       struct fsl_dspi_priv *priv;
> +       struct dm_spi_slave_platdata *slave_plat =
> dev_get_parent_platdata(dev);
> +       struct udevice *bus;
> +       u16 *spi_rd16 = NULL, *spi_wr16 = NULL;
> +       u8 *spi_rd = NULL, *spi_wr = NULL;
> +       static u32 ctrl;
> +       uint len = bitlen >> 3;
> +
> +       bus = dev->parent;
> +       if (bus)
> +               priv = dev_get_priv(bus);
> +       else
> +               return -ENODEV;

This must have a parent. There is no need for this check. If someone
calls this with the root device they deserve trouble! If you like you
could have

assert(bus);

> +
> +       if (priv->charbit == 16) {
> +               bitlen >>= 1;
> +               spi_wr16 = (u16 *)dout;
> +               spi_rd16 = (u16 *)din;
> +       } else {
> +               spi_wr = (u8 *)dout;
> +               spi_rd = (u8 *)din;
> +       }
> +
> +       if ((flags & SPI_XFER_BEGIN) == SPI_XFER_BEGIN)
> +               ctrl |= DSPI_TFR_CONT;
> +
> +       ctrl = ctrl & DSPI_TFR_CONT;
> +       ctrl = ctrl | DSPI_TFR_CTAS(0) | DSPI_TFR_PCS(slave_plat->cs);
> +
> +       if (len > 1) {
> +               int tmp_len = len - 1;
> +               while (tmp_len--) {
> +                       if (dout != NULL) {
> +                               if (priv->charbit == 16)
> +                                       dspi_tx(bus, ctrl, *spi_wr16++);
> +                               else
> +                                       dspi_tx(bus, ctrl, *spi_wr++);
> +                               dspi_rx(bus);
> +                       }
> +
> +                       if (din != NULL) {
> +                               dspi_tx(bus, ctrl, DSPI_IDLE_VAL);
> +                               if (priv->charbit == 16)
> +                                       *spi_rd16++ = dspi_rx(bus);
> +                               else
> +                                       *spi_rd++ = dspi_rx(bus);
> +                       }
> +               }
> +
> +               len = 1;        /* remaining byte */
> +       }
> +
> +       if ((flags & SPI_XFER_END) == SPI_XFER_END)
> +               ctrl &= ~DSPI_TFR_CONT;
> +
> +       if (len) {
> +               if (dout != NULL) {
> +                       if (priv->charbit == 16)
> +                               dspi_tx(bus, ctrl, *spi_wr16);
> +                       else
> +                               dspi_tx(bus, ctrl, *spi_wr);
> +                       dspi_rx(bus);
> +               }
> +
> +               if (din != NULL) {
> +                       dspi_tx(bus, ctrl, DSPI_IDLE_VAL);
> +                       if (priv->charbit == 16)
> +                               *spi_rd16 = dspi_rx(bus);
> +                       else
> +                               *spi_rd = dspi_rx(bus);
> +               }
> +       } else {
> +               /* dummy read */
> +               dspi_tx(bus, ctrl, DSPI_IDLE_VAL);
> +               dspi_rx(bus);
> +       }
> +
> +       return 0;
> +}
> +
> +static int fsl_dspi_hz_to_spi_baud(int *pbr, int *br,
> +               int speed_hz, uint clkrate)

Should document args here.

> +{
> +       /* Valid baud rate pre-scaler values */
> +       int pbr_tbl[4] = {2, 3, 5, 7};
> +       int brs[16] = {2, 4, 6, 8,
> +               16, 32, 64, 128,
> +               256, 512, 1024, 2048,
> +               4096, 8192, 16384, 32768};
> +       int temp, i = 0, j = 0;
> +
> +       temp = clkrate / speed_hz;
> +
> +       for (i = 0; i < ARRAY_SIZE(pbr_tbl); i++)
> +               for (j = 0; j < ARRAY_SIZE(brs); j++) {
> +                       if (pbr_tbl[i] * brs[j] >= temp) {
> +                               *pbr = i;
> +                               *br = j;
> +                               return 0;
> +                       }
> +               }
> +
> +       printf("Can not find valid baud rate,speed_hz is %d, ", speed_hz);
> +       printf("clkrate is %d, we use the max prescaler value.\n", clkrate);
> +
> +       *pbr = ARRAY_SIZE(pbr_tbl) - 1;
> +       *br =  ARRAY_SIZE(brs) - 1;
> +       return -EINVAL;
> +}
> +
> +static int fsl_dspi_set_speed(struct udevice *bus, uint speed)
> +{
> +       u8 cs;
> +       int ret;
> +       uint bus_setup;
> +       int best_i, best_j, bus_clk;
> +       struct fsl_dspi_priv *priv = dev_get_priv(bus);
> +       struct dm_spi_slave_platdata *slave_plat = priv->cur_slave_plat;
> +
> +       cs = slave_plat->cs;

You can't do this. Is the problem that you have a different speed
setting for each chip select? If so then we should either fix the
uclass interface to support this, or you could store the speed and
adjust it just before the transfer. But in this function you don't
know the chip select and should not try to guess it.

> +
> +       bus_clk = priv->bus_clk;
> +
> +       debug("DSPI set_speed: expected SCK speed %u, bus_clk %u.\n",
> +             speed, bus_clk);
> +
> +       bus_setup = dspi_read32(bus, DSPI_CTAR(0));
> +       bus_setup &= ~(DSPI_CTAR_DBR | DSPI_CTAR_PBR(0x3) |
> DSPI_CTAR_BR(0xf));
> +       bus_setup |= priv->ctar_val[cs];
> +
> +       ret = fsl_dspi_hz_to_spi_baud(&best_i, &best_j, speed, bus_clk);
> +       if (ret) {
> +               speed = priv->baudrate;
> +               debug("DSPI set_speed use default SCK rate %u.\n", speed);
> +               fsl_dspi_hz_to_spi_baud(&best_i, &best_j, speed, bus_clk);
> +       }
> +
> +       bus_setup |= (DSPI_CTAR_PBR(best_i) | DSPI_CTAR_BR(best_j));
> +       dspi_write32(bus, DSPI_CTAR(0), bus_setup);
> +
> +       priv->baudrate = speed;
> +
> +       return 0;
> +}
> +
> +static int fsl_dspi_set_mode(struct udevice *bus, uint mode)
> +{
> +       u8 cs;
> +       uint bus_setup;
> +       uint mcr_val;
> +       struct fsl_dspi_priv *priv = dev_get_priv(bus);
> +       struct dm_spi_slave_platdata *slave_plat = priv->cur_slave_plat;
> +
> +       debug("DSPI set_mode: mode 0x%x.\n", mode);
> +
> +       cs = slave_plat->cs;

Similar point here.

> +
> +       bus_setup = dspi_read32(bus, DSPI_CTAR(0));
> +       bus_setup &= ~DSPI_CTAR_SET_MODE_MASK;
> +       bus_setup |= priv->ctar_val[cs];
> +       bus_setup &= ~(DSPI_CTAR_CPOL | DSPI_CTAR_CPHA | DSPI_CTAR_LSBFE);
> +
> +       if (mode & SPI_CPOL)
> +               bus_setup |= DSPI_CTAR_CPOL;
> +       if (mode & SPI_CPHA)
> +               bus_setup |= DSPI_CTAR_CPHA;
> +       if (mode & SPI_LSB_FIRST)
> +               bus_setup |= DSPI_CTAR_LSBFE;
> +
> +       dspi_write32(bus, DSPI_CTAR(0), bus_setup);
> +
> +       if (mode & SPI_CS_HIGH) {
> +               dspi_halt(bus, 1);
> +               mcr_val = dspi_read32(bus, DSPI_MCR);
> +               /* CSx inactive state is low */
> +               mcr_val &= ~DSPI_MCR_PCSIS(cs);
> +               dspi_write32(bus, DSPI_MCR, mcr_val);
> +               dspi_halt(bus, 0);
> +       }
> +
> +       priv->mode = mode;
> +       priv->charbit = ((dspi_read32(bus, DSPI_CTAR(0)) & 0x78000000)
> +                == 0x78000000) ? 16 : 8;
> +
> +       return 0;
> +}
> +
> +static int fsl_dspi_bind(struct udevice *bus)
> +{
> +       debug("%s assigned req_seq %d.\n", bus->name, bus->req_seq);
> +       return 0;

You can drop this unless you want it for debugging. If the latter,
please add a comment like /* This doesn't do anything except help with
debugging */

> +}
> +
> +static int fsl_dspi_ofdata_to_platdata(struct udevice *bus)
> +{
> +       uint addr;
> +       struct fsl_dspi_platdata *plat = bus->platdata;
> +       const void *blob = gd->fdt_blob;
> +       int node = bus->of_offset;
> +
> +       plat->flag = 0;

It will already be zero. All data allocated by driver model is zeroed.

> +       if (fdtdec_get_bool(blob, node, "big-endian"))
> +               plat->flag |= DSPI_FLAG_REGMAP_ENDIAN_BIG;
> +
Extra black line here

> +
> +       plat->num_chipselect =
> +               fdtdec_get_int(blob, node, "num-cs",
> FSL_DSPI_MAX_CHIPSELECT);
> +
> +       addr = fdtdec_get_addr(blob, node, "reg");

You can use dev_get_addr(dev) if you like.

> +       if (addr == FDT_ADDR_T_NONE) {
> +               printf("DSPI: Can't get base address or size\n");
> +               return -ENOMEM;
> +       }
> +       plat->regs = addr;
> +
> +       plat->baudrate =
> +               fdtdec_get_int(blob, node, "spi-max-frequency", 10000000);

The name 'baudrate' bothers me, but that if that is what you normally
call it on your hardware, then that is fine.

> +
> +       debug("DSPI: regs=0x%x, max-frequency=%d, endianess=%s,
> num-cs=%d\n",
> +             plat->regs, plat->baudrate,
> +             plat->flag & DSPI_FLAG_REGMAP_ENDIAN_BIG ? "be" : "le",
> +             plat->num_chipselect);
> +
> +       return 0;
> +}
> +static const struct dm_spi_ops fsl_dspi_ops = {
> +       .claim_bus      = fsl_dspi_claim_bus,
> +       .release_bus    = fsl_dspi_release_bus,
> +       .xfer           = fsl_dspi_xfer,
> +       .set_speed      = fsl_dspi_set_speed,
> +       .set_mode       = fsl_dspi_set_mode,
> +};
> +
> +static const struct udevice_id fsl_dspi_ids[] = {
> +       { .compatible = "fsl,vf610-dspi" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(fsl_dspi) = {
> +       .name   = "fsl_dspi",
> +       .id     = UCLASS_SPI,
> +       .of_match = fsl_dspi_ids,
> +       .ops    = &fsl_dspi_ops,
> +       .ofdata_to_platdata = fsl_dspi_ofdata_to_platdata,
> +       .platdata_auto_alloc_size = sizeof(struct fsl_dspi_platdata),
> +       .priv_auto_alloc_size = sizeof(struct fsl_dspi_priv),
> +       .probe  = fsl_dspi_probe,
> +       .child_pre_probe = fsl_dspi_child_pre_probe,
> +       .child_post_remove = fsl_dspi_child_post_remove,
> +       .bind = fsl_dspi_bind,
> +};
> diff --git a/include/fsl_dspi.h b/include/fsl_dspi.h
> new file mode 100644
> index 0000000..8cf02d3
> --- /dev/null
> +++ b/include/fsl_dspi.h
> @@ -0,0 +1,156 @@
> +/*
> + * Freescale DSPI Module Defines
> + *
> + * Copyright (C) 2004-2007, 2015 Freescale Semiconductor, Inc.
> + * TsiChung Liew (Tsi-Chung.Liew at freescale.com)
> + * Chao Fu (B44548 at freesacle.com)
> + * Haikun Wang (B53464 at freescale.com)
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef _FSL_DSPI_H_
> +#define _FSL_DSPI_H_
> +
> +/* DMA Serial Peripheral Interface (DSPI) */
> +struct dspi {
> +       u32 mcr;        /* 0x00 */
> +       u32 resv0;      /* 0x04 */
> +       u32 tcr;        /* 0x08 */
> +       u32 ctar[8];    /* 0x0C - 0x28 */
> +       u32 sr;         /* 0x2C */
> +       u32 irsr;       /* 0x30 */
> +       u32 tfr;        /* 0x34 - PUSHR */
> +       u32 rfr;        /* 0x38 - POPR */
> +#ifdef CONFIG_MCF547x_8x
> +       u32 tfdr[4];    /* 0x3C */
> +       u8 resv2[0x30]; /* 0x40 */
> +       u32 rfdr[4];    /* 0x7C */
> +#else
> +       u32 tfdr[16];   /* 0x3C */
> +       u32 rfdr[16];   /* 0x7C */
> +#endif

Since you have this structure, can you not use it in your code for
register access?

> +};
> +
> +/* Module configuration */
> +#define DSPI_MCR                        (0x0)
> +#define DSPI_MCR_MSTR                  (0x80000000)
> +#define DSPI_MCR_CSCK                  (0x40000000)
> +#define DSPI_MCR_DCONF(x)              (((x)&0x03)<<28)
> +#define DSPI_MCR_FRZ                   (0x08000000)
> +#define DSPI_MCR_MTFE                  (0x04000000)
> +#define DSPI_MCR_PCSSE                 (0x02000000)
> +#define DSPI_MCR_ROOE                  (0x01000000)
> +#define DSPI_MCR_PCSIS(x)               (1<<(16+(x)))
> +#define DSPI_MCR_PCSIS_MASK             (0xff<<16)
> +#define DSPI_MCR_CSIS7                 (0x00800000)
> +#define DSPI_MCR_CSIS6                 (0x00400000)
> +#define DSPI_MCR_CSIS5                 (0x00200000)
> +#define DSPI_MCR_CSIS4                 (0x00100000)
> +#define DSPI_MCR_CSIS3                 (0x00080000)

Please remove brackets where not needed

> +#define DSPI_MCR_CSIS2                 (0x00040000)
> +#define DSPI_MCR_CSIS1                 (0x00020000)
> +#define DSPI_MCR_CSIS0                 (0x00010000)
> +#define DSPI_MCR_DOZE                  (0x00008000)
> +#define DSPI_MCR_MDIS                  (0x00004000)
> +#define DSPI_MCR_DTXF                  (0x00002000)
> +#define DSPI_MCR_DRXF                  (0x00001000)
> +#define DSPI_MCR_CTXF                  (0x00000800)
> +#define DSPI_MCR_CRXF                  (0x00000400)
> +#define DSPI_MCR_SMPL_PT(x)            (((x)&0x03)<<8)

Spaces around operators

Also can you please use tabs instead of spaces to indent things? It
looks like these are spaces.

> +#define DSPI_MCR_FCPCS                 (0x00000001)
> +#define DSPI_MCR_PES                   (0x00000001)
> +#define DSPI_MCR_HALT                  (0x00000001)
> +
> +/* Transfer count */
> +#define DSPI_TCR                        (0x8)
> +#define DSPI_TCR_SPI_TCNT(x)           (((x)&0x0000FFFF)<<16)
> +
> +/* Clock and transfer attributes */
> +#define DSPI_CTAR(x)                    (0x0c + (x * 4))
> +#define DSPI_CTAR_DBR                  (0x80000000)
> +#define DSPI_CTAR_TRSZ(x)              (((x)&0x0F)<<27)
> +#define DSPI_CTAR_CPOL                 (0x04000000)
> +#define DSPI_CTAR_CPHA                 (0x02000000)
> +#define DSPI_CTAR_LSBFE                        (0x01000000)
> +#define DSPI_CTAR_PCSSCK(x)            (((x)&0x03)<<22)
> +#define DSPI_CTAR_PCSSCK_7CLK          (0x00A00000)
> +#define DSPI_CTAR_PCSSCK_5CLK          (0x00800000)
> +#define DSPI_CTAR_PCSSCK_3CLK          (0x00400000)
> +#define DSPI_CTAR_PCSSCK_1CLK          (0x00000000)
> +#define DSPI_CTAR_PASC(x)              (((x)&0x03)<<20)
> +#define DSPI_CTAR_PASC_7CLK            (0x00300000)
> +#define DSPI_CTAR_PASC_5CLK            (0x00200000)
> +#define DSPI_CTAR_PASC_3CLK            (0x00100000)
> +#define DSPI_CTAR_PASC_1CLK            (0x00000000)
> +#define DSPI_CTAR_PDT(x)               (((x)&0x03)<<18)
> +#define DSPI_CTAR_PDT_7CLK             (0x000A0000)
> +#define DSPI_CTAR_PDT_5CLK             (0x00080000)
> +#define DSPI_CTAR_PDT_3CLK             (0x00040000)
> +#define DSPI_CTAR_PDT_1CLK             (0x00000000)
> +#define DSPI_CTAR_PBR(x)               (((x)&0x03)<<16)
> +#define DSPI_CTAR_PBR_7CLK             (0x00030000)
> +#define DSPI_CTAR_PBR_5CLK             (0x00020000)
> +#define DSPI_CTAR_PBR_3CLK             (0x00010000)
> +#define DSPI_CTAR_PBR_1CLK             (0x00000000)
> +#define DSPI_CTAR_CSSCK(x)             (((x)&0x0F)<<12)
> +#define DSPI_CTAR_ASC(x)               (((x)&0x0F)<<8)
> +#define DSPI_CTAR_DT(x)                        (((x)&0x0F)<<4)
> +#define DSPI_CTAR_BR(x)                        (((x)&0x0F))
> +
> +/* Status */
> +#define DSPI_SR                         (0x2c)
> +#define DSPI_SR_TCF                    (0x80000000)
> +#define DSPI_SR_TXRXS                  (0x40000000)
> +#define DSPI_SR_EOQF                   (0x10000000)
> +#define DSPI_SR_TFUF                   (0x08000000)
> +#define DSPI_SR_TFFF                   (0x02000000)
> +#define DSPI_SR_RFOF                   (0x00080000)
> +#define DSPI_SR_RFDF                   (0x00020000)
> +#define DSPI_SR_TXCTR(x)               (((x)&0x0F)<<12)
> +#define DSPI_SR_TXPTR(x)               (((x)&0x0F)<<8)
> +#define DSPI_SR_RXCTR(x)               (((x)&0x0F)<<4)
> +#define DSPI_SR_RXPTR(x)               (((x)&0x0F))
> +
> +/* DMA/interrupt request selct and enable */
> +#define DSPI_IRSR                       (0x30)
> +#define DSPI_IRSR_TCFE                 (0x80000000)
> +#define DSPI_IRSR_EOQFE                        (0x10000000)
> +#define DSPI_IRSR_TFUFE                        (0x08000000)
> +#define DSPI_IRSR_TFFFE                        (0x02000000)
> +#define DSPI_IRSR_TFFFS                        (0x01000000)
> +#define DSPI_IRSR_RFOFE                        (0x00080000)
> +#define DSPI_IRSR_RFDFE                        (0x00020000)
> +#define DSPI_IRSR_RFDFS                        (0x00010000)
> +
> +/* Transfer control - 32-bit access */
> +#define DSPI_TFR                        (0x34)
> +#define DSPI_TFR_PCS(x)                 (((1 << x) & 0x0000003f) << 16)
> +#define DSPI_TFR_CONT                  (0x80000000)
> +#define DSPI_TFR_CTAS(x)               (((x)&0x07)<<28)
> +#define DSPI_TFR_EOQ                   (0x08000000)
> +#define DSPI_TFR_CTCNT                 (0x04000000)
> +#define DSPI_TFR_CS7                   (0x00800000)
> +#define DSPI_TFR_CS6                   (0x00400000)
> +#define DSPI_TFR_CS5                   (0x00200000)
> +#define DSPI_TFR_CS4                   (0x00100000)
> +#define DSPI_TFR_CS3                   (0x00080000)
> +#define DSPI_TFR_CS2                   (0x00040000)
> +#define DSPI_TFR_CS1                   (0x00020000)
> +#define DSPI_TFR_CS0                   (0x00010000)
> +
> +/* Transfer Fifo */
> +#define DSPI_TFR_TXDATA(x)             (((x)&0xFFFF))
> +
> +/* Bit definitions and macros for DRFR */
> +#define DSPI_RFR                        (0x38)
> +#define DSPI_RFR_RXDATA(x)             (((x)&0xFFFF))
> +
> +/* Bit definitions and macros for DTFDR group */
> +#define DSPI_TFDR_TXDATA(x)            (((x)&0x0000FFFF))
> +#define DSPI_TFDR_TXCMD(x)             (((x)&0x0000FFFF)<<16)
> +
> +/* Bit definitions and macros for DRFDR group */
> +#define DSPI_RFDR_RXDATA(x)            (((x)&0x0000FFFF))
> +
> +#endif                         /* _FSL_DSPI_H_ */
> --
> 2.1.0.27.g96db324
>

Regards,
Simon


More information about the U-Boot mailing list