SPI: switch to spi-bcm2835 - what would it require?

[msperl]

Hi!

Quick question with regards to the spi drivers:

What is the reason for not switching from the spi-bcm2708 to the upstream spi-bcm2835 driver?

I wonder because I have a few improvements that I want to push upstream which might be quite helpful for a few people who are using the SPI sub-system (especially around latencies). Also I have been talking with notro to get DMA incorporated into never versions(as well as upstream) so that the TFT displays that you are starting to support do not put such a huge interrupt burden on the RPI leaving more CPU to do actual work.

The only difference I see is the LoSSI support in spi-bcm2708, which - as far as I can tell - is basically broken because there is no correct support for those 24bit/32bit reads in the first place (depending on the command sent). Actually notro (the author of the LoSSI patch for the spi-bcm2708) told me that even he is not using it with the tft drivers, because it is not really working as he assumed it would and went back to do 9x8 bit transfers instead 8x 9bit transfers.

Also from the state of the implementation I doubt that the LoSSI patches would EVER get incorporated into the upstream Linux kernel.

For things that I have lined up with the spi-bcm2835 driver please look at the wiki: https://github.com/msperl/spi-bcm2835/wiki and the corresponding open and closed issues.
These still need to get upstream, but I am starting to work on that.

Thanks, Martin

[popcornmix]

Moving to upstream drivers is desired.
Submit a PR and convince us that the change won't break lots of things and we'll accept it.

Obviously the "not breaking things" is the tricky part, as it's hard to be sure exactly what people are doing with the spi-bcm2708 driver.

SPI is not something I use, so I'll have to listen to advice from the devs who use it.
Obviously you and @notro. I'd be interested in comments from @bootc who provided the original driver.

If we make a change and lots of users complain that things are broken, we'll have to revert or help the users get working again. If you are willing to help resolve issues, then that will make the switch more likely to happen.

So, if we did this, what would you expect spi users to experience after the update? What would break?

ping @pelwell for opinion.

[notro]

I suggest the following:

• Enable building of spi-bcm2835
• Keep building spi-bcm2708
• Enable spi-bcm2835 instead of spi-bcm2708 in the Device Tree

If someone starts having trouble, it's quite easy to hack together an overlay that switches to spi-bcm2708 instead. Or just provide a fallback overlay upfront.

[msperl]

Why not compile both (each one has a separate .compatible string)?

• to start have a device overlay to set the compatibility to spi-bcm2835, so everyone would need to enable it explicitly.
• Then at a later point in time switch over and provide the old as an overlay
• Finally if nobody complains for some time: remove the old one (by that time we should hopefully be Device Tree only)

That could mean a transition with fallback for those complaining and giving us a window to fix issues...

Martin

[pelwell]

I would prefer adding an overlay to enable it for the first release, then we can allow the advanced users some time to try it out.

This would be a good occasion to use the Release Note/Warning on Update feature, if we had one.

[pelwell]

Jinx.

[msperl]

Note the overlay could look like this (also enabling SPI when loading it):

/*
 * Device tree overlay for spm-2835
 */

/dts-v1/;
/plugin/;

/ {
    compatible = "brcm,bcm2835", "brcm,bcm2836", "brcm,bcm2708", "brcm,bcm2709";
    fragment@0 {
        target = <&spi0>;
        __overlay__ {
            status = "okay";
            compatible = "brcm,bcm2835-spi";
            };
        };
    };
};

[pelwell]

It looks like you've got this figured out. Are you up to creating a pull request? (Note that I'm assuming the drivers behave the same with respect to their DT nodes.)

[msperl]

Yes - fortunately they use the same layout (even though the GPIO portion is not really necessary as the "2708" version explicitly sets ALT4 on the GPIOs regardless of what DeviceTree does (but that is probably still there for systems without DeviceTree)

I can look into that - only thing is: I am not sure where to find those overlays in the source-code...

[msperl]

ok - found that portion: arch/arm/boot/dts/

[msperl]

Created pull-request
BTW: another approach could be to move ALL the spi configs into overlays so that you would have to explicitly have to use dtoverlay=spi-bcm2835-overlayor dtoverlay=spi-bcm2708-overlay
but that would mean a deviation to what is in place right now with dtparam=spi=on

[pelwell]

It shouldn't matter to most users which SPI driver is being used, and this way we get to control what driver they are actually using.

BTW, you don't need to include the "-overlay" when using them - that is assumed.

[msperl]

merged