RFC: DEFINE_DT_DEVICE_SUB for definition of sub-devices that do not have state or power management, as it is served by main device

[de-nordic]

Introduction

The goal of the proposal is to gather input and comments on addition of DEFINE_DT_DEVICE_SUB themed macros that would allow to define sub-device, which is device defined within bounds of other device, and is a device that itself does not have power management nor state as all of that features are actually owned by the main device.
Sub-device here may be considered software construct that is representing some functionality within a main device.
The proposal here links to PR that shows possible use and pinpoints current problems with implementations.

Problem description

There are device that actually part of other device, for example soc-nv-flash compatible devices are defined, often, within flash-controller device and do not have own state, as their state depends on the state of controller.
Other examples of devices may be RTC devices that often combine RTS, battery backed SRAM and small amount of EEPROM, which all rely on the power state of the device but themselves have no such state.

Lack of support for such devices is confusing when attempting to using DEVICE_DT_GET or similar macros as you do not target the device you are trying to work with, but rather a parent, for example when accessing flash0 labeled node that is soc-nv-flash compatible you have to do:

DEVICE_DT_GET(DT_PARENT(DT_NODELABLE(flash0)))

while when the same label is placed on SPI NOR, the invocation would look like:

DEVICE_DT_GET(DT_NODELABLE(flash0))

Proposed change

Proposed change is to add subset of DEFINE_DT_DEVICE macros that would allow to define sub devices that defines no power management state and takes device state from parent/main device.

Detailed RFC

Here I am providing source code #64089 that shows how such sub-device can be defined, with the soc-nv-flash compatible device being the target of the experiment.

Proposed change (Detailed)

This section is freeform - you should describe your change in as much detail
as possible. Please also ensure to include any context or background info here.
For example, do we have existing components which can be reused or altered.

By reading this section, each team member should be able to know what exactly
you're planning to change and how.

Dependencies

The change would require removing static from the definition of Z_DEVICE_STATE_DEFINE to allow referencing it from the sub-device.

Concerns and Unresolved Questions

The device does not have PM state and relies on the state of main device. This means that application may not be able to set power state of the sub device. The problem is that such device should not have own state as its state is the state of main device.
The problem with current implementation of PM is that to be able to pass PM calls from the sub-device to the main device, as should happen here, there is need for some significant pm_device type object to be attached to a device that does not support it at all, to just have a callback function that would be able to pass the PM actions to the main device.

There may be preliminary work required with moving the action_cb out of struct pm_device directly into struct device, and some rework of pm_device_action_run as the function does some basic checks on PM state object before calling the callback.

Note also that support for callback separate from the PM object may require a lot of changes to the code, as the PM callback would have to be provided via DEVICE_DT_DEFINE macros, which means that argument list for these macros needs to be expanded.

Alternatives

Leaving things as they are. The problem with such approach is that often it is hard to add instances of devices, considered here sub-devices, as they are actually represented by the main device. The main device, as for example flash controller mentioned above, often exists as a single instance in DTS, so making it multiple instance device in driver would be rather bad solution.

[tbursztyka]

This would require a thorough review of all possible use-cases to get so a relevant solution. It's an issue that has been here forever. It would be indeed nice to tackle this.
Anyway, you assume that it does not need PM, but I remember seeing devices that could have sub-devices with the ability to be put off/on.
Another issue that could perhaps exist is a driver for a device that can exist either as a standalone device or as a sub-device. Then we need a way (through DTS) to mitigate this.
There are other issues, but these are mostly due to the way things are done currently so not from your idea per-se.

[de-nordic]

This would require a thorough review of all possible use-cases to get so a relevant solution. It's an issue that has been here forever.

I have logged this to just gather that info and get some interest. I am not PM expert in Zephyr, but I have stumbled upon this when attempting to do rework of soc_nrf_flash device and while trying to do software devices.

It would be indeed nice to tackle this. Anyway, you assume that it does not need PM, but I remember seeing devices that could have sub-devices with the ability to be put off/on. Another issue that could perhaps exist is a driver for a device that can exist either as a standalone device or as a sub-device. Then we need a way (through DTS) to mitigate this. There are other issues, but these are mostly due to the way things are done currently so not from your idea per-se.

Yeah, so that is why the RFC is here to gather info like that, and straighten out what I got wrong.

Actually I think that you should be able to ask sub-devices to go to any state you want, because that is what you want to do to save energy (wake -> pm_device_runtime_get, make_sleep -> pm_device_runtime_put).:

    wake(some_dev);
    some_api_call(some_dev);
    ...
    some_other_api_call(some_dev);
    make_sleep(some_dev);

right? User knows best when device is no longer used in the code, at the same time want to have the same approach being used whether device is sub-device or not. By user I mean user code, sub-device drivers and sub-systems, for example VFS.
The difference would be that sub-device, as we call it now, would pass the request up, to the parent.
This means that the parent, probably, needs to be able to keep track on level of wakeup, so that something else, using other sub-device, would not put it to sleep while in use. But does it? Is it possible to put device asleep from other thread while used?
Note also that this is what exactly sub-devices would do when accessing parent device, they would wake it up when needed.

What I mean by no need to hold own state is to hold own state managed by a system, they may have own internal state or, somehow, get one from parent. If you have N sub-devices that are always on when parent is on and off when parent is off, defining state for N devices makes no sense: there is only one device that can be turned on/off.
(that was already kinda discussed here: #60709).

Now, of course, there is a problem on how do we apply state to every device in a system, do we also iterate over sub-devices? Does it makes sense if they will pass the state to parent? Or maybe we should iterate over the sub-devices only? No that does not make sense, not all devices will have sub-devices. So maybe on real/parent devices only? But then these have to ask the sub-devices if they still need power state.

The cases I have been fighting with

First case: SoC flash controllers, soc_flash_nrf in particular - when you address a flash device in SoC you actually address the SoC flash/NVMS controller rather than device; this gives at least two problems:

1. Counter to intuitive DTS definition you do not address the flash device you address a parent; this means that when you have several flash device definitions, under the controller.
2. The driver for flash controller has to figure out what you are trying to access. In case of the soc_nrf_flash driver you have two devices under that controller: the device flash and the UICR region. Because you can not have separate devices for them what happens you have ugly code that basically decides by device offset what you are trying to access. This gets ugly on the driver side and user side, as user can not simply get UICR device and write to it using API, it gets flash controller API, then gets UICR offset and then writes to flash controller device at that offset.
   Here is what, for example, flash write implementation does in that driver:
   

   zephyr/drivers/flash/soc_flash_nrf.c

   Lines 174 to 185 in c33743e

   	static int flash_nrf_write(const struct device *dev, off_t addr,
   	const void *data, size_t len)
   	{
   	int ret;
   	if (is_regular_addr_valid(addr, len)) {
   	addr += DT_REG_ADDR(SOC_NV_FLASH_NODE);
   	} else if (!is_uicr_addr_valid(addr, len)) {
   	LOG_ERR("invalid address: 0x%08lx:%zu",
   	(unsigned long)addr, len);
   	return -EINVAL;
   	}

   
   note that UICR check has to happen everywhere where op is taken.
   I can make that two devices, each with one state. Just two additional pointless objects the system will have to know about. And they actually will be incorrect often: for example if i make UICR on, the flash PM object will indicate it off. In reality they are both on, because the flash-controller is the one that has the state.

Second case: software flash drivers to replace flash map. So here we have device that have no state at all, they are completely software construct. still generated from DTS definitions. Basically they are constrains on flash access that limits user to specific
area, allowing user to access flash map the same way as flash device - using Flash API. Flash area devices were supposed to completely represent properties of a device the partition has been defined on, except for size:
https://github.com/zephyrproject-rtos/zephyr/pull/36487/files#diff-4385f578d197abbc9e90480ff29b8525b26400ce17c79b1f82d6124ca7c648d0
As you probably guess that software fixed partition device would not call any action on parent till it got with all internal checks, so it would not wake the parent till needed either.
So if you ask to wake up a flash area device, as in the code far above this comment, what should probably happen is that a flash area device should pass that request to parent, but it should only do that when it really needs so.

Other known case: monotonic counters and DS18x20

So both are external devices that provide more than one function. In first case you will have some counter that goes forever and additional small SRAM and probably EEPROM; in the second case you have similar stuff, except there is thermometer instead of the counter.
So we can either look at them as two devices of their own kind with special API that access all features or combined devices that have several sub-devices representing each feature independently - this way we assign the features to different APIs and have smaller API for the primary function.
Whatever we do, there is only single power state owner for them - it is the device that has the solder pads.

Case I have met as a "customer"

There is combined external device that is hanging on one of external serial buses, let say SPI. The device main purpose, in this case, is to provide 1PPS signal calibrated and precisely synchronized with other devices at various locations.
The device itself has two power states: on and off.
The 1PPS generator is a sub-device, and can be turned on/off independently from the entire device, and there are several other devices that represent some ROM, EEPROMs (configuration, etc.), different SRAM devs (states, rolling buffers, temperature, etc).
To access any of above you have to have device on; this means that before you do access you wake up device on every driver for above features and make it sleep when no longer used; that is whether 1PPS is enabled or not, because when you have the 1PPS enabled then the device is on all the time all sub devices are accessible, yet you are still going to do the same wake-up/access/make sleep sequences everywhere - because they are independent sub devices, sharing one power state.

Controversial thought

No state defined for any device, just a callback to function that affects state. OK, calm down, I know state is needed, what I mean is that it is not defined with device and accessed by system directly: the driver defines own state object and manipulates it according to events sent by the power management callback the driver provides.
This means that drivers are free to define state objects as they wish, but what they provide to others is only a way to send an event.

[de-nordic]

And the third thing I was trying to do but completely forgot about: I was trying to change soc-nv-flash into generic driver that would serve all soc-nv-flash devices by passing calls to parent. This way you could have multiple instance of the device, even with different controllers.