Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Device (de)initialization use case #40385

Closed
tbursztyka opened this issue Nov 16, 2021 · 21 comments
Closed

Device (de)initialization use case #40385

tbursztyka opened this issue Nov 16, 2021 · 21 comments
Labels
Architecture Review Discussion in the Architecture WG required area: API Changes to public APIs area: Device Management area: Device Model Enhancement Changes/Updates/Additions to existing features

Comments

@tbursztyka
Copy link
Collaborator

There seems to be a cross-over of uses-cases/requirements between 2 issues around device initialization, so gathering both here for tracking and discussion.

Let's summarize the requirements:

Add the ability to manually initialize a device:

This means adding 2 things:

  • marking a device as manually initialized
  • exposing a device_init() function

Add the ability to de-initialize a device:

  • exposing a device_deinit() function (or some other relevant name)

Cross-over requirements:

  • Forcing initialization: device_deinit() needs to be called prior to device_init()
  • In case PM is enabled: device_deninit() should call PM to put the device OFF (so all dependencies will also be properly affected)
  • In case PM is not enabled: we will need a better error-management so if a device is put down, things will not just explode (dependencies still trying to access the device etc...). Device APIs do not check for device status, so currently calling a device that has been de-initialized, would lead to unknown state.
@tbursztyka tbursztyka added Enhancement Changes/Updates/Additions to existing features area: Device Management area: Device Model labels Nov 16, 2021
@carlescufi carlescufi added the area: API Changes to public APIs label Nov 16, 2021
@carlescufi carlescufi changed the title Devices: initialization use case and enhancements (umbrella issue) Device (de)initialization use case Nov 16, 2021
@gregshue
Copy link

Is device de/initialization only concerned with the FW structures, or also the HW state?

Consider when application code inherits a running device initialized by boot code, and de-initialization is not an option. The simplest example of this is GPIOs, but it is effectively true of every HW block touched by boot code.

Consider also the scope of de-initialization. Does it return the HW block to a reset state? Does it clear out all pending interrupts? Does it make sure no pending DMA transactions are left hanging? What state should the HW be left in before boot code calls the application code?

@danielstuart14
Copy link
Contributor

This would be huge!
I have a use case here that I found, that has proven to be impossible using zephyr's SPI driver:

I'm porting Zephyr to a board that has an odd SPI communication between the main controller and the ble chipset (nordic nrf52832).

The BLE chipset (which I'm porting Zephyr) is the slave in the SPI bus, but it also uses the chipselect as a wake pin for the main controller.
When it wants to wake the main controller, the device disables the SPI peripheral, changes the CS pin direction and sends a low signal. After 50us, it changes back the CS pin direction and enable again the SPI. It does all of that using the NRFX SPIS driver.

One dev has suggest the use of dynamic pinctrl, but currently, as we don't have de-initialization, I can't use that.

@keith-zephyr
Copy link
Contributor

Looking the requirements listed, the "Cross-over requirements" seem to only apply to the de-initialization use case.

If the deferred initialization device_init() verifies that the specified device is not already initialized, then I believe the deferred initialization could be implemented now with no dependencies on the de-initialization.

The deferred initialization could always be updated if de-initialization support is added later.

I have a use case for supporting multiple board configurations with a single image. With deferred init, a devicetree could do the following:

device_a: device_a {
    compatible = "vnd,device-a";
    status = "okay";
    deferred-init;
};
device_b: device_b {
    compatible = "vnd,device-b";
    status = "okay";
    deferred-init;
}'

The application can the chose the device to initialize:

static int init_my_devices(void)
{
    if (board_revision == BOARD_REV_A) {
        rc = device_init(DEVICE_DT_GET(DT_NODELABEL(device_a));
    } else {
        rc = device_init(DEVICE_DT_GET(DT_NODELABEL(device_b));
    }
    return rc;
}
SYS_INIT(init_my_devices, POST_KERNEL, 50);

I'm happy to create an RFC and draft PR for the deferred initialization only use case.

@yashi
Copy link
Collaborator

yashi commented Apr 23, 2023

@lmajewski
Copy link
Collaborator

lmajewski commented Jul 10, 2023

Looking the requirements listed, the "Cross-over requirements" seem to only apply to the de-initialization use case.

If the deferred initialization device_init() verifies that the specified device is not already initialized, then I believe the deferred initialization could be implemented now with no dependencies on the de-initialization.

The deferred initialization could always be updated if de-initialization support is added later.

I have a use case for supporting multiple board configurations with a single image. With deferred init, a devicetree could do the following:

device_a: device_a {
    compatible = "vnd,device-a";
    status = "okay";
    deferred-init;
};
device_b: device_b {
    compatible = "vnd,device-b";
    status = "okay";
    deferred-init;
}'

The application can the chose the device to initialize:

static int init_my_devices(void)
{
    if (board_revision == BOARD_REV_A) {
        rc = device_init(DEVICE_DT_GET(DT_NODELABEL(device_a));
    } else {
        rc = device_init(DEVICE_DT_GET(DT_NODELABEL(device_b));
    }
    return rc;
}
SYS_INIT(init_my_devices, POST_KERNEL, 50);

I'm happy to create an RFC and draft PR for the deferred initialization only use case.

I'm also interested in such use case (more description here: #60209).

I also tend to agree that deferred (on demand) initialization in user programs shall be separated from de-init of device, as it would fulfill above requirement and would be easier to implement.

@lmajewski
Copy link
Collaborator

@carlescufi @keith-zephyr @tbursztyka - Has been there any ongoing development or at least some kind of a "road map" for the deferred initialization of drivers?

@carlescufi
Copy link
Member

@carlescufi @keith-zephyr @tbursztyka - Has been there any ongoing development or at least some kind of a "road map" for the deferred initialization of drivers?

We (Nordic) have plans to look at this in the near future @lmajewski. But we will welcome any help we can get!

@lmajewski
Copy link
Collaborator

@carlescufi @keith-zephyr @tbursztyka - Has been there any ongoing development or at least some kind of a "road map" for the deferred initialization of drivers?

We (Nordic) have plans to look at this in the near future @lmajewski. But we will welcome any help we can get!

The "near future" :-) doesn't seem like a fixed date ...

I would be interested in adding support for in-user application initialization of DTS disabled (but defined) devices on-demand (according to @keith-zephyr pseudo code example).

For my use case (described at #60209 in details) - I don't need the de-initialization feature.
Moreover, I think that this shall be separated from PM subsystem, as it is only for on-the-boot initialization (and this configuration would stay till the reboot).

@carlescufi - do you think that such approach (and tasks division) would be feasible?

@keith-zephyr
Copy link
Contributor

@carlescufi Can you add this discussion topic to the next Arch WG?

Specifically, can we get some consensus on a roadmap for some of the device model changes (deferrred init, de-init, PM handling, etc).

@carlescufi
Copy link
Member

@carlescufi Can you add this discussion topic to the next Arch WG?

Specifically, can we get some consensus on a roadmap for some of the device model changes (deferrred init, de-init, PM handling, etc).

@lmajewski @keith-zephyr sorry, was on vacation. I will schedule this for next week's arch wg meeting (August 1st). There we can discuss how to decouple it from PM and how to plan the work.

@lmajewski
Copy link
Collaborator

@carlescufi - is there any input or decision regarding this issue after the meeting?

@fabiobaltieri
Copy link
Member

@carlescufi - is there any input or decision regarding this issue after the meeting?

FYI this did not make it to last week meeting, let's tag it for one of the next ones.

@fabiobaltieri fabiobaltieri added the Architecture Review Discussion in the Architecture WG required label Aug 8, 2023
@lmajewski
Copy link
Collaborator

@carlescufi @fabiobaltieri - Is there any schedule for those meetings? Would it be possible to attend them ? (I do guess that those are held on discord?)

@fabiobaltieri
Copy link
Member

@nashif
Copy link
Member

nashif commented Oct 16, 2023

@keith-zephyr

device_a: device_a {
compatible = "vnd,device-a";
status = "okay";
deferred-init;
};
device_b: device_b {
compatible = "vnd,device-b";
status = "okay";
deferred-init;
}'

if you set the status to "reserved", it will not be started in whatever init level it is put, this seems to work right now already, so we could just focus on exposing the init API and do the right thing for such reserved instances later in the boot process.

@lmajewski
Copy link
Collaborator

if you set the status to "reserved", it will not be started in whatever init level it is put, this seems to work right now already, so we could just focus on exposing the init API and do the right thing for such reserved instances later in the boot process.

@nashif Is there any work-in-progress code or sketch of proposed API design ?

@nashif
Copy link
Member

nashif commented Oct 17, 2023

@nashif Is there any work-in-progress code or sketch of proposed API design ?

not yet.

@nashif
Copy link
Member

nashif commented Oct 17, 2023

device_a: device_a {
compatible = "vnd,device-a";
status = "okay";
deferred-init;
};
device_b: device_b {
compatible = "vnd,device-b";
status = "okay";
deferred-init;
}'

Thinking about this appraoch, while it appears to provide a solution that does not require us to define a new behaviour with something like "reserved", it might not give us what we want given how init of drivers is done right now.

With status set to okay, we have many things happening other than just starting a driver at a certain level, taking i2c_mcux driver as an example:


static int i2c_mcux_init(const struct device *dev)
{
.....
}


static const struct i2c_driver_api i2c_mcux_driver_api = {
        .configure = i2c_mcux_configure,
        .transfer = i2c_mcux_transfer,
#ifdef CONFIG_I2C_CALLBACK
        .transfer_cb = i2c_mcux_transfer_cb,
#endif
};

#define I2C_DEVICE_INIT_MCUX(n)                 \
        PINCTRL_DT_INST_DEFINE(n);                                      \
                                                                        \
        static void i2c_mcux_config_func_ ## n(const struct device *dev); \
                                                                        \
        static const struct i2c_mcux_config i2c_mcux_config_ ## n = {   \
                .base = (I2C_Type *)DT_INST_REG_ADDR(n),\
                .clock_source = I2C ## n ## _CLK_SRC,                   \
                .irq_config_func = i2c_mcux_config_func_ ## n,          \
                .bitrate = DT_INST_PROP(n, clock_frequency),            \
                .pincfg = PINCTRL_DT_INST_DEV_CONFIG_GET(n),            \
        };                                                              \
                                                                        \
        static struct i2c_mcux_data i2c_mcux_data_ ## n;                \
                                                                        \
        I2C_DEVICE_DT_INST_DEFINE(n,                                    \
                        i2c_mcux_init, NULL,                            \
                        &i2c_mcux_data_ ## n,                           \
                        &i2c_mcux_config_ ## n, POST_KERNEL,            \
                        CONFIG_I2C_INIT_PRIORITY,                       \
                        &i2c_mcux_driver_api);                          \
                                                                        \
        static void i2c_mcux_config_func_ ## n(const struct device *dev) \
        {                                                               \
                IRQ_CONNECT(DT_INST_IRQN(n),                            \
                        DT_INST_IRQ(n, priority),                       \
                        i2c_mcux_isr,                                   \
                        DEVICE_DT_INST_GET(n), 0);                      \
                                                                        \
                irq_enable(DT_INST_IRQN(n));                            \
        }

DT_INST_FOREACH_STATUS_OKAY(I2C_DEVICE_INIT_MCUX)

So if we want to use deferred-init to not add t the driver instance to the boot sequence, it will still do many other things in the case where the status of the instance is set to "okay", so this might be a problem. So deferred-init should just skip all of the above to avoid having structure created that might not be needed later (if we decide not to start anything based on detected hardware).

@HenrikKaa
Copy link

Not mentioned yet that this is pretty much a must-have for power management too. In many microcontrollers peripherals get reset in power down modes and trying to re-initialise them with the current code is not really possible.

@nashif
Copy link
Member

nashif commented Oct 17, 2023

ok, IMO we should close this issue and refer to the 2 issues being discuss in their respective open issues, otherwise it will continue to be confusing:

@carlescufi
Copy link
Member

ok, IMO we should close this issue and refer to the 2 issues being discuss in their respective open issues, otherwise it will continue to be confusing:

As per in the Architecture WG, closing this issue in benefit of #39896 and #20012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Review Discussion in the Architecture WG required area: API Changes to public APIs area: Device Management area: Device Model Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

10 participants