Adding support for multi-functional-devices

[bjarki-andreasen]

Introduction

Currently, only one API can be mapped to each instance of a struct device. This proposal provides two solutions which together allow for multiple APIs pr device instance, and the creation of devices which consist of multiple sub devices.

Problem description

Some devices provide multiple functions, which require multiple APIs pr single device instance. Some devices consist of multiple internal devices, which all require their own handles and APIs. There is no general design guideline for creating such devices, and creating devices with multiple APIs pr instance is not currently possible. This is preventing the creation of proper drivers for cellular and GNSS modems which do provide many varying features which can not be efficiently covered by a single API, since the functionalities are ever expanding and vary highly between devices.

Proposed change

The first solution is simply a design guideline for devices which consist of multiple sub devices. The change includes adding a documentation page describing this guideline, and adding some macros to help keep the driver design consistent. The specific docs and macros are contained in this PR #48932

   bus {
       parent {
          child1 {
          }; 
          child2 {
          };
          child3 {
          };
       };
   };

The second solution creates one struct device for each API implemented by a single device instance. The struct devices share all data except for the API pointer. Their names reflect the API they implement, and they are referenced using a function similar to DEVICE_DT_GET(node_id), called DEVICE_DT_API_GET(node_id, api_type), which returns the struct device which implements the specified API type if it has been defined by a driver, using DEVICE_DT_API_DEFINE(api_type, node_id, api_ptr)

struct api_foo;
struct api_bar;

DEVICE_DT_DEFINE(node_id, ... NULL)
DEVICE_DT_API_DEFINE(foo, node_id, &api_foo)
DEVICE_DT_API_DEFINE(bar, node_id, &api_bar)

struct device *foo_dev = DEVICE_DT_API_GET(foo, node_id);
struct device *bar_dev = DEVICE_DT_API_GET(bar, node_id);

The API struct devices are stored in ROM, grouped by API type, allowing for iterating through all devices which support a specific API type, which is handy for the shell for example.

Solution 2 requires no changes to the existing API headers, adds no overhead to the usage of API wrappers, while allowing for multiple APIs pr device instance

int ret = foo_var_get(&foo_dev);
ret = bar_dev_set(&bar_dev);

Detailed RFC

The first solution requires little extra explaining, since it is literally a docs page :)

The second solution requires updating the linker files to add sections for the API struct devices, and an update to the device.h file to include the new macros for defining and getting struct devices. Adding convenience macros to the driver API headers like LOCATION_DEVICE_DT_API_DEFINE(node_id, api_ptr) should also be done, but is not required.

struct api_loc;

DEVICE_DT_DEFINE(node_id, ... NULL)
LOCATION_DEVICE_DT_API_DEFINE(node_id, &api_loc)

Proposed change (Detailed)

See #48817 for example implementation of an API using solution 2

1. Add documentation and macros for solution 1
2. Add linker file which will contain API iterable sections
3. Update device.h to include new macros for solution 2
4. Add convenience macros to driver API headers
5. Start transition period from old solution to these solutions
6. Update in-tree drivers to use DEVICE_DT_API_DEFINE for APIs, and set DEVICE_DT_DEFINE api_ptr to NULL
7. Transition period ends
8. Remove api_ptr from DEVICE_DT_DEFINE
9. Use api ptr in struct device from DEVICE_DT_DEFINE as init function
10. update linker and device.c to use struct devices with init instead of an additional list as is does now
11. Success

Dependencies

The only dependency which we can not control is the out-of-tree drivers which must be updated in the transition period.

Concerns and Unresolved Questions

Alternatives

Come forth with your alternatives here :)

[galak]

Want to make sure we reconcile various cases that exist in tree today:

• networking - NET_DEVICE_DT_INST_DEFINE*
• EMUL - EMUL_DT_INST_DEFINE / EMUL_DT_GET
• I2C / CAN - CAN_DEVICE_DT_DEFINE/CAN_DEVICE_DT_INST_DEFINE
• pm - PM_DEVICE_DT_INST_DEFINE, PM_DEVICE_DT_GET

[gmarull]

Adding one use case to the discussion.

STM32 RCC / GD32 RCU (Reset and clock control unit) is a single block of hardware that controls both clocks and reset lines for multiple peripherals. From a Devicetree perspective, it is described as a single node:

zephyr/dts/arm/st/h7/stm32h7.dtsi

Lines 114 to 118 in 9a7e4b1

	rcc: rcc@58024400 {
	compatible = "st,stm32h7-rcc";
	#clock-cells = <2>;
	reg = <0x58024400 0x400>;
	};

or in Linux:

		rcc: reset-clock-controller@58024400 {
			compatible = "st,stm32h743-rcc", "st,stm32-rcc";
			reg = <0x58024400 0x400>;
			#clock-cells = <1>;
			#reset-cells = <1>;
			clocks = <&clk_hse>, <&clk_lse>, <&clk_i2s>;
			st,syscfg = <&pwrcfg>;
		};

ref. https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/stm32h743.dtsi#L537

It is used like this:

zephyr/dts/arm/st/h7/stm32h7.dtsi

Lines 369 to 378 in fd14236

	spi2: spi@40003800 {
	compatible = "st,stm32h7-spi", "st,stm32-spi-fifo", "st,stm32-spi";
	#address-cells = <1>;
	#size-cells = <0>;
	reg = <0x40003800 0x400>;
	clocks = <&rcc STM32_CLOCK_BUS_APB1 0x00004000>,
	<&rcc STM32_SRC_PLL1_Q SPI123_SEL(0)>;
	interrupts = <36 0>;
	status = "disabled";
	};

or in Linux:

		spi2: spi@40003800 {
			#address-cells = <1>;
			#size-cells = <0>;
			compatible = "st,stm32h7-spi";
			reg = <0x40003800 0x400>;
			interrupts = <36>;
			resets = <&rcc STM32H7_APB1L_RESET(SPI2)>;
			clocks = <&rcc SPI2_CK>;
			status = "disabled";
		};

ref. https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/stm32h743.dtsi#L107

Note that Zephyr doesn't have a reset driver for STM32 yet, so we can't find any resets = ... in Zephyr. And... here comes the problem. In the current device model, we have 1 device per DT node. While it makes sense in most cases, this is an arbitrary decision forced by the Zephyr device model. That means, DEVICE_DT_GET(DT_NODELABEL(rcc)) can only resolve to a single device instance (either clock or reset, but not both). If one expects to maintain DT compatibility with Linux, Zephyr device model doesn't work for this case.

One could argue that RCC is a multi-function device (mfd), like e.g. syscon, meaning it could have 2 subnodes one for clock control and the other for reset control. Something like:

rcc: reset-clock-controller@58024400 {
	  compatible = "st,stm32-rcc";
	  reg = <0x58024400 0x400>;

	  rcc_clk: clock-controller {
		    compatible = "st,stm32-rcc-clock";
		    #clock-cells = <1>;
		    clocks = <&clk_hse>, <&clk_lse>, <&clk_i2s>;
	  };

	  rcc_reset: reset-controller {
		    compatible = "st,stm32-rcc-reset";
		    #reset-cells = <1>;
	  };
};

spi2: spi@40003800 {
	  #address-cells = <1>;
	  #size-cells = <0>;
	  compatible = "st,stm32h7-spi";
	  reg = <0x40003800 0x400>;
	  interrupts = <36>;
	  resets = <&rcc_reset STM32H7_APB1L_RESET(SPI2)>;
	  clocks = <&rcc_clock SPI2_CK>;
	  status = "disabled";
};

This would work with Zephyr device model because we have again 1 device per DT node. But it breaks DT compatibility with Linux (note that for STM32 it's not compatible nowadays, e.g. for pinctrl).

[gmarull]

Some more notes I think can be useful to the discussion.

In Linux one can find MFD (multi-function devices):

Multi-Function Devices (MFD)

These devices comprise a nexus for heterogeneous hardware blocks containing
more than one non-unique yet varying hardware functionality.

A typical MFD can be:

- A mixed signal ASIC on an external bus, sometimes a PMIC (Power Management
  Integrated Circuit) that is manufactured in a lower technology node (rough
  silicon) that handles analog drivers for things like audio amplifiers, LED
  drivers, level shifters, PHY (physical interfaces to things like USB or
  ethernet), regulators etc.

- A range of memory registers containing "miscellaneous system registers" also
  known as a system controller "syscon" or any other memory range containing a
  mix of unrelated hardware devices.

Optional properties:

- compatible : "simple-mfd" - this signifies that the operating system should
  consider all subnodes of the MFD device as separate devices akin to how
  "simple-bus" indicates when to see subnodes as children for a simple
  memory-mapped bus. For more complex devices, when the nexus driver has to
  probe registers to figure out what child devices exist etc, this should not
  be used. In the latter case the child devices will be determined by the
  operating system.

- ranges: Describes the address mapping relationship to the parent. Should set
  the child's base address to 0, the physical address within parent's address
  space, and the length of the address map.

- #address-cells: Specifies the number of cells used to represent physical base
  addresses. Must be present if ranges is used.

- #size-cells: Specifies the number of cells used to represent the size of an
  address. Must be present if ranges is used.

Example:

foo@1000 {
	compatible = "syscon", "simple-mfd";
	reg = <0x01000 0x1000>;

	led@8.0 {
		compatible = "register-bit-led";
		offset = <0x08>;
		mask = <0x01>;
		label = "myled";
		default-state = "on";
	};
};

ref. https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/mfd/mfd.txt

An example:

    #include <dt-bindings/clock/bcm-nsp.h>
    cru-bus@1800c100 {
        compatible = "brcm,ns-cru", "simple-mfd";
        reg = <0x1800c100 0x1d0>;
        ranges;
        #address-cells = <1>;
        #size-cells = <1>;

        clock-controller@100 {
            #clock-cells = <1>;
            compatible = "brcm,nsp-lcpll0";
            reg = <0x100 0x14>;
            clocks = <&osc>;
            clock-output-names = "lcpll0", "pcie_phy", "sdio", "ddr_phy";
        };

        clock-controller@140 {
            #clock-cells = <1>;
            compatible = "brcm,nsp-genpll";
            reg = <0x140 0x24>;
            clocks = <&osc>;
            clock-output-names = "genpll", "phy", "ethernetclk", "usbclk",
                                 "iprocfast", "sata1", "sata2";
        };

        phy@164 {
            compatible = "brcm,ns-usb2-phy";
            reg = <0x164 0x4>;
            brcm,syscon-clkset = <&clkset>;
            clocks = <&genpll BCM_NSP_GENPLL_USB_PHY_REF_CLK>;
            clock-names = "phy-ref-clk";
            #phy-cells = <0>;
        };

        clkset: syscon@180 {
            compatible = "brcm,cru-clkset", "syscon";
            reg = <0x180 0x4>;
        };

        pinctrl@1c0 {
            compatible = "brcm,bcm4708-pinmux";
            reg = <0x1c0 0x24>;
            reg-names = "cru_gpio_control";
        };

        thermal@2c0 {
            compatible = "brcm,ns-thermal";
            reg = <0x2c0 0x10>;
            #thermal-sensor-cells = <0>;
        };
    };

So, it looks like the modem case described a few days ago would fall into this category. In practice, this means that we could simply have multiple regular devices, each one implementing a particular API, so no need to have devices with multiple APIs.

Another point is power management. Some MFD devices may allow turning off its units independently (again, may because I have no idea, if assumption is wrong forget about this). If that's true, a single device with multiple APIs would be problematic because the PM API operates on a device basis.

[bjarki-andreasen]

@gmarull The point of having both solutions is exactly because some multi functional devices are better described as independent nodes, and some devices are single nodes, but require a multitude of APIs to fully be utilized.

The solution 1 fits perfectly with Linux MFDs.
The second solution is for devices like cellular modems, creating a single API for such a device is impractical, since some support GPS, SMS, Networking, Cellular properties like imei etc. The second solution allows for creating tailored composite APIs for a device, like a C# class inheriting a bunch of interfaces.

[gmarull]

@gmarull The point of having both solutions is exactly because some multi functional devices are better described as independent nodes, and some devices are single nodes, but require a multitude of APIs to fully be utilized.

So if they're independent nodes, why can't we have a device per node?

The solution 1 fits perfectly with Linux MFDs. The second solution is for devices like cellular modems, creating a single API for such a device is impractical, since some support GPS, SMS, Networking, Cellular properties like imei etc. The second solution allows for creating tailored composite APIs for a device, like a C# class inheriting a bunch of interfaces.

Btw, it would be useful if you could share the datasheet of such modem.

[bjarki-andreasen]

@gmarull The point of having both solutions is exactly because some multi functional devices are better described as independent nodes, and some devices are single nodes, but require a multitude of APIs to fully be utilized.

So if they're independent nodes, why can't we have a device per node?
We can?

The solution 1 fits perfectly with Linux MFDs. The second solution is for devices like cellular modems, creating a single API for such a device is impractical, since some support GPS, SMS, Networking, Cellular properties like imei etc. The second solution allows for creating tailored composite APIs for a device, like a C# class inheriting a bunch of interfaces.

Btw, it would be useful if you could share the datasheet of such modem.
You can look to the driver for the simcom7080 modem, specifically the amount of APIs that have to be in a custom header since the device is limited to one zephyr/drivers/modem/simcom7080

[bjarki-andreasen]

The working implementation of the (possible) future of the device model :) #49374 Please review it at your leisure :) Automated tests are should be done before next dev meeting

[gmarull]

So I've been thinking about this use case, and I have the following suggestion that requires no modifications to the current device model.

First for DT:

&uart0 {
	vnd-modem@.... {
		compatible = "vnd,modem";
		modem-prop = <0xff>;
		...
		gps {
			compatible = "vnd,modem-gps";
			gps-prop = <0xee>;
			...
		};

		gpio-controller {
			compatible = "vnd,modem-gpio";
			gpio-controller;
			#gpio-cells = <2>;
			...
		};
	};
};

Then, you can create one device per each MFD entry and still share parent driver's data/config:

/* modem data/config (will be used by all child devices) */
struct vnd_modem_data {
	/* potentially arbitration (mutex/sem) for bus access */
	...
};

struct vnd_modem_config {
	const struct device *uart;
	int modem_prop;
};

/* gps data/config */
struct vnd_modem_gps_data {
	...
};

struct vnd_modem_gps_config {
	/* reference to parent modem device */
	const struct device *modem;
	int gps_prop;
};

/* gpio data/config */
struct vnd_modem_gpio_data {
	...
};

struct vnd_modem_gpio_config {
	/* reference to parent modem device */
	const struct device *modem;
};

/* modem init function */
static int vnd_modem_init(const struct device *dev)
{
	const struct vnd_modem_config *config = dev->config;

	if (!device_is_ready(config->uart)) {
		return -ENODEV;
	}

	/* any common init code/sequence here */
}

/* modem GPS init function */
static int vnd_modem_gps_init(const struct device *dev)
{
	const struct vnd_modem_gps_config *config = dev->config;

	if (!device_is_ready(config->modem)) {
		return -ENODEV;
	}

	/* any GPS init code/sequence here */
	/* we can access `config->modem` data/config! */
}

/* modem GPIO init function */
static int vnd_modem_gpio_init(const struct device *dev)
{
	const struct vnd_modem_gpio_config *config = dev->config;

	if (!device_is_ready(config->modem)) {
		return -ENODEV;
	}

	/* any GPIO init code/sequence here */
	/* we can access `config->modem` data/config! */
}

/* modem init function */
static int vnd_modem_init(const struct device *dev)
{
	const struct vnd_modem_config *config = dev->config;

	if (!device_is_ready(config->uart)) {
		return -ENODEV;
	}

	/* any common init code/sequence here */
}

#define VND_MODEM_DEFINE(n)                                                    \
	struct vnd_modem_data vnd_modem_data##n;                               \
                                                                               \
	const struct vnd_modem_config modem_config##n = {                      \
		.uart = DT_INST_BUS(n),                                        \
	};                                                                     \
                                                                               \
	DEVICE_DT_INST_DEFINE(n, vnd_modem_init, NULL, &vnd_modem_data##n,     \
			      &vnd_modem_config##n, POST_KERNEL, N, NULL);

#define VND_MODEM_GPS_DEFINE(n)                                                \
	struct vnd_modem_gps_data vnd_modem_gps_data##n;                       \
                                                                               \
	const struct vnd_modem_gps_config modem_gps_config##n = {              \
		.modem = DEVICE_DT_GET(DT_INST_BUS(n)),                        \
		.gps_prop = DT_INST_PROP(n, gps_prop),                         \
	};                                                                     \
                                                                               \
	DEVICE_DT_INST_DEFINE(                                                 \
		n, vnd_modem_gps_init, NULL, &vnd_modem_gps_data##n,           \
		&vnd_modem_gps_config##n, POST_KERNEL, N + 1, &gps_api);

#define VND_MODEM_GPIO_DEFINE(n)                                               \
	struct vnd_modem_gpio_data vnd_modem_gpio_data##n;                     \
                                                                               \
	const struct vnd_modem_gpio_config modem_gpio_config##n = {            \
		.modem = DEVICE_DT_GET(DT_INST_BUS(n)),                        \
	};                                                                     \
                                                                               \
	DEVICE_DT_INST_DEFINE(                                                 \
		n, vnd_modem_gpio_init, NULL, &vnd_modem_gpio_data##n,         \
		&vnd_modem_gpio_config##n, POST_KERNEL, N + 1, &gpio_api);

/* instantiate modem (parent) device */
#define DT_DRV_COMPAT vnd_modem
DT_INST_FOREACH_STATUS_OKAY(VND_MODEM_DEFINE)
#undef DT_DRV_COMPAT

/* instantiate modem gps (child) device */
#define DT_DRV_COMPAT vnd_modem_gps
DT_INST_FOREACH_STATUS_OKAY(VND_MODEM_GPS_DEFINE)
#undef DT_DRV_COMPAT

/* instantiate modem gpio (child) device */
#define DT_DRV_COMPAT vnd_modem_gpio
DT_INST_FOREACH_STATUS_OKAY(VND_MODEM_GPIO_DEFINE)
#undef DT_DRV_COMPAT

[bjarki-andreasen]

@gmarull

This is the nearly identical to the solution proposed in this PR #48932, which does indeed not require any changes.

The multi API model however is solving the usecase for multiple APIs pr instance, this solution solves MFDs, by creating three distinct instances, which share context. The Multi API device model is trying to create C++ style objects, which can derive multiple APIs, like I2C and I3C and PM.

The multi API device model also creates three destinct instances in ROM which share the same context, but they all use the same devicetree node identifier, and since they are declared in properties files, the build system knows to generate macros and externs for them.