rpi-4.9.y: spi driver reports bogus maximum frequency

[skoehler]

A bug was reported for the MRAA library that the SPI frequency cannot be set higher than 500kHz:
eclipse/mraa#255

It turns out that this boils down to a potential problem with the Raspberry Pi's SPI driver. In response to the SPI_IOC_RD_MAX_SPEED_HZ ioctl, the kernel driver reports some frequency slightly above 500kHz. MRAA then rounds down any requested frequency to the reported maximum speed.

It is known that the Raspberry Pi support speeds of up to 125MHz. So the value reported by the driver should be incorrect. I suggest that the driver is fixed to report the actual maximum speed obtained when the divisor is equal to 2.

Meanwhile, MRAA has been patched to ignore the maximum speed reported by the Raspberry Pi driver. Yet, I though that I should report the bug here so that the kernel driver is actually fixed.

While I did not test it, the bug may also affect other branches such as rpi-4.10.y, rpi-4.11.y, rpi-4.12.y, and rpi-4.13.y.

[pelwell]

I can confirm the symptoms, but fortunately the error comes from Device Tree. Since the introduction of Raspberry Pi Device Tree in rpi-3.15.y, the standard spidev nodes have had an spi-max-frequency (the DT equivalent of max_speed_hz) of 500000. This value has even propagated to the overlays enabling spi1 and spi2.

I'm slightly nervous that changing the maximum now will cause applications currently passing in zero as a speed (or querying the maximum and using that) to fail because their devices can't run at 125MHz. However, that counts as bad coding, and hopefully the usual gradual rollout (rpi-update, apt-get upgrade, major Raspbian release) would catch at least some of them before the change goes mainstream.

I'm inclined to change all of the RPi spi-max-frequencys to 125000000 - any objections, @popcornmix?

[popcornmix]

The 500kHz value is clearly wrong so should be changed.
Are we sure that 125MHz is actually achievable?
While I suspect the peripheral can internally handle that speed, I am suspecting the gpio slew may make that speed unusable.
@P33M any thoughts?

[P33M]

125MHz is probably out past the realms of sanity from a hardware perspective. For example, the SD card interface (4-bit) can just about run at 100MHz and this is with much shorter PCB traces that don't stretch across the board to a pin header.

I'd say sub-100MHz (say 62.5MHz) would be the maximum speed I'd expect to work at all reliably.

[pelwell]

I don't think we should be artificially restricting the range to what is going to be usable. The DT bindings say describe spi-max-frequency as:

Maximum SPI clocking speed of device in Hz.

But since this device is spidev (a userspace API), the value is effectively the maximum bus speed. The SPI host driver would not restrict a kernel SPI device driver to a lower value, so why should spidev be treated any differently?

[popcornmix]

Okay, if max freq is treated as theoretical limit (much like arm_freq=1500), rather than a usable default then I'm happy with the change to 125MHz.

[pelwell]

I've pushed a patch to rpi-4.9.y, and will update the other branches tomorrow unless someone complains.

[popcornmix]

Latest rpi-update kernel includes the patch @pelwell pushed earlier.

[notro]

Wouldn't it better to add a dtparam to change this?
I suspect a lot of setups that don't care about speed rely on the default value.

spi-max-frequency specifies the maximum a certain spi client can handle (and in the spidev case we don't know the client), and setting it to 125MHz certainly is a stretch :-)
I know of only one spi display (CPLD in front) that can actually handle that and in 4k bursts only.

Or better yet, put the spidev node in an overlay and let 'dtparam=spi=on' load it for backwards compatibility.

Userspace can also change the spidev speed using SPI_IOC_WR_MAX_SPEED_HZ:
http://elixir.free-electrons.com/linux/v4.9/source/drivers/spi/spidev.c#L477

[pelwell]

I'm not keen on expecting the userspace code to know that the default maximum speed isn't actually the maximum - the MRAA check seems sensible - which is why I was happy to increase the default.

I would like to change spidev node in the DT to be disabled by default to remove the need for real SPI device overlays to disable the corresponding spidev node (i.e. opt-in to spidev rather than opt-out), but that would require spidev users to also indicate which they want to enable - all or nothing is too coarse, and we can't remove the spidev-disabling fragments until they are guaranteed not to be needed.

We could defer the selective spidev enabling, requested by dtparam=spi=on, until all overlays have been processed, but that puts even more special case code in the firmware when I'd like to reduce it. spidev is a bit of a mess altogether - I2C handles user-space access (/dev/i2c-) in a cleaner way.