periph / host

Go·Hardware·Lean - Host drivers
https://periph.io
Apache License 2.0
57 stars 32 forks source link

bcm283x (or new pkg): support alternate functions of the bcm2711 #24

Closed knieriem closed 1 year ago

knieriem commented 1 year ago

In an RPi 4 project I needed to switch pin functions of GPIO4/5 at runtime between the Input function and the ALT4 alternate function TXD3/RXD3.

While rpi and bcm283x packages mostly appear to work fine on an RPi 4 regarding GPIO functionality, the RPi 4, which is based on the bcm2711, provides quite some more alternate functions that are not covered by the mapping table in bcm283x/gpio.c.

As a workaround, I simply added entries for UART3_TX and UART3_RX to that table:

--- a/bcm283x/gpio.go
+++ b/bcm283x/gpio.go
@@ -975,8 +975,8 @@ var mapping = [][6]pin.Func{
    {"I2C0_SCL"},
    {"I2C1_SDA"},
    {"I2C1_SCL"},
-   {"CLK0"},
-   {"CLK1"}, // 5
+   {"CLK0", "", "", "", "UART3_TX"},
+   {"CLK1", "", "", "", "UART3_RX"}, // 5
    {"CLK2"},
    {"SPI0_CS1"},
    {"SPI0_CS0"},

What would be an appropriate way to add the more extensive settings for the bcm2711? Alternate function tables of bcm2835 and bcm2711 suggest that the bcm2711 is mostly compatible, an extension, to the bcm2835 in this regard. So the mapping table could just be extended to contain the additional functions of the bcm2711. The problem with this approach is that the table then would be misleading for users of RPis smaller than 4, as it would contain alternate functions that are not available on the bcm2835. Code calling rpi.P1_7.(pin.PinFunc).SetFunc("UART_TX") would not work as expected when running it on an RPi 3.

Perhaps an approach could be to extend mapping table entries by a prefix, like in the following example:

    {"CLK0", "", "", "", "bcm2711:UART3_TX"},
    {"CLK1", "", "", "", "bcm2711:UART3_RX"}, // 5

// or, less verbose:

    {"CLK0", "", "", "", "+UART3_TX"},
    {"CLK1", "", "", "", "+UART3_RX"}, // 5

... with prefixes bcm2711: or + meaning available only on the bcm2711 resp. extended MCUs. Depending on a package bcm283x global variable, configured during host.Init(), the additional alt functions then would be available, or not. (Additional code would be needed to strip the prefix within bcm283x/gpio.go where needed)

Do you see another approach that you would prefer?

maruel commented 1 year ago

The mapping is intentionally a private variable. Duplicate it, and chose the right table based on the CPU. Better than trying to make one table address both configurations.

Something like:

var mapping = [][6]pin.Func
var mapping2835 = [...][6]pin.Func{...}
var mapping2711 = [...][6]pin.Func{...}

if isRPi4 {
  mapping = mapping2711
} else {
  mapping = mapping2835
}

It's just simpler for everyone IMO.

knieriem commented 1 year ago

Thanks for suggesting to create a second table. I have tried to implement that approach in the commit linked above. If appropriate, I will create a pull request from it. The new mapping2711 table has been generated from the specification using scripts from knieriem/bcm2711-alt-funcs-gen.

maruel commented 1 year ago

Please send as a PR, it's hard for me to visualize the end result. Thanks.