[U-Boot] [PATCH] spi: davinci: Full dm conversion
Jagan Teki
jagan at amarulasolutions.com
Sat Aug 11 18:24:35 UTC 2018
On Sat, Aug 11, 2018 at 6:12 PM, Adam Ford <aford173 at gmail.com> wrote:
> On Fri, Aug 10, 2018 at 2:58 PM Adam Ford <aford173 at gmail.com> wrote:
>>
>> On Fri, Aug 10, 2018 at 7:42 AM Jagan Teki <jagan at amarulasolutions.com> wrote:
>> >
>> > On Fri, Aug 10, 2018 at 3:50 PM, Adam Ford <aford173 at gmail.com> wrote:
>> > > On Fri, Aug 10, 2018 at 12:14 AM Jagan Teki <jagan at amarulasolutions.com> wrote:
>> > >>
>> > >> On Wed, Aug 8, 2018 at 6:47 PM, Adam Ford <aford173 at gmail.com> wrote:
>> > >> > On Tue, Aug 7, 2018 at 1:29 AM Jagan Teki <jagan at 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 at gmail.com>
>> > >> >> Cc: Vitaly Andrianov <vitalya at ti.com>
>> > >> >> Cc: Stefano Babic <sbabic at denx.de>
>> > >> >> Cc: Peter Howard <phoward at gme.net.au>
>> > >> >> Cc: Tom Rini <trini at konsulko.com>
>> > >> >> Signed-off-by: Jagan Teki <jagan at 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
More information about the U-Boot
mailing list