Adding support for multi-functional-devices #48934
Comments
bjarki-andreasen
added
the
RFC
|
Want to make sure we reconcile various cases that exist in tree today:
|
|
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 or in Linux: 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 or in Linux: 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 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: 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). |
|
Some more notes I think can be useful to the discussion. In Linux one can find MFD (multi-function devices): ref. https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/mfd/mfd.txt An example: 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. |
|
@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. |
So if they're independent nodes, why can't we have a device per node?
Btw, it would be useful if you could share the datasheet of such modem. |
|
|
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 |
|
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: 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 |
|
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. |
The example above also solves the same problem. This is important to note, since than means Zephyr can solve your problem, even if not the way your prefer. It also allows us to weight in solutions that involve breaking the device model, including adding runtime overhead. Is it worth doing all these changes for such particular usecase? To me clearly not, unless I’m given a more convincing example.
Afaik C++ does not work exactly the same way multi-API proposal is in its current state.
Again, in the code above parent context is also shared. |
You misunderstood my comment. I am not arguing that any particular implementation of this specific solution is better, I am highlighting that the MFD solutions are essentially the same, and how they differ from the multi API solution. This is a good thing, as it implies agreement. The PR #48932 contains an addition to the docs describing the solution you suggested, please review it and help formulating the solution. I am well aware how C++ solves the issue, maybe I should have said "any object oriented programming language" |
|
I feel like solution 2 covers the range of needs a bit better, it more simply allows APIs to share context because of the shared struct data. This is really important for many of the multi-function devices that are not necessarily able to do multiple things at once, despite it making sense to split the APIs based on the different functionality. For example the SX1276 is a multi-modulation radio, which is currently supported under the LoRa APIs, however this chip is capable of many more. No additional modulations are currently supported but if they were, they cannot be supported at the same time, the device would need to switch configuration. Having them supported within the same device instance but with separate APIs would allow the driver to intelligently switch the device configuration when the different APIs are used. Of course that specific functionality could be omitted, but perhaps it would be valuable to include API hooks for setup and tear down functions for each API, to formalise support for such devices? I can think of many devices that would require this functionality This is different to the GPS/GSM modem example you have given where both can happen at the same time, or at least the modem deals with the coexistence of the two internally. |
|
Related but not yet linked here: there is a discussion with a good overview of the problem and its solutions so far in #50641. |
ghost
mentioned this issue
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
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)
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
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.
Proposed change (Detailed)
See #48817 for example implementation of an API using solution 2
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 :)
The text was updated successfully, but these errors were encountered: