[U-Boot] [PATCH v1]dm : spi: Convert Freescale DSPI to driver model
Wang Haikun
haikun.wang at freescale.com
Thu Mar 5 10:43:33 CET 2015
On 3/3/2015 2:56 AM, Simon Glass wrote:
> 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).
fine.
>
>> +
>> +/* 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:
fine.
>
> /**
> * 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.
OK, I will move it.
>
> 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.
There is no obvious difference between the two methods. And I think the
code seems more clean in current way.
>
>> +}
>> +
>> +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.
OK.
>
>> + 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?
fine, I will handle the timeout case. Max data size of every transfer is
16bits, so I use u16.
>
>> +
>> + 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);
I will remove.
>
>> +
>> + 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.
Will add.
>
>> +{
>> + /* 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.
fine, CS is really not needed in this function, I will improve.
>
>> +
>> + 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.
[] Here is different from the above function, I need know the cs here,
or I remove some configuration to other function?
There are two reasons about this.
A. We need configure transfer mode and clock attributes in CTAR register
before transfer.
There are some other attributes except CPOL, CPHA, LSBFE in CTAR register.
And those attributes value may be different between every chipselect
device.
I get CPOL, CPHA, LSBFE from SPI uclass.
I can get the rest attributes' value by two methods, defining macros or
reading from device tree.
I adopt the first method in this version.
Later I plan to add a attribute in flash device dts node.
My current process flow:
1. define macro for CTAR pre-configure value, CTAR pre-configure values
may be different between every chipselect device and I only define one
in this version, need improve.
2. Save CTAR pre-configure value in function 'child_pre_probe' identify
with chipselect number.
3. And then write the CTAR pre-register value to register in function
'set_mode' according corresponding chipselect number.
B. when configure the state of PCSx active, I also need know the
chipselect number because DSPI have more than one chipselect signal and
they need be configured independently.
>
>> +
>> + 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 */
I will add the comment.
>
>> +}
>> +
>> +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;
I will remove.
>
> 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.
any difference?
>
>> + 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.
I will rename with "speed_hz", how about?
>
>> +
>> + 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?
I copy it from the old driver. I will remove it if we keep the current
registers access method.
>
>> +};
>> +
>> +/* 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
OK.
>
> Also can you please use tabs instead of spaces to indent things? It
> looks like these are spaces.
My mistake.
--
Best regards,
Wang Haikun
>
>> +#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