2023-07-05 18:17:53

by Valentin Caron

[permalink] [raw]
Subject: [PATCH v2 0/7] rtc: stm32: multiple bug fixes and improvements

This series implements some bug fixes for theses issues:
- typo issues
- register read sequence issue
- irq during pm callbacks issue

This series implements also some improvements:
- don't switch to INIT mode uselessly
- improve error printing during probe
- improve rtc precision on stm32mp1

Since v1:
- Exchange "fix issues of stm32_rtc_valid_alrm" and
"fix unnecessary parentheses" patches to optimize this series.

Antonio Borneo (2):
rtc: stm32: use the proper register sequence to read date/time
rtc: stm32: don't stop time counter if not needed

Christophe Guibout (1):
rtc: stm32: improve rtc precision

Gabriel Fernandez (1):
rtc: stm32: change PM callbacks to "_noirq()"

Valentin Caron (3):
rtc: stm32: don't print an error on probe deferral
rtc: stm32: fix issues of stm32_rtc_valid_alrm function
rtc: stm32: fix unnecessary parentheses

drivers/rtc/rtc-stm32.c | 138 +++++++++++++++++++++++++---------------
1 file changed, 85 insertions(+), 53 deletions(-)

--
2.25.1



2023-07-05 18:17:57

by Valentin Caron

[permalink] [raw]
Subject: [PATCH v2 7/7] rtc: stm32: fix unnecessary parentheses

Fix a few style issues reported by checkpatch.pl:
- Unnecessary parentheses
- Lines should not end with a '('

Signed-off-by: Valentin Caron <[email protected]>
---
drivers/rtc/rtc-stm32.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
index 836d39a124dd..85689192fa7a 100644
--- a/drivers/rtc/rtc-stm32.c
+++ b/drivers/rtc/rtc-stm32.c
@@ -163,10 +163,9 @@ static int stm32_rtc_enter_init_mode(struct stm32_rtc *rtc)
* slowest rtc_ck frequency may be 32kHz and highest should be
* 1MHz, we poll every 10 us with a timeout of 100ms.
*/
- return readl_relaxed_poll_timeout_atomic(
- rtc->base + regs->isr,
- isr, (isr & STM32_RTC_ISR_INITF),
- 10, 100000);
+ return readl_relaxed_poll_timeout_atomic(rtc->base + regs->isr, isr,
+ (isr & STM32_RTC_ISR_INITF),
+ 10, 100000);
}

return 0;
@@ -671,7 +670,7 @@ static int stm32_rtc_init(struct platform_device *pdev,
* Can't find a 1Hz, so give priority to RTC power consumption
* by choosing the higher possible value for prediv_a
*/
- if ((pred_s > pred_s_max) || (pred_a > pred_a_max)) {
+ if (pred_s > pred_s_max || pred_a > pred_a_max) {
pred_a = pred_a_max;
pred_s = (rate / (pred_a + 1)) - 1;

--
2.25.1


2023-07-05 18:18:34

by Valentin Caron

[permalink] [raw]
Subject: [PATCH v2 3/7] rtc: stm32: improve rtc precision

From: Christophe Guibout <[email protected]>

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 <[email protected]>
Signed-off-by: Valentin Caron <[email protected]>
---
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);
+ 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


2023-07-05 18:18:39

by Valentin Caron

[permalink] [raw]
Subject: [PATCH v2 6/7] rtc: stm32: fix issues of stm32_rtc_valid_alrm function

stm32_rtc_valid_alrm function has some issues :
- arithmetical operations are impossible on BCD values
- "cur_mon + 1" can overflow
- the use case with the next month, the same day/hour/minutes went wrong

To solve that, we prefer to use timestamp comparison.
e.g. : On 5 Dec. 2021, the alarm limit is 5 Jan. 2022 (+31 days)
On 31 Jan 2021, the alarm limit is 28 Feb. 2022 (+28 days)

Signed-off-by: Valentin Caron <[email protected]>
---
drivers/rtc/rtc-stm32.c | 61 ++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
index 17e549806784..836d39a124dd 100644
--- a/drivers/rtc/rtc-stm32.c
+++ b/drivers/rtc/rtc-stm32.c
@@ -90,6 +90,9 @@
/* Max STM32 RTC register offset is 0x3FC */
#define UNDEF_REG 0xFFFF

+/* STM32 RTC driver time helpers */
+#define SEC_PER_DAY (24 * 60 * 60)
+
struct stm32_rtc;

struct stm32_rtc_registers {
@@ -427,40 +430,42 @@ static int stm32_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
return 0;
}

-static int stm32_rtc_valid_alrm(struct stm32_rtc *rtc, struct rtc_time *tm)
+static int stm32_rtc_valid_alrm(struct device *dev, struct rtc_time *tm)
{
- const struct stm32_rtc_registers *regs = &rtc->data->regs;
- int cur_day, cur_mon, cur_year, cur_hour, cur_min, cur_sec;
- unsigned int tr = readl_relaxed(rtc->base + regs->tr);
- unsigned int dr = readl_relaxed(rtc->base + regs->dr);
-
- cur_day = (dr & STM32_RTC_DR_DATE) >> STM32_RTC_DR_DATE_SHIFT;
- cur_mon = (dr & STM32_RTC_DR_MONTH) >> STM32_RTC_DR_MONTH_SHIFT;
- cur_year = (dr & STM32_RTC_DR_YEAR) >> STM32_RTC_DR_YEAR_SHIFT;
- cur_sec = (tr & STM32_RTC_TR_SEC) >> STM32_RTC_TR_SEC_SHIFT;
- cur_min = (tr & STM32_RTC_TR_MIN) >> STM32_RTC_TR_MIN_SHIFT;
- cur_hour = (tr & STM32_RTC_TR_HOUR) >> STM32_RTC_TR_HOUR_SHIFT;
+ static struct rtc_time now;
+ time64_t max_alarm_time64;
+ int max_day_forward;
+ int next_month;
+ int next_year;

/*
* Assuming current date is M-D-Y H:M:S.
* RTC alarm can't be set on a specific month and year.
* So the valid alarm range is:
* M-D-Y H:M:S < alarm <= (M+1)-D-Y H:M:S
- * with a specific case for December...
*/
- if ((((tm->tm_year > cur_year) &&
- (tm->tm_mon == 0x1) && (cur_mon == 0x12)) ||
- ((tm->tm_year == cur_year) &&
- (tm->tm_mon <= cur_mon + 1))) &&
- ((tm->tm_mday > cur_day) ||
- ((tm->tm_mday == cur_day) &&
- ((tm->tm_hour > cur_hour) ||
- ((tm->tm_hour == cur_hour) && (tm->tm_min > cur_min)) ||
- ((tm->tm_hour == cur_hour) && (tm->tm_min == cur_min) &&
- (tm->tm_sec >= cur_sec))))))
- return 0;
+ stm32_rtc_read_time(dev, &now);
+
+ /*
+ * Find the next month and the year of the next month.
+ * Note: tm_mon and next_month are from 0 to 11
+ */
+ next_month = now.tm_mon + 1;
+ if (next_month == 12) {
+ next_month = 0;
+ next_year = now.tm_year + 1;
+ } else {
+ next_year = now.tm_year;
+ }

- return -EINVAL;
+ /* Find the maximum limit of alarm in days. */
+ max_day_forward = rtc_month_days(now.tm_mon, now.tm_year)
+ - now.tm_mday
+ + min(rtc_month_days(next_month, next_year), now.tm_mday);
+
+ /* Convert to timestamp and compare the alarm time and its upper limit */
+ max_alarm_time64 = rtc_tm_to_time64(&now) + max_day_forward * SEC_PER_DAY;
+ return rtc_tm_to_time64(tm) <= max_alarm_time64 ? 0 : -EINVAL;
}

static int stm32_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
@@ -471,17 +476,17 @@ static int stm32_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
unsigned int cr, isr, alrmar;
int ret = 0;

- tm2bcd(tm);
-
/*
* RTC alarm can't be set on a specific date, unless this date is
* up to the same day of month next month.
*/
- if (stm32_rtc_valid_alrm(rtc, tm) < 0) {
+ if (stm32_rtc_valid_alrm(dev, tm) < 0) {
dev_err(dev, "Alarm can be set only on upcoming month.\n");
return -EINVAL;
}

+ tm2bcd(tm);
+
alrmar = 0;
/* tm_year and tm_mon are not used because not supported by RTC */
alrmar |= (tm->tm_mday << STM32_RTC_ALRMXR_DATE_SHIFT) &
--
2.25.1


2023-07-27 22:21:37

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] rtc: stm32: multiple bug fixes and improvements


On Wed, 05 Jul 2023 19:43:50 +0200, Valentin Caron wrote:
> This series implements some bug fixes for theses issues:
> - typo issues
> - register read sequence issue
> - irq during pm callbacks issue
>
> This series implements also some improvements:
> - don't switch to INIT mode uselessly
> - improve error printing during probe
> - improve rtc precision on stm32mp1
>
> [...]

Applied, thanks!

[1/7] rtc: stm32: use the proper register sequence to read date/time
commit: f69cb2d6034ddf8dae6848d29b9d4efba8cd4df9
[2/7] rtc: stm32: don't stop time counter if not needed
commit: 1c18b8ec52396af6a6e20cd3450dc9bff0781ab8
[3/7] rtc: stm32: improve rtc precision
commit: 2487925731b75961cf4b7d1d0d28d204b63787b9
[4/7] rtc: stm32: don't print an error on probe deferral
commit: 95f7679c3ab2d032d935692426b6d9f7e681fd60
[5/7] rtc: stm32: change PM callbacks to "_noirq()"
commit: fb9a7e5360dc8089097337a9685f6fed350a310f
[6/7] rtc: stm32: fix issues of stm32_rtc_valid_alrm function
commit: 46828a5f89044b8e057f6bbb50ae2bac926a0fa2
[7/7] rtc: stm32: fix unnecessary parentheses
commit: 650915ecd8f8cbb58e1ef55430f9e15ae03fd7d8

Best regards,

--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com