Re: [U-Boot] [PATCH v3 8/9] spi: davinci: add support for multiple bus and chip select

On Thu, Mar 27, 2014 at 1:19 AM, Karicheri, Muralidharan m-karicheri2@ti.com wrote:

...

-----Original Message----- From: Jagan Teki [mailto:jagannadh.teki@gmail.com] Sent: Sunday, March 23, 2014 7:07 AM To: Karicheri, Muralidharan Cc: Rini, Tom; u-boot@lists.denx.de; Chang, Rex Subject: Re: [U-Boot] [PATCH v3 8/9] spi: davinci: add support for multiple bus and chip select

Hi,

On Sat, Mar 22, 2014 at 2:21 AM, Murali Karicheri m-karicheri2@ti.com wrote:

...

Currently davinci spi driver supports only bus 0 cs 0. This patch allows driver to support bus 1 and bus 2 with configurable number of chip selects. Also defaults are selected in a way to avoid regression on other platforms that uses davinci spi driver and has only one spi bus.

Signed-off-by: Rex Chang rchang@ti.com Signed-off-by: Murali Karicheri m-karicheri2@ti.com Acked-by: Tom Rini trini@ti.com

---

drivers/spi/davinci_spi.c | 60

++++++++++++++++++++++++++++++++++++++++++---

...

drivers/spi/davinci_spi.h | 33 +++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c index e3fb321..b682bc4 100644 --- a/drivers/spi/davinci_spi.c +++ b/drivers/spi/davinci_spi.c @@ -32,7 +32,27 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int

cs,

...

    if (!ds)
            return NULL;

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

•   ds->slave.bus = bus;
  

•   ds->slave.cs = cs;
  

•   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 *)SPI0_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;
  
    return &ds->slave;
  

@@ -59,7 +79,7 @@ int spi_claim_bus(struct spi_slave *slave) writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1);

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

•   writel((SPIPC0_EN0FUN_MASK | SPIPC0_CLKFUN_MASK |
  

•   writel(((1 << slave->cs) | SPIPC0_CLKFUN_MASK |
            SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK),
  

&ds->regs->pc0);

    /* setup format */

@@ -262,9 +282,43 @@ out: return 0; }

+#ifdef CONFIG_SYS_SPI1 +static int bus1_cs_valid(unsigned int bus, unsigned int cs) {

•   if ((bus == SPI1_BUS) && (cs < SPI1_NUM_CS))
  

•           return 1;
  

•   return 0;
  

+} +#else +static int bus1_cs_valid(unsigned int bus, unsigned int cs) {

•   return 0;
  

+} +#endif

+#ifdef CONFIG_SYS_SPI2 +static int bus2_cs_valid(unsigned int bus, unsigned int cs) {

•   if ((bus == SPI2_BUS) && (cs < SPI2_NUM_CS))
  

•           return 1;
  

•   return 0;
  

+} +#else +static int bus2_cs_valid(unsigned int bus, unsigned int cs) {

•   return 0;
  

+} +#endif

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

•   return bus == 0 && cs == 0;
  

•   if ((bus == SPI0_BUS) && (cs < SPI0_NUM_CS))
  

•           return 1;
  

•   else if (bus1_cs_valid(bus, cs))
  

•           return 1;
  

•   else if (bus2_cs_valid(bus, cs))
  

•           return 1;
  

•   return 0;
  

}

void spi_cs_activate(struct spi_slave *slave) diff --git a/drivers/spi/davinci_spi.h b/drivers/spi/davinci_spi.h index 33f69b5..d4612d3 100644 --- a/drivers/spi/davinci_spi.h +++ b/drivers/spi/davinci_spi.h @@ -74,6 +74,39 @@ struct davinci_spi_regs { /* SPIDEF */ #define SPIDEF_CSDEF0_MASK BIT(0)

+#define SPI0_BUS 0 +#define SPI0_BASE CONFIG_SYS_SPI_BASE +/*

• • Define default SPI0_NUM_CS as 1 for existing platforms that uses

+this

• • driver. Platform can configure number of CS using

+CONFIG_SYS_SPI0_NUM_CS

• • if more than one CS is supported and by defining CONFIG_SYS_SPI0.
• */

+#ifndef CONFIG_SYS_SPI0 +#define SPI0_NUM_CS 1 +#else +#define SPI0_NUM_CS CONFIG_SYS_SPI0_NUM_CS +#endif

+/*

• • define CONFIG_SYS_SPI1 when platform has spi-1 device (bus #1) and
• • CONFIG_SYS_SPI1_NUM_CS defines number of CS on this bus */ #ifdef

+CONFIG_SYS_SPI1 +#define SPI1_BUS 1 +#define SPI1_NUM_CS CONFIG_SYS_SPI1_NUM_CS +#define SPI1_BASE CONFIG_SYS_SPI1_BASE +#endif

+/*

• • define CONFIG_SYS_SPI2 when platform has spi-2 device (bus #2) and
• • CONFIG_SYS_SPI2_NUM_CS defines number of CS on this bus */ #ifdef

+CONFIG_SYS_SPI2 +#define SPI2_BUS 2 +#define SPI2_NUM_CS CONFIG_SYS_SPI2_NUM_CS +#define SPI2_BASE CONFIG_SYS_SPI2_BASE +#endif

struct davinci_spi_slave { struct spi_slave slave; struct davinci_spi_regs *regs; -- 1.7.9.5

This code looks more static, can you try get the bus and cs from sf probe and according assign the reg_base.

fyi: look at drivers/spi/zynq_spi.c where based on the bus number we assign the reg_base and prior to that we can do the same for bus and cs.

Jagan,

Thanks for your comments.

I looked at the driver code. In the example driver you provided the reg_base is chosen based on device number and the hardware.h define the base0 and base1 address and assign it based on device number. davinci_spi is re-used on many devices. All of the DaVinci devices such as dm644x, dm355, dm365 etc that mostly has one SPI device vs keystone devices such k2h/k that has 3 devices. If I follow this approach, I need to define BASE1 and BASE2 in the hardware.h of all devices that this software must run. This is not right since DaVinci devices has only one SPI device

I think this is what best we can do given that the driver needs to be re-used across multiple devices and static is the way to go unless you have better suggestion to handle this scenario.

Agree with your comments as reg_base is not possible to define at one place due to this you may not try as I suggested.

OK, atleast can you aviod using busx_cs_valid() calls, try to manage spi_cs_is_valid() itself.

thanks!