Skip to content

Conversation

@ChristianHirsch
Copy link
Contributor

use u16_t prescaler instead of u8_t prescaler in init_rtc to be able to use the full 12 bit prescaler values.
fixes #22014.

@zephyrbot
Copy link

zephyrbot commented Jan 17, 2020

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@carlescufi
Copy link
Member

@ChristianHirsch thanks for the PR! Can you follow the instructions here to fix Gitlint?

Copy link
Contributor

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

I've seen something like this somewhere else very recently. I don't believe there's any benefit to specifying a formal parameter that is smaller than u32_t; in fact there will be a cost, because code must be generated to truncate values to the specified representation. Here nrf_rtc_prescaler_set() takes a u32_t when receiving this value, so it's really unjustified.

I recommend that somebody from Nordic go through all these APIs make sure that an appropriate and standardized size is used when passing values.

@ioannisg ioannisg added bug The issue is a bug, or the PR is fixing a bug platform: Actions Actions Semi platform: nRF Nordic nRFx and removed platform: Actions Actions Semi labels Jan 19, 2020
@ioannisg ioannisg added this to the v2.2.0 milestone Jan 19, 2020
@ChristianHirsch
Copy link
Contributor Author

ChristianHirsch commented Jan 20, 2020

@pabigot thanks for the hints. should I change the line to u32_t? unfotunately, I did not find any header files to see what nrf_rtc_prescaler_set() expects.
@carlescufi yes, I will fix the Gitlint issues.

@nordic-krch
Copy link
Contributor

nordic-krch commented Jan 20, 2020

looks ok, fix commit message and i'll approve.

should I change the line to u32_t

Please do.

use u32_t in init_rtc to be able to use the full 12 bit prescaler values
fixes zephyrproject-rtos#22014

Signed-off-by: Christian Hirsch <christian.hirsch@tuwien.ac.at>
@ioannisg
Copy link
Member

@nordic-krch

@ioannisg
Copy link
Member

Thanks for your contribution @ChristianHirsch

@ioannisg ioannisg merged commit 5fcb5d0 into zephyrproject-rtos:master Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Counter bug The issue is a bug, or the PR is fixing a bug platform: nRF Nordic nRFx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RTC prescaler overflow on nRF(52)

6 participants