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

soc: st: stm32: stm32wbax: STM32WBA Cube 1.4.1 FW integration #80068

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

asm5878
Copy link

@asm5878 asm5878 commented Oct 18, 2024

STM32WBA Cube FW 1.4.1 integration

Added new functions required by STM32WBA Cube FW 1.4.1 Link Layer for WFI notification management and clock source selection

@zephyrbot
Copy link
Collaborator

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_stm32 zephyrproject-rtos/hal_stm32@6f0e5f7 (main) zephyrproject-rtos/hal_stm32#237 zephyrproject-rtos/hal_stm32#237/files

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest manifest-hal_stm32 DNM This PR should not be merged (Do Not Merge) labels Oct 18, 2024
Comment on lines 34 to 36
uint8_t AHB5_SwitchedOff = 0;
uint32_t radio_sleep_timer_val = 0;

Copy link
Member

Choose a reason for hiding this comment

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

This is strange to have these defined here and set as extern in Cube.
I'd expect the opposite (Cube code isn't supposed to depends on Zephyr code )

Copy link
Author

Choose a reason for hiding this comment

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

Just retrieving code form Cube FW....

@asm5878 asm5878 marked this pull request as draft October 21, 2024 13:18
@asm5878 asm5878 force-pushed the STM32WBA_1_4_1_Cube_Alignment branch 4 times, most recently from b9fb0a9 to 393fdd3 Compare October 21, 2024 16:10
Comment on lines 325 to 338
void LINKLAYER_PLAT_NotifyWFIEnter(void)
{
/* Check if Radio state will allow the AHB5 clock to be cut */

/* AHB5 clock will be cut in the following cases:
* - 2.4GHz radio is not in ACTIVE mode (in SLEEP or DEEPSLEEP mode).
* - RADIOSMEN and STRADIOCLKON bits are at 0.
*/
if ((LL_PWR_GetRadioMode() != LL_PWR_RADIO_ACTIVE_MODE) ||
((__HAL_RCC_RADIO_IS_CLK_SLEEP_ENABLED() == 0) &&
(LL_RCC_RADIO_IsEnabledSleepTimerClock() == 0))) {
AHB5_SwitchedOff = 1;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This is a STM32Cube function implemented using STM32Cube APIs, it should exist in STM32Cube package and matching file should be included as part of https://github.com/zephyrproject-rtos/hal_stm32/tree/main/lib/stm32wba/hci.

This should STM32Cube specific code should be maintained outside of Zephyr and forked.
Risk of including such code is that maintenance will be complex as we can't get code evolutions.
Also, it raises License questions.

Same for functions introduced in soc/st/stm32/stm32wbax/hci_if/ll_sys_if.c.

Copy link
Author

Choose a reason for hiding this comment

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

A better separation has been done to move STM32CUBE API based code inside https://github.com/zephyrproject-rtos/hal_stm32/tree/main/lib/stm32wba/hci

@asm5878 asm5878 requested a review from erwango October 23, 2024 15:22
@asm5878 asm5878 marked this pull request as ready for review October 23, 2024 15:22
@erwango erwango added this to the v4.0.0 milestone Oct 24, 2024
Files renaming done to better isolate zephyr related
functions from stm32 hal related functions

Signed-off-by: Alessandro Manganaro <[email protected]>
@asm5878 asm5878 force-pushed the STM32WBA_1_4_1_Cube_Alignment branch from 269e26f to 3646af7 Compare October 24, 2024 15:37
Comment on lines 210 to 214
void LINKLAYER_PLAT_Assert(uint8_t condition)
{
__ASSERT_NO_MSG(condition);
}

Copy link
Member

Choose a reason for hiding this comment

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

This should be kept.
It allows to generate Zephyr ASSERTS

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

Comment on lines -7 to 9
#include <zephyr/irq.h>
#include <zephyr/kernel.h>
#include <zephyr/arch/cpu.h>
#include <zephyr/sys/util.h>
#include <zephyr/drivers/entropy.h>
#include <zephyr/logging/log.h>
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure file will compile w/o this ?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, It compiles

Removed unnecessary pure HAL stm32 functions

Headers cleanup

Signed-off-by: Alessandro Manganaro <[email protected]>
@asm5878 asm5878 force-pushed the STM32WBA_1_4_1_Cube_Alignment branch from 3646af7 to e27d3a4 Compare October 24, 2024 22:30
@asm5878 asm5878 requested a review from erwango October 24, 2024 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DNM This PR should not be merged (Do Not Merge) manifest manifest-hal_stm32 platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants