Proposal: Amend 'standard practice' config defaults model

[kenbell]

The current standard practice for default configuration in TinyGo (and drivers) is:

1. Expose a Config struct
2. Expose a Configure(Config) method
3. In Configure see if individual fields are uninitialized and if so, set defaults

This is causing a few problems:

• The rules for what uninitialized means aren't clear. For integer-derived fields is 0 default, or did the caller explicitly want the 0 value? Should the driver/peripheral logic always make sure 0 is used only for uninitialized even when the underlying hardware uses 0 specifically?
• In the machine package it causes backwards dependencies where board-agnostic code (e.g. for a family of chips) depends on board-specific code (default I2C pins)

Proposal

1. For all drivers and peripherals where useful defaults are needed, expose a DefaultConfig variable that has those defaults present. This should be a non pointer variable
2. Document the correct way to get defaults is xxx.Configure(yyy.DefaultConfig)
3. After some period (may be a release or two of TinyGo), strip all the logic trying to handle uninitialized config fields

How would this look?

To take the I2C pin defaults, currently calling code typically looks like this:

machine.I2C0.Configure(machine.I2CConfig{})

That would now look like this:

machine.I2C0.Configure(machine.DefaultI2C0Config)

For code that wants to just set 'some' of the peripheral config fields, it currently looks like this:

machine.I2C0.Configure(machine.I2CConfig{Frequency: 100000})

That would now look like this:

cfg := machine.DefaultI2C0Config
cfg.Frequency = 100000
machine.I2C0.Configure(cfg)

The implementation would be something like this:

// in board_feather_rp2040.go

const (
	I2C0_SDA_PIN = GPIO24
	I2C0_SCL_PIN = GPIO25

	I2C1_SDA_PIN = GPIO2
	I2C1_SCL_PIN = GPIO3
)

var (
	DefaultI2C0Config = I2CConfig {Frequency: 100_000, SDA: I2C0_SDA_PIN, SCL: I2C0_SCL_PIN}
	DefaultI2C1Config = I2CConfig {Frequency: 100_000, SDA: I2C1_SDA_PIN, SCL: I2C1_SCL_PIN}
)

// in machine_rp2040_i2c.go

func (i2c *I2C) Configure(config I2CConfig) error {

	// Config default logic stripped from here

	config.SDA.Configure(PinConfig{PinI2C})
	config.SCL.Configure(PinConfig{PinI2C})
	return i2c.init(config)
}

A similar approach could be taken for drivers in https://github.com/tinygo-org/drivers

[ysoldak]

I like the idea of doing something with default values and code decoupling.

The API shall be straight forward and hard to make mistakes. In your proposal, what shall happen when some developer mixes I2C0 and I2C1, like following?

machine.I2C1.Configure(machine.DefaultI2C0Config)

[kenbell]

Thanks for reviewing @ysoldak. In my proposal it would be as-if the developer specified the wrong pins specifically (i.e. would compile, but wouldn't function as expected).

[aykevl]

I like this idea! It looks a lot cleaner than my proposal here: https://github.com/tinygo-org/tinygo/pull/593 (although some of the ideas of #593 have been implemented in a more limited form).

A nice side effect is that this makes it a lot easier to build for a chip, instead of a board. Right now, if you want to build for a custom board, your best bet is to simply compile the code with the -target set to an existing board that is sufficiently similar to your custom board. The main reason for this are the board-specific constants that are used in peripheral initialization.

[aykevl]

Related: https://github.com/tinygo-org/tinygo/issues/2607

[kenbell]

I've taken a quick look at implementing. There's a few nits I found so far as to allowing external board definitions:

• CPUFrequency() is an interesting case - it's return value although hard-coded is typically a combination of an external oscillator (part of the 'board') and internal PLL and/or divider config (part of the MCU). It's used some by MCU logic for delay loops.
• Serial is defined in terms of DefaultUART in serial-uart.go, which is part of the board definition.
• USB has some back-dependencies all of it's own (needs more investigation / thought)

Serial can be worked around by doing -serial none, but would need some solution to let custom boards specify which UART to use for serial IO

For CPUFrequency, I can see a few options:

1. Having something in target.json, and figure a way for the build system to inject external oscillator frequency
2. Linker hack like //go:linkname for the oscillator frequency, so custom boards can define machine.ExternalOscillatorFrequency
3. Default it for a given MCU and assume most (but not all?) boards in practice use a common frequency for any given MCU