tinygo icon indicating copy to clipboard operation
tinygo copied to clipboard

WIP machine: explicitly pass pin numbers to serial peripherals

Open aykevl opened this issue 4 years ago • 2 comments

This is an attempt to change the API so that you would only need to pass pin numbers to serial peripherals (UART, SPI, I2C).

func (UART) Configure(tx, rx Pin, config UARTConfig) error
func (SPI) Configure(sck, mosi, miso Pin, config SPIConfig) error
func (I2C) Configure(scl, sda Pin, config I2CConfig) error

This change has several benefits:

  1. It is far more flexible. All available pins per device can be used, instead of a few that are fixed in the machine package.
  2. It is much simpler to use: just pass the pin numbers. Especially on devices like SAMD, you don't need to worry about pin modes (sercom/sercom-alt), pad numbers, or whatever. The documentation can state what kind of pins are supported per SERCOM and you can just pick those.
  3. Pin number 0 can be used. Previously, if you actually wanted to use pin 0, it might be considered an unset pin and the default pin number might be used instead.
  4. Simulation is much easier. Instead of having to simulate the whole pin multiplexer, just use the given pin numbers.
  5. It is possible to get an error message back when you passed an invalid pin number.
  6. It's impossible to forget to set the correct pin by leaving it out of the config struct.

Of course, this is a breaking change so will have to be discussed more widely before applying.

aykevl avatar Sep 25 '19 18:09 aykevl

I think the overall idea is sound, although it adds a little more to simple code, the benefit is that it makes pin usage more explicit. We can still have well-known default constant values for platform independent code, for example:

uart0.Configure(machine.UART_TX_PIN, machine.UART_RX_PIN, machine.UART0ConfigDefault)
spi0.Configure(machine.SPI_SCK_PIN, machine.SPI_MOSI_PIN, machine.SPI_MISO_PIN, machine.SPI0ConfigDefault)
i2c0.Configure(machine.I2C_SCL_PIN, machine.I2C_SDA_PIN, machine.I2C0ConfigDefault)

One thing that will have to be resolved is that IRQ handling for the SAM SERCOM peripherals is not quite as configurable as what you might wish, for example:

https://github.com/tinygo-org/tinygo/blob/master/src/machine/board_arduino_nano33.go#L84 and https://github.com/tinygo-org/tinygo/blob/master/src/machine/board_arduino_nano33.go#L98

But there are a few different ways that could be handled without even adding the interrupt handing proposal you have made separately.

I was looking at the playground and simulators in general and your point about simulator using simple pin numbers and mostly ignoring the config data for those processors can reduce a lot of the complexity that I was imagining coming up even just with the SAMD21.

Anyhow, count me in favor of it. It is a breaking change for only a single line per peripheral subsystem, the rest of the APIs are not changing, so no need to change any drivers.

deadprogram avatar Sep 26 '19 12:09 deadprogram

IRQ handling is outside of the scope of this change. While I have plans for that, this proposal is only about how to pass pin numbers to a peripheral.

My idea is to start by making samd21 pin configuration only look at pin changes, with a mostly backwards compatible API change (an added error return). I'll see how far I can get it, but I think it is doable (but not trivial to do).

aykevl avatar Sep 26 '19 12:09 aykevl