RIOT-OS / RIOT

RIOT - The friendly OS for IoT
https://riot-os.org
GNU Lesser General Public License v2.1
4.9k stars 1.98k forks source link

[RFC] Common device handler + device power management #15298

Closed jia200x closed 2 years ago

jia200x commented 3 years ago

Description

Some OS (Zephyr OS, Mynewt, Linux, etc) define a "device handler" that represents an abstract device (network device, peripheral, etc). This is basically a device_t structure from which all devices can inherit and which would be useful for the following purposes:

Regarding Power Management, we could use such structures to implement centralized power management (e.g the OS indicates that the CPU can go to LPM and therefore, that indication function could try to turn off all devices) or decentralized power management (each applications or component takes care of the underlying peripheral).

I think the idea of having device handlers has been around for some time. I remember having some offline conversations with @MichelRottleuthner and @haukepetersen . It would be nice if something concrete could come up :).

An example with the IEEE 802.15.4 Radio HAL

A radio could extend the device handler like this:

typedef struct {
    int                (*pm_set)(device_t *dev, pm_mode_t mode);
    pm_mode_t (*pm_get)(device_t *dev);
} pm_driver_t;

typedef struct {
    const pm_driver_t driver;
    const uint8_t pm_caps; /* stores the supported power modes */
    bool busy; /* Check if the device is busy and therefore, cannot go to low power mode */
    int id; /* device identifier */
} device_t;

/* ieee802154/radio.h ---------------------------------------------------------------------------------------- */

typedef struct {
    device_t dev; /* extends device */
    /* all the other fields of the Radio HAL go here */
} ieee802154_dev_t;

/* my_ieee802154_radio.h  ----------------------------------------------------------------------------------------*/
typedef struct {
    ieee802154_dev_t dev; /* extends radio HAL */
    int foo;
} my_ieee802154_radio_t;

/* device.h  ----------------------------------------------------------------------------------------*/

/* set the Power Mode of a given device */
int device_pm_set(device_t *dev, pm_mode_t mode);

/* get the Power Mode of a given device */
pm_mode_t device_pm_get(device_t *dev);

/* check if a device can go to a low power mode */
bool device_pm_is_busy(device_t *dev);

/* iterate over all device */
device_t *device_iter(device_t *last_dev);

/* main.c  ----------------------------------------------------------------------------------------*/

int main(void) {
    /* Get first device */
    device_t *device = device_iter(NULL);
    if (device->id = MY_IEEE802154_RADIO) {
        ieee802154_dev_t *hal = (device_t*) device;
        /* do something with the radio ... */
    }

    /* Try to go to sleep if it's not in the middle of a transaction */
    if (!device_pm_is_busy(device)) {
        device_pm_set(device, DEEP_SLEEP);
    }
}

Note this is only a draft to roughly illustrate the concept.

Please also note

Does the concept make sense? Opinions?

fjmolinas commented 3 years ago

I like the general concept you are proposing here, I have indeed seen this abstraction in many other RTOS.

But how does power management approach fits with the current pm_layered approach>

One thing is controlling the devices power modes, but how can the cpu power management system (pm_layered as of now0 know which of the devices power modes are compatible with its own power modes. An examples of this would be:

benpicco commented 3 years ago

We also need a way to tag a device as a wake-up source. So when the device goes to sleep, it would e.g. shut off it's radio before, but keep an accelerometer running to wake the device up if it detects movement. Although it might also enter a special sleep configuration where the polling rate is reduced, so no special tagging, just an (optional) sleep config would be needed.

jia200x commented 3 years ago

But how does power management approach fits with the current pm_layered approach>

I think both should be complementary. In some cases there are indeed overlaps (e.g integrated radios), but I think it should be always possible to cooperate between pm_layered and device handlers. For example, a radio could "block" the SPI device (also a device_t), which could block at the same time the power modes of the CPU. So, the CPU knows that it cannot go to sleep because the radio is doing something.

maribu commented 3 years ago

I also like the idea. However, I would like the generic device to be more minimal and only contain an id and flags. The flags could be used to indicate if the device e.g. has a power management integration. Maybe something like this:

typedef struct {
     /**
      * @brief A locally unique and stable device identifier
      *
      * Layout:
      * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      * Generic layout:
      *    0                   1                   2                   3
      *    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      *   |  Device Type  |    Other fields depending on device type      |
      *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      *
      * Default layout (unless other layout given for a specific device type,
      * this layout is used):
      *    0                   1                   2                   3
      *    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      *   |  Device Type  |  Manufacturer | Part ID               | P_IDX |
      *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      *
      *   Device Type  = ID of the device type
      *   Manufacturer = ID of the manufacturer - only unique within the same device type
      *   Part ID      = ID of the part - only unique in conjunction with the type and manufacturer
      *   P_IDX        = Index in the drivers params array
      * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      *
      */
    uint32_t id;
    uint32_t flags; /**< Flags indicating the features / subsequent fields of the devices */
} device_t;

A unique and stable device ID is something that would be immediately useful e.g. for generation of stable L2 addresses. I suggest to construct the ID from a type id, manufacturer id, part id, and the index in the params array the device was initialized with. If the manufacturer id would only be needed to be unique within its type, I think we can get away with 8 bit. And most of the time less than 4096 different devices of the same type by the same manufacturer are expected and less than 32 instances of the same item connected to the board. However, there is no need to specify this structure once for every device type. E.g. for the device type "MCU" we won't need a params index field, but maybe an CPU architecture field would be handy.

I also believe that not every device is relevant for power management. Let's say we have an MCU that is equipped with a globally unique EUI-64 somewhere in the ROM. I would like that to also have a device_t - but that won't work if the device_t is big or mandates features like power management.

Btw: I'm very much looking forward to an lshw shell command for RIOT :-) I have wished for that a long time. A widespread adoption of a device_t would make this very much feasible.

I believe @fengelhardt will like this proposal very much :-)

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.