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

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

Closed
jia200x opened this issue Oct 26, 2020 · 6 comments
Closed

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

jia200x opened this issue Oct 26, 2020 · 6 comments
Assignees
Labels
Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR State: stale State: The issue / PR has no activity for >185 days

Comments

@jia200x
Copy link
Member

jia200x commented Oct 26, 2020

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:

  • Device registry: be able to fetch a device from a list of device. This can be used, for example, to bootstrap devices, offload ISR or get device descriptors.
  • Power Management: the power modes of a device could be controlled in a device independent way using generic device handlers.

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

  • The on and off functions of the HAL would be replaced by the device_t functions. This is specially useful for devices that expose more than one functionality (e.g some network devices expose a radio, a temperature sensor and a crypto accelerator in the same device)
  • The driver would block/unblock in order to tell the upper layer if it's feasible to go to low power.

Does the concept make sense? Opinions?

@leandrolanzieri leandrolanzieri added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Oct 27, 2020
@fjmolinas
Copy link
Contributor

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:

  • PMO (that would switch off or change the SPI clock speed) and a radio waiting for a message
  • Integrated radios/drivers and power modes that might gate or change the clock speed for those drivers
  • Power modes that change GPIO states

@benpicco
Copy link
Contributor

benpicco commented Oct 27, 2020

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
Copy link
Member Author

jia200x commented Oct 27, 2020

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
Copy link
Member

maribu commented Oct 28, 2020

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
Copy link

stale bot commented Jun 3, 2021

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 stale bot added the State: stale State: The issue / PR has no activity for >185 days label Jun 3, 2021
@fjmolinas fjmolinas removed the State: stale State: The issue / PR has no activity for >185 days label Jun 4, 2021
@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 22, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@stale
Copy link

stale bot commented Mar 2, 2022

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 stale bot added the State: stale State: The issue / PR has no activity for >185 days label Mar 2, 2022
@stale stale bot closed this as completed Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR State: stale State: The issue / PR has no activity for >185 days
Projects
None yet
Development

No branches or pull requests

9 participants