LINUX HAL - SPI chip selects and GPIOs

[John Williams]

Hi,

I'm looking into the AP_HAL_Linux SPI driver implementation, and it seems that for current targets at least, all SPI chip selects are assumed to be on GPIO pins, and as a consequence userspace/mmap GPIO bit banging is the only mode supported.

A consequence of this is that it's only partially using SPIDEV - /dev/spidev<X>.0 is always used instead of <X>.N for the Nth chip select.

For Zynq this will be a little different, the on-board SPI controllers (two available) each have between 1 and 3 dedicated CS pins, and these CS pins are controlled by the SPI controller  and Linux driver.  It's also fine to instantiate GPIOs and drive CS pins with them if more are required.

I understand using direct control of the GPIO to avoid kernel latency.  However, there are device tree bindings that allow the controlling GPIO pin to be registered as a SPI Chip select, see here:

https://www.kernel.org/doc/Documentation/devicetree/bindings/spi/spi-bus.txt

Here's a fragment that shows a mix of controller and GPIO-managed chip selects:

So if for example the controller has 2 CS lines, and the cs-gpios
property looks like this:

cs-gpios = <&gpio1 0 0> <0> <&gpio1 1 0> <&gpio1 2 0>;

Then it should be configured so that num_chipselect = 4 with the
following mapping:

cs0 : &gpio1 0 0
cs1 : native
cs2 : &gpio1 1 0
cs3 : &gpio1 2 0

Of course the kernel needs to own the GPIO controller in this scenario, and given that the existing platforms appear to use the same GPIO banks for CS and GPIO duties, this could get a little tricky!

I see a few options:
 * Zynq use GPIOs exclusively, and I wire the CS lines to GPIO pins instead of the inbuilt ones.  Distasteful but it will work.  It circumvents the Linux SPI API and so nothing will work except with the dedicated code in AP_HAL_Linux/SPIDriver to bit-bang the CS.
 * Zynq uses the full device tree method for SPI, with a dedicated GPIO controller instantiated just for any 'left over' GPIO CS pins.  The kernel owns this GPIO and the others are bit-banged from APM.  This is clean but I'll have to duplicate or refactor the existing SPIDriver class to take out the explicit chip select behaviour and use proper spidevX.N bindings.
 * Port the existing platforms (including Zynq) across to use the kernel-managed GPIO/CS approach, and take a chance that the kernel GPIO drivers are clean enough not to stomp on bits which are being managed by userspace through bitbanging, and that we never hit a read/modify/write race between kernel and userspace :)

Appreciate any thoughts on this before I go diving in.

Cheers,

John


 

[Andrew Tridgell]

Hi John,

> I'm looking into the AP_HAL_Linux SPI driver implementation, and it seems
> that for current targets at least, all SPI chip selects are assumed to be
> on GPIO pins, and as a consequence userspace/mmap GPIO bit banging is the
> only mode supported.

yes, though I'm very happy to generalise the SPI driver code to support
other approaches

> I understand using direct control of the GPIO to avoid kernel latency.
> However, there are device tree bindings that allow the controlling GPIO pin
> to be registered as a SPI Chip select, see here:
>
> https://www.kernel.org/doc/Documentation/devicetree/bindings/spi/spi-bus.txt

yes, and we did try this. I spent quite a bit of time fiddling on the
BBB to try to get separate SPI devices registered for each CS line in
/dev. Getting it to do two was fine. We never managed to get 3 to work
on one bus.

It is perfectly possible that we just didn't understand the device tree
well enough, or perhaps it is something to do with the particular 3.8
kernels we were using (BBB only transitioned to using device trees quite
recently).

> Of course the kernel needs to own the GPIO controller in this
> scenario, and given that the existing platforms appear to use the same
> GPIO banks for CS and GPIO duties, this could get a little tricky!

yep!

> I see a few options:
> * Zynq use GPIOs exclusively, and I wire the CS lines to GPIO pins
> instead of the inbuilt ones. Distasteful but it will work. It
> circumvents the Linux SPI API and so nothing will work except with the
> dedicated code in AP_HAL_Linux/SPIDriver to bit-bang the CS.

acceptable, but as you say its a bit icky

> * Zynq uses the full device tree method for SPI, with a dedicated
> GPIO controller instantiated just for any 'left over' GPIO CS pins.
> The kernel owns this GPIO and the others are bit-banged from APM.
> This is clean but I'll have to duplicate or refactor the existing
> SPIDriver class to take out the explicit chip select behaviour and use
> proper spidevX.N bindings.

yep.

I think adding an extra parameter in the LinuxSPIDeviceDriver
constructor should allow that to work fairly cleanly. Or
change cs_pin to be int16_t and have a special reserved value for
in-kernel CS handling.

> * Port the existing platforms (including Zynq) across to use the
> kernel-managed GPIO/CS approach, and take a chance that the kernel
> GPIO drivers are clean enough not to stomp on bits which are being
> managed by userspace through bitbanging, and that we never hit a
> read/modify/write race between kernel and userspace :)

if you could make this work for BBB I'd be interested to see it. We did
try previously, but I must admit this is my first foray into the
wonderful world of dts files so maybe I just got it wrong :-)

btw, I'm not sure you've realised just how ugly the current BBB solution
is. We couldn't find a easy way to actually disable the use of the CS
pin by the kernel driver, so we had to redirect it to a different pin
where it would do no harm. It currently is mapped to a couple of LEDs
that blink on SPI activity. I guess we could claim that is a feature :-)

Cheers, Tridge

[John Williams]

Hi Andrew,

[resend as google-groups bounced for some reason]

Thanks for the quick reply

On Thu, Oct 30, 2014 at 4:21 PM, Andrew Tridgell <and...@tridgell.net> wrote:

 

> I understand using direct control of the GPIO to avoid kernel latency.
> However, there are device tree bindings that allow the controlling GPIO pin
> to be registered as a SPI Chip select, see here:
>
> https://www.kernel.org/doc/Documentation/devicetree/bindings/spi/spi-bus.txt

yes, and we did try this. I spent quite a bit of time fiddling on the
BBB to try to get separate SPI devices registered for each CS line in
/dev. Getting it to do two was fine. We never managed to get 3 to work
on one bus.

It is perfectly possible that we just didn't understand the device tree
well enough, or perhaps it is something to do with the particular 3.8
kernels we were using (BBB only transitioned to using device trees quite
recently).

Interesting.  I'll have a bash on this with Zynq, see if I learn something that can help the BBB situation.

Does the fact you tried this also suggest you were OK with having dual-managed GPIO pins and the dragons that lurk there?  Or didn't it get that far? :)

Longer term I can imagine device trees having a lot of value in the Linux platforms.  You can enable /proc/device-tree in the kernel and then userspace can use it for device discovery.  Especially in Zynq where people can put their IP cores at any address they like, this will be essential otherwise the code for userspace managed devices quickly gets littered with horrible #ifdefs and hardcoded base addresses.

With the actual devices themselves (not just the controllers) in the device tree then it should remove the need for any Linux platforms to have any hard coded addresses, chip selects, I2C addresses etc.  Everything is in the device tree and you just have a different DTS for each board, and the Linux HAL's device discovery gets much more generic and clean.

We can save that for another day :)

 

  

> Of course the kernel needs to own the GPIO controller in this
> scenario, and given that the existing platforms appear to use the same
> GPIO banks for CS and GPIO duties, this could get a little tricky!

yep!

> I see a few options:
>  * Zynq use GPIOs exclusively, and I wire the CS lines to GPIO pins
> instead of the inbuilt ones.  Distasteful but it will work.  It
> circumvents the Linux SPI API and so nothing will work except with the
> dedicated code in AP_HAL_Linux/SPIDriver to bit-bang the CS.

acceptable, but as you say its a bit icky

>  * Zynq uses the full device tree method for SPI, with a dedicated
> GPIO controller instantiated just for any 'left over' GPIO CS pins.
> The kernel owns this GPIO and the others are bit-banged from APM.
> This is clean but I'll have to duplicate or refactor the existing
> SPIDriver class to take out the explicit chip select behaviour and use
> proper spidevX.N bindings.

yep.

I think adding an extra parameter in the LinuxSPIDeviceDriver
constructor should allow that to work fairly cleanly. Or
change cs_pin to be int16_t and have a special reserved value for
in-kernel CS handling.

Yep I think this is the go.  Existing platforms could then move over to it if it's feasible.

>  * Port the existing platforms (including Zynq) across to use the
> kernel-managed GPIO/CS approach, and take a chance that the kernel
> GPIO drivers are clean enough not to stomp on bits which are being
> managed by userspace through bitbanging, and that we never hit a
> read/modify/write race between kernel and userspace :)

if you could make this work for BBB I'd be interested to see it. We did
try previously, but I must admit this is my first foray into the
wonderful world of dts files so maybe I just got it wrong :-)

Sure, but it won't change the fact that you'll have userspace and kernel-managed pins on the same controller.

I don't have a BBB but once I figure out the controller/gpio SPI chip select thing for Zynq I might have some suggestions for you to try.

Still waiting on those sensor breakout boards to arrive, so a little bit stalled there.

btw, I'm not sure you've realised just how ugly the current BBB solution
is. We couldn't find a easy way to actually disable the use of the CS
pin by the kernel driver, so we had to redirect it to a different pin
where it would do no harm. It currently is mapped to a couple of LEDs
that blink on SPI activity. I guess we could claim that is a feature :-)

Oh that's definitely a feature!

John

[Andrew Tridgell]

Hi John,

> Does the fact you tried this also suggest you were OK with having
> dual-managed GPIO pins and the dragons that lurk there? Or didn't it get
> that far? :)

We didn't really get that far. I'd expect it to work fine, but I haven't
actually looked into the details in the kernel. The BBB only has one
core, so there probably isn't too much that could go wrong, at least
with my naive understanding of how it probably works. Maybe just wishful
thinking :)

> Longer term I can imagine device trees having a lot of value in the Linux
> platforms. You can enable /proc/device-tree in the kernel and then
> userspace can use it for device discovery. Especially in Zynq where people
> can put their IP cores at any address they like, this will be essential
> otherwise the code for userspace managed devices quickly gets littered with
> horrible #ifdefs and hardcoded base addresses.

yep, makes sense.

> With the actual devices themselves (not just the controllers) in the device
> tree then it should remove the need for any Linux platforms to have any
> hard coded addresses, chip selects, I2C addresses etc. Everything is in
> the device tree and you just have a different DTS for each board, and the
> Linux HAL's device discovery gets much more generic and clean.

indeed - if we can make it work!

> Yep I think this is the go. Existing platforms could then move over to it
> if it's feasible.

yep, and it keeps them flying while we grapple with DTS magic :)

> Still waiting on those sensor breakout boards to arrive, so a little bit
> stalled there.

darn!

Cheers, Tridge