McSPI questions pertaining to GPIO chip select support

GoogleGroups
1

Hey all!

Recently I've been thinking about adding support to the McSPI driver for
using GPIO pins as chip selects. As a starting point, I've browsed the
driver source trying to identify what changes would be necessary to add
this support. It seems like the rough idea is,

  a) add a cs_gpio field to the device struct (omap2_mcspi)
  b) modify omap2_mcspi_force_cs() to use cs_gpio instead of the
     CHxCONF[FORCE] bit if cs_gpio is valid

Unfortunately, I'm having a bit of difficulty seeing how the existing
configuration works. In master mode, it seems that the controller is
configured in single forced channel mode (MCSPI_MODULECTRL_SINGLE). As
far as I can tell, this means that the driver is responsible for
managing the chip selects through the MCSPI_CHxCONF[FORCE] bits. It
seems that this is ideal as little will need to be changed to
incorporate GPIO CS support.

Nevertheless, the timing of the various operations isn't making sense to
me. As far as I understand it, the callgraph is as follows,

omap2_mcspi_work():
  loop over messages
    omap2_mcspi_set_enable(1)
    loop over transfers:
      configure bus parameters
      if (!cs_active) cs = 1
      if (dma):
        omap2_mcspi_txrx_dma()
      else:
        omap2_mcspi_txrx_pio()
      if (cs_change) cs = 0
    end loop
  
    if (cs_active) cs = 0
    omap2_mcspi_set_enable(0)
  end loop

It seems that omap2_mcspi_set_enable() is called to enable the channel
(MCSPI_CHCTRLx[EN] is set) before force_cs() is called. If I'm
interpreting the hardware documentation correctly, the enable bit (among
other things) starts the SPI clock, meaning that the chip will begin
clocking out zeros to the device before CS is brought high.
Furthermore, even after CS is brought high, it seems there could be a
fair amount of time before the chip begins actually clocking out valid
data.

As I am writing this, it now occurs to me that perhaps the chip doesn't
start SPI_CLK until it has data to send. It seems that if this were the
case my concerns would be resolved. However, it is not at all clear that
this is the case from the documentation. When exactly does the
controller start the bus clock? Is this stated explicitly anywhere in
the documentation?

Any input you could offer would be greatly appreciated.
Thanks for your time.

- Ben

P.S. The more and more I think about it, it seems like my hypothesis is
the only logical behavior. It would be great if someone could confirm my
suspicions, however. It really seems like this should be in the
documentation either way.

P.P.S. I apologize if this is something someone familiar with SPI should
just know. My knowledge is a little spotty on this sort of thing.

2

Ben Gamari wrote:

As I am writing this, it now occurs to me that perhaps the chip doesn't
start SPI_CLK until it has data to send.

... or receive. This is indeed the case.

b.g.

3

Excerpts from Bill Gatliff's message of Wed Jan 27 23:15:05 -0500 2010:

Ben Gamari wrote:
> As I am writing this, it now occurs to me that perhaps the chip doesn't
> start SPI_CLK until it has data to send.

... or receive. This is indeed the case.

Thanks for your prompt reply. Hopefully you'll see a patch from me about
this in the next few days. I'm assuming this functionality would be
welcome in mainline?

- Ben

4

Ben Gamari wrote:

Thanks for your prompt reply. Hopefully you'll see a patch from me about
this in the next few days. I'm assuming this functionality would be
welcome in mainline?
  
Heck if I know. :slight_smile:

But seriously, seems like SPI slave-selects are always in short supply.
So yeah, people will find your patches very useful.

b.g.

5

You may loot at drivers/spi/spi_s3c64xx.c in Grant Likely's tree.

It doesn't put limit on the number of CS and not even on whether
the CS mechanism is simple GPIO toggling(though the callback
type is defined to match gpio_set_value).
For platform side of it, you need to look in to Ben Dooks' tree.

hth

6

As you may recall, about a year ago I wrote wondering whether it was possible
to use GPIO lines as chip selects in the McSPI driver. A few of you pointed out
that this functionality had been added to other drivers and it should be quite
simple to do the same for McSPI. Two semesters then passed without a followup
from me.

Thankfully, classes are over and I've had some downtime due to sickness,
finally freeing up some time to look at this (just in time as we'll soon need
this functionality). This morning I looked through the relevant code and put
together this patch to add support for GPIO CSs. As was suggested, I modelled
this after the approach taken in the s3c24xx-spi driver. Let me know whether
this looks sane. It has yet to see hardware, but in principle things look
alright (famous last words).

Cheers,

- Ben

P.S. In case people are curious to see an example of usage I have a patch
adding support for multiplexed chip selects to support a data acquisition board
for the BeagleBoard that I've been working on[1]. I can send these out if
people are interested.

[1] http://goldnerlab.physics.umass.edu/wiki/BeagleBoardDaq

7

This mechanism is in large part stolen from the s3c64xx-spi module. To
use this functionality, one simply must define a set_level function to
set the CS state and a omap2_mcspi_csinfo struct for each chip select in
the board file.

Each spi_board_info.controller_data should then be set
to point to the appropriate csinfo struct. This will cause the driver to
call the csinfo->set_level function instead of toggling the McSPI chip
select lines.

Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>

8

* Ben Gamari <bgamari.foss@gmail.com> [101221 09:56]:

This mechanism is in large part stolen from the s3c64xx-spi module. To
use this functionality, one simply must define a set_level function to
set the CS state and a omap2_mcspi_csinfo struct for each chip select in
the board file.

Each spi_board_info.controller_data should then be set
to point to the appropriate csinfo struct. This will cause the driver to
call the csinfo->set_level function instead of toggling the McSPI chip
select lines.

Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>

This one should go via the SPI list:

Acked-by: Tony Lindgren <tony@atomide.com>

9

This mechanism is in large part stolen from the s3c64xx-spi module. To
use this functionality, one simply must define a set_level function to
set the CS state and a omap2_mcspi_csinfo struct for each chip select in
the board file.

Each spi_board_info.controller_data should then be set
to point to the appropriate csinfo struct. This will cause the driver to
call the csinfo->set_level function instead of toggling the McSPI chip
select lines.

Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>

I'd rather see the spi driver modified to use the gpio api directly.
The drivers are already tending in that direction and it doesn't
require machine specific set_level functions to be defined.

g.

10

The reason I left this up to the board is it's easy to foresee cases
where you want a non-trivial mapping between logical CS numbers and CS
pin states. In my case, I using a 2-to-4 multiplexer as the source of
chip select.

- Ben

11

Hi Ben,

I understand and appreciate the motivation. However in practice, the
gpio api is sufficient for pretty much any use case, even when the
backing gpio controller driver ends up driving some oddball device
with different constraints. The big downside to using a callback is
that it forces all users to do the extra work of implementing the
callback. With the gpio api, only the oddball cases have to do extra
code (to adapt the custom device to the gpio api).

g.

12

I understand your concerns, but I'm not sure how to satisfy them without
crippling the design's ability to accomodate my use-case. I can't pass a
GPIO line per spi_board_info since in my case of a multiplexed CS
configuration a single pin's state does not uniquely determine the
desired CS. The only other option I can think of is that we somehow
provide a list of GPIOs for each bus and map the CS numbers to
permutations of GPIO states. Unfortunately, I don't know of any suitable
structure to put this GPIO list in. Perhaps I'm missing something obvious?

- Ben

13

I see. I'm still not convinced that this is the route to take,
however. It seems like this virtual gpio interface is not only pretty
clunky (simple board file glue turns into an entire gpio chip driver),
it seems like this is a very inaccurate and not very useful way to
expose a multiplexed CS configuration; e.g. what is this chip driver to
do if the user tries to set two CS pins at once? Is there precedent for
using the GPIO subsystem in this way?

Cheers,

- Ben

14

> > I understand your concerns, but I'm not sure how to satisfy them without
> > crippling the design's ability to accomodate my use-case. I can't pass a
> > GPIO line per spi_board_info since in my case of a multiplexed CS
> > configuration a single pin's state does not uniquely determine the
> > desired CS. The only other option I can think of is that we somehow
> > provide a list of GPIOs for each bus and map the CS numbers to
> > permutations of GPIO states. Unfortunately, I don't know of any suitable
> > structure to put this GPIO list in. Perhaps I'm missing something obvious?
>
> Close, but not quite. Assign one gpio number to each cs state, and
> write a gpio controller driver that maps the linux-gpio number to the
> physical gpio state permutation. The mapping from gpio# to ss# is
> 1:1, but the driver behind the gpio# can do whatever you need it to
> do.
>
I see. I'm still not convinced that this is the route to take,
however. It seems like this virtual gpio interface is not only pretty
clunky (simple board file glue turns into an entire gpio chip driver),
it seems like this is a very inaccurate and not very useful way to
expose a multiplexed CS configuration; e.g. what is this chip driver to
do if the user tries to set two CS pins at once?

(Sorry for the really laggy reply)

I don't see it being that big a deal. gpio drivers are pretty
lightweight and it is fine to have domain-specific limitations on how
gpios for a particular "gpio controller" behave. In your example, if
the hardware doesn't support enabling 2 CS pins at once, then you make
a choice, either setting one when the other is already set simply does
not work; or it could reset the other state. Either choice would be
fine. The SPI driver must do the right thing regardless and deselect
the previous CS line before enabling a new one.

The other alternative would be to implement a new SPI chipselect
common infrastructure, but IMO, that would just end up looking like a
duplication of the gpio infrastructure.

Regardless, my point still stands, platform callbacks for what amounts
to gpio manipulations doesn't make a whole lot of sense.

Is there precedent for
using the GPIO subsystem in this way?

Yes, in drivers/spi look at mpc52xx_spi.c, davinci_spi.c, ath79_spi.c,
atmel_spi.c, and many more. Grep for gpio in drivers/spi.

g.

15

(Sorry for the really laggy reply)

No worries, I was just starting to think about this again myself.

I don't see it being that big a deal. gpio drivers are pretty
lightweight and it is fine to have domain-specific limitations on how
gpios for a particular "gpio controller" behave. In your example, if
the hardware doesn't support enabling 2 CS pins at once, then you make
a choice, either setting one when the other is already set simply does
not work; or it could reset the other state. Either choice would be
fine. The SPI driver must do the right thing regardless and deselect
the previous CS line before enabling a new one.

Having now finally having had a chance to implement a gpio_chip driver,
I agree. It's quite simple and seems to fill the need pretty well

The other alternative would be to implement a new SPI chipselect
common infrastructure, but IMO, that would just end up looking like a
duplication of the gpio infrastructure.

I think you are probably right. GPIO seems to be a good fit.

Patch coming shortly. I use spi_board_info.controller_data to pass the
gpio number to the controller which doesn't seem like the best way
forward, but I wasn't sure how else to proceed. Also, since the gpio
numbers are dynamically determined spi_board_info.controller_data must
be filled in at runtime, which is also a bit of a bummer (although not
easily solved as far as I can tell). Let me know what you think.

- Ben