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

fix(PeriphDrivers, Other): Fix RTC clock source selection #1315

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

Conversation

mertvatansever
Copy link
Contributor

Description

This PR solves RTC clock source selection problem for ME30. It adds a function to set clock source and creates wrapper function for MXC_RTC_Init to select clock source for ME30.

@github-actions github-actions bot added MAX32657 Related to the MAX32655 (ME30) Zephyr MSDK Zephyr related change. labels Dec 23, 2024
@mertvatansever mertvatansever force-pushed the fix-me30-rtc-clock-source branch 2 times, most recently from 2c603e4 to f5ad283 Compare December 24, 2024 06:51

switch (clk_src) {
case MXC_RTC_ERTCO_CLK:
MXC_GCR->clkctrl |= MXC_F_GCR_CLKCTRL_ERTCO_EN;
Copy link
Contributor

@hfakkiz hfakkiz Dec 24, 2024

Choose a reason for hiding this comment

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

Please use "MSDK_NO_GPIO_CLK_INIT" macro in here. Also, I think you should use "MXC_SYS_ClockSourceEnable(...)" function to enable clock sources.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks

return clk_src;
}

static inline int WRAP_MXC_RTC_Init(uint32_t sec, uint16_t ssec, uint8_t clock_source)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update wrapper function names as "Wrap_MXC..." to have same name structure with other wrapper files.


static inline int WRAP_MXC_RTC_Init(uint32_t sec, uint16_t ssec, uint8_t clock_source)
{
int ret;
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
int ret;
(void)clock_source;
int ret;

Please add this line for unused parameters.

Copy link
Contributor

@hfakkiz hfakkiz Dec 24, 2024

Choose a reason for hiding this comment

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

I checked Zephyr side now and I think that adding if (clock_source != ADI_MAX32_PRPH_CLK_SRC_ERTCO) { return -1; } line would be nice to handle a bad parameter case.

Comment on lines 75 to 76
MXC_MCR->ctrl &= ~MXC_F_MCR_CTRL_CLKSEL;
MXC_MCR->ctrl |= MXC_S_MCR_CTRL_CLKSEL_ERTCO;
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
MXC_MCR->ctrl &= ~MXC_F_MCR_CTRL_CLKSEL;
MXC_MCR->ctrl |= MXC_S_MCR_CTRL_CLKSEL_ERTCO;
MXC_SETFIELD(MXC_MCR->ctrl, MXC_F_MCR_CTRL_CLKSEL, MXC_S_MCR_CTRL_CLKSEL_ERTCO);

I think you should use MXC_SETFIELD macro here.

Comment on lines 80 to 81
MXC_MCR->ctrl &= ~MXC_F_MCR_CTRL_CLKSEL;
MXC_MCR->ctrl |= MXC_S_MCR_CTRL_CLKSEL_INRO_DIV4;
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
MXC_MCR->ctrl &= ~MXC_F_MCR_CTRL_CLKSEL;
MXC_MCR->ctrl |= MXC_S_MCR_CTRL_CLKSEL_INRO_DIV4;
MXC_SETFIELD(MXC_MCR->ctrl, MXC_F_MCR_CTRL_CLKSEL, MXC_S_MCR_CTRL_CLKSEL_INRO_DIV4);

Same above.


clk_src = wrap_get_clock_source_instance(clock_source);

MXC_RTC_SetClockSource(clk_src);
Copy link
Contributor

@hfakkiz hfakkiz Dec 24, 2024

Choose a reason for hiding this comment

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

Please check "MXC_RTC_SetClockSource" function's return value and then according to its result, call "MXC_RTC_Init" function.

This commit creates an enum for rtc clock sources and
adds a function to set clock source for ME30 rtc.

Signed-off-by: Mert Vatansever <[email protected]>
This commit adds a wrapper function for MXC_RTC_Init
to set clock source for MAX32657.

Signed-off-by: Mert Vatansever <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MAX32657 Related to the MAX32655 (ME30) Zephyr MSDK Zephyr related change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants