[U-Boot] [PATCH v3 03/11] drivers: spi: cf_spi: convert to driver model
Angelo Dureghello
angelo at sysam.it
Mon Dec 10 20:47:11 UTC 2018
Hi Jagan,
still sorry for the delay,
fixed all the points except 2, ready to send patch v4, please read below.
On Tue, Oct 23, 2018 at 04:58:22PM +0530, Jagan Teki wrote:
> On Sun, Oct 14, 2018 at 1:00 PM Angelo Dureghello <angelo at sysam.it> wrote:
> >
> > Converting to driver model and removes non-dm code.
> >
> > Signed-off-by: Angelo Dureghello <angelo at sysam.it>
> > ---
> > Changes for v2:
> > - removed non DM code part
> > - add default setup of CTAR registers
> > - add DT CTAR register setup support
> > Changes for v3:
> > - changed commit head
> > - removed spi_slave reference
> > - add #ifdefs for the case OF_PLATDATA is used
> > ---
> > drivers/spi/cf_spi.c | 517 +++++++++++++++---------
> > include/dm/platform_data/spi_coldfire.h | 29 ++
> > 2 files changed, 352 insertions(+), 194 deletions(-)
> > create mode 100644 include/dm/platform_data/spi_coldfire.h
> >
> > diff --git a/drivers/spi/cf_spi.c b/drivers/spi/cf_spi.c
> > index 522631cbbf..bc6a156df9 100644
> > --- a/drivers/spi/cf_spi.c
> > +++ b/drivers/spi/cf_spi.c
> > @@ -6,16 +6,25 @@
> > *
> > * Copyright (C) 2004-2009 Freescale Semiconductor, Inc.
> > * TsiChung Liew (Tsi-Chung.Liew at freescale.com)
> > + *
> > + * Support for DM and DT, non-DM code removed.
> > + * Copyright (C) 2018 Angelo Dureghello <angelo at sysam.it>
> > + *
> > + * TODO: fsl_dspi.c should work as a driver for the DSPI module.
> > */
> >
> > #include <common.h>
> > +#include <dm.h>
> > +#include <dm/platform_data/spi_coldfire.h>
> > #include <spi.h>
> > #include <malloc.h>
> > #include <asm/immap.h>
> > +#include <asm/io.h>
> >
> > -struct cf_spi_slave {
> > - struct spi_slave slave;
> > +struct coldfire_spi_priv {
> > + struct dspi *regs;
> > uint baudrate;
> > + int mode;
> > int charbit;
> > };
> >
> > @@ -38,14 +47,30 @@ DECLARE_GLOBAL_DATA_PTR;
> > #define SPI_MODE_MOD 0x00200000
> > #define SPI_DBLRATE 0x00100000
> >
> > -static inline struct cf_spi_slave *to_cf_spi_slave(struct spi_slave *slave)
> > -{
> > - return container_of(slave, struct cf_spi_slave, slave);
> > -}
> > +#define MCF_DSPI_MAX_CTAR_REGS 8
> >
> > -static void cfspi_init(void)
> > +/* Default values */
> > +#define MCF_DSPI_DEFAULT_SCK_FREQ 10000000
> > +#define MCF_DSPI_DEFAULT_MAX_CS 4
> > +#define MCF_DSPI_DEFAULT_MODE 0
> > +
> > +#define MCF_DSPI_DEFAULT_CTAR (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(1) | \
> > + DSPI_CTAR_BR(6))
> > +
> > - volatile dspi_t *dspi = (dspi_t *) MMAP_DSPI;
> > + struct dspi *dspi = cfspi->regs;
> > + int i;
> > +
> > + /* Default CTARs */
> > + for (i = 0; i < MCF_DSPI_MAX_CTAR_REGS; i++)
> > + writel(MCF_DSPI_DEFAULT_CTAR, &dspi->ctar[i]);
> >
> > cfspi_port_conf(); /* port configuration */
> >
> > @@ -53,128 +78,9 @@ static void cfspi_init(void)
> > DSPI_MCR_CSIS5 | DSPI_MCR_CSIS4 | DSPI_MCR_CSIS3 |
> > DSPI_MCR_CSIS2 | DSPI_MCR_CSIS1 | DSPI_MCR_CSIS0 |
> > DSPI_MCR_CRXF | DSPI_MCR_CTXF;
> > -
> > - /* Default setting in platform configuration */
> > -#ifdef CONFIG_SYS_DSPI_CTAR0
> > - dspi->ctar[0] = CONFIG_SYS_DSPI_CTAR0;
> > -#endif
> > -#ifdef CONFIG_SYS_DSPI_CTAR1
> > - dspi->ctar[1] = CONFIG_SYS_DSPI_CTAR1;
> > -#endif
> > -#ifdef CONFIG_SYS_DSPI_CTAR2
> > - dspi->ctar[2] = CONFIG_SYS_DSPI_CTAR2;
> > -#endif
> > -#ifdef CONFIG_SYS_DSPI_CTAR3
> > - dspi->ctar[3] = CONFIG_SYS_DSPI_CTAR3;
> > -#endif
> > -#ifdef CONFIG_SYS_DSPI_CTAR4
> > - dspi->ctar[4] = CONFIG_SYS_DSPI_CTAR4;
> > -#endif
> > -#ifdef CONFIG_SYS_DSPI_CTAR5
> > - dspi->ctar[5] = CONFIG_SYS_DSPI_CTAR5;
> > -#endif
> > -#ifdef CONFIG_SYS_DSPI_CTAR6
> > - dspi->ctar[6] = CONFIG_SYS_DSPI_CTAR6;
> > -#endif
> > -#ifdef CONFIG_SYS_DSPI_CTAR7
> > - dspi->ctar[7] = CONFIG_SYS_DSPI_CTAR7;
> > -#endif
> > -}
> > -
> > -static void cfspi_tx(u32 ctrl, u16 data)
> > -{
> > - volatile dspi_t *dspi = (dspi_t *) MMAP_DSPI;
> > -
> > - while ((dspi->sr & 0x0000F000) >= 4) ;
> > -
> > - dspi->tfr = (ctrl | data);
> > }
> >
> > -static u16 cfspi_rx(void)
> > -{
> > - volatile dspi_t *dspi = (dspi_t *) MMAP_DSPI;
> > -
> > - while ((dspi->sr & 0x000000F0) == 0) ;
> > -
> > - return (dspi->rfr & 0xFFFF);
> > -}
> > -
> > -static int cfspi_xfer(struct spi_slave *slave, uint bitlen, const void *dout,
> > - void *din, ulong flags)
> > -{
> > - struct cf_spi_slave *cfslave = to_cf_spi_slave(slave);
> > - u16 *spi_rd16 = NULL, *spi_wr16 = NULL;
> > - u8 *spi_rd = NULL, *spi_wr = NULL;
> > - static u32 ctrl = 0;
> > - uint len = bitlen >> 3;
> > -
> > - if (cfslave->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 & 0xFF000000) | ((1 << slave->cs) << 16);
> > -
> > - if (len > 1) {
> > - int tmp_len = len - 1;
> > - while (tmp_len--) {
> > - if (dout != NULL) {
> > - if (cfslave->charbit == 16)
> > - cfspi_tx(ctrl, *spi_wr16++);
> > - else
> > - cfspi_tx(ctrl, *spi_wr++);
> > - cfspi_rx();
> > - }
> > -
> > - if (din != NULL) {
> > - cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL);
> > - if (cfslave->charbit == 16)
> > - *spi_rd16++ = cfspi_rx();
> > - else
> > - *spi_rd++ = cfspi_rx();
> > - }
> > - }
> > -
> > - len = 1; /* remaining byte */
> > - }
> > -
> > - if ((flags & SPI_XFER_END) == SPI_XFER_END)
> > - ctrl &= ~DSPI_TFR_CONT;
> > -
> > - if (len) {
> > - if (dout != NULL) {
> > - if (cfslave->charbit == 16)
> > - cfspi_tx(ctrl, *spi_wr16);
> > - else
> > - cfspi_tx(ctrl, *spi_wr);
> > - cfspi_rx();
> > - }
> > -
> > - if (din != NULL) {
> > - cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL);
> > - if (cfslave->charbit == 16)
> > - *spi_rd16 = cfspi_rx();
> > - else
> > - *spi_rd = cfspi_rx();
> > - }
> > - } else {
> > - /* dummy read */
> > - cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL);
> > - cfspi_rx();
> > - }
> > -
> > - return 0;
> > -}
> > -
> > -static struct spi_slave *cfspi_setup_slave(struct cf_spi_slave *cfslave,
> > - uint mode)
> > +int __spi_set_speed(struct coldfire_spi_priv *cfspi, uint bus)
> > {
> > /*
> > * bit definition for mode:
> > @@ -189,7 +95,7 @@ static struct spi_slave *cfspi_setup_slave(struct cf_spi_slave *cfslave,
> > * 11 - 8: Delay after transfer scaler
> > * 7 - 0: SPI_CPHA, SPI_CPOL, SPI_LSB_FIRST
> > */
> > - volatile dspi_t *dspi = (dspi_t *) MMAP_DSPI;
> > + struct dspi *dspi = cfspi->regs;
> > int prescaler[] = { 2, 3, 5, 7 };
> > int scaler[] = {
> > 2, 4, 6, 8,
> > @@ -199,56 +105,38 @@ static struct spi_slave *cfspi_setup_slave(struct cf_spi_slave *cfslave,
> > };
> > int i, j, pbrcnt, brcnt, diff, tmp, dbr = 0;
> > int best_i, best_j, bestmatch = 0x7FFFFFFF, baud_speed;
> > - u32 bus_setup = 0;
> > + u32 bus_setup;
> > +
> > + /* Read current setup */
> > + bus_setup = readl(&dspi->ctar[bus]);
> >
> > tmp = (prescaler[3] * scaler[15]);
> > /* Maximum and minimum baudrate it can handle */
> > - if ((cfslave->baudrate > (gd->bus_clk >> 1)) ||
> > - (cfslave->baudrate < (gd->bus_clk / tmp))) {
> > + if ((cfspi->baudrate > (gd->bus_clk >> 1)) ||
> > + (cfspi->baudrate < (gd->bus_clk / tmp))) {
> > printf("Exceed baudrate limitation: Max %d - Min %d\n",
> > (int)(gd->bus_clk >> 1), (int)(gd->bus_clk / tmp));
> > - return NULL;
> > + return -1;
> > }
> >
> > /* Activate Double Baud when it exceed 1/4 the bus clk */
> > - if ((CONFIG_SYS_DSPI_CTAR0 & DSPI_CTAR_DBR) ||
> > - (cfslave->baudrate > (gd->bus_clk / (prescaler[0] * scaler[0])))) {
> > + if ((bus_setup & DSPI_CTAR_DBR) ||
> > + (cfspi->baudrate > (gd->bus_clk / (prescaler[0] * scaler[0])))) {
> > bus_setup |= DSPI_CTAR_DBR;
> > dbr = 1;
> > }
> >
> > - 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;
> > -
> > /* Overwrite default value set in platform configuration file */
> > - if (mode & SPI_MODE_MOD) {
> > -
> > - if ((mode & 0xF0000000) == 0)
> > - bus_setup |=
> > - dspi->ctar[cfslave->slave.bus] & 0x78000000;
> > - else
> > - bus_setup |= ((mode & 0xF0000000) >> 1);
> > -
> > + if (cfspi->mode & SPI_MODE_MOD) {
> > /*
> > * Check to see if it is enabled by default in platform
> > * config, or manual setting passed by mode parameter
> > */
> > - if (mode & SPI_DBLRATE) {
> > + if (cfspi->mode & SPI_DBLRATE) {
> > bus_setup |= DSPI_CTAR_DBR;
> > dbr = 1;
> > }
> > - bus_setup |= (mode & 0x0FC00000) >> 4; /* PSCSCK, PASC, PDT */
> > - bus_setup |= (mode & 0x000FFF00) >> 4; /* CSSCK, ASC, DT */
> > - } else
> > - bus_setup |= (dspi->ctar[cfslave->slave.bus] & 0x78FCFFF0);
> > -
> > - cfslave->charbit =
> > - ((dspi->ctar[cfslave->slave.bus] & 0x78000000) ==
> > - 0x78000000) ? 16 : 8;
> > + }
> >
> > pbrcnt = sizeof(prescaler) / sizeof(int);
> > brcnt = sizeof(scaler) / sizeof(int);
> > @@ -259,10 +147,10 @@ static struct spi_slave *cfspi_setup_slave(struct cf_spi_slave *cfslave,
> > for (j = 0; j < brcnt; j++) {
> > tmp = (baud_speed / scaler[j]) * (1 + dbr);
> >
> > - if (tmp > cfslave->baudrate)
> > - diff = tmp - cfslave->baudrate;
> > + if (tmp > cfspi->baudrate)
> > + diff = tmp - cfspi->baudrate;
> > else
> > - diff = cfslave->baudrate - tmp;
> > + diff = cfspi->baudrate - tmp;
> >
> > if (diff < bestmatch) {
> > bestmatch = diff;
> > @@ -271,65 +159,306 @@ static struct spi_slave *cfspi_setup_slave(struct cf_spi_slave *cfslave,
> > }
> > }
> > }
> > +
> > + bus_setup &= ~(DSPI_CTAR_PBR(0x03) | DSPI_CTAR_BR(0x0f));
> > bus_setup |= (DSPI_CTAR_PBR(best_i) | DSPI_CTAR_BR(best_j));
> > - dspi->ctar[cfslave->slave.bus] = bus_setup;
> > + writel(bus_setup, &dspi->ctar[bus]);
> > +
> > + return 0;
> > +}
> > +
> > +static int __spi_set_mode(struct coldfire_spi_priv *cfspi, uint bus)
> > +{
> > + struct dspi *dspi = cfspi->regs;
> > + u32 bus_setup = 0;
> > +
> > + if (cfspi->mode & SPI_CPOL)
> > + bus_setup |= DSPI_CTAR_CPOL;
> > + if (cfspi->mode & SPI_CPHA)
> > + bus_setup |= DSPI_CTAR_CPHA;
> > + if (cfspi->mode & SPI_LSB_FIRST)
> > + bus_setup |= DSPI_CTAR_LSBFE;
> > +
> > + /* Overwrite default value set in platform configuration file */
> > + if (cfspi->mode & SPI_MODE_MOD) {
> > + if ((cfspi->mode & 0xF0000000) == 0)
> > + bus_setup |=
> > + readl(&dspi->ctar[bus]) & 0x78000000;
> > + else
> > + bus_setup |= ((cfspi->mode & 0xF0000000) >> 1);
> > +
> > + /* PSCSCK, PASC, PDT */
> > + bus_setup |= (cfspi->mode & 0x0FC00000) >> 4;
> > + /* CSSCK, ASC, DT */
> > + bus_setup |= (cfspi->mode & 0x000FFF00) >> 4;
> > + } else {
> > + bus_setup |= (readl(&dspi->ctar[bus]) & 0x78FCFFF0);
> > + }
> > +
> > + cfspi->charbit =
> > + ((readl(&dspi->ctar[bus]) & 0x78000000) == 0x78000000) ? 16 : 8;
> >
> > - return &cfslave->slave;
> > + setbits_be32(&dspi->ctar[bus], bus_setup);
> > +
> > + return 0;
> > }
> > -#endif /* CONFIG_CF_DSPI */
> >
> > -#ifdef CONFIG_CMD_SPI
> > -int spi_cs_is_valid(unsigned int bus, unsigned int cs)
> > +static inline void __cfspi_tx(struct coldfire_spi_priv *cfspi,
> > + u32 ctrl, u16 data)
> > {
> > - if (((cs >= 0) && (cs < 8)) && ((bus >= 0) && (bus < 8)))
> > - return 1;
> > - else
> > - return 0;
> > + while ((readl(&cfspi->regs->sr) & 0x0000F000) >= 4)
> > + ;
> > +
> > + writel(ctrl | data, &cfspi->regs->tfr);
> > +}
> > +
> > +static inline u16 __cfspi_rx(struct coldfire_spi_priv *cfspi)
> > +{
> > + while ((readl(&cfspi->regs->sr) & 0x000000F0) == 0)
> > + ;
> > +
> > + return readw(&cfspi->regs->rfr);
> > +}
>
> Use wait_for bit calls instead of infine loops.
>
Seems i cannot use wait_for_bits for both the above cases.
The first case is a fifo level check ( >= 4) (on the other hand, it was also
wrong as a chek, so fixed it), while the second case waits for at least
one bit to 1, and so wait_for_bit can't apply too.
> > +
> > +static int __spi_xfer(struct coldfire_spi_priv *cfspi, uint cs, uint bitlen,
>
> no need for __ we don't have non-dm code
>
done
> > + const void *dout, void *din, ulong flags)
> > +{
> > + u16 *spi_rd16 = NULL, *spi_wr16 = NULL;
> > + u8 *spi_rd = NULL, *spi_wr = NULL;
> > + static u32 ctrl;
> > + uint len = bitlen >> 3;
> > +
> > + if (cfspi->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 & 0xFF000000) | ((1 << cs) << 16);
>
> macros?
>
done
> > +
> > + if (len > 1) {
> > + int tmp_len = len - 1;
> > +
> > + while (tmp_len--) {
> > + if (dout) {
> > + if (cfspi->charbit == 16)
> > + __cfspi_tx(cfspi, ctrl, *spi_wr16++);
> > + else
> > + __cfspi_tx(cfspi, ctrl, *spi_wr++);
> > + __cfspi_rx(cfspi);
> > + }
> > +
> > + if (din) {
> > + __cfspi_tx(cfspi, ctrl, CONFIG_SPI_IDLE_VAL);
>
> same here?
>
done
> > + if (cfspi->charbit == 16)
> > + *spi_rd16++ = __cfspi_rx(cfspi);
> > + else
> > + *spi_rd++ = __cfspi_rx(cfspi);
> > + }
> > + }
> > +
> > + len = 1; /* remaining byte */
> > + }
> > +
> > + if ((flags & SPI_XFER_END) == SPI_XFER_END)
>
> Why we need to check the macro again flags & SPI_XFER_END enogh?
>
fixed
> > + ctrl &= ~DSPI_TFR_CONT;
> > +
> > + if (len) {
> > + if (dout) {
> > + if (cfspi->charbit == 16)
> > + __cfspi_tx(cfspi, ctrl, *spi_wr16);
> > + else
> > + __cfspi_tx(cfspi, ctrl, *spi_wr);
> > + __cfspi_rx(cfspi);
> > + }
> > +
> > + if (din) {
> > + __cfspi_tx(cfspi, ctrl, CONFIG_SPI_IDLE_VAL);
> > + if (cfspi->charbit == 16)
> > + *spi_rd16 = __cfspi_rx(cfspi);
> > + else
> > + *spi_rd = __cfspi_rx(cfspi);
> > + }
> > + } else {
> > + /* dummy read */
> > + __cfspi_tx(cfspi, ctrl, CONFIG_SPI_IDLE_VAL);
> > + __cfspi_rx(cfspi);
>
> same here, no __
>
done
> > + }
> > +
> > + return 0;
> > }
> >
> > void spi_init(void)
> > {
> > - cfspi_init();
> > }
>
> remove spi_init if not used, can be another patch.
>
still cannot remove spi_init, left empty, but anyway moved cfspi_init() code
inside _probe() and removed cfspi_init().
> >
> > -struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> > - unsigned int max_hz, unsigned int mode)
> > +static int coldfire_spi_claim_bus(struct udevice *dev)
> > +{
> > + struct udevice *bus = dev->parent;
> > + struct dm_spi_slave_platdata *slave_plat =
> > + dev_get_parent_platdata(dev);
> > +
> > + return cfspi_claim_bus(bus->seq, slave_plat->cs);
> > +}
> > +
> > +static int coldfire_spi_release_bus(struct udevice *dev)
> > {
> > - struct cf_spi_slave *cfslave;
> > + struct udevice *bus = dev->parent;
> > + struct dm_spi_slave_platdata *slave_plat =
> > + dev_get_parent_platdata(dev);
> >
> > - if (!spi_cs_is_valid(bus, cs))
> > - return NULL;
> > + cfspi_release_bus(bus->seq, slave_plat->cs);
> >
> > - cfslave = spi_alloc_slave(struct cf_spi_slave, bus, cs);
> > - if (!cfslave)
> > - return NULL;
> > + return 0;
> > +}
> >
> > - cfslave->baudrate = max_hz;
> > +static int coldfire_spi_xfer(struct udevice *dev, unsigned int bitlen,
> > + const void *dout, void *din,
> > + unsigned long flags)
> > +{
> > + struct udevice *bus = dev_get_parent(dev);
> > + struct coldfire_spi_priv *cfspi = dev_get_priv(bus);
> > + struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
> >
> > - /* specific setup */
> > - return cfspi_setup_slave(cfslave, mode);
> > + return __spi_xfer(cfspi, slave_plat->cs, bitlen, dout, din, flags);
> > }
> >
> > -void spi_free_slave(struct spi_slave *slave)
> > +static int coldfire_spi_set_speed(struct udevice *bus, uint max_hz)
> > {
> > - struct cf_spi_slave *cfslave = to_cf_spi_slave(slave);
> > + struct coldfire_spi_priv *cfspi = dev_get_priv(bus);
> > +
> > + cfspi->baudrate = max_hz;
> >
> > - free(cfslave);
> > + return __spi_set_speed(cfspi, bus->seq);
>
> no __ and do it withough function.
>
>
> > }
> >
> > -int spi_claim_bus(struct spi_slave *slave)
> > +static int coldfire_spi_set_mode(struct udevice *bus, uint mode)
> > {
> > - return cfspi_claim_bus(slave->bus, slave->cs);
> > + struct coldfire_spi_priv *cfspi = dev_get_priv(bus);
> > +
> > + cfspi->mode = mode;
> > +
> > + return __spi_set_mode(cfspi, bus->seq);
>
> same here.
>
done
> > }
> >
> > -void spi_release_bus(struct spi_slave *slave)
> > +static int coldfire_spi_probe(struct udevice *bus)
> > {
> > - cfspi_release_bus(slave->bus, slave->cs);
> > + struct coldfire_spi_platdata *plat = dev_get_platdata(bus);
> > + struct coldfire_spi_priv *cfspi = dev_get_priv(bus);
> > + int i;
> > +
> > + cfspi->regs = (struct dspi *)plat->regs_addr;
> > +
> > + cfspi->baudrate = plat->speed_hz;
> > + cfspi->mode = plat->mode;
> > +
> > + for (i = 0; i < MCF_DSPI_MAX_CTAR_REGS; i++) {
> > + unsigned int ctar = 0;
> > +
> > + if (plat->ctar[i][0] == 0)
> > + break;
> > +
> > + ctar = DSPI_CTAR_TRSZ(plat->ctar[i][0]) |
> > + DSPI_CTAR_PCSSCK(plat->ctar[i][1]) |
> > + DSPI_CTAR_PASC(plat->ctar[i][2]) |
> > + DSPI_CTAR_PDT(plat->ctar[i][3]) |
> > + DSPI_CTAR_CSSCK(plat->ctar[i][4]) |
> > + DSPI_CTAR_ASC(plat->ctar[i][5]) |
> > + DSPI_CTAR_DT(plat->ctar[i][6]) |
> > + DSPI_CTAR_BR(plat->ctar[i][7]);
> > +
> > + writel(ctar, &cfspi->regs->ctar[i]);
> > + }
> > +
> > + __spi_init(cfspi);
>
> no __
>
done
> > +
> > + return 0;
> > }
> >
> > -int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> > - void *din, unsigned long flags)
> > +#if !CONFIG_IS_ENABLED(OF_PLATDATA)
>
> valid check is
> #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
>
done
> > +static int coldfire_dspi_ofdata_to_platdata(struct udevice *bus)
> > {
> > - return cfspi_xfer(slave, bitlen, dout, din, flags);
> > + fdt_addr_t addr;
> > + struct coldfire_spi_platdata *plat = bus->platdata;
> > + const void *blob = gd->fdt_blob;
> > + int node = dev_of_offset(bus);
> > + int *ctar, len;
> > +
> > + addr = devfdt_get_addr(bus);
> > + if (addr == FDT_ADDR_T_NONE) {
> > + debug("DSPI: Can't get base address or size\n");
>
> no need this debug.
>
done
> > + return -ENOMEM;
> > + }
> > + plat->regs_addr = addr;
> > +
> > + plat->num_cs = fdtdec_get_int(blob, node, "num-cs",
> > + MCF_DSPI_DEFAULT_MAX_CS);
> > +
> > + plat->speed_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
> > + MCF_DSPI_DEFAULT_SCK_FREQ);
> > +
> > + plat->mode = fdtdec_get_int(blob, node, "spi-mode",
> > + MCF_DSPI_DEFAULT_MODE);
> > +
> > + memset(plat->ctar, 0, sizeof(plat->ctar));
> > +
> > + ctar = (int *)fdt_getprop(blob, node, "ctar-params", &len);
> > +
> > + if (ctar && len) {
> > + int i, q, ctar_regs;
> > +
> > + ctar_regs = len / sizeof(unsigned int) / MAX_CTAR_FIELDS;
> > +
> > + if (ctar_regs > MAX_CTAR_REGS)
> > + ctar_regs = MAX_CTAR_REGS;
> > +
> > + for (i = 0; i < ctar_regs; i++) {
> > + for (q = 0; q < MAX_CTAR_FIELDS; q++)
> > + plat->ctar[i][q] = *ctar++;
> > + }
> > + }
> > +
> > + debug("DSPI: regs=%pa, max-frequency=%d, num-cs=%d, mode=%d\n",
> > + (void *)plat->regs_addr,
> > + plat->speed_hz, plat->num_cs, plat->mode);
> > +
> > + return 0;
> > }
> > -#endif /* CONFIG_CMD_SPI */
> > +#endif
> > +
> > +static const struct dm_spi_ops coldfire_spi_ops = {
> > + .claim_bus = coldfire_spi_claim_bus,
> > + .release_bus = coldfire_spi_release_bus,
> > + .xfer = coldfire_spi_xfer,
> > + .set_speed = coldfire_spi_set_speed,
> > + .set_mode = coldfire_spi_set_mode,
> > +};
> > +
> > +#if !CONFIG_IS_ENABLED(OF_PLATDATA)
>
> group this check related code at once.
>
done
> > +static const struct udevice_id coldfire_spi_ids[] = {
> > + { .compatible = "fsl,mcf-dspi" },
> > + { }
> > +};
> > +#endif
> > +
> > +U_BOOT_DRIVER(coldfire_spi) = {
> > + .name = "spi_coldfire",
> > + .id = UCLASS_SPI,
> > +#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> > + .of_match = coldfire_spi_ids,
> > +#endif
> > + .probe = coldfire_spi_probe,
> > + .ops = &coldfire_spi_ops,
> > + .platdata_auto_alloc_size = sizeof(struct coldfire_spi_platdata),
> > +#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> > + .ofdata_to_platdata = coldfire_dspi_ofdata_to_platdata,
> > +#endif
>
> group it above ifdef and move in once place.
>
done
> > + .priv_auto_alloc_size = sizeof(struct coldfire_spi_priv),
> > +};
> > +#endif /* CONFIG_CF_DSPI */
>
> I think we don't need this.
>
done
> Apart from above, there are extern functions in this driver which are
> using spi code in arch area so better to move here or handle via
> respective drivers.
I had a look but proper place for those 3 functions seems
/u-boot-coldfire/arch/m68k/cpu/mcf5445x/cpu_init.c
since there are some cpu sub-family #ifdef checks there. Just let me know, i
can move them here anyway.
Regards,
Angelo
More information about the U-Boot
mailing list