| Message ID | 20230615092753.323844-4-valentin.caron@foss.st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rtc: stm32: multiple bug fixes and improvements | expand |
On 15/06/2023 11:27:49+0200, Valentin Caron wrote: > From: Christophe Guibout <christophe.guibout@foss.st.com> > > The rtc is used to update the stgen counter on wake up from > low power modes, so it needs to be as much accurate as possible. > > The maximization of asynchronous divider leads to a 4ms rtc > precision clock. > By decreasing pred_a to 0, it will have pred_s=32767 (when > need_accuracy is true), so stgen clock becomes more accurate > with 30us precision. > Nevertheless this will leads to an increase of power consumption. > > Signed-off-by: Christophe Guibout <christophe.guibout@foss.st.com> > Signed-off-by: Valentin Caron <valentin.caron@foss.st.com> > --- > drivers/rtc/rtc-stm32.c | 26 ++++++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c > index bd7a59a07537..cad88668bcfb 100644 > --- a/drivers/rtc/rtc-stm32.c > +++ b/drivers/rtc/rtc-stm32.c > @@ -114,6 +114,7 @@ struct stm32_rtc_data { > void (*clear_events)(struct stm32_rtc *rtc, unsigned int flags); > bool has_pclk; > bool need_dbp; > + bool need_accuracy; > }; > > struct stm32_rtc { > @@ -545,6 +546,7 @@ static void stm32_rtc_clear_events(struct stm32_rtc *rtc, > static const struct stm32_rtc_data stm32_rtc_data = { > .has_pclk = false, > .need_dbp = true, > + .need_accuracy = false, > .regs = { > .tr = 0x00, > .dr = 0x04, > @@ -566,6 +568,7 @@ static const struct stm32_rtc_data stm32_rtc_data = { > static const struct stm32_rtc_data stm32h7_rtc_data = { > .has_pclk = true, > .need_dbp = true, > + .need_accuracy = false, > .regs = { > .tr = 0x00, > .dr = 0x04, > @@ -596,6 +599,7 @@ static void stm32mp1_rtc_clear_events(struct stm32_rtc *rtc, > static const struct stm32_rtc_data stm32mp1_data = { > .has_pclk = true, > .need_dbp = false, > + .need_accuracy = true, > .regs = { > .tr = 0x00, > .dr = 0x04, > @@ -636,11 +640,25 @@ static int stm32_rtc_init(struct platform_device *pdev, > pred_a_max = STM32_RTC_PRER_PRED_A >> STM32_RTC_PRER_PRED_A_SHIFT; > pred_s_max = STM32_RTC_PRER_PRED_S >> STM32_RTC_PRER_PRED_S_SHIFT; > > - for (pred_a = pred_a_max; pred_a + 1 > 0; pred_a--) { > - pred_s = (rate / (pred_a + 1)) - 1; > + if (rate > (pred_a_max + 1) * (pred_s_max + 1)) { > + dev_err(&pdev->dev, "rtc_ck rate is too high: %dHz\n", rate); What is the expect user action after seeing this message? > + return -EINVAL; > + } > + > + if (rtc->data->need_accuracy) { > + for (pred_a = 0; pred_a <= pred_a_max; pred_a++) { > + pred_s = (rate / (pred_a + 1)) - 1; > > - if (((pred_s + 1) * (pred_a + 1)) == rate) > - break; > + if (pred_s <= pred_s_max && ((pred_s + 1) * (pred_a + 1)) == rate) > + break; > + } > + } else { > + for (pred_a = pred_a_max; pred_a + 1 > 0; pred_a--) { > + pred_s = (rate / (pred_a + 1)) - 1; > + > + if (((pred_s + 1) * (pred_a + 1)) == rate) > + break; > + } > } > > /* > -- > 2.25.1 >
Hi Alexandre, Sorry for the delay On 6/26/23 01:14, Alexandre Belloni wrote: > On 15/06/2023 11:27:49+0200, Valentin Caron wrote: >> From: Christophe Guibout <christophe.guibout@foss.st.com> >> >> The rtc is used to update the stgen counter on wake up from >> low power modes, so it needs to be as much accurate as possible. >> >> The maximization of asynchronous divider leads to a 4ms rtc >> precision clock. >> By decreasing pred_a to 0, it will have pred_s=32767 (when >> need_accuracy is true), so stgen clock becomes more accurate >> with 30us precision. >> Nevertheless this will leads to an increase of power consumption. >> >> Signed-off-by: Christophe Guibout <christophe.guibout@foss.st.com> >> Signed-off-by: Valentin Caron <valentin.caron@foss.st.com> >> --- >> drivers/rtc/rtc-stm32.c | 26 ++++++++++++++++++++++---- >> 1 file changed, 22 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c >> index bd7a59a07537..cad88668bcfb 100644 >> --- a/drivers/rtc/rtc-stm32.c >> +++ b/drivers/rtc/rtc-stm32.c >> @@ -114,6 +114,7 @@ struct stm32_rtc_data { >> void (*clear_events)(struct stm32_rtc *rtc, unsigned int flags); >> bool has_pclk; >> bool need_dbp; >> + bool need_accuracy; >> }; >> >> struct stm32_rtc { >> @@ -545,6 +546,7 @@ static void stm32_rtc_clear_events(struct stm32_rtc *rtc, >> static const struct stm32_rtc_data stm32_rtc_data = { >> .has_pclk = false, >> .need_dbp = true, >> + .need_accuracy = false, >> .regs = { >> .tr = 0x00, >> .dr = 0x04, >> @@ -566,6 +568,7 @@ static const struct stm32_rtc_data stm32_rtc_data = { >> static const struct stm32_rtc_data stm32h7_rtc_data = { >> .has_pclk = true, >> .need_dbp = true, >> + .need_accuracy = false, >> .regs = { >> .tr = 0x00, >> .dr = 0x04, >> @@ -596,6 +599,7 @@ static void stm32mp1_rtc_clear_events(struct stm32_rtc *rtc, >> static const struct stm32_rtc_data stm32mp1_data = { >> .has_pclk = true, >> .need_dbp = false, >> + .need_accuracy = true, >> .regs = { >> .tr = 0x00, >> .dr = 0x04, >> @@ -636,11 +640,25 @@ static int stm32_rtc_init(struct platform_device *pdev, >> pred_a_max = STM32_RTC_PRER_PRED_A >> STM32_RTC_PRER_PRED_A_SHIFT; >> pred_s_max = STM32_RTC_PRER_PRED_S >> STM32_RTC_PRER_PRED_S_SHIFT; >> >> - for (pred_a = pred_a_max; pred_a + 1 > 0; pred_a--) { >> - pred_s = (rate / (pred_a + 1)) - 1; >> + if (rate > (pred_a_max + 1) * (pred_s_max + 1)) { >> + dev_err(&pdev->dev, "rtc_ck rate is too high: %dHz\n", rate); > What is the expect user action after seeing this message? User could change of source clock by a smaller one (in terms of frequency) or adjust the clock divider in amount of RTC. e.g. on STM32MP1, RTC source clock could be: - LSE clock - LSI clock - HSE clock * an adjustable divider. > >> + return -EINVAL; >> + } >> + >> + if (rtc->data->need_accuracy) { >> + for (pred_a = 0; pred_a <= pred_a_max; pred_a++) { >> + pred_s = (rate / (pred_a + 1)) - 1; >> >> - if (((pred_s + 1) * (pred_a + 1)) == rate) >> - break; >> + if (pred_s <= pred_s_max && ((pred_s + 1) * (pred_a + 1)) == rate) >> + break; >> + } >> + } else { >> + for (pred_a = pred_a_max; pred_a + 1 > 0; pred_a--) { >> + pred_s = (rate / (pred_a + 1)) - 1; >> + >> + if (((pred_s + 1) * (pred_a + 1)) == rate) >> + break; >> + } >> } >> >> /* >> -- >> 2.25.1 >> Regards, Valentin
diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c index bd7a59a07537..cad88668bcfb 100644 --- a/drivers/rtc/rtc-stm32.c +++ b/drivers/rtc/rtc-stm32.c @@ -114,6 +114,7 @@ struct stm32_rtc_data { void (*clear_events)(struct stm32_rtc *rtc, unsigned int flags); bool has_pclk; bool need_dbp; + bool need_accuracy; }; struct stm32_rtc { @@ -545,6 +546,7 @@ static void stm32_rtc_clear_events(struct stm32_rtc *rtc, static const struct stm32_rtc_data stm32_rtc_data = { .has_pclk = false, .need_dbp = true, + .need_accuracy = false, .regs = { .tr = 0x00, .dr = 0x04, @@ -566,6 +568,7 @@ static const struct stm32_rtc_data stm32_rtc_data = { static const struct stm32_rtc_data stm32h7_rtc_data = { .has_pclk = true, .need_dbp = true, + .need_accuracy = false, .regs = { .tr = 0x00, .dr = 0x04, @@ -596,6 +599,7 @@ static void stm32mp1_rtc_clear_events(struct stm32_rtc *rtc, static const struct stm32_rtc_data stm32mp1_data = { .has_pclk = true, .need_dbp = false, + .need_accuracy = true, .regs = { .tr = 0x00, .dr = 0x04, @@ -636,11 +640,25 @@ static int stm32_rtc_init(struct platform_device *pdev, pred_a_max = STM32_RTC_PRER_PRED_A >> STM32_RTC_PRER_PRED_A_SHIFT; pred_s_max = STM32_RTC_PRER_PRED_S >> STM32_RTC_PRER_PRED_S_SHIFT; - for (pred_a = pred_a_max; pred_a + 1 > 0; pred_a--) { - pred_s = (rate / (pred_a + 1)) - 1; + if (rate > (pred_a_max + 1) * (pred_s_max + 1)) { + dev_err(&pdev->dev, "rtc_ck rate is too high: %dHz\n", rate); + return -EINVAL; + } + + if (rtc->data->need_accuracy) { + for (pred_a = 0; pred_a <= pred_a_max; pred_a++) { + pred_s = (rate / (pred_a + 1)) - 1; - if (((pred_s + 1) * (pred_a + 1)) == rate) - break; + if (pred_s <= pred_s_max && ((pred_s + 1) * (pred_a + 1)) == rate) + break; + } + } else { + for (pred_a = pred_a_max; pred_a + 1 > 0; pred_a--) { + pred_s = (rate / (pred_a + 1)) - 1; + + if (((pred_s + 1) * (pred_a + 1)) == rate) + break; + } } /*