On Thu, Mar 27, 2014 at 1:19 AM, Karicheri, Muralidharan <m-kariche...@ti.com> wrote: >>-----Original Message----- >>From: Jagan Teki [mailto:jagannadh.t...@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-kariche...@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 <rch...@ti.com> >>> Signed-off-by: Murali Karicheri <m-kariche...@ti.com> >>> Acked-by: Tom Rini <tr...@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! -- Jagan. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
