* [RFC][PATCH] rtc: sunxi: use external 32k oscillator if provided
@ 2023-07-27 15:01 Mans Rullgard
2023-07-27 16:53 ` Andre Przywara
0 siblings, 1 reply; 3+ messages in thread
From: Mans Rullgard @ 2023-07-27 15:01 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland
Cc: linux-rtc, linux-sunxi, linux-kernel
Set the OSC32K_SRC_SEL bit in the LOSC control register if a clock is
specified in the devicetree.
Signed-off-by: Mans Rullgard <mans@mansr.com>
---
The newer sun6i rtc driver is a proper clk provider with parent
selection. Doing the same thing in this driver would be difficult
while staying compatible with existing devicetrees. For that reason,
this simpler approach seems reasonable.
---
drivers/rtc/rtc-sunxi.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/rtc/rtc-sunxi.c b/drivers/rtc/rtc-sunxi.c
index 5d019e3a835a..4f1053eab778 100644
--- a/drivers/rtc/rtc-sunxi.c
+++ b/drivers/rtc/rtc-sunxi.c
@@ -5,6 +5,7 @@
* Copyright (c) 2013, Carlo Caione <carlo.caione@gmail.com>
*/
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/fs.h>
@@ -21,8 +22,10 @@
#include <linux/types.h>
#define SUNXI_LOSC_CTRL 0x0000
+#define SUNXI_LOSC_CTRL_KEY (0x16aa << 16)
#define SUNXI_LOSC_CTRL_RTC_HMS_ACC BIT(8)
#define SUNXI_LOSC_CTRL_RTC_YMD_ACC BIT(7)
+#define SUNXI_LOSC_CTRL_OSC32K_SRC_SEL BIT(0)
#define SUNXI_RTC_YMD 0x0004
@@ -422,6 +425,7 @@ MODULE_DEVICE_TABLE(of, sunxi_rtc_dt_ids);
static int sunxi_rtc_probe(struct platform_device *pdev)
{
struct sunxi_rtc_dev *chip;
+ struct clk *extclk;
int ret;
chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
@@ -455,6 +459,14 @@ static int sunxi_rtc_probe(struct platform_device *pdev)
return -ENODEV;
}
+ /* use external oscillator if present */
+ extclk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
+ if (IS_ERR(extclk))
+ return PTR_ERR(extclk);
+ if (extclk)
+ writel(SUNXI_LOSC_CTRL_KEY | SUNXI_LOSC_CTRL_OSC32K_SRC_SEL,
+ chip->base + SUNXI_LOSC_CTRL);
+
/* clear the alarm count value */
writel(0, chip->base + SUNXI_ALRM_DHMS);
--
2.41.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [RFC][PATCH] rtc: sunxi: use external 32k oscillator if provided
2023-07-27 15:01 [RFC][PATCH] rtc: sunxi: use external 32k oscillator if provided Mans Rullgard
@ 2023-07-27 16:53 ` Andre Przywara
2023-07-27 18:35 ` Måns Rullgård
0 siblings, 1 reply; 3+ messages in thread
From: Andre Przywara @ 2023-07-27 16:53 UTC (permalink / raw)
To: Mans Rullgard, Alessandro Zummo, Alexandre Belloni, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland
Cc: linux-rtc, linux-sunxi, linux-kernel
Hi Mans,
On 27/07/2023 16:01, Mans Rullgard wrote:
> Set the OSC32K_SRC_SEL bit in the LOSC control register if a clock is
> specified in the devicetree.
>
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
> The newer sun6i rtc driver is a proper clk provider with parent
> selection. Doing the same thing in this driver would be difficult
> while staying compatible with existing devicetrees. For that reason,
> this simpler approach seems reasonable.
> ---
> drivers/rtc/rtc-sunxi.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/rtc/rtc-sunxi.c b/drivers/rtc/rtc-sunxi.c
> index 5d019e3a835a..4f1053eab778 100644
> --- a/drivers/rtc/rtc-sunxi.c
> +++ b/drivers/rtc/rtc-sunxi.c
> @@ -5,6 +5,7 @@
> * Copyright (c) 2013, Carlo Caione <carlo.caione@gmail.com>
> */
>
> +#include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/err.h>
> #include <linux/fs.h>
> @@ -21,8 +22,10 @@
> #include <linux/types.h>
>
> #define SUNXI_LOSC_CTRL 0x0000
> +#define SUNXI_LOSC_CTRL_KEY (0x16aa << 16)
> #define SUNXI_LOSC_CTRL_RTC_HMS_ACC BIT(8)
> #define SUNXI_LOSC_CTRL_RTC_YMD_ACC BIT(7)
> +#define SUNXI_LOSC_CTRL_OSC32K_SRC_SEL BIT(0)
>
> #define SUNXI_RTC_YMD 0x0004
>
> @@ -422,6 +425,7 @@ MODULE_DEVICE_TABLE(of, sunxi_rtc_dt_ids);
> static int sunxi_rtc_probe(struct platform_device *pdev)
> {
> struct sunxi_rtc_dev *chip;
> + struct clk *extclk;
> int ret;
>
> chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> @@ -455,6 +459,14 @@ static int sunxi_rtc_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> + /* use external oscillator if present */
> + extclk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
> + if (IS_ERR(extclk))
> + return PTR_ERR(extclk);
> + if (extclk)
> + writel(SUNXI_LOSC_CTRL_KEY | SUNXI_LOSC_CTRL_OSC32K_SRC_SEL,
> + chip->base + SUNXI_LOSC_CTRL);
This should be a read-modify-write operation, since we don't want to
disturb other bits in this register.
In general this looks OK to me, but would need to be documented in the
bindings docs, to allow an optional clocks property.
Cheers,
Andre
> +
> /* clear the alarm count value */
> writel(0, chip->base + SUNXI_ALRM_DHMS);
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC][PATCH] rtc: sunxi: use external 32k oscillator if provided
2023-07-27 16:53 ` Andre Przywara
@ 2023-07-27 18:35 ` Måns Rullgård
0 siblings, 0 replies; 3+ messages in thread
From: Måns Rullgård @ 2023-07-27 18:35 UTC (permalink / raw)
To: Andre Przywara
Cc: Alessandro Zummo, Alexandre Belloni, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, linux-rtc, linux-sunxi, linux-kernel
Andre Przywara <andre.przywara@arm.com> writes:
> Hi Mans,
>
> On 27/07/2023 16:01, Mans Rullgard wrote:
>> Set the OSC32K_SRC_SEL bit in the LOSC control register if a clock is
>> specified in the devicetree.
>> Signed-off-by: Mans Rullgard <mans@mansr.com>
>> ---
>> The newer sun6i rtc driver is a proper clk provider with parent
>> selection. Doing the same thing in this driver would be difficult
>> while staying compatible with existing devicetrees. For that reason,
>> this simpler approach seems reasonable.
>> ---
>> drivers/rtc/rtc-sunxi.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>> diff --git a/drivers/rtc/rtc-sunxi.c b/drivers/rtc/rtc-sunxi.c
>> index 5d019e3a835a..4f1053eab778 100644
>> --- a/drivers/rtc/rtc-sunxi.c
>> +++ b/drivers/rtc/rtc-sunxi.c
>> @@ -5,6 +5,7 @@
>> * Copyright (c) 2013, Carlo Caione <carlo.caione@gmail.com>
>> */
>> +#include <linux/clk.h>
>> #include <linux/delay.h>
>> #include <linux/err.h>
>> #include <linux/fs.h>
>> @@ -21,8 +22,10 @@
>> #include <linux/types.h>
>> #define SUNXI_LOSC_CTRL 0x0000
>> +#define SUNXI_LOSC_CTRL_KEY (0x16aa << 16)
>> #define SUNXI_LOSC_CTRL_RTC_HMS_ACC BIT(8)
>> #define SUNXI_LOSC_CTRL_RTC_YMD_ACC BIT(7)
>> +#define SUNXI_LOSC_CTRL_OSC32K_SRC_SEL BIT(0)
>> #define SUNXI_RTC_YMD 0x0004
>> @@ -422,6 +425,7 @@ MODULE_DEVICE_TABLE(of, sunxi_rtc_dt_ids);
>> static int sunxi_rtc_probe(struct platform_device *pdev)
>> {
>> struct sunxi_rtc_dev *chip;
>> + struct clk *extclk;
>> int ret;
>> chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
>> @@ -455,6 +459,14 @@ static int sunxi_rtc_probe(struct platform_device *pdev)
>> return -ENODEV;
>> }
>> + /* use external oscillator if present */
>> + extclk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
>> + if (IS_ERR(extclk))
>> + return PTR_ERR(extclk);
>> + if (extclk)
>> + writel(SUNXI_LOSC_CTRL_KEY | SUNXI_LOSC_CTRL_OSC32K_SRC_SEL,
>> + chip->base + SUNXI_LOSC_CTRL);
>
> This should be a read-modify-write operation, since we don't want to disturb
> other bits in this register.
Good point. I guess it's best to leave everything untouched if the
clock isn't specified, just in case someone has a bootloader that sets
this bit.
> In general this looks OK to me, but would need to be documented in the
> bindings docs, to allow an optional clocks property.
Sure, I'll make a patch for that as well.
--
Måns Rullgård
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-07-27 18:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-27 15:01 [RFC][PATCH] rtc: sunxi: use external 32k oscillator if provided Mans Rullgard
2023-07-27 16:53 ` Andre Przywara
2023-07-27 18:35 ` Måns Rullgård
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox