[U-Boot] [PATCH v3 8/9] spi: davinci: add support for multiple bus and chip select
Jagan Teki
jagannadh.teki at gmail.com
Thu Mar 27 18:47:03 CET 2014
On Thu, Mar 27, 2014 at 1:19 AM, Karicheri, Muralidharan
<m-karicheri2 at ti.com> wrote:
>>-----Original Message-----
>>From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>>Sent: Sunday, March 23, 2014 7:07 AM
>>To: Karicheri, Muralidharan
>>Cc: Rini, Tom; u-boot at 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 at 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 at ti.com>
>>> Signed-off-by: Murali Karicheri <m-karicheri2 at ti.com>
>>> Acked-by: Tom Rini <trini at 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.
More information about the U-Boot
mailing list