[RFC] Multi interface devices #47655
Conversation
bjarki-andreasen
requested review from
tbursztyka,
nashif,
mmahadevan108,
dleach02 and
galak
as code owners
|
This is a list of issues that this solution should solve
Feel free to add to this list, or suggest changes to it |
| required: false | ||
| description: Username for network connection context | ||
|
|
||
| password: |
There was a problem hiding this comment.
having a username/password dts feels wrong.
There was a problem hiding this comment.
It is not possible to configure APN during runtime (at least yet), so it has to be here or in a KConfig, the reason it is here is to allow for multiple modem instances, which could use different APNs, I can imagine having two identical modems on different networks for redundancy for example
There was a problem hiding this comment.
A username/password may be needed to integrate with existing hardware, but we should design APIs to comply (support?) with each of the provisions in ETSI EN 303645: CYBER;
Cyber Security for Consumer Internet of Things:
Baseline Requirements (https://www.etsi.org/deliver/etsi_en/303600_303699/303645/02.01.01_60/en_303645v020101p.pdf)
bjarki-andreasen
force-pushed
the
drivers-add-multi-interface-dev
branch
2 times, most recently
from
8a6eab6 to
4618c56
Compare
|
What about multi-function devices? For example, Nuvoton's TCPC (type-C port controller) is an I2C addressable device that includes the TCPC and a GPIO port expander. Would something like this fit into the model you're proposing? |
Yes :) |
Actually driver like one for sc28c94 would provide maximal four UART devices per driver instance, described, configured and enabled in device tree. Obviously also that "a single SPI to translate the in and outbound UART data, so they must be synchronized within a single device" is task of the driver.
What exactly is the problem? How to implement driver for external UART controller? We have drivers for external GPIO controllers, pca9555 and sn74hc595, perhaps these can serve as an example. For BG96, there are tree independent UART interfaces: Usually a GNNS driver would have a specific UART as parent node, or a GNNS service would access UART directly (how the application determines UART is its own problem, e.g. using /chosen node). The same for LTE modem. |
The sc28c94 only has exactly four uarts, you could also use an id like uart@0, uart@1 etc. However, in this example, i chose to go the more descriptive route, informing the user that only 4 distinct UARTs exist. Not sure what the issue with the driver is here, the parent device has the driver, which uses the device tree to find its up to 4 child devices (UARTs) as shown in the draft driver included in this PR
The pin controllers and I2C bus APIs are designed for addressable child devices which all are interacted with identically. Multi-functional-devices are not busses, their multiple functions are not necessarily identical, and are therefore not addressable. Take the UART for example, the API functions only take a device pointer, not an address, so for the sc28c94, you can not choose which of them to send or receive from, contrary to I2C, which takes a pointer to the parent, and then the address of the child. The pin cntrl takes a pin number, which is like an address.
If you look to the example of the BG96, the nmea-uart = &uart1; ties the specific uart to a device, the BG96 parent is placed on the "main" uart, which is used for AT commands. This is the same way clocks, GPIOs etc. are referenced for things like UARTs through params like clocks = <&clock1>;
Yes, for many projects, for modems, the UART is exposed, and the drivers are placed in the application. This is not a portable solution however. The point of this PR is to allow for drivers to handle the AT commands and NMEA frame parsing, exposing APIs like location, socket offloading, GPIO, file systems etc. decoupling the application from the specific modem. The main issue is that setup of the modems, like position update rates, power modes etc. are also handled through the UART, which should be part of the driver. The application should not know if it has to send "$PMTK225,42F" or "$PAIR00339" to turn of the modem, and which pins to toggle, the app should just get the parsed lat or long, and use the PM to turn it on or off. |
There was a problem hiding this comment.
As discussed on the Architecture WG meeting, I don't see this proposed standardization on multi-function devices (MFDs) really bringing in anything new.
We already support MFDs in-tree using a similar approach, but without the limitations proposed here (see e.g. https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/adc/ti%2Clmp90100.yaml and https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/gpio/ti%2Clmp90xxx-gpio.yaml, there are others too).
I don't see any reason for restricting DTS bindings for the same MFD to be located in the same subfolder. I'd much rather have them in their respective device class dts bindings directory, as this makes it easier to get an overview of e.g. all our GPIO controller bindings.
What we really need to consider is whether the current Zephyr device driver model, which enforces a one-to-one relationship between a devicetree node and a struct device along with a one-to-one relationship between a struct device and an API struct, is sufficient to support MFDs going forward. This limitation causes us to having to reflect the division of MFD support into various drivers in the devicetree (as also shown here). This causes a MFD devicetree for use in Zephyr having to deviate from the devicetree for the same MFD used in the Linux kernel, as the Linux kernel does not have this restriction (one devicenode can support multiple device classes/APIs).
The new thing being added is the standardization itself, a page in the Documentation (and two helper macros to the device.h), describing best practices for creating MFDs using the currently available features of the Zephyr project. It is supposed to bring clarity to developers wishing to create MFDs, so they don't all have to go through the same discussions, designs etc. as I have done. If this documentation existed, I would not have needed to bring it up, and we could have started writing the driver for the BG96 immediately. The further discussion regarding changing the 1-1 binding is largely irrelevant to adding this documentation, since it is not currently possible and it is not required to create MFDs as you demonstrated in your example (and I in mine). It is an important discussion, and something that would be nice to have, but we can't wait until that becomes a reality (if it does). I agree that the rule to place parent and child nodes in the same folder was a bad idea, and i have replaced it with simply placing it on a bus with the name of the parent like in your example. |
bjarki-andreasen
force-pushed
the
drivers-add-multi-interface-dev
branch
from
4618c56 to
21984dc
Compare
I don't see any documentation being added by this RFC? The two helper macros seem fine, but they can easily be implemented where needed by using the existing DT macros and as such do not solve the issue of implementing MFD drivers. The The idea of instantiating all devices for a MFD in the same driver is already used in-tree, please see https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/gpio/gpio_xlnx_axi.c for an example. |
Cool, so the proposed solution is already used in Zephyr, nearly as described in this RFC, and the solution is scalable to fit all of the usecases I have listed during the discussions, and no even better solution has been proposed. Absolutely perfect. Having the helper functions in the device.h will help link to the MFD documentation, which is primarily where it will be used. Otherwise we must create a new file or use another existing file for the macros, IMO this is a worse solution. The worst solution would be to repeat this macro in every MFD driver. With the standardization of the design, the macros will be used extensively, helping with readability and understanding, since the macros in general are not easy to read, and have little to no description in the drivers. This RFC does not contain any formal documentation, as it is meant for discussion on the driver level, the formal documentation will be based on this RFC, and be in its own pull request. I forgot to move the draft driver to the serial folder, that has been updated. |
Some devices require more than one interface to present all of their functionalies. This can be a SPI to 4x UART transceiver, which presents 4 distinct uart devices. A struct device only provides one API, so 4 devices must be created to allow for the application to use them. However, the IC uses a single SPI to translate the in and outbound UART data, so they must be synchronized within a single device. This proposal provides a standardized simple solution to this problem, using only what is already provided by the zephyr build system, in a transparent and intuitive fasion. Summarized, the solution is to create a parent device, which then exposes the additional interfaces through its child devices. To ensure that it is obvious to a developer that the device is a composite device (single device with multiple interfaces), these rules must be followed: 1. A composite device has the bus binding set to its name, it is placed in the folder which matches its primary function if it has one, otherwise it is placed in a folder named mfd 2. A component device uses the on-bus binding to indicate that it belongs to the composite device, it is placed in the folder which matches its function. 3. A component (child device) has the same name as the composite device, followed by - and the name of the component 4. The composite bindings yaml file ends with a commented out section, which shows a detailed example of the composite device and its components in a fake device tree Following these rules, it should be easy to spot a composite device, and easy to figure out how to include it in a device tree The proposal also includes a draft implementation of the bindings for the nxp,sc28c94 and bg96 composite devices, alongside a draft driver for the nxp,sc28c94 to show how the driver could be implemented. The BG96 is a cellular modem with built in GNSS, which requires two interfaces, and if cellular location is used, three interfaces. Two new functions have also been added to the device.h file which allows for easily getting the child devices of a composite device Please come with any suggestions to improve this solution, or propose a complimentary one for discussion :) Signed-off-by: Bjarki Arge Andreasen <bjarkix123@gmail.com>
bjarki-andreasen
force-pushed
the
drivers-add-multi-interface-dev
branch
from
21984dc to
509140a
Compare
|
I think the general rules look good. Have some small tweaks in some cases, but will make those comments once you create a PR with a doc update that includes them. |
| username: | ||
| type: string | ||
| required: false | ||
| description: Username for network connection context | ||
|
|
||
| password: | ||
| type: string | ||
| required: false | ||
| description: Password for network connection context |
There was a problem hiding this comment.
Without commenting on the meat of the PR: a friendly but unambiguous no way to these properties :). Not now, not ever, not on my watch :). CC @d3zd3z
- While this isn't strictly speaking a security hole since the password property will not automatically be stored in the firmware, it is just too tempting a target. Someone somewhere will save into ROM "just for now"; I can't in good conscience encourage this antipattern.
- IMO this is a pain to manage from a provisioning standpoint. Are we really going to expect every single device to get its own firmware build just to pass in an overlay for the unique password (the password is unique per device, right? 🙂 )
- This is also a pain from a flexibility standpoint. This might work for you, but other people are going to want to configure their software differently, e.g. getting the password from a special piece of on-chip flash with anti-readout protections, or from an EEPROM, or ...
TL;DR this is the sort of software configuration that doesn't belong in DT, and it is a security smell to boot.
There was a problem hiding this comment.
This is currently done using KConfig, which to my knowledge has the exact same issues as you mention here :) The username and password are like WIFI passwords, tied to the mobile network provider. All units using the same provider will use the same username/password, if required. Note that the username and password are largely no longer used, and most APNs and their accompanying passwords are public information https://www.hw-group.com/support/international-apn-settings-for-mobile-broadband-network-operators, so i doubt it is a security risk to store them outside of EEPROM or similar.
Usually, mass manufactured devices have soldered on ESIM chips which have a fixed APN/Username/Password, so it can be tied directly with the device through firmware.
Overall, this is just a parameter, like baudrate, I can not see what would differentiate this property from any other property directly tied to an instance of a device.
With that said, i do agree that it should be possible to change during runtime, which would remove it from KConfig and the device tree, which would also open up the possibility of creating routers or similar, which require the ability to change SIM card and APN during runtime, but for now, this is how it is done.
There was a problem hiding this comment.
This is currently done using KConfig, which to my knowledge has the exact same issues as you mention here :)
The presence of bad behavior elsewhere is not a justification for introducing it into a devicetree binding.
The username and password are like WIFI passwords, tied to the mobile network provider. All units using the same provider will use the same username/password, if required.
Then it doesn't seem like this is per-instance modem configuration, but rather per-mobile-network information, which also argues against having them here.
most APNs and their accompanying passwords are public information https://www.hw-group.com/support/international-apn-settings-for-mobile-broadband-network-operators
OK, thanks for explaining that. That does indeed change the security aspect of this in most cases, I guess. However, "most" is not "all".
Usually, mass manufactured devices have soldered on ESIM chips which have a fixed APN/Username/Password, so it can be tied directly with the device through firmware.
Right, which is exactly my point about flexibility, which is basically that putting this in DT doesn't really make sense if people are going to want to do it in other ways, and in fact that seems to be the case here, which is not surprising.
which require the ability to change SIM card and APN during runtime,
In general, devicetree describes the hardware and its boot time configuration. Just because something can change at runtime is not -- in general -- a reason to remove its boot time value from the DT. However again I still don't think this should go here.
but for now, this is how it is done.
My -1 remains, sorry
There was a problem hiding this comment.
Well, can you propose a better solution to the issue? Keep in mind that two modems will use two different SIM cards potentially from different network providers for redundancy.
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
The goal of this proposal is to create a standardized design
and to set a precedent for how to create drivers and models
for multi-functional-devices, which expose one or more APIs
pr device.
The formal documentation will be created based on this RFC,
in its own pull request once the design details are in place.
Some devices require more than one interface to present
all of their functionalies. This can be a SPI to 4x UART
transceiver, which presents 4 distinct uart devices. A
struct device only provides one API, so 4 devices must
be created to allow for the application to use them.
However, the IC uses a single SPI to translate the
in and outbound UART data, so they must be synchronized
within a single device.
This proposal provides a standardized simple solution to
this problem, using only what is already provided by
the zephyr build system, in a transparent and intuitive
fasion.
Summarized, the solution is to create a parent device,
which then exposes the additional interfaces through
its child devices.
To ensure that it is obvious to a developer that the
device is a composite device (single device with
multiple interfaces), these rules must be followed:
name, it is placed in the folder which matches its
primary function if it has one, otherwise it is placed
in a folder named mfd
indicate that it belongs to the composite device, it
is placed in the folder which matches its function.
the composite device, followed by - and the name
of the component
commented out section, which shows a detailed
example of the composite device and its components
in a fake device tree
Following these rules, it should be easy to spot a
composite device, and easy to figure out how to include
it in a device tree
The proposal also includes a draft implementation of the
bindings for the nxp,sc28c94 and bg96 composite devices,
alongside a draft driver for the nxp,sc28c94 to show how
the driver could be implemented.
Furthermore, the design descibed here is already in use by
the following driver:
https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/gpio/gpio_xlnx_axi.c
and in the following bindings:
https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/adc/ti%2Clmp90100.yaml
https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/gpio/ti%2Clmp90xxx-gpio.yaml
Note here that there is only one driver pr multi functional
device, which first instanciates all child devices, then itself,
the child devices are just used for storing cfgs, data and the
APIs.
The BG96 is a cellular modem with built in GNSS, which
requires two interfaces, and if cellular location is used,
three interfaces.
Two new functions have also been added to the device.h file
which allows for easily getting the child devices of a
composite device
Please come with any suggestions to improve this solution, or
propose a complimentary one for discussion :)
Signed-off-by: Bjarki Arge Andreasen bjarkix123@gmail.com