+Tom for the coding style question Hi,
On 4 March 2015 at 04:22, [email protected] <[email protected]> wrote: > >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] On Behalf Of Simon Glass >> Sent: Tuesday, March 03, 2015 2:56 AM >> To: Wang Haikun-B53464 >> Cc: [email protected]; Jagan Teki >> Subject: Re: [U-Boot] [PATCH v1]dm : spi: Convert Freescale DSPI to >> driver model >> >> Hi, >> >> On 28 February 2015 at 03:54, [email protected] >> <[email protected]> wrote: >> > Move the Freescale DSPI driver over to driver model. >> > >> > Signed-off-by: Haikun Wang <[email protected]> >> > --- >> > >> > 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, [email protected]. >> > + * >> > + * Copyright (C) 2004-2009, 2015 Freescale Semiconductor, Inc. >> > + * TsiChung Liew ([email protected]) >> > + * Chao Fu ([email protected]) >> > + * Haikun Wang ([email protected]) >> > + * >> > + * 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. See the bottom of this page. http://www.denx.de/wiki/U-Boot/CodingStyle >> >> > +} >> > + >> > +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); > [] fine, 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. > [] fine, I 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. The current driver model SPI uclass only supports a single speed/mode for the whole bus. It does not understand different modes. One option is to stop the speed/mode in the set_mode/set_speed() function and then action it the in the activate() method. By that point, you know the chip select and can do the right thing. If you do that, then fsl_dspi_set_mode() will only store the value in priv, it will not touch the hardware. >> >> > + >> > + 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 >> */ > [] fine, 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; >> >> It will already be zero. All data allocated by driver model is zeroed. > [] fine, will remove. >> >> > + 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. > [] fine, I will rename with "speed_hz", how about? Sure. >> >> > + >> > + 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 ([email protected]) >> > + * Chao Fu ([email protected]) >> > + * Haikun Wang ([email protected]) >> > + * >> > + * 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. >> That seems to go against U-Boot style to me. Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
