On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford aford173@gmail.com wrote:

On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki jagan@amarulasolutions.com wrote:

...

davinci_spi now support dt along with platform data, respective boards need to switch into dm for the same.

Cc: Adam Ford aford173@gmail.com Cc: Vitaly Andrianov vitalya@ti.com Cc: Stefano Babic sbabic@denx.de Cc: Peter Howard phoward@gme.net.au Cc: Tom Rini trini@konsulko.com Signed-off-by: Jagan Teki jagan@amarulasolutions.com

---

drivers/spi/Kconfig | 12 +- drivers/spi/davinci_spi.c | 289 +++++++------------------ include/dm/platform_data/spi_davinci.h | 15 ++ 3 files changed, 97 insertions(+), 219 deletions(-) create mode 100644 include/dm/platform_data/spi_davinci.h

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index d046e919b4..18ebff0231 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -80,6 +80,12 @@ config CADENCE_QSPI used to access the SPI NOR flash on platforms embedding this Cadence IP core.

+config DAVINCI_SPI

•   bool "Davinci & Keystone SPI driver"
  

•   depends on ARCH_DAVINCI || ARCH_KEYSTONE
  

•   help
  

•     Enable the Davinci SPI driver
  

config DESIGNWARE_SPI bool "Designware SPI driver" help @@ -281,12 +287,6 @@ config FSL_QSPI used to access the SPI NOR flash on platforms embedding this Freescale IP core.

-config DAVINCI_SPI

•   bool "Davinci & Keystone SPI driver"
  

•   depends on ARCH_DAVINCI || ARCH_KEYSTONE
  

•   help
  

•     Enable the Davinci SPI driver
  

config SH_SPI bool "SuperH SPI driver" help diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c index a822858323..5007e6c618 100644 --- a/drivers/spi/davinci_spi.c +++ b/drivers/spi/davinci_spi.c @@ -14,6 +14,7 @@ #include <asm/io.h> #include <asm/arch/hardware.h> #include <dm.h> +#include <dm/platform_data/spi_davinci.h>

/* SPIGCR0 */ #define SPIGCR0_SPIENA_MASK 0x1 @@ -118,9 +119,6 @@ struct davinci_spi_regs {

/* davinci spi slave */ struct davinci_spi_slave { -#ifndef CONFIG_DM_SPI

•   struct spi_slave slave;
  

-#endif struct davinci_spi_regs *regs; unsigned int freq; /* current SPI bus frequency */ unsigned int mode; /* current SPI mode used */ @@ -240,11 +238,43 @@ static int davinci_spi_read_write(struct davinci_spi_slave *ds, unsigned return 0; }

+static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) +{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  
  

-static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs)

•   debug("%s speed %u\n", __func__, max_hz);
  

•   if (max_hz > CONFIG_SYS_SPI_CLK / 2)
  

•           return -EINVAL;
  

•   ds->freq = max_hz;
  

•   return 0;
  

+}

+static int davinci_spi_set_mode(struct udevice *bus, uint mode) +{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s mode %u\n", __func__, mode);
  

•   ds->mode = mode;
  

•   return 0;
  

+}

+static int davinci_spi_claim_bus(struct udevice *dev) {

•   struct dm_spi_slave_platdata *slave_plat =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
    unsigned int mode = 0, scalar;
  
  

•   if (slave_plat->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->half_duplex = slave_plat->mode & SPI_PREAMBLE;
  

•   /* Enable the SPI hardware */
    writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
    udelay(1000);
  

@@ -254,7 +284,7 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1);

    /* CS, CLK, SIMO and SOMI are functional pins */

•   writel(((1 << cs) | SPIPC0_CLKFUN_MASK |
  

•   writel(((1 << slave_plat->cs) | SPIPC0_CLKFUN_MASK |
            SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0);
  
    /* setup format */
  

@@ -292,20 +322,32 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) return 0; }

-static int __davinci_spi_release_bus(struct davinci_spi_slave *ds) +static int davinci_spi_release_bus(struct udevice *dev) {

•   struct davinci_spi_slave *ds = dev_get_priv(dev->parent);
  

•   /* Disable the SPI hardware */
    writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
  
    return 0;
  

}

-static int __davinci_spi_xfer(struct davinci_spi_slave *ds,

•           unsigned int bitlen,  const void *dout, void *din,
  

•           unsigned long flags)
  

+static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen,

•                       const void *dout, void *din,
  

•                       unsigned long flags)
  

{

•   struct dm_spi_slave_platdata *slave =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
    unsigned int len;
  
  

•   if (slave->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->cur_cs = slave->cs;
  

•   if (bitlen == 0)
            /* Finish any previously submitted transfers */
            goto out;
  

@@ -339,240 +381,61 @@ out: u8 dummy = 0; davinci_spi_write(ds, 1, &dummy, flags); }

•   return 0;
  

-}

-#ifndef CONFIG_DM_SPI

-static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) -{

•   return container_of(slave, struct davinci_spi_slave, slave);
  

-}

-int spi_cs_is_valid(unsigned int bus, unsigned int cs) -{

•   int ret = 0;
  

•   switch (bus) {
  

•   case SPI0_BUS:
  

•           if (cs < SPI0_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#ifdef CONFIG_SYS_SPI1

•   case SPI1_BUS:
  

•           if (cs < SPI1_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#endif -#ifdef CONFIG_SYS_SPI2

•   case SPI2_BUS:
  

•           if (cs < SPI2_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#endif

•   default:
  

•           /* Invalid bus number. Do nothing */
  

•           break;
  

•   }
  

•   return ret;
  

-}

-void spi_cs_activate(struct spi_slave *slave) -{

•   /* do nothing */
  

-}

-void spi_cs_deactivate(struct spi_slave *slave) -{

•   /* do nothing */
  

-}

-void spi_init(void) -{

•   /* do nothing */
  

-}

-struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,

•                   unsigned int max_hz, unsigned int mode)
  

-{

•   struct davinci_spi_slave        *ds;
  

•   if (!spi_cs_is_valid(bus, cs))
  

•           return NULL;
  

•   ds = spi_alloc_slave(struct davinci_spi_slave, bus, cs);
  

•   if (!ds)
  

•           return NULL;
  

•   switch (bus) {
  

•   case SPI0_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI0_BASE;
  

•           break;
  

-#ifdef CONFIG_SYS_SPI1

•   case SPI1_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI1_BASE;
  

•           break;
  

-#endif -#ifdef CONFIG_SYS_SPI2

•   case SPI2_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI2_BASE;
  

•           break;
  

-#endif

•   default: /* Invalid bus number */
  

•           return NULL;
  

•   }
  

•   ds->freq = max_hz;
  

•   ds->mode = mode;
  

•   return &ds->slave;
  

-}

-void spi_free_slave(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   free(ds);
  

-}

-int spi_xfer(struct spi_slave *slave, unsigned int bitlen,

•        const void *dout, void *din, unsigned long flags)
  

-{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   ds->cur_cs = slave->cs;
  

•   return __davinci_spi_xfer(ds, bitlen, dout, din, flags);
  

-}

-int spi_claim_bus(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

-#ifdef CONFIG_SPI_HALF_DUPLEX

•   ds->half_duplex = true;
  

-#else

•   ds->half_duplex = false;
  

-#endif

•   return __davinci_spi_claim_bus(ds, ds->slave.cs);
  

-}

-void spi_release_bus(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   __davinci_spi_release_bus(ds);
  

-}

-#else -static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) -{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s speed %u\n", __func__, max_hz);
  

•   if (max_hz > CONFIG_SYS_SPI_CLK / 2)
  

•           return -EINVAL;
  

•   ds->freq = max_hz;
  
    return 0;
  

}

-static int davinci_spi_set_mode(struct udevice *bus, uint mode) -{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s mode %u\n", __func__, mode);
  

•   ds->mode = mode;
  

•   return 0;
  

-}

-static int davinci_spi_claim_bus(struct udevice *dev) -{

•   struct dm_spi_slave_platdata *slave_plat =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   if (slave_plat->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->half_duplex = slave_plat->mode & SPI_PREAMBLE;
  

•   return __davinci_spi_claim_bus(ds, slave_plat->cs);
  

-}

-static int davinci_spi_release_bus(struct udevice *dev) -{

•   struct davinci_spi_slave *ds = dev_get_priv(dev->parent);
  

•   return __davinci_spi_release_bus(ds);
  

-} +static const struct dm_spi_ops davinci_spi_ops = {

•   .claim_bus      = davinci_spi_claim_bus,
  

•   .release_bus    = davinci_spi_release_bus,
  

•   .xfer           = davinci_spi_xfer,
  

•   .set_speed      = davinci_spi_set_speed,
  

•   .set_mode       = davinci_spi_set_mode,
  

+};

-static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen,

•                       const void *dout, void *din,
  

•                       unsigned long flags)
  

+static int davinci_spi_probe(struct udevice *bus) {

•   struct dm_spi_slave_platdata *slave =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
    struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   struct davinci_spi_platdata *plat = bus->platdata;
  

•   ds->regs = plat->regs;
  

•   ds->num_cs = plat->num_cs;
  
  

•   if (slave->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->cur_cs = slave->cs;
  

•   return __davinci_spi_xfer(ds, bitlen, dout, din, flags);
  

-}

-static int davinci_spi_probe(struct udevice *bus) -{

•   /* Nothing to do */
    return 0;
  

}

+#if CONFIG_IS_ENABLED(OF_CONTROL)

Looking at other drivers, I wonder if this should be +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

...

static int davinci_ofdata_to_platadata(struct udevice *bus) {

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   const void *blob = gd->fdt_blob;
  

•   int node = dev_of_offset(bus);
  

•   struct davinci_spi_platdata *plat = bus->platdata;
  

•   fdt_addr_t addr;
  
  

•   ds->regs = devfdt_map_physmem(bus, sizeof(struct davinci_spi_regs));
  

•   if (!ds->regs) {
  

•           printf("%s: could not map device address\n", __func__);
  

•   addr = devfdt_get_addr(bus);
  

•   if (addr == FDT_ADDR_T_NONE)
            return -EINVAL;
  

•   }
  

•   ds->num_cs = fdtdec_get_int(blob, node, "num-cs", 4);
  

•   plat->regs = (struct davinci_spi_regs *)addr;
  

•   plat->num_cs = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus), "num-cs", 4);
  
    return 0;
  

}

-static const struct dm_spi_ops davinci_spi_ops = {

•   .claim_bus      = davinci_spi_claim_bus,
  

•   .release_bus    = davinci_spi_release_bus,
  

•   .xfer           = davinci_spi_xfer,
  

•   .set_speed      = davinci_spi_set_speed,
  

•   .set_mode       = davinci_spi_set_mode,
  

-};

static const struct udevice_id davinci_spi_ids[] = { { .compatible = "ti,keystone-spi" }, { .compatible = "ti,dm6441-spi" }, { .compatible = "ti,da830-spi" }, { } }; +#endif

U_BOOT_DRIVER(davinci_spi) = { .name = "davinci_spi", .id = UCLASS_SPI, +#if CONFIG_IS_ENABLED(OF_CONTROL)

Like above, should this be: +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

With limited SPL resources, I cannot build OF_CONTROL in SPL and disabling OF_CONTROL in SPL doesn't build either. With the modification, I can build with OF_PLATDATA enabled.

...

    .of_match = davinci_spi_ids,

•   .ops = &davinci_spi_ops,
    .ofdata_to_platdata = davinci_ofdata_to_platadata,
  

•   .priv_auto_alloc_size = sizeof(struct davinci_spi_slave),
  

•    .platdata_auto_alloc_size = sizeof(struct davinci_spi_platdata),
  

+#endif .probe = davinci_spi_probe,

•   .ops = &davinci_spi_ops,
  

•   .priv_auto_alloc_size = sizeof(struct davinci_spi_slave),
  

}; -#endif

With the above changes, I can build U-Boot, but I cannot boot with DM_SPL enabled.

Can you send U-Boot 'sf' log.

On Fri, Aug 10, 2018 at 3:50 PM, Adam Ford aford173@gmail.com wrote:

On Fri, Aug 10, 2018 at 12:14 AM Jagan Teki jagan@amarulasolutions.com wrote:

...

On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford aford173@gmail.com wrote:

...

On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki jagan@amarulasolutions.com wrote:

...

davinci_spi now support dt along with platform data, respective boards need to switch into dm for the same.

Cc: Adam Ford aford173@gmail.com Cc: Vitaly Andrianov vitalya@ti.com Cc: Stefano Babic sbabic@denx.de Cc: Peter Howard phoward@gme.net.au Cc: Tom Rini trini@konsulko.com Signed-off-by: Jagan Teki jagan@amarulasolutions.com

---

drivers/spi/Kconfig | 12 +- drivers/spi/davinci_spi.c | 289 +++++++------------------ include/dm/platform_data/spi_davinci.h | 15 ++ 3 files changed, 97 insertions(+), 219 deletions(-) create mode 100644 include/dm/platform_data/spi_davinci.h

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index d046e919b4..18ebff0231 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -80,6 +80,12 @@ config CADENCE_QSPI used to access the SPI NOR flash on platforms embedding this Cadence IP core.

+config DAVINCI_SPI

•   bool "Davinci & Keystone SPI driver"
  

•   depends on ARCH_DAVINCI || ARCH_KEYSTONE
  

•   help
  

•     Enable the Davinci SPI driver
  

config DESIGNWARE_SPI bool "Designware SPI driver" help @@ -281,12 +287,6 @@ config FSL_QSPI used to access the SPI NOR flash on platforms embedding this Freescale IP core.

-config DAVINCI_SPI

•   bool "Davinci & Keystone SPI driver"
  

•   depends on ARCH_DAVINCI || ARCH_KEYSTONE
  

•   help
  

•     Enable the Davinci SPI driver
  

config SH_SPI bool "SuperH SPI driver" help diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c index a822858323..5007e6c618 100644 --- a/drivers/spi/davinci_spi.c +++ b/drivers/spi/davinci_spi.c @@ -14,6 +14,7 @@ #include <asm/io.h> #include <asm/arch/hardware.h> #include <dm.h> +#include <dm/platform_data/spi_davinci.h>

/* SPIGCR0 */ #define SPIGCR0_SPIENA_MASK 0x1 @@ -118,9 +119,6 @@ struct davinci_spi_regs {

/* davinci spi slave */ struct davinci_spi_slave { -#ifndef CONFIG_DM_SPI

•   struct spi_slave slave;
  

-#endif struct davinci_spi_regs *regs; unsigned int freq; /* current SPI bus frequency */ unsigned int mode; /* current SPI mode used */ @@ -240,11 +238,43 @@ static int davinci_spi_read_write(struct davinci_spi_slave *ds, unsigned return 0; }

+static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) +{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  
  

-static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs)

•   debug("%s speed %u\n", __func__, max_hz);
  

•   if (max_hz > CONFIG_SYS_SPI_CLK / 2)
  

•           return -EINVAL;
  

•   ds->freq = max_hz;
  

•   return 0;
  

+}

+static int davinci_spi_set_mode(struct udevice *bus, uint mode) +{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s mode %u\n", __func__, mode);
  

•   ds->mode = mode;
  

•   return 0;
  

+}

+static int davinci_spi_claim_bus(struct udevice *dev) {

•   struct dm_spi_slave_platdata *slave_plat =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
    unsigned int mode = 0, scalar;
  
  

•   if (slave_plat->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->half_duplex = slave_plat->mode & SPI_PREAMBLE;
  

•   /* Enable the SPI hardware */
    writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
    udelay(1000);
  

@@ -254,7 +284,7 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1);

    /* CS, CLK, SIMO and SOMI are functional pins */

•   writel(((1 << cs) | SPIPC0_CLKFUN_MASK |
  

•   writel(((1 << slave_plat->cs) | SPIPC0_CLKFUN_MASK |
            SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0);
  
    /* setup format */
  

@@ -292,20 +322,32 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) return 0; }

-static int __davinci_spi_release_bus(struct davinci_spi_slave *ds) +static int davinci_spi_release_bus(struct udevice *dev) {

•   struct davinci_spi_slave *ds = dev_get_priv(dev->parent);
  

•   /* Disable the SPI hardware */
    writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
  
    return 0;
  

}

-static int __davinci_spi_xfer(struct davinci_spi_slave *ds,

•           unsigned int bitlen,  const void *dout, void *din,
  

•           unsigned long flags)
  

+static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen,

•                       const void *dout, void *din,
  

•                       unsigned long flags)
  

{

•   struct dm_spi_slave_platdata *slave =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
    unsigned int len;
  
  

•   if (slave->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->cur_cs = slave->cs;
  

•   if (bitlen == 0)
            /* Finish any previously submitted transfers */
            goto out;
  

@@ -339,240 +381,61 @@ out: u8 dummy = 0; davinci_spi_write(ds, 1, &dummy, flags); }

•   return 0;
  

-}

-#ifndef CONFIG_DM_SPI

-static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) -{

•   return container_of(slave, struct davinci_spi_slave, slave);
  

-}

-int spi_cs_is_valid(unsigned int bus, unsigned int cs) -{

•   int ret = 0;
  

•   switch (bus) {
  

•   case SPI0_BUS:
  

•           if (cs < SPI0_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#ifdef CONFIG_SYS_SPI1

•   case SPI1_BUS:
  

•           if (cs < SPI1_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#endif -#ifdef CONFIG_SYS_SPI2

•   case SPI2_BUS:
  

•           if (cs < SPI2_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#endif

•   default:
  

•           /* Invalid bus number. Do nothing */
  

•           break;
  

•   }
  

•   return ret;
  

-}

-void spi_cs_activate(struct spi_slave *slave) -{

•   /* do nothing */
  

-}

-void spi_cs_deactivate(struct spi_slave *slave) -{

•   /* do nothing */
  

-}

-void spi_init(void) -{

•   /* do nothing */
  

-}

-struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,

•                   unsigned int max_hz, unsigned int mode)
  

-{

•   struct davinci_spi_slave        *ds;
  

•   if (!spi_cs_is_valid(bus, cs))
  

•           return NULL;
  

•   ds = spi_alloc_slave(struct davinci_spi_slave, bus, cs);
  

•   if (!ds)
  

•           return NULL;
  

•   switch (bus) {
  

•   case SPI0_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI0_BASE;
  

•           break;
  

-#ifdef CONFIG_SYS_SPI1

•   case SPI1_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI1_BASE;
  

•           break;
  

-#endif -#ifdef CONFIG_SYS_SPI2

•   case SPI2_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI2_BASE;
  

•           break;
  

-#endif

•   default: /* Invalid bus number */
  

•           return NULL;
  

•   }
  

•   ds->freq = max_hz;
  

•   ds->mode = mode;
  

•   return &ds->slave;
  

-}

-void spi_free_slave(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   free(ds);
  

-}

-int spi_xfer(struct spi_slave *slave, unsigned int bitlen,

•        const void *dout, void *din, unsigned long flags)
  

-{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   ds->cur_cs = slave->cs;
  

•   return __davinci_spi_xfer(ds, bitlen, dout, din, flags);
  

-}

-int spi_claim_bus(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

-#ifdef CONFIG_SPI_HALF_DUPLEX

•   ds->half_duplex = true;
  

-#else

•   ds->half_duplex = false;
  

-#endif

•   return __davinci_spi_claim_bus(ds, ds->slave.cs);
  

-}

-void spi_release_bus(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   __davinci_spi_release_bus(ds);
  

-}

-#else -static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) -{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s speed %u\n", __func__, max_hz);
  

•   if (max_hz > CONFIG_SYS_SPI_CLK / 2)
  

•           return -EINVAL;
  

•   ds->freq = max_hz;
  
    return 0;
  

}

-static int davinci_spi_set_mode(struct udevice *bus, uint mode) -{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s mode %u\n", __func__, mode);
  

•   ds->mode = mode;
  

•   return 0;
  

-}

-static int davinci_spi_claim_bus(struct udevice *dev) -{

•   struct dm_spi_slave_platdata *slave_plat =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   if (slave_plat->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->half_duplex = slave_plat->mode & SPI_PREAMBLE;
  

•   return __davinci_spi_claim_bus(ds, slave_plat->cs);
  

-}

-static int davinci_spi_release_bus(struct udevice *dev) -{

•   struct davinci_spi_slave *ds = dev_get_priv(dev->parent);
  

•   return __davinci_spi_release_bus(ds);
  

-} +static const struct dm_spi_ops davinci_spi_ops = {

•   .claim_bus      = davinci_spi_claim_bus,
  

•   .release_bus    = davinci_spi_release_bus,
  

•   .xfer           = davinci_spi_xfer,
  

•   .set_speed      = davinci_spi_set_speed,
  

•   .set_mode       = davinci_spi_set_mode,
  

+};

-static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen,

•                       const void *dout, void *din,
  

•                       unsigned long flags)
  

+static int davinci_spi_probe(struct udevice *bus) {

•   struct dm_spi_slave_platdata *slave =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
    struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   struct davinci_spi_platdata *plat = bus->platdata;
  

•   ds->regs = plat->regs;
  

•   ds->num_cs = plat->num_cs;
  
  

•   if (slave->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->cur_cs = slave->cs;
  

•   return __davinci_spi_xfer(ds, bitlen, dout, din, flags);
  

-}

-static int davinci_spi_probe(struct udevice *bus) -{

•   /* Nothing to do */
    return 0;
  

}

+#if CONFIG_IS_ENABLED(OF_CONTROL)

Looking at other drivers, I wonder if this should be +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

...

static int davinci_ofdata_to_platadata(struct udevice *bus) {

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   const void *blob = gd->fdt_blob;
  

•   int node = dev_of_offset(bus);
  

•   struct davinci_spi_platdata *plat = bus->platdata;
  

•   fdt_addr_t addr;
  
  

•   ds->regs = devfdt_map_physmem(bus, sizeof(struct davinci_spi_regs));
  

•   if (!ds->regs) {
  

•           printf("%s: could not map device address\n", __func__);
  

•   addr = devfdt_get_addr(bus);
  

•   if (addr == FDT_ADDR_T_NONE)
            return -EINVAL;
  

•   }
  

•   ds->num_cs = fdtdec_get_int(blob, node, "num-cs", 4);
  

•   plat->regs = (struct davinci_spi_regs *)addr;
  

•   plat->num_cs = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus), "num-cs", 4);
  
    return 0;
  

}

-static const struct dm_spi_ops davinci_spi_ops = {

•   .claim_bus      = davinci_spi_claim_bus,
  

•   .release_bus    = davinci_spi_release_bus,
  

•   .xfer           = davinci_spi_xfer,
  

•   .set_speed      = davinci_spi_set_speed,
  

•   .set_mode       = davinci_spi_set_mode,
  

-};

static const struct udevice_id davinci_spi_ids[] = { { .compatible = "ti,keystone-spi" }, { .compatible = "ti,dm6441-spi" }, { .compatible = "ti,da830-spi" }, { } }; +#endif

U_BOOT_DRIVER(davinci_spi) = { .name = "davinci_spi", .id = UCLASS_SPI, +#if CONFIG_IS_ENABLED(OF_CONTROL)

Like above, should this be: +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

With limited SPL resources, I cannot build OF_CONTROL in SPL and disabling OF_CONTROL in SPL doesn't build either. With the modification, I can build with OF_PLATDATA enabled.

Is it possible to enable DM_SPI in SPL? da850evm_direct_nor_defconfig is able to build since it has it.

On Fri, Aug 10, 2018 at 2:58 PM Adam Ford aford173@gmail.com wrote:

On Fri, Aug 10, 2018 at 7:42 AM Jagan Teki jagan@amarulasolutions.com wrote:

...

On Fri, Aug 10, 2018 at 3:50 PM, Adam Ford aford173@gmail.com wrote:

...

On Fri, Aug 10, 2018 at 12:14 AM Jagan Teki jagan@amarulasolutions.com wrote:

...

On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford aford173@gmail.com wrote:

...

On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki jagan@amarulasolutions.com wrote:

...

davinci_spi now support dt along with platform data, respective boards need to switch into dm for the same.

Cc: Adam Ford aford173@gmail.com Cc: Vitaly Andrianov vitalya@ti.com Cc: Stefano Babic sbabic@denx.de Cc: Peter Howard phoward@gme.net.au Cc: Tom Rini trini@konsulko.com Signed-off-by: Jagan Teki jagan@amarulasolutions.com

---

drivers/spi/Kconfig | 12 +- drivers/spi/davinci_spi.c | 289 +++++++------------------ include/dm/platform_data/spi_davinci.h | 15 ++ 3 files changed, 97 insertions(+), 219 deletions(-) create mode 100644 include/dm/platform_data/spi_davinci.h

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index d046e919b4..18ebff0231 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -80,6 +80,12 @@ config CADENCE_QSPI used to access the SPI NOR flash on platforms embedding this Cadence IP core.

+config DAVINCI_SPI

•   bool "Davinci & Keystone SPI driver"
  

•   depends on ARCH_DAVINCI || ARCH_KEYSTONE
  

•   help
  

•     Enable the Davinci SPI driver
  

config DESIGNWARE_SPI bool "Designware SPI driver" help @@ -281,12 +287,6 @@ config FSL_QSPI used to access the SPI NOR flash on platforms embedding this Freescale IP core.

-config DAVINCI_SPI

•   bool "Davinci & Keystone SPI driver"
  

•   depends on ARCH_DAVINCI || ARCH_KEYSTONE
  

•   help
  

•     Enable the Davinci SPI driver
  

config SH_SPI bool "SuperH SPI driver" help diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c index a822858323..5007e6c618 100644 --- a/drivers/spi/davinci_spi.c +++ b/drivers/spi/davinci_spi.c @@ -14,6 +14,7 @@ #include <asm/io.h> #include <asm/arch/hardware.h> #include <dm.h> +#include <dm/platform_data/spi_davinci.h>

/* SPIGCR0 */ #define SPIGCR0_SPIENA_MASK 0x1 @@ -118,9 +119,6 @@ struct davinci_spi_regs {

/* davinci spi slave */ struct davinci_spi_slave { -#ifndef CONFIG_DM_SPI

•   struct spi_slave slave;
  

-#endif struct davinci_spi_regs *regs; unsigned int freq; /* current SPI bus frequency */ unsigned int mode; /* current SPI mode used */ @@ -240,11 +238,43 @@ static int davinci_spi_read_write(struct davinci_spi_slave *ds, unsigned return 0; }

+static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) +{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  
  

-static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs)

•   debug("%s speed %u\n", __func__, max_hz);
  

•   if (max_hz > CONFIG_SYS_SPI_CLK / 2)
  

•           return -EINVAL;
  

•   ds->freq = max_hz;
  

•   return 0;
  

+}

+static int davinci_spi_set_mode(struct udevice *bus, uint mode) +{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s mode %u\n", __func__, mode);
  

•   ds->mode = mode;
  

•   return 0;
  

+}

+static int davinci_spi_claim_bus(struct udevice *dev) {

•   struct dm_spi_slave_platdata *slave_plat =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
    unsigned int mode = 0, scalar;
  
  

•   if (slave_plat->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->half_duplex = slave_plat->mode & SPI_PREAMBLE;
  

•   /* Enable the SPI hardware */
    writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
    udelay(1000);
  

@@ -254,7 +284,7 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1);

    /* CS, CLK, SIMO and SOMI are functional pins */

•   writel(((1 << cs) | SPIPC0_CLKFUN_MASK |
  

•   writel(((1 << slave_plat->cs) | SPIPC0_CLKFUN_MASK |
            SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0);
  
    /* setup format */
  

@@ -292,20 +322,32 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) return 0; }

-static int __davinci_spi_release_bus(struct davinci_spi_slave *ds) +static int davinci_spi_release_bus(struct udevice *dev) {

•   struct davinci_spi_slave *ds = dev_get_priv(dev->parent);
  

•   /* Disable the SPI hardware */
    writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
  
    return 0;
  

}

-static int __davinci_spi_xfer(struct davinci_spi_slave *ds,

•           unsigned int bitlen,  const void *dout, void *din,
  

•           unsigned long flags)
  

+static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen,

•                       const void *dout, void *din,
  

•                       unsigned long flags)
  

{

•   struct dm_spi_slave_platdata *slave =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
    unsigned int len;
  
  

•   if (slave->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->cur_cs = slave->cs;
  

•   if (bitlen == 0)
            /* Finish any previously submitted transfers */
            goto out;
  

@@ -339,240 +381,61 @@ out: u8 dummy = 0; davinci_spi_write(ds, 1, &dummy, flags); }

•   return 0;
  

-}

-#ifndef CONFIG_DM_SPI

-static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) -{

•   return container_of(slave, struct davinci_spi_slave, slave);
  

-}

-int spi_cs_is_valid(unsigned int bus, unsigned int cs) -{

•   int ret = 0;
  

•   switch (bus) {
  

•   case SPI0_BUS:
  

•           if (cs < SPI0_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#ifdef CONFIG_SYS_SPI1

•   case SPI1_BUS:
  

•           if (cs < SPI1_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#endif -#ifdef CONFIG_SYS_SPI2

•   case SPI2_BUS:
  

•           if (cs < SPI2_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#endif

•   default:
  

•           /* Invalid bus number. Do nothing */
  

•           break;
  

•   }
  

•   return ret;
  

-}

-void spi_cs_activate(struct spi_slave *slave) -{

•   /* do nothing */
  

-}

-void spi_cs_deactivate(struct spi_slave *slave) -{

•   /* do nothing */
  

-}

-void spi_init(void) -{

•   /* do nothing */
  

-}

-struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,

•                   unsigned int max_hz, unsigned int mode)
  

-{

•   struct davinci_spi_slave        *ds;
  

•   if (!spi_cs_is_valid(bus, cs))
  

•           return NULL;
  

•   ds = spi_alloc_slave(struct davinci_spi_slave, bus, cs);
  

•   if (!ds)
  

•           return NULL;
  

•   switch (bus) {
  

•   case SPI0_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI0_BASE;
  

•           break;
  

-#ifdef CONFIG_SYS_SPI1

•   case SPI1_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI1_BASE;
  

•           break;
  

-#endif -#ifdef CONFIG_SYS_SPI2

•   case SPI2_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI2_BASE;
  

•           break;
  

-#endif

•   default: /* Invalid bus number */
  

•           return NULL;
  

•   }
  

•   ds->freq = max_hz;
  

•   ds->mode = mode;
  

•   return &ds->slave;
  

-}

-void spi_free_slave(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   free(ds);
  

-}

-int spi_xfer(struct spi_slave *slave, unsigned int bitlen,

•        const void *dout, void *din, unsigned long flags)
  

-{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   ds->cur_cs = slave->cs;
  

•   return __davinci_spi_xfer(ds, bitlen, dout, din, flags);
  

-}

-int spi_claim_bus(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

-#ifdef CONFIG_SPI_HALF_DUPLEX

•   ds->half_duplex = true;
  

-#else

•   ds->half_duplex = false;
  

-#endif

•   return __davinci_spi_claim_bus(ds, ds->slave.cs);
  

-}

-void spi_release_bus(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   __davinci_spi_release_bus(ds);
  

-}

-#else -static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) -{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s speed %u\n", __func__, max_hz);
  

•   if (max_hz > CONFIG_SYS_SPI_CLK / 2)
  

•           return -EINVAL;
  

•   ds->freq = max_hz;
  
    return 0;
  

}

-static int davinci_spi_set_mode(struct udevice *bus, uint mode) -{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s mode %u\n", __func__, mode);
  

•   ds->mode = mode;
  

•   return 0;
  

-}

-static int davinci_spi_claim_bus(struct udevice *dev) -{

•   struct dm_spi_slave_platdata *slave_plat =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   if (slave_plat->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->half_duplex = slave_plat->mode & SPI_PREAMBLE;
  

•   return __davinci_spi_claim_bus(ds, slave_plat->cs);
  

-}

-static int davinci_spi_release_bus(struct udevice *dev) -{

•   struct davinci_spi_slave *ds = dev_get_priv(dev->parent);
  

•   return __davinci_spi_release_bus(ds);
  

-} +static const struct dm_spi_ops davinci_spi_ops = {

•   .claim_bus      = davinci_spi_claim_bus,
  

•   .release_bus    = davinci_spi_release_bus,
  

•   .xfer           = davinci_spi_xfer,
  

•   .set_speed      = davinci_spi_set_speed,
  

•   .set_mode       = davinci_spi_set_mode,
  

+};

-static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen,

•                       const void *dout, void *din,
  

•                       unsigned long flags)
  

+static int davinci_spi_probe(struct udevice *bus) {

•   struct dm_spi_slave_platdata *slave =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
    struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   struct davinci_spi_platdata *plat = bus->platdata;
  

•   ds->regs = plat->regs;
  

•   ds->num_cs = plat->num_cs;
  
  

•   if (slave->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->cur_cs = slave->cs;
  

•   return __davinci_spi_xfer(ds, bitlen, dout, din, flags);
  

-}

-static int davinci_spi_probe(struct udevice *bus) -{

•   /* Nothing to do */
    return 0;
  

}

+#if CONFIG_IS_ENABLED(OF_CONTROL)

Looking at other drivers, I wonder if this should be +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

...

static int davinci_ofdata_to_platadata(struct udevice *bus) {

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   const void *blob = gd->fdt_blob;
  

•   int node = dev_of_offset(bus);
  

•   struct davinci_spi_platdata *plat = bus->platdata;
  

•   fdt_addr_t addr;
  
  

•   ds->regs = devfdt_map_physmem(bus, sizeof(struct davinci_spi_regs));
  

•   if (!ds->regs) {
  

•           printf("%s: could not map device address\n", __func__);
  

•   addr = devfdt_get_addr(bus);
  

•   if (addr == FDT_ADDR_T_NONE)
            return -EINVAL;
  

•   }
  

•   ds->num_cs = fdtdec_get_int(blob, node, "num-cs", 4);
  

•   plat->regs = (struct davinci_spi_regs *)addr;
  

•   plat->num_cs = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus), "num-cs", 4);
  
    return 0;
  

}

-static const struct dm_spi_ops davinci_spi_ops = {

•   .claim_bus      = davinci_spi_claim_bus,
  

•   .release_bus    = davinci_spi_release_bus,
  

•   .xfer           = davinci_spi_xfer,
  

•   .set_speed      = davinci_spi_set_speed,
  

•   .set_mode       = davinci_spi_set_mode,
  

-};

static const struct udevice_id davinci_spi_ids[] = { { .compatible = "ti,keystone-spi" }, { .compatible = "ti,dm6441-spi" }, { .compatible = "ti,da830-spi" }, { } }; +#endif

U_BOOT_DRIVER(davinci_spi) = { .name = "davinci_spi", .id = UCLASS_SPI, +#if CONFIG_IS_ENABLED(OF_CONTROL)

Like above, should this be: +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

With limited SPL resources, I cannot build OF_CONTROL in SPL and disabling OF_CONTROL in SPL doesn't build either. With the modification, I can build with OF_PLATDATA enabled.

Is it possible to enable DM_SPI in SPL? da850evm_direct_nor_defconfig is able to build since it has it.

Enabling DM_SPI in SPL starts to grow, hence my other comments about SPL_OF_PLATDATA in order to make it fit into SPL. da850evm_direct_nor_defconfig does not enable SPL, but it does enable DM_SPI. I had to get the NOR expansion board in order to try it. I'm trying to get it to work now, but for some reason, I'm having difficulty booting the stock da850evm_direct_nor_defconfig

It would be easiest if we could have both a DM_SPI and non DM_SPI version of the driver so it can fit into SPL or the ability to disable SPI in SPL. I am experimenting with the latter. Several drivers offer the option to be disabled in SPL, so I'm experimenting with that to save space in SPL.

I was able to verify your code works for the da850evm_direct_nor_defconfig version which doesn't use SPL. I spent a significant amount of time yesterday trying to get SPL to work, but just enabling SPL and disabling all drivers except serial, I was not able to boot, so I think something is wrong with DM and SPL. I don't have a debugger at home for this board, so I'll need to get one from work to further troubleshoot.

I don't think any DA850/L138/AM1808 board uses DM in SPL, so I think it would be best to maintain both DM and non-DM code bases for now until we're able to support DM in SPL.

The good news is that when in U-Boot, your code seems to work just fine. Unfortunately, it's limited to boards which can XIP boot to NOR.

adam

adam

On Sat, Aug 11, 2018 at 6:12 PM, Adam Ford aford173@gmail.com wrote:

On Fri, Aug 10, 2018 at 2:58 PM Adam Ford aford173@gmail.com wrote:

...

On Fri, Aug 10, 2018 at 7:42 AM Jagan Teki jagan@amarulasolutions.com wrote:

...

On Fri, Aug 10, 2018 at 3:50 PM, Adam Ford aford173@gmail.com wrote:

...

On Fri, Aug 10, 2018 at 12:14 AM Jagan Teki jagan@amarulasolutions.com wrote:

...

On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford aford173@gmail.com wrote:

...

On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki jagan@amarulasolutions.com wrote: > > davinci_spi now support dt along with platform data, > respective boards need to switch into dm for the same. > > Cc: Adam Ford aford173@gmail.com > Cc: Vitaly Andrianov vitalya@ti.com > Cc: Stefano Babic sbabic@denx.de > Cc: Peter Howard phoward@gme.net.au > Cc: Tom Rini trini@konsulko.com > Signed-off-by: Jagan Teki jagan@amarulasolutions.com > --- > drivers/spi/Kconfig | 12 +- > drivers/spi/davinci_spi.c | 289 +++++++------------------ > include/dm/platform_data/spi_davinci.h | 15 ++ > 3 files changed, 97 insertions(+), 219 deletions(-) > create mode 100644 include/dm/platform_data/spi_davinci.h > > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig > index d046e919b4..18ebff0231 100644 > --- a/drivers/spi/Kconfig > +++ b/drivers/spi/Kconfig > @@ -80,6 +80,12 @@ config CADENCE_QSPI > used to access the SPI NOR flash on platforms embedding this > Cadence IP core. > > +config DAVINCI_SPI > + bool "Davinci & Keystone SPI driver" > + depends on ARCH_DAVINCI || ARCH_KEYSTONE > + help > + Enable the Davinci SPI driver > + > config DESIGNWARE_SPI > bool "Designware SPI driver" > help > @@ -281,12 +287,6 @@ config FSL_QSPI > used to access the SPI NOR flash on platforms embedding this > Freescale IP core. > > -config DAVINCI_SPI > - bool "Davinci & Keystone SPI driver" > - depends on ARCH_DAVINCI || ARCH_KEYSTONE > - help > - Enable the Davinci SPI driver > - > config SH_SPI > bool "SuperH SPI driver" > help > diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c > index a822858323..5007e6c618 100644 > --- a/drivers/spi/davinci_spi.c > +++ b/drivers/spi/davinci_spi.c > @@ -14,6 +14,7 @@ > #include <asm/io.h> > #include <asm/arch/hardware.h> > #include <dm.h> > +#include <dm/platform_data/spi_davinci.h> > > /* SPIGCR0 */ > #define SPIGCR0_SPIENA_MASK 0x1 > @@ -118,9 +119,6 @@ struct davinci_spi_regs { > > /* davinci spi slave */ > struct davinci_spi_slave { > -#ifndef CONFIG_DM_SPI > - struct spi_slave slave; > -#endif > struct davinci_spi_regs *regs; > unsigned int freq; /* current SPI bus frequency */ > unsigned int mode; /* current SPI mode used */ > @@ -240,11 +238,43 @@ static int davinci_spi_read_write(struct davinci_spi_slave *ds, unsigned > return 0; > } > > +static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) > +{ > + struct davinci_spi_slave *ds = dev_get_priv(bus); > > -static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) > + debug("%s speed %u\n", __func__, max_hz); > + if (max_hz > CONFIG_SYS_SPI_CLK / 2) > + return -EINVAL; > + > + ds->freq = max_hz; > + > + return 0; > +} > + > +static int davinci_spi_set_mode(struct udevice *bus, uint mode) > +{ > + struct davinci_spi_slave *ds = dev_get_priv(bus); > + > + debug("%s mode %u\n", __func__, mode); > + ds->mode = mode; > + > + return 0; > +} > + > +static int davinci_spi_claim_bus(struct udevice *dev) > { > + struct dm_spi_slave_platdata *slave_plat = > + dev_get_parent_platdata(dev); > + struct udevice *bus = dev->parent; > + struct davinci_spi_slave *ds = dev_get_priv(bus); > unsigned int mode = 0, scalar; > > + if (slave_plat->cs >= ds->num_cs) { > + printf("Invalid SPI chipselect\n"); > + return -EINVAL; > + } > + ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; > + > /* Enable the SPI hardware */ > writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); > udelay(1000); > @@ -254,7 +284,7 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) > writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1); > > /* CS, CLK, SIMO and SOMI are functional pins */ > - writel(((1 << cs) | SPIPC0_CLKFUN_MASK | > + writel(((1 << slave_plat->cs) | SPIPC0_CLKFUN_MASK | > SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0); > > /* setup format */ > @@ -292,20 +322,32 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) > return 0; > } > > -static int __davinci_spi_release_bus(struct davinci_spi_slave *ds) > +static int davinci_spi_release_bus(struct udevice *dev) > { > + struct davinci_spi_slave *ds = dev_get_priv(dev->parent); > + > /* Disable the SPI hardware */ > writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); > > return 0; > } > > -static int __davinci_spi_xfer(struct davinci_spi_slave *ds, > - unsigned int bitlen, const void *dout, void *din, > - unsigned long flags) > +static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, > + const void *dout, void *din, > + unsigned long flags) > { > + struct dm_spi_slave_platdata *slave = > + dev_get_parent_platdata(dev); > + struct udevice *bus = dev->parent; > + struct davinci_spi_slave *ds = dev_get_priv(bus); > unsigned int len; > > + if (slave->cs >= ds->num_cs) { > + printf("Invalid SPI chipselect\n"); > + return -EINVAL; > + } > + ds->cur_cs = slave->cs; > + > if (bitlen == 0) > /* Finish any previously submitted transfers */ > goto out; > @@ -339,240 +381,61 @@ out: > u8 dummy = 0; > davinci_spi_write(ds, 1, &dummy, flags); > } > - return 0; > -} > - > -#ifndef CONFIG_DM_SPI > - > -static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) > -{ > - return container_of(slave, struct davinci_spi_slave, slave); > -} > - > -int spi_cs_is_valid(unsigned int bus, unsigned int cs) > -{ > - int ret = 0; > - > - switch (bus) { > - case SPI0_BUS: > - if (cs < SPI0_NUM_CS) > - ret = 1; > - break; > -#ifdef CONFIG_SYS_SPI1 > - case SPI1_BUS: > - if (cs < SPI1_NUM_CS) > - ret = 1; > - break; > -#endif > -#ifdef CONFIG_SYS_SPI2 > - case SPI2_BUS: > - if (cs < SPI2_NUM_CS) > - ret = 1; > - break; > -#endif > - default: > - /* Invalid bus number. Do nothing */ > - break; > - } > - return ret; > -} > - > -void spi_cs_activate(struct spi_slave *slave) > -{ > - /* do nothing */ > -} > - > -void spi_cs_deactivate(struct spi_slave *slave) > -{ > - /* do nothing */ > -} > - > -void spi_init(void) > -{ > - /* do nothing */ > -} > - > -struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, > - unsigned int max_hz, unsigned int mode) > -{ > - struct davinci_spi_slave *ds; > - > - if (!spi_cs_is_valid(bus, cs)) > - return NULL; > - > - ds = spi_alloc_slave(struct davinci_spi_slave, bus, cs); > - if (!ds) > - return NULL; > - > - switch (bus) { > - case SPI0_BUS: > - ds->regs = (struct davinci_spi_regs *)SPI0_BASE; > - break; > -#ifdef CONFIG_SYS_SPI1 > - case SPI1_BUS: > - ds->regs = (struct davinci_spi_regs *)SPI1_BASE; > - break; > -#endif > -#ifdef CONFIG_SYS_SPI2 > - case SPI2_BUS: > - ds->regs = (struct davinci_spi_regs *)SPI2_BASE; > - break; > -#endif > - default: /* Invalid bus number */ > - return NULL; > - } > - > - ds->freq = max_hz; > - ds->mode = mode; > - > - return &ds->slave; > -} > - > -void spi_free_slave(struct spi_slave *slave) > -{ > - struct davinci_spi_slave *ds = to_davinci_spi(slave); > - > - free(ds); > -} > - > -int spi_xfer(struct spi_slave *slave, unsigned int bitlen, > - const void *dout, void *din, unsigned long flags) > -{ > - struct davinci_spi_slave *ds = to_davinci_spi(slave); > - > - ds->cur_cs = slave->cs; > - > - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); > -} > - > -int spi_claim_bus(struct spi_slave *slave) > -{ > - struct davinci_spi_slave *ds = to_davinci_spi(slave); > - > -#ifdef CONFIG_SPI_HALF_DUPLEX > - ds->half_duplex = true; > -#else > - ds->half_duplex = false; > -#endif > - return __davinci_spi_claim_bus(ds, ds->slave.cs); > -} > - > -void spi_release_bus(struct spi_slave *slave) > -{ > - struct davinci_spi_slave *ds = to_davinci_spi(slave); > - > - __davinci_spi_release_bus(ds); > -} > - > -#else > -static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) > -{ > - struct davinci_spi_slave *ds = dev_get_priv(bus); > - > - debug("%s speed %u\n", __func__, max_hz); > - if (max_hz > CONFIG_SYS_SPI_CLK / 2) > - return -EINVAL; > - > - ds->freq = max_hz; > > return 0; > } > > -static int davinci_spi_set_mode(struct udevice *bus, uint mode) > -{ > - struct davinci_spi_slave *ds = dev_get_priv(bus); > - > - debug("%s mode %u\n", __func__, mode); > - ds->mode = mode; > - > - return 0; > -} > - > -static int davinci_spi_claim_bus(struct udevice *dev) > -{ > - struct dm_spi_slave_platdata *slave_plat = > - dev_get_parent_platdata(dev); > - struct udevice *bus = dev->parent; > - struct davinci_spi_slave *ds = dev_get_priv(bus); > - > - if (slave_plat->cs >= ds->num_cs) { > - printf("Invalid SPI chipselect\n"); > - return -EINVAL; > - } > - ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; > - > - return __davinci_spi_claim_bus(ds, slave_plat->cs); > -} > - > -static int davinci_spi_release_bus(struct udevice *dev) > -{ > - struct davinci_spi_slave *ds = dev_get_priv(dev->parent); > - > - return __davinci_spi_release_bus(ds); > -} > +static const struct dm_spi_ops davinci_spi_ops = { > + .claim_bus = davinci_spi_claim_bus, > + .release_bus = davinci_spi_release_bus, > + .xfer = davinci_spi_xfer, > + .set_speed = davinci_spi_set_speed, > + .set_mode = davinci_spi_set_mode, > +}; > > -static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, > - const void *dout, void *din, > - unsigned long flags) > +static int davinci_spi_probe(struct udevice *bus) > { > - struct dm_spi_slave_platdata *slave = > - dev_get_parent_platdata(dev); > - struct udevice *bus = dev->parent; > struct davinci_spi_slave *ds = dev_get_priv(bus); > + struct davinci_spi_platdata *plat = bus->platdata; > + ds->regs = plat->regs; > + ds->num_cs = plat->num_cs; > > - if (slave->cs >= ds->num_cs) { > - printf("Invalid SPI chipselect\n"); > - return -EINVAL; > - } > - ds->cur_cs = slave->cs; > - > - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); > -} > - > -static int davinci_spi_probe(struct udevice *bus) > -{ > - /* Nothing to do */ > return 0; > } > > +#if CONFIG_IS_ENABLED(OF_CONTROL)

Looking at other drivers, I wonder if this should be +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

> static int davinci_ofdata_to_platadata(struct udevice *bus) > { > - struct davinci_spi_slave *ds = dev_get_priv(bus); > - const void *blob = gd->fdt_blob; > - int node = dev_of_offset(bus); > + struct davinci_spi_platdata *plat = bus->platdata; > + fdt_addr_t addr; > > - ds->regs = devfdt_map_physmem(bus, sizeof(struct davinci_spi_regs)); > - if (!ds->regs) { > - printf("%s: could not map device address\n", __func__); > + addr = devfdt_get_addr(bus); > + if (addr == FDT_ADDR_T_NONE) > return -EINVAL; > - } > - ds->num_cs = fdtdec_get_int(blob, node, "num-cs", 4); > + > + plat->regs = (struct davinci_spi_regs *)addr; > + plat->num_cs = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus), "num-cs", 4); > > return 0; > } > > -static const struct dm_spi_ops davinci_spi_ops = { > - .claim_bus = davinci_spi_claim_bus, > - .release_bus = davinci_spi_release_bus, > - .xfer = davinci_spi_xfer, > - .set_speed = davinci_spi_set_speed, > - .set_mode = davinci_spi_set_mode, > -}; > - > static const struct udevice_id davinci_spi_ids[] = { > { .compatible = "ti,keystone-spi" }, > { .compatible = "ti,dm6441-spi" }, > { .compatible = "ti,da830-spi" }, > { } > }; > +#endif > > U_BOOT_DRIVER(davinci_spi) = { > .name = "davinci_spi", > .id = UCLASS_SPI, > +#if CONFIG_IS_ENABLED(OF_CONTROL)

Like above, should this be: +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

With limited SPL resources, I cannot build OF_CONTROL in SPL and disabling OF_CONTROL in SPL doesn't build either. With the modification, I can build with OF_PLATDATA enabled.

Is it possible to enable DM_SPI in SPL? da850evm_direct_nor_defconfig is able to build since it has it.

Enabling DM_SPI in SPL starts to grow, hence my other comments about SPL_OF_PLATDATA in order to make it fit into SPL. da850evm_direct_nor_defconfig does not enable SPL, but it does enable DM_SPI. I had to get the NOR expansion board in order to try it. I'm trying to get it to work now, but for some reason, I'm having difficulty booting the stock da850evm_direct_nor_defconfig

It would be easiest if we could have both a DM_SPI and non DM_SPI version of the driver so it can fit into SPL or the ability to disable SPI in SPL. I am experimenting with the latter. Several drivers offer the option to be disabled in SPL, so I'm experimenting with that to save space in SPL.

I was able to verify your code works for the da850evm_direct_nor_defconfig version which doesn't use SPL. I spent a significant amount of time yesterday trying to get SPL to work, but just enabling SPL and disabling all drivers except serial, I was not able to boot, so I think something is wrong with DM and SPL. I don't have a debugger at home for this board, so I'll need to get one from work to further troubleshoot.

I don't think it is much difficult to get serial up here. I made few changes for serial and spi platdata for SPL. If haven't try these please check the same and better change proper clock value for uart if added one is improper.

I don't think any DA850/L138/AM1808 board uses DM in SPL, so I think it would be best to maintain both DM and non-DM code bases for now until we're able to support DM in SPL.

But the whole idea is to drop nod-dm SPI as much as possible.

[1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/spi-dm-migrate

On Sat, Aug 11, 2018 at 3:09 PM Adam Ford aford173@gmail.com wrote:

On Sat, Aug 11, 2018, 1:24 PM Jagan Teki jagan@amarulasolutions.com wrote:

...

On Sat, Aug 11, 2018 at 6:12 PM, Adam Ford aford173@gmail.com wrote:

...

On Fri, Aug 10, 2018 at 2:58 PM Adam Ford aford173@gmail.com wrote:

...

On Fri, Aug 10, 2018 at 7:42 AM Jagan Teki jagan@amarulasolutions.com wrote:

...

On Fri, Aug 10, 2018 at 3:50 PM, Adam Ford aford173@gmail.com wrote:

...

On Fri, Aug 10, 2018 at 12:14 AM Jagan Teki jagan@amarulasolutions.com wrote: > > On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford aford173@gmail.com wrote: > > On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki jagan@amarulasolutions.com wrote: > >> > >> davinci_spi now support dt along with platform data, > >> respective boards need to switch into dm for the same. > >> > >> Cc: Adam Ford aford173@gmail.com > >> Cc: Vitaly Andrianov vitalya@ti.com > >> Cc: Stefano Babic sbabic@denx.de > >> Cc: Peter Howard phoward@gme.net.au > >> Cc: Tom Rini trini@konsulko.com > >> Signed-off-by: Jagan Teki jagan@amarulasolutions.com > >> --- > >> drivers/spi/Kconfig | 12 +- > >> drivers/spi/davinci_spi.c | 289 +++++++------------------ > >> include/dm/platform_data/spi_davinci.h | 15 ++ > >> 3 files changed, 97 insertions(+), 219 deletions(-) > >> create mode 100644 include/dm/platform_data/spi_davinci.h > >> > >> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig > >> index d046e919b4..18ebff0231 100644 > >> --- a/drivers/spi/Kconfig > >> +++ b/drivers/spi/Kconfig > >> @@ -80,6 +80,12 @@ config CADENCE_QSPI > >> used to access the SPI NOR flash on platforms embedding this > >> Cadence IP core. > >> > >> +config DAVINCI_SPI > >> + bool "Davinci & Keystone SPI driver" > >> + depends on ARCH_DAVINCI || ARCH_KEYSTONE > >> + help > >> + Enable the Davinci SPI driver > >> + > >> config DESIGNWARE_SPI > >> bool "Designware SPI driver" > >> help > >> @@ -281,12 +287,6 @@ config FSL_QSPI > >> used to access the SPI NOR flash on platforms embedding this > >> Freescale IP core. > >> > >> -config DAVINCI_SPI > >> - bool "Davinci & Keystone SPI driver" > >> - depends on ARCH_DAVINCI || ARCH_KEYSTONE > >> - help > >> - Enable the Davinci SPI driver > >> - > >> config SH_SPI > >> bool "SuperH SPI driver" > >> help > >> diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c > >> index a822858323..5007e6c618 100644 > >> --- a/drivers/spi/davinci_spi.c > >> +++ b/drivers/spi/davinci_spi.c > >> @@ -14,6 +14,7 @@ > >> #include <asm/io.h> > >> #include <asm/arch/hardware.h> > >> #include <dm.h> > >> +#include <dm/platform_data/spi_davinci.h> > >> > >> /* SPIGCR0 */ > >> #define SPIGCR0_SPIENA_MASK 0x1 > >> @@ -118,9 +119,6 @@ struct davinci_spi_regs { > >> > >> /* davinci spi slave */ > >> struct davinci_spi_slave { > >> -#ifndef CONFIG_DM_SPI > >> - struct spi_slave slave; > >> -#endif > >> struct davinci_spi_regs *regs; > >> unsigned int freq; /* current SPI bus frequency */ > >> unsigned int mode; /* current SPI mode used */ > >> @@ -240,11 +238,43 @@ static int davinci_spi_read_write(struct davinci_spi_slave *ds, unsigned > >> return 0; > >> } > >> > >> +static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) > >> +{ > >> + struct davinci_spi_slave *ds = dev_get_priv(bus); > >> > >> -static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) > >> + debug("%s speed %u\n", __func__, max_hz); > >> + if (max_hz > CONFIG_SYS_SPI_CLK / 2) > >> + return -EINVAL; > >> + > >> + ds->freq = max_hz; > >> + > >> + return 0; > >> +} > >> + > >> +static int davinci_spi_set_mode(struct udevice *bus, uint mode) > >> +{ > >> + struct davinci_spi_slave *ds = dev_get_priv(bus); > >> + > >> + debug("%s mode %u\n", __func__, mode); > >> + ds->mode = mode; > >> + > >> + return 0; > >> +} > >> + > >> +static int davinci_spi_claim_bus(struct udevice *dev) > >> { > >> + struct dm_spi_slave_platdata *slave_plat = > >> + dev_get_parent_platdata(dev); > >> + struct udevice *bus = dev->parent; > >> + struct davinci_spi_slave *ds = dev_get_priv(bus); > >> unsigned int mode = 0, scalar; > >> > >> + if (slave_plat->cs >= ds->num_cs) { > >> + printf("Invalid SPI chipselect\n"); > >> + return -EINVAL; > >> + } > >> + ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; > >> + > >> /* Enable the SPI hardware */ > >> writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); > >> udelay(1000); > >> @@ -254,7 +284,7 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) > >> writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1); > >> > >> /* CS, CLK, SIMO and SOMI are functional pins */ > >> - writel(((1 << cs) | SPIPC0_CLKFUN_MASK | > >> + writel(((1 << slave_plat->cs) | SPIPC0_CLKFUN_MASK | > >> SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0); > >> > >> /* setup format */ > >> @@ -292,20 +322,32 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) > >> return 0; > >> } > >> > >> -static int __davinci_spi_release_bus(struct davinci_spi_slave *ds) > >> +static int davinci_spi_release_bus(struct udevice *dev) > >> { > >> + struct davinci_spi_slave *ds = dev_get_priv(dev->parent); > >> + > >> /* Disable the SPI hardware */ > >> writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); > >> > >> return 0; > >> } > >> > >> -static int __davinci_spi_xfer(struct davinci_spi_slave *ds, > >> - unsigned int bitlen, const void *dout, void *din, > >> - unsigned long flags) > >> +static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, > >> + const void *dout, void *din, > >> + unsigned long flags) > >> { > >> + struct dm_spi_slave_platdata *slave = > >> + dev_get_parent_platdata(dev); > >> + struct udevice *bus = dev->parent; > >> + struct davinci_spi_slave *ds = dev_get_priv(bus); > >> unsigned int len; > >> > >> + if (slave->cs >= ds->num_cs) { > >> + printf("Invalid SPI chipselect\n"); > >> + return -EINVAL; > >> + } > >> + ds->cur_cs = slave->cs; > >> + > >> if (bitlen == 0) > >> /* Finish any previously submitted transfers */ > >> goto out; > >> @@ -339,240 +381,61 @@ out: > >> u8 dummy = 0; > >> davinci_spi_write(ds, 1, &dummy, flags); > >> } > >> - return 0; > >> -} > >> - > >> -#ifndef CONFIG_DM_SPI > >> - > >> -static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) > >> -{ > >> - return container_of(slave, struct davinci_spi_slave, slave); > >> -} > >> - > >> -int spi_cs_is_valid(unsigned int bus, unsigned int cs) > >> -{ > >> - int ret = 0; > >> - > >> - switch (bus) { > >> - case SPI0_BUS: > >> - if (cs < SPI0_NUM_CS) > >> - ret = 1; > >> - break; > >> -#ifdef CONFIG_SYS_SPI1 > >> - case SPI1_BUS: > >> - if (cs < SPI1_NUM_CS) > >> - ret = 1; > >> - break; > >> -#endif > >> -#ifdef CONFIG_SYS_SPI2 > >> - case SPI2_BUS: > >> - if (cs < SPI2_NUM_CS) > >> - ret = 1; > >> - break; > >> -#endif > >> - default: > >> - /* Invalid bus number. Do nothing */ > >> - break; > >> - } > >> - return ret; > >> -} > >> - > >> -void spi_cs_activate(struct spi_slave *slave) > >> -{ > >> - /* do nothing */ > >> -} > >> - > >> -void spi_cs_deactivate(struct spi_slave *slave) > >> -{ > >> - /* do nothing */ > >> -} > >> - > >> -void spi_init(void) > >> -{ > >> - /* do nothing */ > >> -} > >> - > >> -struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, > >> - unsigned int max_hz, unsigned int mode) > >> -{ > >> - struct davinci_spi_slave *ds; > >> - > >> - if (!spi_cs_is_valid(bus, cs)) > >> - return NULL; > >> - > >> - ds = spi_alloc_slave(struct davinci_spi_slave, bus, cs); > >> - if (!ds) > >> - return NULL; > >> - > >> - switch (bus) { > >> - case SPI0_BUS: > >> - ds->regs = (struct davinci_spi_regs *)SPI0_BASE; > >> - break; > >> -#ifdef CONFIG_SYS_SPI1 > >> - case SPI1_BUS: > >> - ds->regs = (struct davinci_spi_regs *)SPI1_BASE; > >> - break; > >> -#endif > >> -#ifdef CONFIG_SYS_SPI2 > >> - case SPI2_BUS: > >> - ds->regs = (struct davinci_spi_regs *)SPI2_BASE; > >> - break; > >> -#endif > >> - default: /* Invalid bus number */ > >> - return NULL; > >> - } > >> - > >> - ds->freq = max_hz; > >> - ds->mode = mode; > >> - > >> - return &ds->slave; > >> -} > >> - > >> -void spi_free_slave(struct spi_slave *slave) > >> -{ > >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); > >> - > >> - free(ds); > >> -} > >> - > >> -int spi_xfer(struct spi_slave *slave, unsigned int bitlen, > >> - const void *dout, void *din, unsigned long flags) > >> -{ > >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); > >> - > >> - ds->cur_cs = slave->cs; > >> - > >> - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); > >> -} > >> - > >> -int spi_claim_bus(struct spi_slave *slave) > >> -{ > >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); > >> - > >> -#ifdef CONFIG_SPI_HALF_DUPLEX > >> - ds->half_duplex = true; > >> -#else > >> - ds->half_duplex = false; > >> -#endif > >> - return __davinci_spi_claim_bus(ds, ds->slave.cs); > >> -} > >> - > >> -void spi_release_bus(struct spi_slave *slave) > >> -{ > >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); > >> - > >> - __davinci_spi_release_bus(ds); > >> -} > >> - > >> -#else > >> -static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) > >> -{ > >> - struct davinci_spi_slave *ds = dev_get_priv(bus); > >> - > >> - debug("%s speed %u\n", __func__, max_hz); > >> - if (max_hz > CONFIG_SYS_SPI_CLK / 2) > >> - return -EINVAL; > >> - > >> - ds->freq = max_hz; > >> > >> return 0; > >> } > >> > >> -static int davinci_spi_set_mode(struct udevice *bus, uint mode) > >> -{ > >> - struct davinci_spi_slave *ds = dev_get_priv(bus); > >> - > >> - debug("%s mode %u\n", __func__, mode); > >> - ds->mode = mode; > >> - > >> - return 0; > >> -} > >> - > >> -static int davinci_spi_claim_bus(struct udevice *dev) > >> -{ > >> - struct dm_spi_slave_platdata *slave_plat = > >> - dev_get_parent_platdata(dev); > >> - struct udevice *bus = dev->parent; > >> - struct davinci_spi_slave *ds = dev_get_priv(bus); > >> - > >> - if (slave_plat->cs >= ds->num_cs) { > >> - printf("Invalid SPI chipselect\n"); > >> - return -EINVAL; > >> - } > >> - ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; > >> - > >> - return __davinci_spi_claim_bus(ds, slave_plat->cs); > >> -} > >> - > >> -static int davinci_spi_release_bus(struct udevice *dev) > >> -{ > >> - struct davinci_spi_slave *ds = dev_get_priv(dev->parent); > >> - > >> - return __davinci_spi_release_bus(ds); > >> -} > >> +static const struct dm_spi_ops davinci_spi_ops = { > >> + .claim_bus = davinci_spi_claim_bus, > >> + .release_bus = davinci_spi_release_bus, > >> + .xfer = davinci_spi_xfer, > >> + .set_speed = davinci_spi_set_speed, > >> + .set_mode = davinci_spi_set_mode, > >> +}; > >> > >> -static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, > >> - const void *dout, void *din, > >> - unsigned long flags) > >> +static int davinci_spi_probe(struct udevice *bus) > >> { > >> - struct dm_spi_slave_platdata *slave = > >> - dev_get_parent_platdata(dev); > >> - struct udevice *bus = dev->parent; > >> struct davinci_spi_slave *ds = dev_get_priv(bus); > >> + struct davinci_spi_platdata *plat = bus->platdata; > >> + ds->regs = plat->regs; > >> + ds->num_cs = plat->num_cs; > >> > >> - if (slave->cs >= ds->num_cs) { > >> - printf("Invalid SPI chipselect\n"); > >> - return -EINVAL; > >> - } > >> - ds->cur_cs = slave->cs; > >> - > >> - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); > >> -} > >> - > >> -static int davinci_spi_probe(struct udevice *bus) > >> -{ > >> - /* Nothing to do */ > >> return 0; > >> } > >> > >> +#if CONFIG_IS_ENABLED(OF_CONTROL) > > > > Looking at other drivers, I wonder if this should be > > +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) > > > > > >> static int davinci_ofdata_to_platadata(struct udevice *bus) > >> { > >> - struct davinci_spi_slave *ds = dev_get_priv(bus); > >> - const void *blob = gd->fdt_blob; > >> - int node = dev_of_offset(bus); > >> + struct davinci_spi_platdata *plat = bus->platdata; > >> + fdt_addr_t addr; > >> > >> - ds->regs = devfdt_map_physmem(bus, sizeof(struct davinci_spi_regs)); > >> - if (!ds->regs) { > >> - printf("%s: could not map device address\n", __func__); > >> + addr = devfdt_get_addr(bus); > >> + if (addr == FDT_ADDR_T_NONE) > >> return -EINVAL; > >> - } > >> - ds->num_cs = fdtdec_get_int(blob, node, "num-cs", 4); > >> + > >> + plat->regs = (struct davinci_spi_regs *)addr; > >> + plat->num_cs = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus), "num-cs", 4); > >> > >> return 0; > >> } > >> > >> -static const struct dm_spi_ops davinci_spi_ops = { > >> - .claim_bus = davinci_spi_claim_bus, > >> - .release_bus = davinci_spi_release_bus, > >> - .xfer = davinci_spi_xfer, > >> - .set_speed = davinci_spi_set_speed, > >> - .set_mode = davinci_spi_set_mode, > >> -}; > >> - > >> static const struct udevice_id davinci_spi_ids[] = { > >> { .compatible = "ti,keystone-spi" }, > >> { .compatible = "ti,dm6441-spi" }, > >> { .compatible = "ti,da830-spi" }, > >> { } > >> }; > >> +#endif > >> > >> U_BOOT_DRIVER(davinci_spi) = { > >> .name = "davinci_spi", > >> .id = UCLASS_SPI, > >> +#if CONFIG_IS_ENABLED(OF_CONTROL) > > > > Like above, should this be: > > +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) > > > > With limited SPL resources, I cannot build OF_CONTROL in SPL and > > disabling OF_CONTROL in SPL doesn't build either. > > With the modification, I can build with OF_PLATDATA enabled.

Is it possible to enable DM_SPI in SPL? da850evm_direct_nor_defconfig is able to build since it has it.

Enabling DM_SPI in SPL starts to grow, hence my other comments about SPL_OF_PLATDATA in order to make it fit into SPL. da850evm_direct_nor_defconfig does not enable SPL, but it does enable DM_SPI. I had to get the NOR expansion board in order to try it. I'm trying to get it to work now, but for some reason, I'm having difficulty booting the stock da850evm_direct_nor_defconfig

It would be easiest if we could have both a DM_SPI and non DM_SPI version of the driver so it can fit into SPL or the ability to disable SPI in SPL. I am experimenting with the latter. Several drivers offer the option to be disabled in SPL, so I'm experimenting with that to save space in SPL.

I was able to verify your code works for the da850evm_direct_nor_defconfig version which doesn't use SPL. I spent a significant amount of time yesterday trying to get SPL to work, but just enabling SPL and disabling all drivers except serial, I was not able to boot, so I think something is wrong with DM and SPL. I don't have a debugger at home for this board, so I'll need to get one from work to further troubleshoot.

I don't think it is much difficult to get serial up here. I made few changes for serial and spi platdata for SPL. If haven't try these please check the same and better change proper clock value for uart if added one is improper.

...

I don't think any DA850/L138/AM1808 board uses DM in SPL, so I think it would be best to maintain both DM and non-DM code bases for now until we're able to support DM in SPL.

But the whole idea is to drop nod-dm SPI as much as possible.

What your saying makes sense.

...

[1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/spi-dm-migrate

I am traveling today, but I will try to look at that tomorrow. I did something similar already without success, but I will try again with your patch to see if it's any better.

Is there a document somewhere that shows the order of operations during SPL? I'm wondering if some of the SPL Davinci code should be refactored to make sure the hardware is ready in the order that SPL is expecting it when used with DM.

Your patch adding the UART stuff and enabling DM still didn't work. As soon as I enable DM in SPL, I no longer get any of the SPL text stuff that normally gets displayed immediately on boot. I will try to take a look at the suggestion from Simon Goldschmidt to see if I can adapt any of his work to the Davinci platform.

adam

Adam

...

On Mon, Aug 13, 2018 at 3:46 PM Alex Kiernan alex.kiernan@gmail.com wrote:

On Mon, Aug 13, 2018 at 1:40 PM Adam Ford aford173@gmail.com wrote:

...

On Sat, Aug 11, 2018 at 3:09 PM Adam Ford aford173@gmail.com wrote:

...

On Sat, Aug 11, 2018, 1:24 PM Jagan Teki jagan@amarulasolutions.com wrote:

...

On Sat, Aug 11, 2018 at 6:12 PM, Adam Ford aford173@gmail.com wrote:

...

On Fri, Aug 10, 2018 at 2:58 PM Adam Ford aford173@gmail.com wrote:

...

On Fri, Aug 10, 2018 at 7:42 AM Jagan Teki jagan@amarulasolutions.com wrote: > > On Fri, Aug 10, 2018 at 3:50 PM, Adam Ford aford173@gmail.com wrote: > > On Fri, Aug 10, 2018 at 12:14 AM Jagan Teki jagan@amarulasolutions.com wrote: > >> > >> On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford aford173@gmail.com wrote: > >> > On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki jagan@amarulasolutions.com wrote: > >> >> > >> >> davinci_spi now support dt along with platform data, > >> >> respective boards need to switch into dm for the same. > >> >> > >> >> Cc: Adam Ford aford173@gmail.com > >> >> Cc: Vitaly Andrianov vitalya@ti.com > >> >> Cc: Stefano Babic sbabic@denx.de > >> >> Cc: Peter Howard phoward@gme.net.au > >> >> Cc: Tom Rini trini@konsulko.com > >> >> Signed-off-by: Jagan Teki jagan@amarulasolutions.com > >> >> --- > >> >> drivers/spi/Kconfig | 12 +- > >> >> drivers/spi/davinci_spi.c | 289 +++++++------------------ > >> >> include/dm/platform_data/spi_davinci.h | 15 ++ > >> >> 3 files changed, 97 insertions(+), 219 deletions(-) > >> >> create mode 100644 include/dm/platform_data/spi_davinci.h > >> >> > >> >> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig > >> >> index d046e919b4..18ebff0231 100644 > >> >> --- a/drivers/spi/Kconfig > >> >> +++ b/drivers/spi/Kconfig > >> >> @@ -80,6 +80,12 @@ config CADENCE_QSPI > >> >> used to access the SPI NOR flash on platforms embedding this > >> >> Cadence IP core. > >> >> > >> >> +config DAVINCI_SPI > >> >> + bool "Davinci & Keystone SPI driver" > >> >> + depends on ARCH_DAVINCI || ARCH_KEYSTONE > >> >> + help > >> >> + Enable the Davinci SPI driver > >> >> + > >> >> config DESIGNWARE_SPI > >> >> bool "Designware SPI driver" > >> >> help > >> >> @@ -281,12 +287,6 @@ config FSL_QSPI > >> >> used to access the SPI NOR flash on platforms embedding this > >> >> Freescale IP core. > >> >> > >> >> -config DAVINCI_SPI > >> >> - bool "Davinci & Keystone SPI driver" > >> >> - depends on ARCH_DAVINCI || ARCH_KEYSTONE > >> >> - help > >> >> - Enable the Davinci SPI driver > >> >> - > >> >> config SH_SPI > >> >> bool "SuperH SPI driver" > >> >> help > >> >> diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c > >> >> index a822858323..5007e6c618 100644 > >> >> --- a/drivers/spi/davinci_spi.c > >> >> +++ b/drivers/spi/davinci_spi.c > >> >> @@ -14,6 +14,7 @@ > >> >> #include <asm/io.h> > >> >> #include <asm/arch/hardware.h> > >> >> #include <dm.h> > >> >> +#include <dm/platform_data/spi_davinci.h> > >> >> > >> >> /* SPIGCR0 */ > >> >> #define SPIGCR0_SPIENA_MASK 0x1 > >> >> @@ -118,9 +119,6 @@ struct davinci_spi_regs { > >> >> > >> >> /* davinci spi slave */ > >> >> struct davinci_spi_slave { > >> >> -#ifndef CONFIG_DM_SPI > >> >> - struct spi_slave slave; > >> >> -#endif > >> >> struct davinci_spi_regs *regs; > >> >> unsigned int freq; /* current SPI bus frequency */ > >> >> unsigned int mode; /* current SPI mode used */ > >> >> @@ -240,11 +238,43 @@ static int davinci_spi_read_write(struct davinci_spi_slave *ds, unsigned > >> >> return 0; > >> >> } > >> >> > >> >> +static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) > >> >> +{ > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); > >> >> > >> >> -static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) > >> >> + debug("%s speed %u\n", __func__, max_hz); > >> >> + if (max_hz > CONFIG_SYS_SPI_CLK / 2) > >> >> + return -EINVAL; > >> >> + > >> >> + ds->freq = max_hz; > >> >> + > >> >> + return 0; > >> >> +} > >> >> + > >> >> +static int davinci_spi_set_mode(struct udevice *bus, uint mode) > >> >> +{ > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); > >> >> + > >> >> + debug("%s mode %u\n", __func__, mode); > >> >> + ds->mode = mode; > >> >> + > >> >> + return 0; > >> >> +} > >> >> + > >> >> +static int davinci_spi_claim_bus(struct udevice *dev) > >> >> { > >> >> + struct dm_spi_slave_platdata *slave_plat = > >> >> + dev_get_parent_platdata(dev); > >> >> + struct udevice *bus = dev->parent; > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); > >> >> unsigned int mode = 0, scalar; > >> >> > >> >> + if (slave_plat->cs >= ds->num_cs) { > >> >> + printf("Invalid SPI chipselect\n"); > >> >> + return -EINVAL; > >> >> + } > >> >> + ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; > >> >> + > >> >> /* Enable the SPI hardware */ > >> >> writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); > >> >> udelay(1000); > >> >> @@ -254,7 +284,7 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) > >> >> writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1); > >> >> > >> >> /* CS, CLK, SIMO and SOMI are functional pins */ > >> >> - writel(((1 << cs) | SPIPC0_CLKFUN_MASK | > >> >> + writel(((1 << slave_plat->cs) | SPIPC0_CLKFUN_MASK | > >> >> SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0); > >> >> > >> >> /* setup format */ > >> >> @@ -292,20 +322,32 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) > >> >> return 0; > >> >> } > >> >> > >> >> -static int __davinci_spi_release_bus(struct davinci_spi_slave *ds) > >> >> +static int davinci_spi_release_bus(struct udevice *dev) > >> >> { > >> >> + struct davinci_spi_slave *ds = dev_get_priv(dev->parent); > >> >> + > >> >> /* Disable the SPI hardware */ > >> >> writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); > >> >> > >> >> return 0; > >> >> } > >> >> > >> >> -static int __davinci_spi_xfer(struct davinci_spi_slave *ds, > >> >> - unsigned int bitlen, const void *dout, void *din, > >> >> - unsigned long flags) > >> >> +static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, > >> >> + const void *dout, void *din, > >> >> + unsigned long flags) > >> >> { > >> >> + struct dm_spi_slave_platdata *slave = > >> >> + dev_get_parent_platdata(dev); > >> >> + struct udevice *bus = dev->parent; > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); > >> >> unsigned int len; > >> >> > >> >> + if (slave->cs >= ds->num_cs) { > >> >> + printf("Invalid SPI chipselect\n"); > >> >> + return -EINVAL; > >> >> + } > >> >> + ds->cur_cs = slave->cs; > >> >> + > >> >> if (bitlen == 0) > >> >> /* Finish any previously submitted transfers */ > >> >> goto out; > >> >> @@ -339,240 +381,61 @@ out: > >> >> u8 dummy = 0; > >> >> davinci_spi_write(ds, 1, &dummy, flags); > >> >> } > >> >> - return 0; > >> >> -} > >> >> - > >> >> -#ifndef CONFIG_DM_SPI > >> >> - > >> >> -static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) > >> >> -{ > >> >> - return container_of(slave, struct davinci_spi_slave, slave); > >> >> -} > >> >> - > >> >> -int spi_cs_is_valid(unsigned int bus, unsigned int cs) > >> >> -{ > >> >> - int ret = 0; > >> >> - > >> >> - switch (bus) { > >> >> - case SPI0_BUS: > >> >> - if (cs < SPI0_NUM_CS) > >> >> - ret = 1; > >> >> - break; > >> >> -#ifdef CONFIG_SYS_SPI1 > >> >> - case SPI1_BUS: > >> >> - if (cs < SPI1_NUM_CS) > >> >> - ret = 1; > >> >> - break; > >> >> -#endif > >> >> -#ifdef CONFIG_SYS_SPI2 > >> >> - case SPI2_BUS: > >> >> - if (cs < SPI2_NUM_CS) > >> >> - ret = 1; > >> >> - break; > >> >> -#endif > >> >> - default: > >> >> - /* Invalid bus number. Do nothing */ > >> >> - break; > >> >> - } > >> >> - return ret; > >> >> -} > >> >> - > >> >> -void spi_cs_activate(struct spi_slave *slave) > >> >> -{ > >> >> - /* do nothing */ > >> >> -} > >> >> - > >> >> -void spi_cs_deactivate(struct spi_slave *slave) > >> >> -{ > >> >> - /* do nothing */ > >> >> -} > >> >> - > >> >> -void spi_init(void) > >> >> -{ > >> >> - /* do nothing */ > >> >> -} > >> >> - > >> >> -struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, > >> >> - unsigned int max_hz, unsigned int mode) > >> >> -{ > >> >> - struct davinci_spi_slave *ds; > >> >> - > >> >> - if (!spi_cs_is_valid(bus, cs)) > >> >> - return NULL; > >> >> - > >> >> - ds = spi_alloc_slave(struct davinci_spi_slave, bus, cs); > >> >> - if (!ds) > >> >> - return NULL; > >> >> - > >> >> - switch (bus) { > >> >> - case SPI0_BUS: > >> >> - ds->regs = (struct davinci_spi_regs *)SPI0_BASE; > >> >> - break; > >> >> -#ifdef CONFIG_SYS_SPI1 > >> >> - case SPI1_BUS: > >> >> - ds->regs = (struct davinci_spi_regs *)SPI1_BASE; > >> >> - break; > >> >> -#endif > >> >> -#ifdef CONFIG_SYS_SPI2 > >> >> - case SPI2_BUS: > >> >> - ds->regs = (struct davinci_spi_regs *)SPI2_BASE; > >> >> - break; > >> >> -#endif > >> >> - default: /* Invalid bus number */ > >> >> - return NULL; > >> >> - } > >> >> - > >> >> - ds->freq = max_hz; > >> >> - ds->mode = mode; > >> >> - > >> >> - return &ds->slave; > >> >> -} > >> >> - > >> >> -void spi_free_slave(struct spi_slave *slave) > >> >> -{ > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); > >> >> - > >> >> - free(ds); > >> >> -} > >> >> - > >> >> -int spi_xfer(struct spi_slave *slave, unsigned int bitlen, > >> >> - const void *dout, void *din, unsigned long flags) > >> >> -{ > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); > >> >> - > >> >> - ds->cur_cs = slave->cs; > >> >> - > >> >> - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); > >> >> -} > >> >> - > >> >> -int spi_claim_bus(struct spi_slave *slave) > >> >> -{ > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); > >> >> - > >> >> -#ifdef CONFIG_SPI_HALF_DUPLEX > >> >> - ds->half_duplex = true; > >> >> -#else > >> >> - ds->half_duplex = false; > >> >> -#endif > >> >> - return __davinci_spi_claim_bus(ds, ds->slave.cs); > >> >> -} > >> >> - > >> >> -void spi_release_bus(struct spi_slave *slave) > >> >> -{ > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); > >> >> - > >> >> - __davinci_spi_release_bus(ds); > >> >> -} > >> >> - > >> >> -#else > >> >> -static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) > >> >> -{ > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); > >> >> - > >> >> - debug("%s speed %u\n", __func__, max_hz); > >> >> - if (max_hz > CONFIG_SYS_SPI_CLK / 2) > >> >> - return -EINVAL; > >> >> - > >> >> - ds->freq = max_hz; > >> >> > >> >> return 0; > >> >> } > >> >> > >> >> -static int davinci_spi_set_mode(struct udevice *bus, uint mode) > >> >> -{ > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); > >> >> - > >> >> - debug("%s mode %u\n", __func__, mode); > >> >> - ds->mode = mode; > >> >> - > >> >> - return 0; > >> >> -} > >> >> - > >> >> -static int davinci_spi_claim_bus(struct udevice *dev) > >> >> -{ > >> >> - struct dm_spi_slave_platdata *slave_plat = > >> >> - dev_get_parent_platdata(dev); > >> >> - struct udevice *bus = dev->parent; > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); > >> >> - > >> >> - if (slave_plat->cs >= ds->num_cs) { > >> >> - printf("Invalid SPI chipselect\n"); > >> >> - return -EINVAL; > >> >> - } > >> >> - ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; > >> >> - > >> >> - return __davinci_spi_claim_bus(ds, slave_plat->cs); > >> >> -} > >> >> - > >> >> -static int davinci_spi_release_bus(struct udevice *dev) > >> >> -{ > >> >> - struct davinci_spi_slave *ds = dev_get_priv(dev->parent); > >> >> - > >> >> - return __davinci_spi_release_bus(ds); > >> >> -} > >> >> +static const struct dm_spi_ops davinci_spi_ops = { > >> >> + .claim_bus = davinci_spi_claim_bus, > >> >> + .release_bus = davinci_spi_release_bus, > >> >> + .xfer = davinci_spi_xfer, > >> >> + .set_speed = davinci_spi_set_speed, > >> >> + .set_mode = davinci_spi_set_mode, > >> >> +}; > >> >> > >> >> -static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, > >> >> - const void *dout, void *din, > >> >> - unsigned long flags) > >> >> +static int davinci_spi_probe(struct udevice *bus) > >> >> { > >> >> - struct dm_spi_slave_platdata *slave = > >> >> - dev_get_parent_platdata(dev); > >> >> - struct udevice *bus = dev->parent; > >> >> struct davinci_spi_slave *ds = dev_get_priv(bus); > >> >> + struct davinci_spi_platdata *plat = bus->platdata; > >> >> + ds->regs = plat->regs; > >> >> + ds->num_cs = plat->num_cs; > >> >> > >> >> - if (slave->cs >= ds->num_cs) { > >> >> - printf("Invalid SPI chipselect\n"); > >> >> - return -EINVAL; > >> >> - } > >> >> - ds->cur_cs = slave->cs; > >> >> - > >> >> - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); > >> >> -} > >> >> - > >> >> -static int davinci_spi_probe(struct udevice *bus) > >> >> -{ > >> >> - /* Nothing to do */ > >> >> return 0; > >> >> } > >> >> > >> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) > >> > > >> > Looking at other drivers, I wonder if this should be > >> > +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) > >> > > >> > > >> >> static int davinci_ofdata_to_platadata(struct udevice *bus) > >> >> { > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); > >> >> - const void *blob = gd->fdt_blob; > >> >> - int node = dev_of_offset(bus); > >> >> + struct davinci_spi_platdata *plat = bus->platdata; > >> >> + fdt_addr_t addr; > >> >> > >> >> - ds->regs = devfdt_map_physmem(bus, sizeof(struct davinci_spi_regs)); > >> >> - if (!ds->regs) { > >> >> - printf("%s: could not map device address\n", __func__); > >> >> + addr = devfdt_get_addr(bus); > >> >> + if (addr == FDT_ADDR_T_NONE) > >> >> return -EINVAL; > >> >> - } > >> >> - ds->num_cs = fdtdec_get_int(blob, node, "num-cs", 4); > >> >> + > >> >> + plat->regs = (struct davinci_spi_regs *)addr; > >> >> + plat->num_cs = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus), "num-cs", 4); > >> >> > >> >> return 0; > >> >> } > >> >> > >> >> -static const struct dm_spi_ops davinci_spi_ops = { > >> >> - .claim_bus = davinci_spi_claim_bus, > >> >> - .release_bus = davinci_spi_release_bus, > >> >> - .xfer = davinci_spi_xfer, > >> >> - .set_speed = davinci_spi_set_speed, > >> >> - .set_mode = davinci_spi_set_mode, > >> >> -}; > >> >> - > >> >> static const struct udevice_id davinci_spi_ids[] = { > >> >> { .compatible = "ti,keystone-spi" }, > >> >> { .compatible = "ti,dm6441-spi" }, > >> >> { .compatible = "ti,da830-spi" }, > >> >> { } > >> >> }; > >> >> +#endif > >> >> > >> >> U_BOOT_DRIVER(davinci_spi) = { > >> >> .name = "davinci_spi", > >> >> .id = UCLASS_SPI, > >> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) > >> > > >> > Like above, should this be: > >> > +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) > >> > > >> > With limited SPL resources, I cannot build OF_CONTROL in SPL and > >> > disabling OF_CONTROL in SPL doesn't build either. > >> > With the modification, I can build with OF_PLATDATA enabled. > > Is it possible to enable DM_SPI in SPL? da850evm_direct_nor_defconfig > is able to build since it has it.

Enabling DM_SPI in SPL starts to grow, hence my other comments about SPL_OF_PLATDATA in order to make it fit into SPL. da850evm_direct_nor_defconfig does not enable SPL, but it does enable DM_SPI. I had to get the NOR expansion board in order to try it. I'm trying to get it to work now, but for some reason, I'm having difficulty booting the stock da850evm_direct_nor_defconfig

It would be easiest if we could have both a DM_SPI and non DM_SPI version of the driver so it can fit into SPL or the ability to disable SPI in SPL. I am experimenting with the latter. Several drivers offer the option to be disabled in SPL, so I'm experimenting with that to save space in SPL.

I was able to verify your code works for the da850evm_direct_nor_defconfig version which doesn't use SPL. I spent a significant amount of time yesterday trying to get SPL to work, but just enabling SPL and disabling all drivers except serial, I was not able to boot, so I think something is wrong with DM and SPL. I don't have a debugger at home for this board, so I'll need to get one from work to further troubleshoot.

I don't think it is much difficult to get serial up here. I made few changes for serial and spi platdata for SPL. If haven't try these please check the same and better change proper clock value for uart if added one is improper.

...

I don't think any DA850/L138/AM1808 board uses DM in SPL, so I think it would be best to maintain both DM and non-DM code bases for now until we're able to support DM in SPL.

But the whole idea is to drop nod-dm SPI as much as possible.

What your saying makes sense.

...

[1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/spi-dm-migrate

I am traveling today, but I will try to look at that tomorrow. I did something similar already without success, but I will try again with your patch to see if it's any better.

Is there a document somewhere that shows the order of operations during SPL? I'm wondering if some of the SPL Davinci code should be refactored to make sure the hardware is ready in the order that SPL is expecting it when used with DM.

Your patch adding the UART stuff and enabling DM still didn't work. As soon as I enable DM in SPL, I no longer get any of the SPL text stuff that normally gets displayed immediately on boot. I will try to take a look at the suggestion from Simon Goldschmidt to see if I can adapt any of his work to the Davinci platform.

In case it's of use as another line to explore, we had the same problem, which we eventually tracked down to some board detection code which doing GPIO bashing using gpio_get_value (and friends) which with SPL_DM ended up using a lot more RAM than we had with SPL_SYS_MALLOC_SIMPLE.

I have sent a patch to fix a data abort when calloc() calls memset on a NULL pointer returned by malloc_simple(). If you don't have enough memory in your "simple" heap, this might be why you don't see anything.

Have you tried enabling the debug uart (which doesn't need DM)?

Simon

On Mon, Aug 13, 2018 at 5:09 PM Adam Ford aford173@gmail.com wrote:

On Mon, Aug 13, 2018 at 1:29 PM Simon Goldschmidt simon.k.r.goldschmidt@gmail.com wrote:

...

On Mon, Aug 13, 2018 at 3:46 PM Alex Kiernan alex.kiernan@gmail.com wrote:

...

On Mon, Aug 13, 2018 at 1:40 PM Adam Ford aford173@gmail.com wrote:

...

On Sat, Aug 11, 2018 at 3:09 PM Adam Ford aford173@gmail.com wrote:

...

On Sat, Aug 11, 2018, 1:24 PM Jagan Teki jagan@amarulasolutions.com wrote:

...

On Sat, Aug 11, 2018 at 6:12 PM, Adam Ford aford173@gmail.com wrote: > On Fri, Aug 10, 2018 at 2:58 PM Adam Ford aford173@gmail.com wrote: >> >> On Fri, Aug 10, 2018 at 7:42 AM Jagan Teki jagan@amarulasolutions.com wrote: >> > >> > On Fri, Aug 10, 2018 at 3:50 PM, Adam Ford aford173@gmail.com wrote: >> > > On Fri, Aug 10, 2018 at 12:14 AM Jagan Teki jagan@amarulasolutions.com wrote: >> > >> >> > >> On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford aford173@gmail.com wrote: >> > >> > On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki jagan@amarulasolutions.com wrote: >> > >> >> >> > >> >> davinci_spi now support dt along with platform data, >> > >> >> respective boards need to switch into dm for the same. >> > >> >> >> > >> >> Cc: Adam Ford aford173@gmail.com >> > >> >> Cc: Vitaly Andrianov vitalya@ti.com >> > >> >> Cc: Stefano Babic sbabic@denx.de >> > >> >> Cc: Peter Howard phoward@gme.net.au >> > >> >> Cc: Tom Rini trini@konsulko.com >> > >> >> Signed-off-by: Jagan Teki jagan@amarulasolutions.com >> > >> >> --- >> > >> >> drivers/spi/Kconfig | 12 +- >> > >> >> drivers/spi/davinci_spi.c | 289 +++++++------------------ >> > >> >> include/dm/platform_data/spi_davinci.h | 15 ++ >> > >> >> 3 files changed, 97 insertions(+), 219 deletions(-) >> > >> >> create mode 100644 include/dm/platform_data/spi_davinci.h >> > >> >> >> > >> >> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig >> > >> >> index d046e919b4..18ebff0231 100644 >> > >> >> --- a/drivers/spi/Kconfig >> > >> >> +++ b/drivers/spi/Kconfig >> > >> >> @@ -80,6 +80,12 @@ config CADENCE_QSPI >> > >> >> used to access the SPI NOR flash on platforms embedding this >> > >> >> Cadence IP core. >> > >> >> >> > >> >> +config DAVINCI_SPI >> > >> >> + bool "Davinci & Keystone SPI driver" >> > >> >> + depends on ARCH_DAVINCI || ARCH_KEYSTONE >> > >> >> + help >> > >> >> + Enable the Davinci SPI driver >> > >> >> + >> > >> >> config DESIGNWARE_SPI >> > >> >> bool "Designware SPI driver" >> > >> >> help >> > >> >> @@ -281,12 +287,6 @@ config FSL_QSPI >> > >> >> used to access the SPI NOR flash on platforms embedding this >> > >> >> Freescale IP core. >> > >> >> >> > >> >> -config DAVINCI_SPI >> > >> >> - bool "Davinci & Keystone SPI driver" >> > >> >> - depends on ARCH_DAVINCI || ARCH_KEYSTONE >> > >> >> - help >> > >> >> - Enable the Davinci SPI driver >> > >> >> - >> > >> >> config SH_SPI >> > >> >> bool "SuperH SPI driver" >> > >> >> help >> > >> >> diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c >> > >> >> index a822858323..5007e6c618 100644 >> > >> >> --- a/drivers/spi/davinci_spi.c >> > >> >> +++ b/drivers/spi/davinci_spi.c >> > >> >> @@ -14,6 +14,7 @@ >> > >> >> #include <asm/io.h> >> > >> >> #include <asm/arch/hardware.h> >> > >> >> #include <dm.h> >> > >> >> +#include <dm/platform_data/spi_davinci.h> >> > >> >> >> > >> >> /* SPIGCR0 */ >> > >> >> #define SPIGCR0_SPIENA_MASK 0x1 >> > >> >> @@ -118,9 +119,6 @@ struct davinci_spi_regs { >> > >> >> >> > >> >> /* davinci spi slave */ >> > >> >> struct davinci_spi_slave { >> > >> >> -#ifndef CONFIG_DM_SPI >> > >> >> - struct spi_slave slave; >> > >> >> -#endif >> > >> >> struct davinci_spi_regs *regs; >> > >> >> unsigned int freq; /* current SPI bus frequency */ >> > >> >> unsigned int mode; /* current SPI mode used */ >> > >> >> @@ -240,11 +238,43 @@ static int davinci_spi_read_write(struct davinci_spi_slave *ds, unsigned >> > >> >> return 0; >> > >> >> } >> > >> >> >> > >> >> +static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) >> > >> >> +{ >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> > >> >> >> > >> >> -static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) >> > >> >> + debug("%s speed %u\n", __func__, max_hz); >> > >> >> + if (max_hz > CONFIG_SYS_SPI_CLK / 2) >> > >> >> + return -EINVAL; >> > >> >> + >> > >> >> + ds->freq = max_hz; >> > >> >> + >> > >> >> + return 0; >> > >> >> +} >> > >> >> + >> > >> >> +static int davinci_spi_set_mode(struct udevice *bus, uint mode) >> > >> >> +{ >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> > >> >> + >> > >> >> + debug("%s mode %u\n", __func__, mode); >> > >> >> + ds->mode = mode; >> > >> >> + >> > >> >> + return 0; >> > >> >> +} >> > >> >> + >> > >> >> +static int davinci_spi_claim_bus(struct udevice *dev) >> > >> >> { >> > >> >> + struct dm_spi_slave_platdata *slave_plat = >> > >> >> + dev_get_parent_platdata(dev); >> > >> >> + struct udevice *bus = dev->parent; >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> > >> >> unsigned int mode = 0, scalar; >> > >> >> >> > >> >> + if (slave_plat->cs >= ds->num_cs) { >> > >> >> + printf("Invalid SPI chipselect\n"); >> > >> >> + return -EINVAL; >> > >> >> + } >> > >> >> + ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; >> > >> >> + >> > >> >> /* Enable the SPI hardware */ >> > >> >> writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); >> > >> >> udelay(1000); >> > >> >> @@ -254,7 +284,7 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) >> > >> >> writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1); >> > >> >> >> > >> >> /* CS, CLK, SIMO and SOMI are functional pins */ >> > >> >> - writel(((1 << cs) | SPIPC0_CLKFUN_MASK | >> > >> >> + writel(((1 << slave_plat->cs) | SPIPC0_CLKFUN_MASK | >> > >> >> SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0); >> > >> >> >> > >> >> /* setup format */ >> > >> >> @@ -292,20 +322,32 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) >> > >> >> return 0; >> > >> >> } >> > >> >> >> > >> >> -static int __davinci_spi_release_bus(struct davinci_spi_slave *ds) >> > >> >> +static int davinci_spi_release_bus(struct udevice *dev) >> > >> >> { >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(dev->parent); >> > >> >> + >> > >> >> /* Disable the SPI hardware */ >> > >> >> writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); >> > >> >> >> > >> >> return 0; >> > >> >> } >> > >> >> >> > >> >> -static int __davinci_spi_xfer(struct davinci_spi_slave *ds, >> > >> >> - unsigned int bitlen, const void *dout, void *din, >> > >> >> - unsigned long flags) >> > >> >> +static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, >> > >> >> + const void *dout, void *din, >> > >> >> + unsigned long flags) >> > >> >> { >> > >> >> + struct dm_spi_slave_platdata *slave = >> > >> >> + dev_get_parent_platdata(dev); >> > >> >> + struct udevice *bus = dev->parent; >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> > >> >> unsigned int len; >> > >> >> >> > >> >> + if (slave->cs >= ds->num_cs) { >> > >> >> + printf("Invalid SPI chipselect\n"); >> > >> >> + return -EINVAL; >> > >> >> + } >> > >> >> + ds->cur_cs = slave->cs; >> > >> >> + >> > >> >> if (bitlen == 0) >> > >> >> /* Finish any previously submitted transfers */ >> > >> >> goto out; >> > >> >> @@ -339,240 +381,61 @@ out: >> > >> >> u8 dummy = 0; >> > >> >> davinci_spi_write(ds, 1, &dummy, flags); >> > >> >> } >> > >> >> - return 0; >> > >> >> -} >> > >> >> - >> > >> >> -#ifndef CONFIG_DM_SPI >> > >> >> - >> > >> >> -static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) >> > >> >> -{ >> > >> >> - return container_of(slave, struct davinci_spi_slave, slave); >> > >> >> -} >> > >> >> - >> > >> >> -int spi_cs_is_valid(unsigned int bus, unsigned int cs) >> > >> >> -{ >> > >> >> - int ret = 0; >> > >> >> - >> > >> >> - switch (bus) { >> > >> >> - case SPI0_BUS: >> > >> >> - if (cs < SPI0_NUM_CS) >> > >> >> - ret = 1; >> > >> >> - break; >> > >> >> -#ifdef CONFIG_SYS_SPI1 >> > >> >> - case SPI1_BUS: >> > >> >> - if (cs < SPI1_NUM_CS) >> > >> >> - ret = 1; >> > >> >> - break; >> > >> >> -#endif >> > >> >> -#ifdef CONFIG_SYS_SPI2 >> > >> >> - case SPI2_BUS: >> > >> >> - if (cs < SPI2_NUM_CS) >> > >> >> - ret = 1; >> > >> >> - break; >> > >> >> -#endif >> > >> >> - default: >> > >> >> - /* Invalid bus number. Do nothing */ >> > >> >> - break; >> > >> >> - } >> > >> >> - return ret; >> > >> >> -} >> > >> >> - >> > >> >> -void spi_cs_activate(struct spi_slave *slave) >> > >> >> -{ >> > >> >> - /* do nothing */ >> > >> >> -} >> > >> >> - >> > >> >> -void spi_cs_deactivate(struct spi_slave *slave) >> > >> >> -{ >> > >> >> - /* do nothing */ >> > >> >> -} >> > >> >> - >> > >> >> -void spi_init(void) >> > >> >> -{ >> > >> >> - /* do nothing */ >> > >> >> -} >> > >> >> - >> > >> >> -struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, >> > >> >> - unsigned int max_hz, unsigned int mode) >> > >> >> -{ >> > >> >> - struct davinci_spi_slave *ds; >> > >> >> - >> > >> >> - if (!spi_cs_is_valid(bus, cs)) >> > >> >> - return NULL; >> > >> >> - >> > >> >> - ds = spi_alloc_slave(struct davinci_spi_slave, bus, cs); >> > >> >> - if (!ds) >> > >> >> - return NULL; >> > >> >> - >> > >> >> - switch (bus) { >> > >> >> - case SPI0_BUS: >> > >> >> - ds->regs = (struct davinci_spi_regs *)SPI0_BASE; >> > >> >> - break; >> > >> >> -#ifdef CONFIG_SYS_SPI1 >> > >> >> - case SPI1_BUS: >> > >> >> - ds->regs = (struct davinci_spi_regs *)SPI1_BASE; >> > >> >> - break; >> > >> >> -#endif >> > >> >> -#ifdef CONFIG_SYS_SPI2 >> > >> >> - case SPI2_BUS: >> > >> >> - ds->regs = (struct davinci_spi_regs *)SPI2_BASE; >> > >> >> - break; >> > >> >> -#endif >> > >> >> - default: /* Invalid bus number */ >> > >> >> - return NULL; >> > >> >> - } >> > >> >> - >> > >> >> - ds->freq = max_hz; >> > >> >> - ds->mode = mode; >> > >> >> - >> > >> >> - return &ds->slave; >> > >> >> -} >> > >> >> - >> > >> >> -void spi_free_slave(struct spi_slave *slave) >> > >> >> -{ >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> > >> >> - >> > >> >> - free(ds); >> > >> >> -} >> > >> >> - >> > >> >> -int spi_xfer(struct spi_slave *slave, unsigned int bitlen, >> > >> >> - const void *dout, void *din, unsigned long flags) >> > >> >> -{ >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> > >> >> - >> > >> >> - ds->cur_cs = slave->cs; >> > >> >> - >> > >> >> - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); >> > >> >> -} >> > >> >> - >> > >> >> -int spi_claim_bus(struct spi_slave *slave) >> > >> >> -{ >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> > >> >> - >> > >> >> -#ifdef CONFIG_SPI_HALF_DUPLEX >> > >> >> - ds->half_duplex = true; >> > >> >> -#else >> > >> >> - ds->half_duplex = false; >> > >> >> -#endif >> > >> >> - return __davinci_spi_claim_bus(ds, ds->slave.cs); >> > >> >> -} >> > >> >> - >> > >> >> -void spi_release_bus(struct spi_slave *slave) >> > >> >> -{ >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> > >> >> - >> > >> >> - __davinci_spi_release_bus(ds); >> > >> >> -} >> > >> >> - >> > >> >> -#else >> > >> >> -static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) >> > >> >> -{ >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> > >> >> - >> > >> >> - debug("%s speed %u\n", __func__, max_hz); >> > >> >> - if (max_hz > CONFIG_SYS_SPI_CLK / 2) >> > >> >> - return -EINVAL; >> > >> >> - >> > >> >> - ds->freq = max_hz; >> > >> >> >> > >> >> return 0; >> > >> >> } >> > >> >> >> > >> >> -static int davinci_spi_set_mode(struct udevice *bus, uint mode) >> > >> >> -{ >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> > >> >> - >> > >> >> - debug("%s mode %u\n", __func__, mode); >> > >> >> - ds->mode = mode; >> > >> >> - >> > >> >> - return 0; >> > >> >> -} >> > >> >> - >> > >> >> -static int davinci_spi_claim_bus(struct udevice *dev) >> > >> >> -{ >> > >> >> - struct dm_spi_slave_platdata *slave_plat = >> > >> >> - dev_get_parent_platdata(dev); >> > >> >> - struct udevice *bus = dev->parent; >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> > >> >> - >> > >> >> - if (slave_plat->cs >= ds->num_cs) { >> > >> >> - printf("Invalid SPI chipselect\n"); >> > >> >> - return -EINVAL; >> > >> >> - } >> > >> >> - ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; >> > >> >> - >> > >> >> - return __davinci_spi_claim_bus(ds, slave_plat->cs); >> > >> >> -} >> > >> >> - >> > >> >> -static int davinci_spi_release_bus(struct udevice *dev) >> > >> >> -{ >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(dev->parent); >> > >> >> - >> > >> >> - return __davinci_spi_release_bus(ds); >> > >> >> -} >> > >> >> +static const struct dm_spi_ops davinci_spi_ops = { >> > >> >> + .claim_bus = davinci_spi_claim_bus, >> > >> >> + .release_bus = davinci_spi_release_bus, >> > >> >> + .xfer = davinci_spi_xfer, >> > >> >> + .set_speed = davinci_spi_set_speed, >> > >> >> + .set_mode = davinci_spi_set_mode, >> > >> >> +}; >> > >> >> >> > >> >> -static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, >> > >> >> - const void *dout, void *din, >> > >> >> - unsigned long flags) >> > >> >> +static int davinci_spi_probe(struct udevice *bus) >> > >> >> { >> > >> >> - struct dm_spi_slave_platdata *slave = >> > >> >> - dev_get_parent_platdata(dev); >> > >> >> - struct udevice *bus = dev->parent; >> > >> >> struct davinci_spi_slave *ds = dev_get_priv(bus); >> > >> >> + struct davinci_spi_platdata *plat = bus->platdata; >> > >> >> + ds->regs = plat->regs; >> > >> >> + ds->num_cs = plat->num_cs; >> > >> >> >> > >> >> - if (slave->cs >= ds->num_cs) { >> > >> >> - printf("Invalid SPI chipselect\n"); >> > >> >> - return -EINVAL; >> > >> >> - } >> > >> >> - ds->cur_cs = slave->cs; >> > >> >> - >> > >> >> - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); >> > >> >> -} >> > >> >> - >> > >> >> -static int davinci_spi_probe(struct udevice *bus) >> > >> >> -{ >> > >> >> - /* Nothing to do */ >> > >> >> return 0; >> > >> >> } >> > >> >> >> > >> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> > >> > >> > >> > Looking at other drivers, I wonder if this should be >> > >> > +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) >> > >> > >> > >> > >> > >> >> static int davinci_ofdata_to_platadata(struct udevice *bus) >> > >> >> { >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> > >> >> - const void *blob = gd->fdt_blob; >> > >> >> - int node = dev_of_offset(bus); >> > >> >> + struct davinci_spi_platdata *plat = bus->platdata; >> > >> >> + fdt_addr_t addr; >> > >> >> >> > >> >> - ds->regs = devfdt_map_physmem(bus, sizeof(struct davinci_spi_regs)); >> > >> >> - if (!ds->regs) { >> > >> >> - printf("%s: could not map device address\n", __func__); >> > >> >> + addr = devfdt_get_addr(bus); >> > >> >> + if (addr == FDT_ADDR_T_NONE) >> > >> >> return -EINVAL; >> > >> >> - } >> > >> >> - ds->num_cs = fdtdec_get_int(blob, node, "num-cs", 4); >> > >> >> + >> > >> >> + plat->regs = (struct davinci_spi_regs *)addr; >> > >> >> + plat->num_cs = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus), "num-cs", 4); >> > >> >> >> > >> >> return 0; >> > >> >> } >> > >> >> >> > >> >> -static const struct dm_spi_ops davinci_spi_ops = { >> > >> >> - .claim_bus = davinci_spi_claim_bus, >> > >> >> - .release_bus = davinci_spi_release_bus, >> > >> >> - .xfer = davinci_spi_xfer, >> > >> >> - .set_speed = davinci_spi_set_speed, >> > >> >> - .set_mode = davinci_spi_set_mode, >> > >> >> -}; >> > >> >> - >> > >> >> static const struct udevice_id davinci_spi_ids[] = { >> > >> >> { .compatible = "ti,keystone-spi" }, >> > >> >> { .compatible = "ti,dm6441-spi" }, >> > >> >> { .compatible = "ti,da830-spi" }, >> > >> >> { } >> > >> >> }; >> > >> >> +#endif >> > >> >> >> > >> >> U_BOOT_DRIVER(davinci_spi) = { >> > >> >> .name = "davinci_spi", >> > >> >> .id = UCLASS_SPI, >> > >> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> > >> > >> > >> > Like above, should this be: >> > >> > +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) >> > >> > >> > >> > With limited SPL resources, I cannot build OF_CONTROL in SPL and >> > >> > disabling OF_CONTROL in SPL doesn't build either. >> > >> > With the modification, I can build with OF_PLATDATA enabled. >> > >> > Is it possible to enable DM_SPI in SPL? da850evm_direct_nor_defconfig >> > is able to build since it has it. >> >> Enabling DM_SPI in SPL starts to grow, hence my other comments about >> SPL_OF_PLATDATA in order to make it fit into SPL. >> da850evm_direct_nor_defconfig does not enable SPL, but it does enable >> DM_SPI. >> I had to get the NOR expansion board in order to try it. I'm trying >> to get it to work now, but for some reason, I'm having difficulty >> booting the stock da850evm_direct_nor_defconfig >> >> It would be easiest if we could have both a DM_SPI and non DM_SPI >> version of the driver so it can fit into SPL or the ability to disable >> SPI in SPL. I am experimenting with the latter. Several drivers >> offer the option to be disabled in SPL, so I'm experimenting with that >> to save space in SPL. >> > > I was able to verify your code works for the > da850evm_direct_nor_defconfig version which doesn't use SPL. > I spent a significant amount of time yesterday trying to get SPL to > work, but just enabling SPL and disabling > all drivers except serial, I was not able to boot, so I think > something is wrong with DM and SPL. I don't have > a debugger at home for this board, so I'll need to get one from work > to further troubleshoot.

I don't think it is much difficult to get serial up here. I made few changes for serial and spi platdata for SPL. If haven't try these please check the same and better change proper clock value for uart if added one is improper.

> > I don't think any DA850/L138/AM1808 board uses DM in SPL, so I think > it would be best to maintain both DM > and non-DM code bases for now until we're able to support DM in SPL.

But the whole idea is to drop nod-dm SPI as much as possible.

What your saying makes sense.

...

[1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/spi-dm-migrate

I am traveling today, but I will try to look at that tomorrow. I did something similar already without success, but I will try again with your patch to see if it's any better.

Is there a document somewhere that shows the order of operations during SPL? I'm wondering if some of the SPL Davinci code should be refactored to make sure the hardware is ready in the order that SPL is expecting it when used with DM.

Your patch adding the UART stuff and enabling DM still didn't work. As soon as I enable DM in SPL, I no longer get any of the SPL text stuff that normally gets displayed immediately on boot. I will try to take a look at the suggestion from Simon Goldschmidt to see if I can adapt any of his work to the Davinci platform.

In case it's of use as another line to explore, we had the same problem, which we eventually tracked down to some board detection code which doing GPIO bashing using gpio_get_value (and friends) which with SPL_DM ended up using a lot more RAM than we had with SPL_SYS_MALLOC_SIMPLE.

I have sent a patch to fix a data abort when calloc() calls memset on a NULL pointer returned by malloc_simple(). If you don't have enough memory in your "simple" heap, this might be why you don't see anything.

I tried your CALLOC fix, but that didn't seem make any difference, but I think it's good to keep it.

...

Have you tried enabling the debug uart (which doesn't need DM)?

I only have one UART, and I don't know what will happen if I try to route debug UART to the same UART, but I can try it.

I looped in since his name is on the da850 lowlevel init.

I tried to use the only uart as a debug UART passing the address for UART2 and the clocking info. I even enabled #define DEBUG hoping some debug info might show somewhere. I even tried printing 'c' after the UART was initialized in da850 lowlevel, but I still go nothing out of the uart with DM enabled during SPL.

I forgot my debugger at work, so I'm going to try and use it later this week. I'm going to be gone for 2 weeks, so if I cannot get it working by the end of the weekend, I won't get back to it until mid-september.

adam

adam

...

Simon

On Wed, Aug 15, 2018 at 6:50 PM, Adam Ford aford173@gmail.com wrote:

On Mon, Aug 13, 2018 at 6:50 PM Adam Ford aford173@gmail.com wrote:

...

On Mon, Aug 13, 2018 at 5:09 PM Adam Ford aford173@gmail.com wrote:

...

On Mon, Aug 13, 2018 at 1:29 PM Simon Goldschmidt simon.k.r.goldschmidt@gmail.com wrote:

...

On Mon, Aug 13, 2018 at 3:46 PM Alex Kiernan alex.kiernan@gmail.com wrote:

...

On Mon, Aug 13, 2018 at 1:40 PM Adam Ford aford173@gmail.com wrote:

...

On Sat, Aug 11, 2018 at 3:09 PM Adam Ford aford173@gmail.com wrote: > > > > On Sat, Aug 11, 2018, 1:24 PM Jagan Teki jagan@amarulasolutions.com wrote: >> >> On Sat, Aug 11, 2018 at 6:12 PM, Adam Ford aford173@gmail.com wrote: >> > On Fri, Aug 10, 2018 at 2:58 PM Adam Ford aford173@gmail.com wrote: >> >> >> >> On Fri, Aug 10, 2018 at 7:42 AM Jagan Teki jagan@amarulasolutions.com wrote: >> >> > >> >> > On Fri, Aug 10, 2018 at 3:50 PM, Adam Ford aford173@gmail.com wrote: >> >> > > On Fri, Aug 10, 2018 at 12:14 AM Jagan Teki jagan@amarulasolutions.com wrote: >> >> > >> >> >> > >> On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford aford173@gmail.com wrote: >> >> > >> > On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki jagan@amarulasolutions.com wrote: >> >> > >> >> >> >> > >> >> davinci_spi now support dt along with platform data, >> >> > >> >> respective boards need to switch into dm for the same. >> >> > >> >> >> >> > >> >> Cc: Adam Ford aford173@gmail.com >> >> > >> >> Cc: Vitaly Andrianov vitalya@ti.com >> >> > >> >> Cc: Stefano Babic sbabic@denx.de >> >> > >> >> Cc: Peter Howard phoward@gme.net.au >> >> > >> >> Cc: Tom Rini trini@konsulko.com >> >> > >> >> Signed-off-by: Jagan Teki jagan@amarulasolutions.com >> >> > >> >> --- >> >> > >> >> drivers/spi/Kconfig | 12 +- >> >> > >> >> drivers/spi/davinci_spi.c | 289 +++++++------------------ >> >> > >> >> include/dm/platform_data/spi_davinci.h | 15 ++ >> >> > >> >> 3 files changed, 97 insertions(+), 219 deletions(-) >> >> > >> >> create mode 100644 include/dm/platform_data/spi_davinci.h >> >> > >> >> >> >> > >> >> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig >> >> > >> >> index d046e919b4..18ebff0231 100644 >> >> > >> >> --- a/drivers/spi/Kconfig >> >> > >> >> +++ b/drivers/spi/Kconfig >> >> > >> >> @@ -80,6 +80,12 @@ config CADENCE_QSPI >> >> > >> >> used to access the SPI NOR flash on platforms embedding this >> >> > >> >> Cadence IP core. >> >> > >> >> >> >> > >> >> +config DAVINCI_SPI >> >> > >> >> + bool "Davinci & Keystone SPI driver" >> >> > >> >> + depends on ARCH_DAVINCI || ARCH_KEYSTONE >> >> > >> >> + help >> >> > >> >> + Enable the Davinci SPI driver >> >> > >> >> + >> >> > >> >> config DESIGNWARE_SPI >> >> > >> >> bool "Designware SPI driver" >> >> > >> >> help >> >> > >> >> @@ -281,12 +287,6 @@ config FSL_QSPI >> >> > >> >> used to access the SPI NOR flash on platforms embedding this >> >> > >> >> Freescale IP core. >> >> > >> >> >> >> > >> >> -config DAVINCI_SPI >> >> > >> >> - bool "Davinci & Keystone SPI driver" >> >> > >> >> - depends on ARCH_DAVINCI || ARCH_KEYSTONE >> >> > >> >> - help >> >> > >> >> - Enable the Davinci SPI driver >> >> > >> >> - >> >> > >> >> config SH_SPI >> >> > >> >> bool "SuperH SPI driver" >> >> > >> >> help >> >> > >> >> diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c >> >> > >> >> index a822858323..5007e6c618 100644 >> >> > >> >> --- a/drivers/spi/davinci_spi.c >> >> > >> >> +++ b/drivers/spi/davinci_spi.c >> >> > >> >> @@ -14,6 +14,7 @@ >> >> > >> >> #include <asm/io.h> >> >> > >> >> #include <asm/arch/hardware.h> >> >> > >> >> #include <dm.h> >> >> > >> >> +#include <dm/platform_data/spi_davinci.h> >> >> > >> >> >> >> > >> >> /* SPIGCR0 */ >> >> > >> >> #define SPIGCR0_SPIENA_MASK 0x1 >> >> > >> >> @@ -118,9 +119,6 @@ struct davinci_spi_regs { >> >> > >> >> >> >> > >> >> /* davinci spi slave */ >> >> > >> >> struct davinci_spi_slave { >> >> > >> >> -#ifndef CONFIG_DM_SPI >> >> > >> >> - struct spi_slave slave; >> >> > >> >> -#endif >> >> > >> >> struct davinci_spi_regs *regs; >> >> > >> >> unsigned int freq; /* current SPI bus frequency */ >> >> > >> >> unsigned int mode; /* current SPI mode used */ >> >> > >> >> @@ -240,11 +238,43 @@ static int davinci_spi_read_write(struct davinci_spi_slave *ds, unsigned >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> +static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) >> >> > >> >> +{ >> >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> >> >> > >> >> -static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) >> >> > >> >> + debug("%s speed %u\n", __func__, max_hz); >> >> > >> >> + if (max_hz > CONFIG_SYS_SPI_CLK / 2) >> >> > >> >> + return -EINVAL; >> >> > >> >> + >> >> > >> >> + ds->freq = max_hz; >> >> > >> >> + >> >> > >> >> + return 0; >> >> > >> >> +} >> >> > >> >> + >> >> > >> >> +static int davinci_spi_set_mode(struct udevice *bus, uint mode) >> >> > >> >> +{ >> >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> + >> >> > >> >> + debug("%s mode %u\n", __func__, mode); >> >> > >> >> + ds->mode = mode; >> >> > >> >> + >> >> > >> >> + return 0; >> >> > >> >> +} >> >> > >> >> + >> >> > >> >> +static int davinci_spi_claim_bus(struct udevice *dev) >> >> > >> >> { >> >> > >> >> + struct dm_spi_slave_platdata *slave_plat = >> >> > >> >> + dev_get_parent_platdata(dev); >> >> > >> >> + struct udevice *bus = dev->parent; >> >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> unsigned int mode = 0, scalar; >> >> > >> >> >> >> > >> >> + if (slave_plat->cs >= ds->num_cs) { >> >> > >> >> + printf("Invalid SPI chipselect\n"); >> >> > >> >> + return -EINVAL; >> >> > >> >> + } >> >> > >> >> + ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; >> >> > >> >> + >> >> > >> >> /* Enable the SPI hardware */ >> >> > >> >> writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); >> >> > >> >> udelay(1000); >> >> > >> >> @@ -254,7 +284,7 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) >> >> > >> >> writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1); >> >> > >> >> >> >> > >> >> /* CS, CLK, SIMO and SOMI are functional pins */ >> >> > >> >> - writel(((1 << cs) | SPIPC0_CLKFUN_MASK | >> >> > >> >> + writel(((1 << slave_plat->cs) | SPIPC0_CLKFUN_MASK | >> >> > >> >> SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0); >> >> > >> >> >> >> > >> >> /* setup format */ >> >> > >> >> @@ -292,20 +322,32 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> -static int __davinci_spi_release_bus(struct davinci_spi_slave *ds) >> >> > >> >> +static int davinci_spi_release_bus(struct udevice *dev) >> >> > >> >> { >> >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(dev->parent); >> >> > >> >> + >> >> > >> >> /* Disable the SPI hardware */ >> >> > >> >> writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); >> >> > >> >> >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> -static int __davinci_spi_xfer(struct davinci_spi_slave *ds, >> >> > >> >> - unsigned int bitlen, const void *dout, void *din, >> >> > >> >> - unsigned long flags) >> >> > >> >> +static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, >> >> > >> >> + const void *dout, void *din, >> >> > >> >> + unsigned long flags) >> >> > >> >> { >> >> > >> >> + struct dm_spi_slave_platdata *slave = >> >> > >> >> + dev_get_parent_platdata(dev); >> >> > >> >> + struct udevice *bus = dev->parent; >> >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> unsigned int len; >> >> > >> >> >> >> > >> >> + if (slave->cs >= ds->num_cs) { >> >> > >> >> + printf("Invalid SPI chipselect\n"); >> >> > >> >> + return -EINVAL; >> >> > >> >> + } >> >> > >> >> + ds->cur_cs = slave->cs; >> >> > >> >> + >> >> > >> >> if (bitlen == 0) >> >> > >> >> /* Finish any previously submitted transfers */ >> >> > >> >> goto out; >> >> > >> >> @@ -339,240 +381,61 @@ out: >> >> > >> >> u8 dummy = 0; >> >> > >> >> davinci_spi_write(ds, 1, &dummy, flags); >> >> > >> >> } >> >> > >> >> - return 0; >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -#ifndef CONFIG_DM_SPI >> >> > >> >> - >> >> > >> >> -static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - return container_of(slave, struct davinci_spi_slave, slave); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -int spi_cs_is_valid(unsigned int bus, unsigned int cs) >> >> > >> >> -{ >> >> > >> >> - int ret = 0; >> >> > >> >> - >> >> > >> >> - switch (bus) { >> >> > >> >> - case SPI0_BUS: >> >> > >> >> - if (cs < SPI0_NUM_CS) >> >> > >> >> - ret = 1; >> >> > >> >> - break; >> >> > >> >> -#ifdef CONFIG_SYS_SPI1 >> >> > >> >> - case SPI1_BUS: >> >> > >> >> - if (cs < SPI1_NUM_CS) >> >> > >> >> - ret = 1; >> >> > >> >> - break; >> >> > >> >> -#endif >> >> > >> >> -#ifdef CONFIG_SYS_SPI2 >> >> > >> >> - case SPI2_BUS: >> >> > >> >> - if (cs < SPI2_NUM_CS) >> >> > >> >> - ret = 1; >> >> > >> >> - break; >> >> > >> >> -#endif >> >> > >> >> - default: >> >> > >> >> - /* Invalid bus number. Do nothing */ >> >> > >> >> - break; >> >> > >> >> - } >> >> > >> >> - return ret; >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -void spi_cs_activate(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - /* do nothing */ >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -void spi_cs_deactivate(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - /* do nothing */ >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -void spi_init(void) >> >> > >> >> -{ >> >> > >> >> - /* do nothing */ >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, >> >> > >> >> - unsigned int max_hz, unsigned int mode) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds; >> >> > >> >> - >> >> > >> >> - if (!spi_cs_is_valid(bus, cs)) >> >> > >> >> - return NULL; >> >> > >> >> - >> >> > >> >> - ds = spi_alloc_slave(struct davinci_spi_slave, bus, cs); >> >> > >> >> - if (!ds) >> >> > >> >> - return NULL; >> >> > >> >> - >> >> > >> >> - switch (bus) { >> >> > >> >> - case SPI0_BUS: >> >> > >> >> - ds->regs = (struct davinci_spi_regs *)SPI0_BASE; >> >> > >> >> - break; >> >> > >> >> -#ifdef CONFIG_SYS_SPI1 >> >> > >> >> - case SPI1_BUS: >> >> > >> >> - ds->regs = (struct davinci_spi_regs *)SPI1_BASE; >> >> > >> >> - break; >> >> > >> >> -#endif >> >> > >> >> -#ifdef CONFIG_SYS_SPI2 >> >> > >> >> - case SPI2_BUS: >> >> > >> >> - ds->regs = (struct davinci_spi_regs *)SPI2_BASE; >> >> > >> >> - break; >> >> > >> >> -#endif >> >> > >> >> - default: /* Invalid bus number */ >> >> > >> >> - return NULL; >> >> > >> >> - } >> >> > >> >> - >> >> > >> >> - ds->freq = max_hz; >> >> > >> >> - ds->mode = mode; >> >> > >> >> - >> >> > >> >> - return &ds->slave; >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -void spi_free_slave(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> >> > >> >> - >> >> > >> >> - free(ds); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -int spi_xfer(struct spi_slave *slave, unsigned int bitlen, >> >> > >> >> - const void *dout, void *din, unsigned long flags) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> >> > >> >> - >> >> > >> >> - ds->cur_cs = slave->cs; >> >> > >> >> - >> >> > >> >> - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -int spi_claim_bus(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> >> > >> >> - >> >> > >> >> -#ifdef CONFIG_SPI_HALF_DUPLEX >> >> > >> >> - ds->half_duplex = true; >> >> > >> >> -#else >> >> > >> >> - ds->half_duplex = false; >> >> > >> >> -#endif >> >> > >> >> - return __davinci_spi_claim_bus(ds, ds->slave.cs); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -void spi_release_bus(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> >> > >> >> - >> >> > >> >> - __davinci_spi_release_bus(ds); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -#else >> >> > >> >> -static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> - >> >> > >> >> - debug("%s speed %u\n", __func__, max_hz); >> >> > >> >> - if (max_hz > CONFIG_SYS_SPI_CLK / 2) >> >> > >> >> - return -EINVAL; >> >> > >> >> - >> >> > >> >> - ds->freq = max_hz; >> >> > >> >> >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> -static int davinci_spi_set_mode(struct udevice *bus, uint mode) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> - >> >> > >> >> - debug("%s mode %u\n", __func__, mode); >> >> > >> >> - ds->mode = mode; >> >> > >> >> - >> >> > >> >> - return 0; >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -static int davinci_spi_claim_bus(struct udevice *dev) >> >> > >> >> -{ >> >> > >> >> - struct dm_spi_slave_platdata *slave_plat = >> >> > >> >> - dev_get_parent_platdata(dev); >> >> > >> >> - struct udevice *bus = dev->parent; >> >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> - >> >> > >> >> - if (slave_plat->cs >= ds->num_cs) { >> >> > >> >> - printf("Invalid SPI chipselect\n"); >> >> > >> >> - return -EINVAL; >> >> > >> >> - } >> >> > >> >> - ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; >> >> > >> >> - >> >> > >> >> - return __davinci_spi_claim_bus(ds, slave_plat->cs); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -static int davinci_spi_release_bus(struct udevice *dev) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(dev->parent); >> >> > >> >> - >> >> > >> >> - return __davinci_spi_release_bus(ds); >> >> > >> >> -} >> >> > >> >> +static const struct dm_spi_ops davinci_spi_ops = { >> >> > >> >> + .claim_bus = davinci_spi_claim_bus, >> >> > >> >> + .release_bus = davinci_spi_release_bus, >> >> > >> >> + .xfer = davinci_spi_xfer, >> >> > >> >> + .set_speed = davinci_spi_set_speed, >> >> > >> >> + .set_mode = davinci_spi_set_mode, >> >> > >> >> +}; >> >> > >> >> >> >> > >> >> -static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, >> >> > >> >> - const void *dout, void *din, >> >> > >> >> - unsigned long flags) >> >> > >> >> +static int davinci_spi_probe(struct udevice *bus) >> >> > >> >> { >> >> > >> >> - struct dm_spi_slave_platdata *slave = >> >> > >> >> - dev_get_parent_platdata(dev); >> >> > >> >> - struct udevice *bus = dev->parent; >> >> > >> >> struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> + struct davinci_spi_platdata *plat = bus->platdata; >> >> > >> >> + ds->regs = plat->regs; >> >> > >> >> + ds->num_cs = plat->num_cs; >> >> > >> >> >> >> > >> >> - if (slave->cs >= ds->num_cs) { >> >> > >> >> - printf("Invalid SPI chipselect\n"); >> >> > >> >> - return -EINVAL; >> >> > >> >> - } >> >> > >> >> - ds->cur_cs = slave->cs; >> >> > >> >> - >> >> > >> >> - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -static int davinci_spi_probe(struct udevice *bus) >> >> > >> >> -{ >> >> > >> >> - /* Nothing to do */ >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> >> > >> > >> >> > >> > Looking at other drivers, I wonder if this should be >> >> > >> > +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) >> >> > >> > >> >> > >> > >> >> > >> >> static int davinci_ofdata_to_platadata(struct udevice *bus) >> >> > >> >> { >> >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> - const void *blob = gd->fdt_blob; >> >> > >> >> - int node = dev_of_offset(bus); >> >> > >> >> + struct davinci_spi_platdata *plat = bus->platdata; >> >> > >> >> + fdt_addr_t addr; >> >> > >> >> >> >> > >> >> - ds->regs = devfdt_map_physmem(bus, sizeof(struct davinci_spi_regs)); >> >> > >> >> - if (!ds->regs) { >> >> > >> >> - printf("%s: could not map device address\n", __func__); >> >> > >> >> + addr = devfdt_get_addr(bus); >> >> > >> >> + if (addr == FDT_ADDR_T_NONE) >> >> > >> >> return -EINVAL; >> >> > >> >> - } >> >> > >> >> - ds->num_cs = fdtdec_get_int(blob, node, "num-cs", 4); >> >> > >> >> + >> >> > >> >> + plat->regs = (struct davinci_spi_regs *)addr; >> >> > >> >> + plat->num_cs = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus), "num-cs", 4); >> >> > >> >> >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> -static const struct dm_spi_ops davinci_spi_ops = { >> >> > >> >> - .claim_bus = davinci_spi_claim_bus, >> >> > >> >> - .release_bus = davinci_spi_release_bus, >> >> > >> >> - .xfer = davinci_spi_xfer, >> >> > >> >> - .set_speed = davinci_spi_set_speed, >> >> > >> >> - .set_mode = davinci_spi_set_mode, >> >> > >> >> -}; >> >> > >> >> - >> >> > >> >> static const struct udevice_id davinci_spi_ids[] = { >> >> > >> >> { .compatible = "ti,keystone-spi" }, >> >> > >> >> { .compatible = "ti,dm6441-spi" }, >> >> > >> >> { .compatible = "ti,da830-spi" }, >> >> > >> >> { } >> >> > >> >> }; >> >> > >> >> +#endif >> >> > >> >> >> >> > >> >> U_BOOT_DRIVER(davinci_spi) = { >> >> > >> >> .name = "davinci_spi", >> >> > >> >> .id = UCLASS_SPI, >> >> > >> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> >> > >> > >> >> > >> > Like above, should this be: >> >> > >> > +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) >> >> > >> > >> >> > >> > With limited SPL resources, I cannot build OF_CONTROL in SPL and >> >> > >> > disabling OF_CONTROL in SPL doesn't build either. >> >> > >> > With the modification, I can build with OF_PLATDATA enabled. >> >> > >> >> > Is it possible to enable DM_SPI in SPL? da850evm_direct_nor_defconfig >> >> > is able to build since it has it. >> >> >> >> Enabling DM_SPI in SPL starts to grow, hence my other comments about >> >> SPL_OF_PLATDATA in order to make it fit into SPL. >> >> da850evm_direct_nor_defconfig does not enable SPL, but it does enable >> >> DM_SPI. >> >> I had to get the NOR expansion board in order to try it. I'm trying >> >> to get it to work now, but for some reason, I'm having difficulty >> >> booting the stock da850evm_direct_nor_defconfig >> >> >> >> It would be easiest if we could have both a DM_SPI and non DM_SPI >> >> version of the driver so it can fit into SPL or the ability to disable >> >> SPI in SPL. I am experimenting with the latter. Several drivers >> >> offer the option to be disabled in SPL, so I'm experimenting with that >> >> to save space in SPL. >> >> >> > >> > I was able to verify your code works for the >> > da850evm_direct_nor_defconfig version which doesn't use SPL. >> > I spent a significant amount of time yesterday trying to get SPL to >> > work, but just enabling SPL and disabling >> > all drivers except serial, I was not able to boot, so I think >> > something is wrong with DM and SPL. I don't have >> > a debugger at home for this board, so I'll need to get one from work >> > to further troubleshoot. >> >> I don't think it is much difficult to get serial up here. I made few >> changes for serial and spi platdata for SPL. If haven't try these >> please check the same and better change proper clock value for uart if >> added one is improper. >> >> > >> > I don't think any DA850/L138/AM1808 board uses DM in SPL, so I think >> > it would be best to maintain both DM >> > and non-DM code bases for now until we're able to support DM in SPL. >> >> But the whole idea is to drop nod-dm SPI as much as possible. > > > What your saying makes sense. >> >> >> [1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/spi-dm-migrate > > > I am traveling today, but I will try to look at that tomorrow. I did something similar already without success, but I will try again with your patch to see if it's any better. > > Is there a document somewhere that shows the order of operations during SPL? I'm wondering if some of the SPL Davinci code should be refactored to make sure the hardware is ready in the order that SPL is expecting it when used with DM. >

Your patch adding the UART stuff and enabling DM still didn't work. As soon as I enable DM in SPL, I no longer get any of the SPL text stuff that normally gets displayed immediately on boot. I will try to take a look at the suggestion from Simon Goldschmidt to see if I can adapt any of his work to the Davinci platform.

In case it's of use as another line to explore, we had the same problem, which we eventually tracked down to some board detection code which doing GPIO bashing using gpio_get_value (and friends) which with SPL_DM ended up using a lot more RAM than we had with SPL_SYS_MALLOC_SIMPLE.

I have sent a patch to fix a data abort when calloc() calls memset on a NULL pointer returned by malloc_simple(). If you don't have enough memory in your "simple" heap, this might be why you don't see anything.

I tried your CALLOC fix, but that didn't seem make any difference, but I think it's good to keep it.

...

Have you tried enabling the debug uart (which doesn't need DM)?

I only have one UART, and I don't know what will happen if I try to route debug UART to the same UART, but I can try it.

I looped in since his name is on the da850 lowlevel init.

I tried to use the only uart as a debug UART passing the address for UART2 and the clocking info. I even enabled #define DEBUG hoping some debug info might show somewhere. I even tried printing 'c' after the UART was initialized in da850 lowlevel, but I still go nothing out of the uart with DM enabled during SPL.

I forgot my debugger at work, so I'm going to try and use it later this week. I'm going to be gone for 2 weeks, so if I cannot get it working by the end of the weekend, I won't get back to it until mid-september.

Some good news. I had to do a few things to Jagan's patch, but I'm getting closer and the serial port is now working. To the config file: +CONFIG_SPL_SYS_MALLOC_SIMPLE=y +CONFIG_USE_TINY_PRINTF=y

I had to change the clock of the serial port to 150MHz

@@ -57,7 +57,7 @@ static const struct ns16550_platdata da850evm_serial = { .base = DAVINCI_UART2_BASE, .reg_shift = 2, //.clock = clk_get(DAVINCI_UART2_CLKID),

•   .clock = 48000000,
  

•   .clock = 150000000,
    .fcr = UART_FCR_DEFVAL,
  

};

I also disabled enabling the serial port in da850 lowlevel since I would expect the driver to do this now. (See https://patchwork.ozlabs.org/patch/957336/ which makes DM_SERIAL work when booting from NOR)

Now, I get the following on startup:

U-Boot SPL 2018.09-rc1-00237-g9f20d7f105-dirty (Aug 15 2018 - 08:07:16 -0500) Trying to boot from SPI SF: unrecognized JEDEC id bytes: 00, 00, 00 SPI probe failed. SPL: failed to boot from all boot devices ### ERROR ### Please RESET the board ###

With serial working, I am hoping to determine what's going on with the SF/SPI Probe.

Jagan - if you have any suggestions, please let me know. I'm going to look into the SPI clocking and startup sequence.

Have you add spi-flash compatible in arch/arm/dts/da850-evm.dts ?

compatible = "m25p64", "spi-flash";

On Wed, Aug 15, 2018 at 8:20 AM Adam Ford aford173@gmail.com wrote:

On Mon, Aug 13, 2018 at 6:50 PM Adam Ford aford173@gmail.com wrote:

...

On Mon, Aug 13, 2018 at 5:09 PM Adam Ford aford173@gmail.com wrote:

...

On Mon, Aug 13, 2018 at 1:29 PM Simon Goldschmidt simon.k.r.goldschmidt@gmail.com wrote:

...

On Mon, Aug 13, 2018 at 3:46 PM Alex Kiernan alex.kiernan@gmail.com wrote:

...

On Mon, Aug 13, 2018 at 1:40 PM Adam Ford aford173@gmail.com wrote:

...

On Sat, Aug 11, 2018 at 3:09 PM Adam Ford aford173@gmail.com wrote: > > > > On Sat, Aug 11, 2018, 1:24 PM Jagan Teki jagan@amarulasolutions.com wrote: >> >> On Sat, Aug 11, 2018 at 6:12 PM, Adam Ford aford173@gmail.com wrote: >> > On Fri, Aug 10, 2018 at 2:58 PM Adam Ford aford173@gmail.com wrote: >> >> >> >> On Fri, Aug 10, 2018 at 7:42 AM Jagan Teki jagan@amarulasolutions.com wrote: >> >> > >> >> > On Fri, Aug 10, 2018 at 3:50 PM, Adam Ford aford173@gmail.com wrote: >> >> > > On Fri, Aug 10, 2018 at 12:14 AM Jagan Teki jagan@amarulasolutions.com wrote: >> >> > >> >> >> > >> On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford aford173@gmail.com wrote: >> >> > >> > On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki jagan@amarulasolutions.com wrote: >> >> > >> >> >> >> > >> >> davinci_spi now support dt along with platform data, >> >> > >> >> respective boards need to switch into dm for the same. >> >> > >> >> >> >> > >> >> Cc: Adam Ford aford173@gmail.com >> >> > >> >> Cc: Vitaly Andrianov vitalya@ti.com >> >> > >> >> Cc: Stefano Babic sbabic@denx.de >> >> > >> >> Cc: Peter Howard phoward@gme.net.au >> >> > >> >> Cc: Tom Rini trini@konsulko.com >> >> > >> >> Signed-off-by: Jagan Teki jagan@amarulasolutions.com >> >> > >> >> --- >> >> > >> >> drivers/spi/Kconfig | 12 +- >> >> > >> >> drivers/spi/davinci_spi.c | 289 +++++++------------------ >> >> > >> >> include/dm/platform_data/spi_davinci.h | 15 ++ >> >> > >> >> 3 files changed, 97 insertions(+), 219 deletions(-) >> >> > >> >> create mode 100644 include/dm/platform_data/spi_davinci.h >> >> > >> >> >> >> > >> >> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig >> >> > >> >> index d046e919b4..18ebff0231 100644 >> >> > >> >> --- a/drivers/spi/Kconfig >> >> > >> >> +++ b/drivers/spi/Kconfig >> >> > >> >> @@ -80,6 +80,12 @@ config CADENCE_QSPI >> >> > >> >> used to access the SPI NOR flash on platforms embedding this >> >> > >> >> Cadence IP core. >> >> > >> >> >> >> > >> >> +config DAVINCI_SPI >> >> > >> >> + bool "Davinci & Keystone SPI driver" >> >> > >> >> + depends on ARCH_DAVINCI || ARCH_KEYSTONE >> >> > >> >> + help >> >> > >> >> + Enable the Davinci SPI driver >> >> > >> >> + >> >> > >> >> config DESIGNWARE_SPI >> >> > >> >> bool "Designware SPI driver" >> >> > >> >> help >> >> > >> >> @@ -281,12 +287,6 @@ config FSL_QSPI >> >> > >> >> used to access the SPI NOR flash on platforms embedding this >> >> > >> >> Freescale IP core. >> >> > >> >> >> >> > >> >> -config DAVINCI_SPI >> >> > >> >> - bool "Davinci & Keystone SPI driver" >> >> > >> >> - depends on ARCH_DAVINCI || ARCH_KEYSTONE >> >> > >> >> - help >> >> > >> >> - Enable the Davinci SPI driver >> >> > >> >> - >> >> > >> >> config SH_SPI >> >> > >> >> bool "SuperH SPI driver" >> >> > >> >> help >> >> > >> >> diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c >> >> > >> >> index a822858323..5007e6c618 100644 >> >> > >> >> --- a/drivers/spi/davinci_spi.c >> >> > >> >> +++ b/drivers/spi/davinci_spi.c >> >> > >> >> @@ -14,6 +14,7 @@ >> >> > >> >> #include <asm/io.h> >> >> > >> >> #include <asm/arch/hardware.h> >> >> > >> >> #include <dm.h> >> >> > >> >> +#include <dm/platform_data/spi_davinci.h> >> >> > >> >> >> >> > >> >> /* SPIGCR0 */ >> >> > >> >> #define SPIGCR0_SPIENA_MASK 0x1 >> >> > >> >> @@ -118,9 +119,6 @@ struct davinci_spi_regs { >> >> > >> >> >> >> > >> >> /* davinci spi slave */ >> >> > >> >> struct davinci_spi_slave { >> >> > >> >> -#ifndef CONFIG_DM_SPI >> >> > >> >> - struct spi_slave slave; >> >> > >> >> -#endif >> >> > >> >> struct davinci_spi_regs *regs; >> >> > >> >> unsigned int freq; /* current SPI bus frequency */ >> >> > >> >> unsigned int mode; /* current SPI mode used */ >> >> > >> >> @@ -240,11 +238,43 @@ static int davinci_spi_read_write(struct davinci_spi_slave *ds, unsigned >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> +static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) >> >> > >> >> +{ >> >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> >> >> > >> >> -static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) >> >> > >> >> + debug("%s speed %u\n", __func__, max_hz); >> >> > >> >> + if (max_hz > CONFIG_SYS_SPI_CLK / 2) >> >> > >> >> + return -EINVAL; >> >> > >> >> + >> >> > >> >> + ds->freq = max_hz; >> >> > >> >> + >> >> > >> >> + return 0; >> >> > >> >> +} >> >> > >> >> + >> >> > >> >> +static int davinci_spi_set_mode(struct udevice *bus, uint mode) >> >> > >> >> +{ >> >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> + >> >> > >> >> + debug("%s mode %u\n", __func__, mode); >> >> > >> >> + ds->mode = mode; >> >> > >> >> + >> >> > >> >> + return 0; >> >> > >> >> +} >> >> > >> >> + >> >> > >> >> +static int davinci_spi_claim_bus(struct udevice *dev) >> >> > >> >> { >> >> > >> >> + struct dm_spi_slave_platdata *slave_plat = >> >> > >> >> + dev_get_parent_platdata(dev); >> >> > >> >> + struct udevice *bus = dev->parent; >> >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> unsigned int mode = 0, scalar; >> >> > >> >> >> >> > >> >> + if (slave_plat->cs >= ds->num_cs) { >> >> > >> >> + printf("Invalid SPI chipselect\n"); >> >> > >> >> + return -EINVAL; >> >> > >> >> + } >> >> > >> >> + ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; >> >> > >> >> + >> >> > >> >> /* Enable the SPI hardware */ >> >> > >> >> writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); >> >> > >> >> udelay(1000); >> >> > >> >> @@ -254,7 +284,7 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) >> >> > >> >> writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1); >> >> > >> >> >> >> > >> >> /* CS, CLK, SIMO and SOMI are functional pins */ >> >> > >> >> - writel(((1 << cs) | SPIPC0_CLKFUN_MASK | >> >> > >> >> + writel(((1 << slave_plat->cs) | SPIPC0_CLKFUN_MASK | >> >> > >> >> SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0); >> >> > >> >> >> >> > >> >> /* setup format */ >> >> > >> >> @@ -292,20 +322,32 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> -static int __davinci_spi_release_bus(struct davinci_spi_slave *ds) >> >> > >> >> +static int davinci_spi_release_bus(struct udevice *dev) >> >> > >> >> { >> >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(dev->parent); >> >> > >> >> + >> >> > >> >> /* Disable the SPI hardware */ >> >> > >> >> writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); >> >> > >> >> >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> -static int __davinci_spi_xfer(struct davinci_spi_slave *ds, >> >> > >> >> - unsigned int bitlen, const void *dout, void *din, >> >> > >> >> - unsigned long flags) >> >> > >> >> +static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, >> >> > >> >> + const void *dout, void *din, >> >> > >> >> + unsigned long flags) >> >> > >> >> { >> >> > >> >> + struct dm_spi_slave_platdata *slave = >> >> > >> >> + dev_get_parent_platdata(dev); >> >> > >> >> + struct udevice *bus = dev->parent; >> >> > >> >> + struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> unsigned int len; >> >> > >> >> >> >> > >> >> + if (slave->cs >= ds->num_cs) { >> >> > >> >> + printf("Invalid SPI chipselect\n"); >> >> > >> >> + return -EINVAL; >> >> > >> >> + } >> >> > >> >> + ds->cur_cs = slave->cs; >> >> > >> >> + >> >> > >> >> if (bitlen == 0) >> >> > >> >> /* Finish any previously submitted transfers */ >> >> > >> >> goto out; >> >> > >> >> @@ -339,240 +381,61 @@ out: >> >> > >> >> u8 dummy = 0; >> >> > >> >> davinci_spi_write(ds, 1, &dummy, flags); >> >> > >> >> } >> >> > >> >> - return 0; >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -#ifndef CONFIG_DM_SPI >> >> > >> >> - >> >> > >> >> -static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - return container_of(slave, struct davinci_spi_slave, slave); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -int spi_cs_is_valid(unsigned int bus, unsigned int cs) >> >> > >> >> -{ >> >> > >> >> - int ret = 0; >> >> > >> >> - >> >> > >> >> - switch (bus) { >> >> > >> >> - case SPI0_BUS: >> >> > >> >> - if (cs < SPI0_NUM_CS) >> >> > >> >> - ret = 1; >> >> > >> >> - break; >> >> > >> >> -#ifdef CONFIG_SYS_SPI1 >> >> > >> >> - case SPI1_BUS: >> >> > >> >> - if (cs < SPI1_NUM_CS) >> >> > >> >> - ret = 1; >> >> > >> >> - break; >> >> > >> >> -#endif >> >> > >> >> -#ifdef CONFIG_SYS_SPI2 >> >> > >> >> - case SPI2_BUS: >> >> > >> >> - if (cs < SPI2_NUM_CS) >> >> > >> >> - ret = 1; >> >> > >> >> - break; >> >> > >> >> -#endif >> >> > >> >> - default: >> >> > >> >> - /* Invalid bus number. Do nothing */ >> >> > >> >> - break; >> >> > >> >> - } >> >> > >> >> - return ret; >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -void spi_cs_activate(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - /* do nothing */ >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -void spi_cs_deactivate(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - /* do nothing */ >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -void spi_init(void) >> >> > >> >> -{ >> >> > >> >> - /* do nothing */ >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, >> >> > >> >> - unsigned int max_hz, unsigned int mode) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds; >> >> > >> >> - >> >> > >> >> - if (!spi_cs_is_valid(bus, cs)) >> >> > >> >> - return NULL; >> >> > >> >> - >> >> > >> >> - ds = spi_alloc_slave(struct davinci_spi_slave, bus, cs); >> >> > >> >> - if (!ds) >> >> > >> >> - return NULL; >> >> > >> >> - >> >> > >> >> - switch (bus) { >> >> > >> >> - case SPI0_BUS: >> >> > >> >> - ds->regs = (struct davinci_spi_regs *)SPI0_BASE; >> >> > >> >> - break; >> >> > >> >> -#ifdef CONFIG_SYS_SPI1 >> >> > >> >> - case SPI1_BUS: >> >> > >> >> - ds->regs = (struct davinci_spi_regs *)SPI1_BASE; >> >> > >> >> - break; >> >> > >> >> -#endif >> >> > >> >> -#ifdef CONFIG_SYS_SPI2 >> >> > >> >> - case SPI2_BUS: >> >> > >> >> - ds->regs = (struct davinci_spi_regs *)SPI2_BASE; >> >> > >> >> - break; >> >> > >> >> -#endif >> >> > >> >> - default: /* Invalid bus number */ >> >> > >> >> - return NULL; >> >> > >> >> - } >> >> > >> >> - >> >> > >> >> - ds->freq = max_hz; >> >> > >> >> - ds->mode = mode; >> >> > >> >> - >> >> > >> >> - return &ds->slave; >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -void spi_free_slave(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> >> > >> >> - >> >> > >> >> - free(ds); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -int spi_xfer(struct spi_slave *slave, unsigned int bitlen, >> >> > >> >> - const void *dout, void *din, unsigned long flags) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> >> > >> >> - >> >> > >> >> - ds->cur_cs = slave->cs; >> >> > >> >> - >> >> > >> >> - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -int spi_claim_bus(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> >> > >> >> - >> >> > >> >> -#ifdef CONFIG_SPI_HALF_DUPLEX >> >> > >> >> - ds->half_duplex = true; >> >> > >> >> -#else >> >> > >> >> - ds->half_duplex = false; >> >> > >> >> -#endif >> >> > >> >> - return __davinci_spi_claim_bus(ds, ds->slave.cs); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -void spi_release_bus(struct spi_slave *slave) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = to_davinci_spi(slave); >> >> > >> >> - >> >> > >> >> - __davinci_spi_release_bus(ds); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -#else >> >> > >> >> -static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> - >> >> > >> >> - debug("%s speed %u\n", __func__, max_hz); >> >> > >> >> - if (max_hz > CONFIG_SYS_SPI_CLK / 2) >> >> > >> >> - return -EINVAL; >> >> > >> >> - >> >> > >> >> - ds->freq = max_hz; >> >> > >> >> >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> -static int davinci_spi_set_mode(struct udevice *bus, uint mode) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> - >> >> > >> >> - debug("%s mode %u\n", __func__, mode); >> >> > >> >> - ds->mode = mode; >> >> > >> >> - >> >> > >> >> - return 0; >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -static int davinci_spi_claim_bus(struct udevice *dev) >> >> > >> >> -{ >> >> > >> >> - struct dm_spi_slave_platdata *slave_plat = >> >> > >> >> - dev_get_parent_platdata(dev); >> >> > >> >> - struct udevice *bus = dev->parent; >> >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> - >> >> > >> >> - if (slave_plat->cs >= ds->num_cs) { >> >> > >> >> - printf("Invalid SPI chipselect\n"); >> >> > >> >> - return -EINVAL; >> >> > >> >> - } >> >> > >> >> - ds->half_duplex = slave_plat->mode & SPI_PREAMBLE; >> >> > >> >> - >> >> > >> >> - return __davinci_spi_claim_bus(ds, slave_plat->cs); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -static int davinci_spi_release_bus(struct udevice *dev) >> >> > >> >> -{ >> >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(dev->parent); >> >> > >> >> - >> >> > >> >> - return __davinci_spi_release_bus(ds); >> >> > >> >> -} >> >> > >> >> +static const struct dm_spi_ops davinci_spi_ops = { >> >> > >> >> + .claim_bus = davinci_spi_claim_bus, >> >> > >> >> + .release_bus = davinci_spi_release_bus, >> >> > >> >> + .xfer = davinci_spi_xfer, >> >> > >> >> + .set_speed = davinci_spi_set_speed, >> >> > >> >> + .set_mode = davinci_spi_set_mode, >> >> > >> >> +}; >> >> > >> >> >> >> > >> >> -static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen, >> >> > >> >> - const void *dout, void *din, >> >> > >> >> - unsigned long flags) >> >> > >> >> +static int davinci_spi_probe(struct udevice *bus) >> >> > >> >> { >> >> > >> >> - struct dm_spi_slave_platdata *slave = >> >> > >> >> - dev_get_parent_platdata(dev); >> >> > >> >> - struct udevice *bus = dev->parent; >> >> > >> >> struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> + struct davinci_spi_platdata *plat = bus->platdata; >> >> > >> >> + ds->regs = plat->regs; >> >> > >> >> + ds->num_cs = plat->num_cs; >> >> > >> >> >> >> > >> >> - if (slave->cs >= ds->num_cs) { >> >> > >> >> - printf("Invalid SPI chipselect\n"); >> >> > >> >> - return -EINVAL; >> >> > >> >> - } >> >> > >> >> - ds->cur_cs = slave->cs; >> >> > >> >> - >> >> > >> >> - return __davinci_spi_xfer(ds, bitlen, dout, din, flags); >> >> > >> >> -} >> >> > >> >> - >> >> > >> >> -static int davinci_spi_probe(struct udevice *bus) >> >> > >> >> -{ >> >> > >> >> - /* Nothing to do */ >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> >> > >> > >> >> > >> > Looking at other drivers, I wonder if this should be >> >> > >> > +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) >> >> > >> > >> >> > >> > >> >> > >> >> static int davinci_ofdata_to_platadata(struct udevice *bus) >> >> > >> >> { >> >> > >> >> - struct davinci_spi_slave *ds = dev_get_priv(bus); >> >> > >> >> - const void *blob = gd->fdt_blob; >> >> > >> >> - int node = dev_of_offset(bus); >> >> > >> >> + struct davinci_spi_platdata *plat = bus->platdata; >> >> > >> >> + fdt_addr_t addr; >> >> > >> >> >> >> > >> >> - ds->regs = devfdt_map_physmem(bus, sizeof(struct davinci_spi_regs)); >> >> > >> >> - if (!ds->regs) { >> >> > >> >> - printf("%s: could not map device address\n", __func__); >> >> > >> >> + addr = devfdt_get_addr(bus); >> >> > >> >> + if (addr == FDT_ADDR_T_NONE) >> >> > >> >> return -EINVAL; >> >> > >> >> - } >> >> > >> >> - ds->num_cs = fdtdec_get_int(blob, node, "num-cs", 4); >> >> > >> >> + >> >> > >> >> + plat->regs = (struct davinci_spi_regs *)addr; >> >> > >> >> + plat->num_cs = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus), "num-cs", 4); >> >> > >> >> >> >> > >> >> return 0; >> >> > >> >> } >> >> > >> >> >> >> > >> >> -static const struct dm_spi_ops davinci_spi_ops = { >> >> > >> >> - .claim_bus = davinci_spi_claim_bus, >> >> > >> >> - .release_bus = davinci_spi_release_bus, >> >> > >> >> - .xfer = davinci_spi_xfer, >> >> > >> >> - .set_speed = davinci_spi_set_speed, >> >> > >> >> - .set_mode = davinci_spi_set_mode, >> >> > >> >> -}; >> >> > >> >> - >> >> > >> >> static const struct udevice_id davinci_spi_ids[] = { >> >> > >> >> { .compatible = "ti,keystone-spi" }, >> >> > >> >> { .compatible = "ti,dm6441-spi" }, >> >> > >> >> { .compatible = "ti,da830-spi" }, >> >> > >> >> { } >> >> > >> >> }; >> >> > >> >> +#endif >> >> > >> >> >> >> > >> >> U_BOOT_DRIVER(davinci_spi) = { >> >> > >> >> .name = "davinci_spi", >> >> > >> >> .id = UCLASS_SPI, >> >> > >> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> >> > >> > >> >> > >> > Like above, should this be: >> >> > >> > +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) >> >> > >> > >> >> > >> > With limited SPL resources, I cannot build OF_CONTROL in SPL and >> >> > >> > disabling OF_CONTROL in SPL doesn't build either. >> >> > >> > With the modification, I can build with OF_PLATDATA enabled. >> >> > >> >> > Is it possible to enable DM_SPI in SPL? da850evm_direct_nor_defconfig >> >> > is able to build since it has it. >> >> >> >> Enabling DM_SPI in SPL starts to grow, hence my other comments about >> >> SPL_OF_PLATDATA in order to make it fit into SPL. >> >> da850evm_direct_nor_defconfig does not enable SPL, but it does enable >> >> DM_SPI. >> >> I had to get the NOR expansion board in order to try it. I'm trying >> >> to get it to work now, but for some reason, I'm having difficulty >> >> booting the stock da850evm_direct_nor_defconfig >> >> >> >> It would be easiest if we could have both a DM_SPI and non DM_SPI >> >> version of the driver so it can fit into SPL or the ability to disable >> >> SPI in SPL. I am experimenting with the latter. Several drivers >> >> offer the option to be disabled in SPL, so I'm experimenting with that >> >> to save space in SPL. >> >> >> > >> > I was able to verify your code works for the >> > da850evm_direct_nor_defconfig version which doesn't use SPL. >> > I spent a significant amount of time yesterday trying to get SPL to >> > work, but just enabling SPL and disabling >> > all drivers except serial, I was not able to boot, so I think >> > something is wrong with DM and SPL. I don't have >> > a debugger at home for this board, so I'll need to get one from work >> > to further troubleshoot. >> >> I don't think it is much difficult to get serial up here. I made few >> changes for serial and spi platdata for SPL. If haven't try these >> please check the same and better change proper clock value for uart if >> added one is improper. >> >> > >> > I don't think any DA850/L138/AM1808 board uses DM in SPL, so I think >> > it would be best to maintain both DM >> > and non-DM code bases for now until we're able to support DM in SPL. >> >> But the whole idea is to drop nod-dm SPI as much as possible. > > > What your saying makes sense. >> >> >> [1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/spi-dm-migrate > > > I am traveling today, but I will try to look at that tomorrow. I did something similar already without success, but I will try again with your patch to see if it's any better. > > Is there a document somewhere that shows the order of operations during SPL? I'm wondering if some of the SPL Davinci code should be refactored to make sure the hardware is ready in the order that SPL is expecting it when used with DM. >

Your patch adding the UART stuff and enabling DM still didn't work. As soon as I enable DM in SPL, I no longer get any of the SPL text stuff that normally gets displayed immediately on boot. I will try to take a look at the suggestion from Simon Goldschmidt to see if I can adapt any of his work to the Davinci platform.

In case it's of use as another line to explore, we had the same problem, which we eventually tracked down to some board detection code which doing GPIO bashing using gpio_get_value (and friends) which with SPL_DM ended up using a lot more RAM than we had with SPL_SYS_MALLOC_SIMPLE.

I have sent a patch to fix a data abort when calloc() calls memset on a NULL pointer returned by malloc_simple(). If you don't have enough memory in your "simple" heap, this might be why you don't see anything.

I tried your CALLOC fix, but that didn't seem make any difference, but I think it's good to keep it.

...

Have you tried enabling the debug uart (which doesn't need DM)?

I only have one UART, and I don't know what will happen if I try to route debug UART to the same UART, but I can try it.

I looped in since his name is on the da850 lowlevel init.

I tried to use the only uart as a debug UART passing the address for UART2 and the clocking info. I even enabled #define DEBUG hoping some debug info might show somewhere. I even tried printing 'c' after the UART was initialized in da850 lowlevel, but I still go nothing out of the uart with DM enabled during SPL.

I forgot my debugger at work, so I'm going to try and use it later this week. I'm going to be gone for 2 weeks, so if I cannot get it working by the end of the weekend, I won't get back to it until mid-september.

Some good news. I had to do a few things to Jagan's patch, but I'm getting closer and the serial port is now working. To the config file: +CONFIG_SPL_SYS_MALLOC_SIMPLE=y +CONFIG_USE_TINY_PRINTF=y

I had to change the clock of the serial port to 150MHz

@@ -57,7 +57,7 @@ static const struct ns16550_platdata da850evm_serial = { .base = DAVINCI_UART2_BASE, .reg_shift = 2, //.clock = clk_get(DAVINCI_UART2_CLKID),

•   .clock = 48000000,
  

•   .clock = 150000000,
    .fcr = UART_FCR_DEFVAL,
  

};

I also disabled enabling the serial port in da850 lowlevel since I would expect the driver to do this now. (See https://patchwork.ozlabs.org/patch/957336/ which makes DM_SERIAL work when booting from NOR)

Now, I get the following on startup:

U-Boot SPL 2018.09-rc1-00237-g9f20d7f105-dirty (Aug 15 2018 - 08:07:16 -0500) Trying to boot from SPI SF: unrecognized JEDEC id bytes: 00, 00, 00 SPI probe failed. SPL: failed to boot from all boot devices ### ERROR ### Please RESET the board ###

With serial working, I am hoping to determine what's going on with the SF/SPI Probe.

Jagan - if you have any suggestions, please let me know. I'm going to look into the SPI clocking and startup sequence.

Apparently, the base address used in your patch is wrong, but when switched to +static const struct davinci_spi_platdata davinci_spi_data = { + .regs = (struct davinci_spi_regs *)0x01f0e000, + .num_cs = 4, +};

It works now!

Thanks for your help.

adam

...

adam

...

adam

...

Simon

On Fri, Aug 10, 2018 at 8:38 AM Jagan Teki jagan@amarulasolutions.com wrote:

On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford aford173@gmail.com wrote:

...

On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki jagan@amarulasolutions.com wrote:

...

davinci_spi now support dt along with platform data, respective boards need to switch into dm for the same.

Cc: Adam Ford aford173@gmail.com Cc: Vitaly Andrianov vitalya@ti.com Cc: Stefano Babic sbabic@denx.de Cc: Peter Howard phoward@gme.net.au Cc: Tom Rini trini@konsulko.com Signed-off-by: Jagan Teki jagan@amarulasolutions.com

---

drivers/spi/Kconfig | 12 +- drivers/spi/davinci_spi.c | 289 +++++++------------------ include/dm/platform_data/spi_davinci.h | 15 ++ 3 files changed, 97 insertions(+), 219 deletions(-) create mode 100644 include/dm/platform_data/spi_davinci.h

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index d046e919b4..18ebff0231 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -80,6 +80,12 @@ config CADENCE_QSPI used to access the SPI NOR flash on platforms embedding this Cadence IP core.

+config DAVINCI_SPI

•   bool "Davinci & Keystone SPI driver"
  

•   depends on ARCH_DAVINCI || ARCH_KEYSTONE
  

•   help
  

•     Enable the Davinci SPI driver
  

config DESIGNWARE_SPI bool "Designware SPI driver" help @@ -281,12 +287,6 @@ config FSL_QSPI used to access the SPI NOR flash on platforms embedding this Freescale IP core.

-config DAVINCI_SPI

•   bool "Davinci & Keystone SPI driver"
  

•   depends on ARCH_DAVINCI || ARCH_KEYSTONE
  

•   help
  

•     Enable the Davinci SPI driver
  

config SH_SPI bool "SuperH SPI driver" help diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c index a822858323..5007e6c618 100644 --- a/drivers/spi/davinci_spi.c +++ b/drivers/spi/davinci_spi.c @@ -14,6 +14,7 @@ #include <asm/io.h> #include <asm/arch/hardware.h> #include <dm.h> +#include <dm/platform_data/spi_davinci.h>

/* SPIGCR0 */ #define SPIGCR0_SPIENA_MASK 0x1 @@ -118,9 +119,6 @@ struct davinci_spi_regs {

/* davinci spi slave */ struct davinci_spi_slave { -#ifndef CONFIG_DM_SPI

•   struct spi_slave slave;
  

-#endif struct davinci_spi_regs *regs; unsigned int freq; /* current SPI bus frequency */ unsigned int mode; /* current SPI mode used */ @@ -240,11 +238,43 @@ static int davinci_spi_read_write(struct davinci_spi_slave *ds, unsigned return 0; }

+static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) +{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  
  

-static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs)

•   debug("%s speed %u\n", __func__, max_hz);
  

•   if (max_hz > CONFIG_SYS_SPI_CLK / 2)
  

•           return -EINVAL;
  

•   ds->freq = max_hz;
  

•   return 0;
  

+}

+static int davinci_spi_set_mode(struct udevice *bus, uint mode) +{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s mode %u\n", __func__, mode);
  

•   ds->mode = mode;
  

•   return 0;
  

+}

+static int davinci_spi_claim_bus(struct udevice *dev) {

•   struct dm_spi_slave_platdata *slave_plat =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
    unsigned int mode = 0, scalar;
  
  

•   if (slave_plat->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->half_duplex = slave_plat->mode & SPI_PREAMBLE;
  

•   /* Enable the SPI hardware */
    writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
    udelay(1000);
  

@@ -254,7 +284,7 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1);

    /* CS, CLK, SIMO and SOMI are functional pins */

•   writel(((1 << cs) | SPIPC0_CLKFUN_MASK |
  

•   writel(((1 << slave_plat->cs) | SPIPC0_CLKFUN_MASK |
            SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0);
  
    /* setup format */
  

@@ -292,20 +322,32 @@ static int __davinci_spi_claim_bus(struct davinci_spi_slave *ds, int cs) return 0; }

-static int __davinci_spi_release_bus(struct davinci_spi_slave *ds) +static int davinci_spi_release_bus(struct udevice *dev) {

•   struct davinci_spi_slave *ds = dev_get_priv(dev->parent);
  

•   /* Disable the SPI hardware */
    writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
  
    return 0;
  

}

-static int __davinci_spi_xfer(struct davinci_spi_slave *ds,

•           unsigned int bitlen,  const void *dout, void *din,
  

•           unsigned long flags)
  

+static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen,

•                       const void *dout, void *din,
  

•                       unsigned long flags)
  

{

•   struct dm_spi_slave_platdata *slave =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
    unsigned int len;
  
  

•   if (slave->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->cur_cs = slave->cs;
  

•   if (bitlen == 0)
            /* Finish any previously submitted transfers */
            goto out;
  

@@ -339,240 +381,61 @@ out: u8 dummy = 0; davinci_spi_write(ds, 1, &dummy, flags); }

•   return 0;
  

-}

-#ifndef CONFIG_DM_SPI

-static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) -{

•   return container_of(slave, struct davinci_spi_slave, slave);
  

-}

-int spi_cs_is_valid(unsigned int bus, unsigned int cs) -{

•   int ret = 0;
  

•   switch (bus) {
  

•   case SPI0_BUS:
  

•           if (cs < SPI0_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#ifdef CONFIG_SYS_SPI1

•   case SPI1_BUS:
  

•           if (cs < SPI1_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#endif -#ifdef CONFIG_SYS_SPI2

•   case SPI2_BUS:
  

•           if (cs < SPI2_NUM_CS)
  

•                   ret = 1;
  

•           break;
  

-#endif

•   default:
  

•           /* Invalid bus number. Do nothing */
  

•           break;
  

•   }
  

•   return ret;
  

-}

-void spi_cs_activate(struct spi_slave *slave) -{

•   /* do nothing */
  

-}

-void spi_cs_deactivate(struct spi_slave *slave) -{

•   /* do nothing */
  

-}

-void spi_init(void) -{

•   /* do nothing */
  

-}

-struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,

•                   unsigned int max_hz, unsigned int mode)
  

-{

•   struct davinci_spi_slave        *ds;
  

•   if (!spi_cs_is_valid(bus, cs))
  

•           return NULL;
  

•   ds = spi_alloc_slave(struct davinci_spi_slave, bus, cs);
  

•   if (!ds)
  

•           return NULL;
  

•   switch (bus) {
  

•   case SPI0_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI0_BASE;
  

•           break;
  

-#ifdef CONFIG_SYS_SPI1

•   case SPI1_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI1_BASE;
  

•           break;
  

-#endif -#ifdef CONFIG_SYS_SPI2

•   case SPI2_BUS:
  

•           ds->regs = (struct davinci_spi_regs *)SPI2_BASE;
  

•           break;
  

-#endif

•   default: /* Invalid bus number */
  

•           return NULL;
  

•   }
  

•   ds->freq = max_hz;
  

•   ds->mode = mode;
  

•   return &ds->slave;
  

-}

-void spi_free_slave(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   free(ds);
  

-}

-int spi_xfer(struct spi_slave *slave, unsigned int bitlen,

•        const void *dout, void *din, unsigned long flags)
  

-{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   ds->cur_cs = slave->cs;
  

•   return __davinci_spi_xfer(ds, bitlen, dout, din, flags);
  

-}

-int spi_claim_bus(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

-#ifdef CONFIG_SPI_HALF_DUPLEX

•   ds->half_duplex = true;
  

-#else

•   ds->half_duplex = false;
  

-#endif

•   return __davinci_spi_claim_bus(ds, ds->slave.cs);
  

-}

-void spi_release_bus(struct spi_slave *slave) -{

•   struct davinci_spi_slave *ds = to_davinci_spi(slave);
  

•   __davinci_spi_release_bus(ds);
  

-}

-#else -static int davinci_spi_set_speed(struct udevice *bus, uint max_hz) -{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s speed %u\n", __func__, max_hz);
  

•   if (max_hz > CONFIG_SYS_SPI_CLK / 2)
  

•           return -EINVAL;
  

•   ds->freq = max_hz;
  
    return 0;
  

}

-static int davinci_spi_set_mode(struct udevice *bus, uint mode) -{

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   debug("%s mode %u\n", __func__, mode);
  

•   ds->mode = mode;
  

•   return 0;
  

-}

-static int davinci_spi_claim_bus(struct udevice *dev) -{

•   struct dm_spi_slave_platdata *slave_plat =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
  

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   if (slave_plat->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->half_duplex = slave_plat->mode & SPI_PREAMBLE;
  

•   return __davinci_spi_claim_bus(ds, slave_plat->cs);
  

-}

-static int davinci_spi_release_bus(struct udevice *dev) -{

•   struct davinci_spi_slave *ds = dev_get_priv(dev->parent);
  

•   return __davinci_spi_release_bus(ds);
  

-} +static const struct dm_spi_ops davinci_spi_ops = {

•   .claim_bus      = davinci_spi_claim_bus,
  

•   .release_bus    = davinci_spi_release_bus,
  

•   .xfer           = davinci_spi_xfer,
  

•   .set_speed      = davinci_spi_set_speed,
  

•   .set_mode       = davinci_spi_set_mode,
  

+};

-static int davinci_spi_xfer(struct udevice *dev, unsigned int bitlen,

•                       const void *dout, void *din,
  

•                       unsigned long flags)
  

+static int davinci_spi_probe(struct udevice *bus) {

•   struct dm_spi_slave_platdata *slave =
  

•           dev_get_parent_platdata(dev);
  

•   struct udevice *bus = dev->parent;
    struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   struct davinci_spi_platdata *plat = bus->platdata;
  

•   ds->regs = plat->regs;
  

•   ds->num_cs = plat->num_cs;
  
  

•   if (slave->cs >= ds->num_cs) {
  

•           printf("Invalid SPI chipselect\n");
  

•           return -EINVAL;
  

•   }
  

•   ds->cur_cs = slave->cs;
  

•   return __davinci_spi_xfer(ds, bitlen, dout, din, flags);
  

-}

-static int davinci_spi_probe(struct udevice *bus) -{

•   /* Nothing to do */
    return 0;
  

}

+#if CONFIG_IS_ENABLED(OF_CONTROL)

Looking at other drivers, I wonder if this should be +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

...

static int davinci_ofdata_to_platadata(struct udevice *bus) {

•   struct davinci_spi_slave *ds = dev_get_priv(bus);
  

•   const void *blob = gd->fdt_blob;
  

•   int node = dev_of_offset(bus);
  

•   struct davinci_spi_platdata *plat = bus->platdata;
  

•   fdt_addr_t addr;
  
  

•   ds->regs = devfdt_map_physmem(bus, sizeof(struct davinci_spi_regs));
  

•   if (!ds->regs) {
  

•           printf("%s: could not map device address\n", __func__);
  

•   addr = devfdt_get_addr(bus);
  

•   if (addr == FDT_ADDR_T_NONE)
            return -EINVAL;
  

•   }
  

•   ds->num_cs = fdtdec_get_int(blob, node, "num-cs", 4);
  

•   plat->regs = (struct davinci_spi_regs *)addr;
  

•   plat->num_cs = fdtdec_get_int(gd->fdt_blob, dev_of_offset(bus), "num-cs", 4);
  
    return 0;
  

}

-static const struct dm_spi_ops davinci_spi_ops = {

•   .claim_bus      = davinci_spi_claim_bus,
  

•   .release_bus    = davinci_spi_release_bus,
  

•   .xfer           = davinci_spi_xfer,
  

•   .set_speed      = davinci_spi_set_speed,
  

•   .set_mode       = davinci_spi_set_mode,
  

-};

static const struct udevice_id davinci_spi_ids[] = { { .compatible = "ti,keystone-spi" }, { .compatible = "ti,dm6441-spi" }, { .compatible = "ti,da830-spi" }, { } }; +#endif

U_BOOT_DRIVER(davinci_spi) = { .name = "davinci_spi", .id = UCLASS_SPI, +#if CONFIG_IS_ENABLED(OF_CONTROL)

Like above, should this be: +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)

With limited SPL resources, I cannot build OF_CONTROL in SPL and disabling OF_CONTROL in SPL doesn't build either. With the modification, I can build with OF_PLATDATA enabled.

...

    .of_match = davinci_spi_ids,

•   .ops = &davinci_spi_ops,
    .ofdata_to_platdata = davinci_ofdata_to_platadata,
  

•   .priv_auto_alloc_size = sizeof(struct davinci_spi_slave),
  

•    .platdata_auto_alloc_size = sizeof(struct davinci_spi_platdata),
  

+#endif .probe = davinci_spi_probe,

•   .ops = &davinci_spi_ops,
  

•   .priv_auto_alloc_size = sizeof(struct davinci_spi_slave),
  

}; -#endif

With the above changes, I can build U-Boot, but I cannot boot with DM_SPL enabled.

For SPL have you initialize pladata some where in board code?