[RFC] Board config: SPI & I2C config simplifications

[bkueng]

Next round after #13871: SPI + I2C

Summary

Similar to the PWM config, we can configure SPI buses and devices:

constexpr px4_spi_bus_t px4_spi_buses[SPI_BUS_MAX_BUS_ITEMS] = {
	initSPIBus(1, {
		initSPIDevice(DRV_GYR_DEVTYPE_ICM20689, SPI::CS{GPIO::PortF, GPIO::Pin2}, SPI::DRDY{GPIO::PortB, GPIO::Pin4}),
		initSPIDevice(DRV_GYR_DEVTYPE_ICM20602, SPI::CS{GPIO::PortF, GPIO::Pin3}, SPI::DRDY{GPIO::PortC, GPIO::Pin5}),
		initSPIDevice(DRV_GYR_DEVTYPE_BMI055, SPI::CS{GPIO::PortF, GPIO::Pin4}, SPI::DRDY{GPIO::PortB, GPIO::Pin14}),
		initSPIDevice(DRV_ACC_DEVTYPE_BMI055, SPI::CS{GPIO::PortG, GPIO::Pin10}, SPI::DRDY{GPIO::PortB, GPIO::Pin15}),
	}, true),
	initSPIBus(2, {
		initSPIDevice(SPIDEV_FLASH(0), SPI::CS{GPIO::PortF, GPIO::Pin5})
	}),
	initSPIBus(4, {
		initSPIDevice(DRV_BARO_DEVTYPE_MS5611, SPI::CS{GPIO::PortF, GPIO::Pin10}),
	}),
	initSPIBusExternal(5, {
		SPI::CS{GPIO::PortI, GPIO::Pin4},
		SPI::CS{GPIO::PortI, GPIO::Pin10},
		SPI::CS{GPIO::PortI, GPIO::Pin11}
	}),
	initSPIBusExternal(6, {
		SPI::CS{GPIO::PortI, GPIO::Pin6},
		SPI::CS{GPIO::PortI, GPIO::Pin7},
		SPI::CS{GPIO::PortI, GPIO::Pin8}
	}),
};

This is then mapped to a constant data-structure that the drivers can query. Currently they use bus_options structs with lots of ifdefs. The PR replaces this by a generic bus iterator class, which can be used to generically select internal/external I2C/SPI buses. For example the 1. external SPI bus can be selected with:

SPIBusIterator bus_iterator(SPIBusIterator::FilterType::ExternalBus, 1, 1);
if (bus_iterator.next()) {
  g_dev = new PMW3901(bus_iterator.bus().bus, bus_iterator.devid(), (enum Rotation)0);
}

Details & Goals

• The matching between driver and configured device is done via existing DRV_*_DEVTYPE_*. This has pro's and con's:
  • pro: It's using already existing ID's, and is simpler to add new ones.
  • con: there's typically different definitions for gyro and accel, and we need to select which one to use consistently (I used the gyro now).
• It's a bit tricky for drivers supporting multiple devices, like ms5611 and ms5607 that do auto-detection. It works, but both sides (driver+board config) need to use the same IDs.
• For external SPI, the chip-select signal can be selected via CLI
• With the current implementation there can only be 1 SPI device of a certain type per bus. This can be extended, but I did not want to make things more complicated than required.
• it works on Linux targets as well
• while having to go through all drivers, I think it makes sense to also add a bit more structure to them. The 2. commit adds an I2C/SPI driver base class, with multi-instance support, reliable driver stopping, and fits into the existing driver structure. However it does not fit into the existing module base structure (or the refactored version) - but it is possible to add instances to the refactored linked list of modules, if we want to.
• cleanup boot output to only show what devices were found
• the configuration can be used to check at runtime if expected devices are running. It will need to be extended with board revision information though.

Steps to complete

Note: this is obviously far from complete. It runs on v3 and v5 with a small set of drivers.
I think we can go in steps with this:

• extend boards with new config
• update drivers
• remove previous board config

@dagar @davids5

[dagar]

• The matching between driver and configured device is done via existing DRV_*_DEVTYPE_*. This has pro's and con's:
  • pro: It's using already existing ID's, and is simpler to add new ones.
  • con: there's typically different definitions for gyro and accel, and we need to select which one to use consistently (I used the gyro now).

We only need one DEVTYPE per physical interface, and in fact handling both has made more of a mess. I think we could migrate them safely with a hook at parameter import time.

It's a bit tricky for drivers supporting multiple devices, like ms5611 and ms5607 that do auto-detection. It works, but both sides (driver+board config) need to use the same IDs.

I think we should consider explicitly not supporting these cases (at the cost of flash short term). In many cases it starts off simple enough, but gradually gets worse over time as we discover the devices aren't so similar after all. We saw this happen when the mpu9250 driver was extended to support the icm20689. Partial list of differences between Invensense drivers (#14064).

Depending on how we want to handle old boards and variants we'll need a mechanism to handle alternate sensors on a chip select. The ms5607 could be started only if the ms5611 fails, or ideally with a different board manifest entirely.

[davids5]

@bkueng - this looks great!

All the resets will need to be verified with a scope.

We have to come to an agreement on what is FMUx compatible. The remapping of CS can be avoided with separate bin files. If we enhance the boot loaded it can do the HW detection an supply the alternate ID. or we just add targets more BL targets.

[davids5]

@bkueng Can you bake in the HW version here and simplify the select logic?

[bkueng]

I added the versions in c3531af. This makes it indeed much cleaner and allows to use the generic chip-select code (not done yet).

[davids5]

These needs to be tested with a scope! Also have a look at v5x it had multiple power domains.

[davids5]

@bkueng How does the code size look?

[UAV-Pilot]

While the SPI/I2C configuration is being improved, I hope it can also be considered to solve 2 issues I encountered while working on code changes in pull request #14168 .

The 1st issue is that currently it's possible for one SPI device driver to execute code that should be only applied to I2C driver of the same device. Here is an example, mpu9250 is started in SPI mode with command like "mpu9250 -S start", and you would expect that the driver would only execute code for SPI driver, not code for I2C driver. However in USE_I2C macro is defined in mpu9250.h as the following:

#if defined(PX4_I2C_OBDEV_MPU9250) || defined(PX4_I2C_BUS_EXPANSION)

define USE_I2C

#endif

When PX4_I2C_BUS_EXPANSION is also defined in board_config.h for another sensor, then USE_I2C will be defined for mpu9250 and I2C portion of the driver code will be executed although you intend to use mpu9250 in SPI code.

The 2nd issue is that SPI driver and I2C driver of the same device cannot be used on the same board. PX4 supports redundant sensors, so this issue could arise. For example, when running out available SPI interfaces, one might want to hook up the sensor using I2C bus. Although usually a different sensor is used as backup sensor, one could choose to use the same sensor with I2C interface. Also there are reports that multiple IMU sensors arranged in different orientations could improve accuracy, so it might be possible to use multiple same IMU devices in the future.

[bkueng]

When PX4_I2C_BUS_EXPANSION is also defined in board_config.h for another sensor, then USE_I2C will be defined for mpu9250 and I2C portion of the driver code will be executed although you intend to use mpu9250 in SPI code.

Yes, this definitely needs to be fixed. I'll look into it.

The 2nd issue is that SPI driver and I2C driver of the same device cannot be used on the same board.

This will be possible as well. Are you referring to the same issue as above (USE_I2C)?

[bkueng]

Closing this, as it is brought in in separate parts.