[RPi] spi.frequency(x) is not working above 500khz

[leonyuhanov]

Ok I'm back, i know I'm annoying...
Running MRAA on an RPi A+ in NodeJs
MRAA Module Version: 'v0.7.3-20-gef28607'
Node Version: v0.12.6
NPM Version: 2.11.2
I manualy compiled mraa on the board as per previous instructions and got it working. However I can not change the SPI frequency above 500Khz. I can put in any number i like into

var mraaModule = require('mraa');
var spiDevice = new mraaModule.Spi(0);
spiDevice.frequency(4000000);

But the frequency sticks at 500khz. I can set it lower and can very that it changes on my scope.
I can also verify that my RPi has a working SPI device as i have tested the pi-spi node module and i can run that at 8mhz with very little issue.

[arfoll]

Reporting bugs != annoying.

@michael-ring I'm away from my rpi, and since you're the expert can you have a look at this?

[michael-ring]

I will try my very best, will look at this in the evening

[michael-ring]

The problem is described very easily:

mraa_spi_frequency queries the driver about the maximum possible speed and allows only setting values up to this speed.

You should have seen a message:
spi: Selected speed reduced to max allowed speed
in syslog.
The value that the driver returns is obviously rubbish as the raspberry can go a lot higher with the SPI clock. Commenting out the following code solves the issue:

// if (ioctl(dev->devfd, SPI_IOC_RD_MAX_SPEED_HZ, &speed) != -1) {
// if (speed < hz) {
// dev->clock = speed;
// syslog(LOG_WARNING, "spi: Selected speed reduced to max allowed speed");
// }

I think I 'invented' this sanity check, arfoll, if you have no objections then I can create a patch to eliminate those lines.

[arfoll]

Actually the sanity check seems quite useful to me, let's keep it but just add an advance function hook to bypass it in the case of rpi. Also a bug should be filed to that SPI driver because it's obviously saying rubbish. Are you ok to do this?

[michael-ring]

Fine with me, I will provide a patch for what you described in the next days.

[leonyuhanov]

Awesome, no rush, im using a different spi node module for the moment, but would prefer to have mraa in the production version
Will i need to recompile this on my RPi again from scratch?

[arfoll]

Sadly yes. However I'm planning on a making a new release soon and that
will have NPM on arch fixed (it's fixed but not pushed on NPM due to no
tag) so you'll be able to just do npm mraa on your rpi :)

On 19 August 2015 at 15:11, Leon notifications@github.com wrote:

Awesome, no rush, im using a different spi node module for the moment, but
would prefer to have mraa in the production version
Will i need to recompile this on my RPi again from scratch?

—
Reply to this email directly or view it on GitHub
#255 (comment)
.

[leonyuhanov]

No stress

[michael-ring]

@arfoll: Hmm, new release... I realized tha my patch for 16bit length spi on raspberry is not merged yet, could you include the patch in the release before it gets bit rot?

[arfoll]

@michael-ring where is that patch? I tried to quickly find it but couldn't...

[michael-ring]

I meant this one:
#153
It's about Soft-SPI

[leonyuhanov]

Guys I'm a bit new to github, how do i know when you have committed the new changes? WIll there be an post here?

[tingleby]

If in the commit message refers to "#255" in some way yes!

[leonyuhanov]

Hey team, i just realized that i could have just done this myself in my local copy. And that seems to have fixed this issue...
I commented out the lines of code as sugested above, recompiled and now I'm running a higher frequency. It looks as though its capping at around 4-5mhz.. But that is good enough for me.
Question: should i CLOSE this bug??

[michael-ring]

Nope, i will fix it correctly, did not find the time last weekend

[leonyuhanov]

@michael-ring Wondering when this will be resolved? I am still commenting out these lines pre compilation:
// if (ioctl(dev->devfd, SPI_IOC_RD_MAX_SPEED_HZ, &speed) != -1) {
// if (speed < hz) {
// dev->clock = speed;
// syslog(LOG_WARNING, "spi: Selected speed reduced to max allowed speed");
// }

[alext-mkrs]

@michael-ring, is there any chance you'll tackle this one?

[leonyuhanov]

Guys I am just commenting this out when I compile and it works just fine. Not sure why this is in there...

[alext-mkrs]

This is to make sure we use the actual max frequency - and the assumption is that driver has the ultimate knowledge. And apparently on rpi the driver is broken in this part, so there's a mechanism in mraa, which allows to implement a workaround.

[leonyuhanov]

I have been able to get 10mhz but anything higher seems the same(but only if i comment out these lines of code)
Iether way,im happy to close this if its not worth looking into

[leonyuhanov]

I'm closing this issue. As stated above, this code depends on the RPI driver reporting correct information back to the MRAA module. It seems like this is not happening properly.
To fix this, all you need to do is comment out:

// if (ioctl(dev->devfd, SPI_IOC_RD_MAX_SPEED_HZ, &speed) != -1) {
// if (speed < hz) {
// dev->clock = speed;
// syslog(LOG_WARNING, "spi: Selected speed reduced to max allowed speed");
// }

[alext-mkrs]

Actually, it's worth it to be fixed properly, and that means exactly what @arfoll discussed with @michael-ring earlier. Namely, to implement a RPi-specific _replace function, which would make it work without commenting suff out and rebuilding + a bug should be filed for the driver to make it work properly.

That's why I pinged @michael-ring earlier, to see if he's still willing to do that. That's quite a simple fix I just don't have a RPi to test and prepare it myself.

[alext-mkrs]

Let me open this one back as I believe this should be fixed (worked around) properly in mraa to avoid user confusion.

@arfoll, feel free to override.

[DavidYon]

Am running into this bug as well, agree with @alext-mkrs and @arfoll, rather than having to comment this out, a simple replace function would fix it for RPi. That opens the door for the replace function being able accommodate changes with the out-of-spec RPi SPI driver over time.

I.e., if the driver's ioctl() was brought in line with the drivers on other embedded systems, the replace function could detect the driver/OS version and implement the appropriate algorithm.

I'd be happy to implement this if you'll accept a pull request. Seems pretty safe from a regression standpoint as:

1. Whereever you want to place the blame, the fact remains that it's broken on RPi as it stands now.
2. The fix is clean, using an existing platform-hook mechanism.
3. It won't affect other plaforms.

[arfoll]

Sure, I totally agree - how about this fix 644064f? If someone tests it and tells me it works I'll merge it.

[skoehler]

I just hit the same issue. Do I understand correctly, that the kernel driver is reporting a value which is much too low? Has somebody reported this issue to the guys at https://github.com/raspberrypi/linux?

[leonyuhanov]

@skoehler just coment out
// if (ioctl(dev->devfd, SPI_IOC_RD_MAX_SPEED_HZ, &speed) != -1) {
// if (speed < hz) {
// dev->clock = speed;
// syslog(LOG_WARNING, "spi: Selected speed reduced to max allowed speed");
// }
And it will work!

[skoehler]

I will patch mraa tomorrow. I'll probably use the patch by arfoll, but my question remains: did anybody report this too the guys maintaining the Raspberry Kernel sources (see my link above)? Because that should be done. If nobody has, I will do it asap.

[leonyuhanov]

@skoehler i have not so feel free to do so. I stopped using mraa a long time ago in favor of rpis native spi C library

[skoehler]

FYI: they patched the device tree such that the maximum frequency reported by the driver is now 125kHz and not 500MHz anymore. On Raspbian, rpi-update kernel should give you the new device tree.