Skip to content

STM32WBAX PM rework to support STOP/STDBY modes with radio activity #92847

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

STM32WBAX PM rework to support STOP/STDBY modes with radio activity #92847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions boards/st/nucleo_wba55cg/nucleo_wba55cg.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@

&clk_hse {
status = "okay";
hse-div2;
};

&clk_hsi {
Expand All @@ -109,9 +108,9 @@

&rcc {
clocks = <&clk_hse>;
clock-frequency = <DT_FREQ_M(16)>;
clock-frequency = <DT_FREQ_M(32)>;
ahb-prescaler = <1>;
ahb5-prescaler = <2>;
ahb5-prescaler = <1>;
apb1-prescaler = <1>;
apb2-prescaler = <2>;
apb7-prescaler = <1>;
Expand Down
9 changes: 6 additions & 3 deletions boards/st/nucleo_wba65ri/nucleo_wba65ri.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,15 @@
};
};

&clk_lsi {
status = "okay";
};

&clk_lse {
status = "okay";
};

&clk_hse {
hse-div2;
status = "okay";
};

Expand All @@ -94,9 +97,9 @@

&rcc {
clocks = <&clk_hse>;
clock-frequency = <DT_FREQ_M(16)>;
clock-frequency = <DT_FREQ_M(32)>;
ahb-prescaler = <1>;
ahb5-prescaler = <2>;
ahb5-prescaler = <1>;
apb1-prescaler = <1>;
apb2-prescaler = <2>;
apb7-prescaler = <1>;
Expand Down
67 changes: 65 additions & 2 deletions drivers/bluetooth/hci/hci_stm32wba.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
#include <zephyr/drivers/bluetooth.h>
#include <zephyr/bluetooth/addr.h>
#include <zephyr/drivers/clock_control/stm32_clock_control.h>
#ifdef CONFIG_PM_DEVICE
#include <zephyr/pm/policy.h>
#include <zephyr/pm/device.h>
#include <zephyr/pm/pm.h>
#include "linklayer_plat.h"
#endif
#include <linklayer_plat_local.h>

#include <zephyr/sys/byteorder.h>
Expand Down Expand Up @@ -454,6 +460,52 @@
}
#endif /* CONFIG_BT_HCI_SETUP */

#ifdef CONFIG_PM_DEVICE
static int radio_pm_action(const struct device *dev, enum pm_device_action action)
{
switch (action) {
case PM_DEVICE_ACTION_RESUME:
LL_AHB5_GRP1_EnableClock(LL_AHB5_GRP1_PERIPH_RADIO);
#if defined(CONFIG_PM_S2RAM)
if (LL_PWR_IsActiveFlag_SB() == 1U) {
/* Put the radio in active state */
link_layer_register_isr();
}
#endif
LINKLAYER_PLAT_NotifyWFIExit();
ll_sys_dp_slp_exit();
break;
case PM_DEVICE_ACTION_SUSPEND:
#if defined(CONFIG_PM_S2RAM)
uint32_t radio_remaining_time = 0;
enum pm_state state = pm_state_next_get(_current_cpu->id)->state;
if (state == PM_STATE_SUSPEND_TO_RAM) {

Check warning on line 482 in drivers/bluetooth/hci/hci_stm32wba.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LINE_SPACING 

drivers/bluetooth/hci/hci_stm32wba.c:482  Missing a blank line after declarations

Check warning on line 482 in drivers/bluetooth/hci/hci_stm32wba.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LINE_SPACING 

drivers/bluetooth/hci/hci_stm32wba.c:482  Missing a blank line after declarations

Check warning on line 482 in drivers/bluetooth/hci/hci_stm32wba.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LINE_SPACING 

drivers/bluetooth/hci/hci_stm32wba.c:482  Missing a blank line after declarations
/* */
uint32_t cmd_status =
ll_intf_le_get_remaining_time_for_next_event(&radio_remaining_time);
UNUSED(cmd_status);
assert_param(cmd_status == SUCCESS);

if (radio_remaining_time == LL_DP_SLP_NO_WAKEUP) {
/* No next radio event scheduled */
(void)ll_sys_dp_slp_enter(LL_DP_SLP_NO_WAKEUP);
} else /* if (radio_remaining_time > RADIO_DEEPSLEEP_WAKEUP_TIME_US) */ {
/* No event in a "near" futur */
(void)ll_sys_dp_slp_enter(radio_remaining_time -
CFG_LPM_STDBY_WAKEUP_TIME);
}
}
#endif
LINKLAYER_PLAT_NotifyWFIEnter();
break;
default:
return -ENOTSUP;
}

return 0;
}
#endif /* CONFIG_PM_DEVICE */

static DEVICE_API(bt_hci, drv) = {
#if defined(CONFIG_BT_HCI_SETUP)
.setup = bt_hci_stm32wba_setup,
Expand All @@ -462,11 +514,22 @@
.send = bt_hci_stm32wba_send,
};


#if defined (CONFIG_PM_DEVICE)

Check warning on line 518 in drivers/bluetooth/hci/hci_stm32wba.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

SPACING 

drivers/bluetooth/hci/hci_stm32wba.c:518  space prohibited between function name and open parenthesis '('

Check warning on line 518 in drivers/bluetooth/hci/hci_stm32wba.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

SPACING 

drivers/bluetooth/hci/hci_stm32wba.c:518  space prohibited between function name and open parenthesis '('

Check warning on line 518 in drivers/bluetooth/hci/hci_stm32wba.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

SPACING 

drivers/bluetooth/hci/hci_stm32wba.c:518  space prohibited between function name and open parenthesis '('
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#if defined (CONFIG_PM_DEVICE)
#if defined(CONFIG_PM_DEVICE)

#define HCI_DEVICE_INIT(inst) \
static struct hci_data hci_data_##inst = {}; \
PM_DEVICE_DT_INST_DEFINE(inst, radio_pm_action); \
DEVICE_DT_INST_DEFINE(inst, NULL, PM_DEVICE_DT_INST_GET(inst), &hci_data_##inst, NULL, \
POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEVICE, &drv)

/* Only one instance supported */
HCI_DEVICE_INIT(0)
#else
#define HCI_DEVICE_INIT(inst) \
static struct hci_data hci_data_##inst = { \
}; \
static struct hci_data hci_data_##inst = {}; \
DEVICE_DT_INST_DEFINE(inst, NULL, NULL, &hci_data_##inst, NULL, \
POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEVICE, &drv)

/* Only one instance supported */
HCI_DEVICE_INIT(0)
#endif
29 changes: 29 additions & 0 deletions samples/bluetooth/peripheral_hr/boards/nucleo_wba55cg.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* Copyright (c) 2023 STMicroelectronics
*/

/ {
/* Change min residency time to ease power consumption measurement */
cpus {
cpu0: cpu@0 {
cpu-power-states = <&stop0 &stop1 &standby>;
};

power-states {
standby: state2 {
compatible = "zephyr,power-state";
power-state-name = "suspend-to-ram";
substate-id = <1>;
min-residency-us = <10000>;
exit-latency-us = <1000>;
};
};
};

};

&lptim1 {
status = "okay";
};
Comment on lines +1 to +29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, all these changes should be moved in the dtsi file of the device since they are not board specific.
In particular, the standby power state should be added where stop 0 and stop 1 are defined.
LPTIM should be selected when CONFIG_PM is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed and for LPTIM we could use PM config options to enable/disable LPTIM (board or soc dts )

28 changes: 28 additions & 0 deletions samples/bluetooth/peripheral_hr/boards/nucleo_wba65ri.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* Copyright (c) 2023 STMicroelectronics
*/

/ {
/* Change min residency time to ease power consumption measurement */
cpus {
cpu0: cpu@0 {
cpu-power-states = <&stop0 &stop1 &standby>;
};

power-states {
standby: state2 {
compatible = "zephyr,power-state";
power-state-name = "suspend-to-ram";
substate-id = <1>;
min-residency-us = <10000>;
exit-latency-us = <1000>;
};
};
};
};

&lptim1 {
status = "okay";
};
Comment on lines +1 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar considerations done for nucleo_wba55cg.overlay

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as before

6 changes: 6 additions & 0 deletions samples/bluetooth/peripheral_hr/prj.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,9 @@ CONFIG_BT_BAS=y
CONFIG_BT_HRS=y
CONFIG_BT_DEVICE_NAME="Zephyr Heartrate Sensor"
CONFIG_BT_DEVICE_APPEARANCE=833

CONFIG_PM=y
CONFIG_PM_DEVICE=y
CONFIG_PM_DEVICE_RUNTIME=y
CONFIG_PM_DEVICE_SYSTEM_MANAGED=y
CONFIG_PM_S2RAM=y
Comment on lines +11 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these should be moved to a specific file related to the relevant NUCLEO, if we want to enable power management by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just leave CONFIG_PM to be selected by user in each sample and use it to conditionally enable all the rest of configs in nucleo_wba65ri_defconfig

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CONFIG_PM_S2RAM could be implyed by CONFIG_PM at SoC level.

Or even better: imply only if the corresponding state is status = "okay" in DTS. (but not sure that status = "disabled" on a power state really disables it)

7 changes: 2 additions & 5 deletions soc/st/stm32/stm32wbax/hci_if/linklayer_plat_adapt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@

#include <stm32_backup_domain.h>

#include "scm.h"
#include "app_conf.h"
#include "bsp.h"

#define LOG_LEVEL CONFIG_SOC_LOG_LEVEL
LOG_MODULE_REGISTER(linklayer_plat_adapt);
Expand Down Expand Up @@ -245,17 +246,13 @@ void LINKLAYER_PLAT_StartRadioEvt(void)
__HAL_RCC_RADIO_CLK_SLEEP_ENABLE();

NVIC_SetPriority((IRQn_Type)RADIO_INTR_NUM, RADIO_INTR_PRIO_HIGH_Z);

scm_notifyradiostate(SCM_RADIO_ACTIVE);
}

void LINKLAYER_PLAT_StopRadioEvt(void)
{
__HAL_RCC_RADIO_CLK_SLEEP_DISABLE();

NVIC_SetPriority((IRQn_Type)RADIO_INTR_NUM, RADIO_INTR_PRIO_LOW_Z);

scm_notifyradiostate(SCM_RADIO_NOT_ACTIVE);
}

/* Link Layer notification for RCO calibration start */
Expand Down
Loading
Loading