spigen overlays for RPI-B, RPI-2 in base (was: possible new port for spigen overlay for Raspberry Pi)

[bobf_mrp3.com]

Summary

spigen overlays to be placed into 'base'. one for RPI-B, the other for RPI-2.

requires loader.conf 'fdt_overlays' entry for the correct overlay (either spigen-rpi-b.dtbo or spigen-rpi2.dtbo)

Implements 3 spigen devices on RPI2, but only 2 on RPi-B. This is because RPi-B uses GPIO16 for an LED, and it therefore cannot be used for SPI.

Needs DS15301 if you want a relationship between the cs pin and the spigen device name.

Test Plan

overlays tested in -CURRENT along with DS15301, all good.

This overlay contains fixes for pins for spi0@7e200000 and spi0_cs_pins. This allows cs==2 on RPI2.

Need to verify that RPI2 overlay also works on RPI3 [since they have the same pin assignments]

[andrew]

Why a port and not put the overlay in the base system?

[bobf_mrp3.com]

it seemed to me that a port was the better choice, although I'm not opposed at all to having it in the base.

It's just that an overlay like this is not something that's necessarily common to all bcm2835 devices, but is intended for the raspberry pi only.

Although, it _could_ become part of the base, I've been told that we're not supposed to edit the dts files, nor patch them, but to use overlays instead. That being said, if the overlay were included in the base, and installed by default, the end-user would still have to add the 2 entries to loader.conf to activate it.

Ideally I'd like to see it installed by default (not requiring loader.conf entries) when you build the kernel+world for the RPI-B and RPI2 [and RPI3, eventually] kernels, but that matter is also up for discussion, I guess. The same goes with the 'spigen' device, which needs kldload before the overlay will work.

[manu]

In D15067#317957, @bobf_mrp3.com wrote:

it seemed to me that a port was the better choice, although I'm not opposed at all to having it in the base.

For things like that (using custom spigen properties) base is better now that we build and install overlays.

It's just that an overlay like this is not something that's necessarily common to all bcm2835 devices, but is intended for the raspberry pi only.

One per board (or compatible board I think that for spi RPI1 = RPI2 = RPI0) is fine.

Although, it _could_ become part of the base, I've been told that we're not supposed to edit the dts files, nor patch them, but to use overlays instead. That being said, if the overlay were included in the base, and installed by default, the end-user would still have to add the 2 entries to loader.conf to activate it.

Ideally I'd like to see it installed by default (not requiring loader.conf entries) when you build the kernel+world for the RPI-B and RPI2 [and RPI3, eventually] kernels, but that matter is also up for discussion, I guess. The same goes with the 'spigen' device, which needs kldload before the overlay will work.

It will be installed but not activated, this isn't something that we want to do, we cannot please everyone.

[bobf_mrp3.com]

devel/rpi-spigen/files/spigen.dts

6 ↗

(On Diff #41468)

-CURRENT requires adding "brcm,bcm2835" to this list

9 ↗

(On Diff #41468)

-CURRENT appears to be using /soc/spi@7e204000 rather than /axi/spi0

additionally, it's worth pointing out that the pins definition for spi0 (or for spi0_gpio7 for that matter) doesn't include the pin for E2 which is GPIO16.

On the model 1, GPIO16 is connected to the LED (inverted polarity) and is therefore not available for use as 'E2' (cs==2). So the overlay for this version should be appropriately done to indicate the correct available pins. Model 1 can do cs==0 and cs==1; model 2 and later can include cs==2 and would have to include pin 16 in the definition for 'pins'.

This suggests that multiple overlays will be needed for the various RPi models, and the fdt_overlays entry in the loader.conf file will need to load the correct one.

[bobf_mrp3.com]

devel/rpi-spigen/pkg-message

12 ↗

(On Diff #41468)

actually it needs to be comma-separated like:

fdt_overlays="other_overlay.dtbo,spigen.dtbo"

(tested yesterday actually, verified it)

[bobf_mrp3.com]

devel/rpi-spigen/files/spigen.dts

16 ↗

(On Diff #41468)

new changes to the bcm2835_spi driver (in -CURRENT) no longer pre-assigns 'ALT0' pin states to the pins associated with the SPI, including the CS pins (this is the correct behavior, in the long run, the old code was a workaround).

As a result, for every one of the CS pins, there will need to be some additional info in the overlay that designates which physical GPIO pin is being used for the CS, as well as the correct 'ALT' state for it, so that the pin 'mux' states are correctly assigned.

The individual CS pins would then need to be defined as being given an 'ALT0' pin status. I'll need to determine whether this info can be within the 'spigen#' blocks or if they need to be separate overlay fragments.

[bobf_mrp3.com]

removing the port diff; putting 2 overlays into base

[bobf_mrp3.com]

removed recommended port; added to base instead, to be installed in /boot/dtb/overlays/

uses -CURRENT definitions for DTB (not compatible with what is being done in 11-STABLE)

implements CS=2 for RPI2 only (RPI-B uses same pin for LED, not compatible).

[kevans]

I think you should go ahead and add these to DTSO= in sys/modules/dtb/rpi/Makefile while you're here to hook them up to be installed at the same time; see modules/dtb/allwinner/Makefile for example.

We'd still need to figure out how to work this in with arm64/GENERIC for RPI-3, I guess, since it doesn't build dtb modules by default and thus no overlays.

[bobf_mrp3.com]

added DTSO= section to sys/modules/dtb/rpi/Makefile (as recommended)

[bobf_mrp3.com]

updated 'dtso' formatting to latest spec in accordance with r332483 commit comment

[manu]

./sys/dts/arm/overlays/spigen-rpi-b.dtso

11 ↗

(On Diff #42144)

spigen0 then spigen1

./sys/dts/arm/overlays/spigen-rpi2.dtso

10 ↗

(On Diff #42144)

spigen0 then 1 then 2

12 ↗

(On Diff #42144)

Not sure the comment is needed, this is a standard device tree binding property.

[bobf_mrp3.com]

./sys/dts/arm/overlays/spigen-rpi-b.dtso

11 ↗

(On Diff #42144)

D15301 should make this possible, and would use spigen@0 rather than spigen0 : spigen0 . As a reminder, overlays are created in reverse order due to the way the lib inserts new devices right after the last property, whereas they're created in the same order with normal dtb loading. The solution is to divorce the device numbering from the cs by using 'spigenN.M' rather than 'spigenU' for the device (i.e. D15301).

./sys/dts/arm/overlays/spigen-rpi2.dtso

12 ↗

(On Diff #42144)

I'll remove it. it was a holdover from my testing.

[bobf_mrp3.com]

Requires D15301 to work properly, i.e. allows the overlay to create the actual devices in reverse order without affecting the ability to relate the 'spigen' device name to its bus and CS number. The fdt library itself causes this to happen; that is, for an overlay the devices are inserted at the beginning of the section, as they are created, resulting in a kind of reversed sort of how they're specified in the overlay. but relying on the order in which the devices are created is undesirable. That is where D15301 comes in, and now the overlays won't have to specify spigen devices in reverse order.

Also removed a comment.

Also might want to (later, after I test it) use 'spigen@0' rather than 'spigen0' (etc.)

[bobf_mrp3.com]

This comment was removed by bobf_mrp3.com.

[bobf_mrp3.com]

whoops - sorry people - I posted that last comment to the wrong Phab (it was for D15787

[bobf_mrp3.com]

verified working ok in r335524 which includes the name change applied to spigen ( https://reviews.freebsd.org/rS335506 ) on RPi 1 B.