Add support for multiple I2C or SPI peripherals

[ladyada]

These are now supported in modern Arduino chipsets. here's an examples of how to create multiple I2C/SPI peripherals.
https://github.com/adafruit/ArduinoCore-samd/blob/master/variants/grand_central_m4/variant.h#L177

they are auto-generated here:
https://github.com/adafruit/ArduinoCore-samd/blob/master/libraries/SPI/SPI.cpp#L469

[matthijskooijman]

I just had a look over the SPI code, and it seems you can already do this by manually creating a new SPI object:

 SPIClass MySPI(mosi_pin, miso_pin, sck_pin, ss_pin);

Note that you pass the pin numbers and then the code figures out what SPI unit is attached to those pins (within the options defined by the variant, which is currently limited to 1 SPI unit for each pin).

What I like very much about this, is the flexibility: The sketch does not need any hardcoded knowledge of the SPI unit to pin mapping, it just needs to know what pins have been used to connect the slave (of course those pins should be a valid combination, of course).

There are still some things I do not completely like about this approach, though. It currently works like this:

• The variant defines pin maps, grouped by pin function (e.g. a list of all pins that can be configured as MOSI pins and for each pin how to configure them and what SPI unit they would then be connected to).
• spi_init is passed a list of pins. It looks up each pin in the pinmap for the appropriate function to see what SPI unit it would be connected to.
• If all pins (except SS, which is optional) are connected, and are connected to the same SPI unit, then that SPI unit is initialized and used.

See

Arduino_Core_STM32/cores/arduino/stm32/spi_com.c

Lines 144 to 165 in 1e2a598

	// Determine the SPI to use
	SPI_TypeDef *spi_mosi = pinmap_peripheral(obj->pin_mosi, PinMap_SPI_MOSI);
	SPI_TypeDef *spi_miso = pinmap_peripheral(obj->pin_miso, PinMap_SPI_MISO);
	SPI_TypeDef *spi_sclk = pinmap_peripheral(obj->pin_sclk, PinMap_SPI_SCLK);
	SPI_TypeDef *spi_ssel = pinmap_peripheral(obj->pin_ssel, PinMap_SPI_SSEL);
	/* Pins MOSI/MISO/SCLK must not be NP. ssel can be NP. */
	if (spi_mosi == NP || spi_miso == NP || spi_sclk == NP) {
	core_debug("ERROR: at least one SPI pin has no peripheral\n");
	return;
	}
	SPI_TypeDef *spi_data = pinmap_merge_peripheral(spi_mosi, spi_miso);
	SPI_TypeDef *spi_cntl = pinmap_merge_peripheral(spi_sclk, spi_ssel);
	obj->spi = pinmap_merge_peripheral(spi_data, spi_cntl);
	// Are all pins connected to the same SPI instance?
	if (obj->spi == NP) {
	core_debug("ERROR: SPI pins mismatch\n");
	return;
	}

and

Arduino_Core_STM32/variants/NUCLEO_F401RE/PeripheralPins.c

Lines 180 to 188 in 1e2a598

	WEAK const PinMap PinMap_SPI_MOSI[] = {
	{PA_7, SPI1, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI1)},
	{PB_5, SPI1, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI1)},
	// {PB_5, SPI3, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF6_SPI3)},
	{PB_15, SPI2, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI2)},
	{PC_3, SPI2, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI2)},
	{PC_12, SPI3, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF6_SPI3)},
	{NC, NP, 0}
	};

What I do not like here is:

• Sometimes, a single pin can be mapped to the same function for multiple units, e.g. PB5 which can be MOSI for SPI1 and SPI3. Currently, the variant has to select which one of these is actually used:

  Arduino_Core_STM32/variants/NUCLEO_F401RE/PeripheralPins.c

  Lines 182 to 183 in 1e2a598

  	{PB_5, SPI1, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI1)},
  	// {PB_5, SPI3, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF6_SPI3)},

  
  This obviously reduces flexibility for the sketch, which is a pity. This limitation is there, because the pin mapping code looks for a given pin number in a given pinmap and then always returns the first one.
• Sometimes, you might want to explicitly assign use a specific SPI unit for something. Usually, a given set of pins will uniquely point at one viable SPI unit, but sometimes (if the previous point is solved), there would be two options (e.g. some set of pins can be used for both SPI1 and SPI3). In isolation, either unit would be fine, but if you use multiple units, you might need to use specific units on specific pins to prevent using the same unit twice.

Some ideas on improving this:

• Allow specifying the same pin multiple times in a pin map (this can be done right away, since existing code will still just use the first entry).
• Then the SPI code can be modified to, instead of looking for the unit belonging to each pin in isolation, find a unit that matches all pins together. This requires some additional smarts in the pinmap module, but should not be too hard.
• Additionally, or alternatively, allow specifying an explicit SPI instance to the constructor. Then any pinmap looks can simply match both the SPI instance and the pin number specified.
• It could also make sense to predefine SPI1, SPI2, etc. like it happens for AVR and SAMD. There would then not be any explicit pin configuration, but that could just assume default pins (either using defines in the variant as happens for the SPI instance now, or just use the first pin that matches the unit in the pinmaps, which might be more robust). One complication here is the the HAL already contains SPI1, SPI2 defines for the instance pointers, but these are always just typecasts of e.g. SPI1_BASE, so they could perhaps be undef'd.
• To really allow automatically allocating the SPI units, we might need to keep track of which SPI units are already in use, so you can find another one connected to the same pins (though I suspect this is something that might be more needed with e.g. PWM pins and timers than SPI). Without specifying explicit SPI units, this might require initializing SPI instances in a specific order to make sure they are allocated correctly.

I suspect that most of the above applies to other peripherals equally (at least timers, probably also I2c and others).

[fpistm]

@matthijskooijman
You're right and I've already think about this but not so easy and will require some more study.
As an example Mbed define PY_n_ALTx pin in the array to differentiate alternative pin capabilities but with Arduino pin number PYn upper layer this become hard to handle.

[matthijskooijman]

As an example Mbed define PY_n_ALTx pin in the array

Do you have a link for that?

One related improvement I just realized: Currently, every transaction calls spi_init() to configure the settings (e.g. clock speed). However, this also completely sets up the SPI device, including scanning the pin map for which SPI unit to use. I think this should be split: Select and set up the SPI device once, on begin(), and then only change the needed settings on each transaction.

[fpistm]

Here the PeripheralPins.c for a Nucleo F411RE:
https://github.com/ARMmbed/mbed-os/blob/816689d1bbd5e82b64aa8af3cb294f6dac7130ec/targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F411xE/TARGET_NUCLEO_F411RE/PeripheralPins.c#L120

[fpistm]

One related improvement I just realized: Currently, every transaction calls spi_init() to configure the settings (e.g. clock speed). However, this also completely sets up the SPI device, including scanning the pin map for which SPI unit to use. I think this should be split: Select and set up the SPI device once, on begin(), and then only change the needed settings on each transaction.

For that I will comment in #257

[Hoek67]

Check out https://stm32f4-discovery.net/api/group___t_m___d_e_l_a_y.html 3rd party library.

I use it for I2C and SPI as it supports multiple pins and in the case of SPI... DMA. I use a STM32F407VET6 which is supported. I altered a few functions so allow me to transfer SPI with DMA and do other stuff while waiting for the DMA to finish.

[uzi18]

@Hoek67 https://github.com/MaJerle/stm32f429/blob/master/00-STM32F429_LIBRARIES/tm_stm32f4_delay.c

[fpistm]

Currently, I'm working (almost finished) on variant rework and make all pins from the pinmap array available. This will remove this restriction pointed by @matthijskooijman

* The variant defines pin maps, grouped by pin function (e.g. a list of all pins that can be configured as MOSI pins and for each pin how to configure them and what SPI unit they would then be connected to).

* spi_init is passed a list of pins. It looks up each pin in the pinmap for the appropriate function to see what SPI unit it would be connected to.

* If all pins (except SS, which is optional) are connected, and are connected to the same SPI unit, then that SPI unit is initialized and used.

See

Arduino_Core_STM32/cores/arduino/stm32/spi_com.c

Lines 144 to 165 in 1e2a598

	// Determine the SPI to use
	SPI_TypeDef *spi_mosi = pinmap_peripheral(obj->pin_mosi, PinMap_SPI_MOSI);
	SPI_TypeDef *spi_miso = pinmap_peripheral(obj->pin_miso, PinMap_SPI_MISO);
	SPI_TypeDef *spi_sclk = pinmap_peripheral(obj->pin_sclk, PinMap_SPI_SCLK);
	SPI_TypeDef *spi_ssel = pinmap_peripheral(obj->pin_ssel, PinMap_SPI_SSEL);
	/* Pins MOSI/MISO/SCLK must not be NP. ssel can be NP. */
	if (spi_mosi == NP || spi_miso == NP || spi_sclk == NP) {
	core_debug("ERROR: at least one SPI pin has no peripheral\n");
	return;
	}
	SPI_TypeDef *spi_data = pinmap_merge_peripheral(spi_mosi, spi_miso);
	SPI_TypeDef *spi_cntl = pinmap_merge_peripheral(spi_sclk, spi_ssel);
	obj->spi = pinmap_merge_peripheral(spi_data, spi_cntl);
	// Are all pins connected to the same SPI instance?
	if (obj->spi == NP) {
	core_debug("ERROR: SPI pins mismatch\n");
	return;
	}

and

Arduino_Core_STM32/variants/NUCLEO_F401RE/PeripheralPins.c

Lines 180 to 188 in 1e2a598

	WEAK const PinMap PinMap_SPI_MOSI[] = {
	{PA_7, SPI1, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI1)},
	{PB_5, SPI1, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI1)},
	// {PB_5, SPI3, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF6_SPI3)},
	{PB_15, SPI2, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI2)},
	{PC_3, SPI2, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF5_SPI2)},
	{PC_12, SPI3, STM_PIN_DATA(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF6_SPI3)},
	{NC, NP, 0}
	};

What I do not like here is:

* Sometimes, a single pin can be mapped to the same function for multiple units, e.g. PB5 which can be MOSI for SPI1 and SPI3. Currently, the variant has to select which one of these is actually used: https://github.com/stm32duino/Arduino_Core_STM32/blob/1e2a5985dabb20a7581a4c284537c3031d968779/variants/NUCLEO_F401RE/PeripheralPins.c#L182-L183
  
  This obviously reduces flexibility for the sketch, which is a pity. This limitation is there, because the pin mapping code looks for a given pin number in a given pinmap and then always returns the first one.

I'm wondering if any official documentation on the *_INTERFACE_COUNT exists?

About:

* One complication here is the the HAL already contains `SPI1`, `SPI2` defines for the instance pointers, but these are always just typecasts of e.g. `SPI1_BASE`, so they could perhaps be undef'd.

Unfortunately, I don't want undef it is really too risky as at sketch level user can use HAL/LL and could need to use SPIx peripheral define in the CMSIS so the new instance from SPIClass will probably be SPI_x.