[v1] rtc: stm32: multiple bug fixes and improvements

[PATCH 3/7] rtc: stm32: improve rtc precision

Posted by Valentin Caron 2 years, 7 months ago

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);
+		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

Re: [PATCH 3/7] rtc: stm32: improve rtc precision

Posted by Alexandre Belloni 2 years, 7 months ago

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
> 

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

Re: [PATCH 3/7] rtc: stm32: improve rtc precision

Posted by Valentin CARON 2 years, 7 months ago

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