linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/1] drivers/rtc: rtc-sun6i: AutoCal Internal OSC Clock
@ 2024-05-02 18:07 Alois Fertl
  2024-05-03 11:02 ` Ryan Walklin
  2024-05-03 11:13 ` Andre Przywara
  0 siblings, 2 replies; 6+ messages in thread
From: Alois Fertl @ 2024-05-02 18:07 UTC (permalink / raw)
  To: a.zummo
  Cc: alexandre.belloni, wens, jernej.skrabec, samuel, linux-rtc,
	linux-sunxi, linux-kernel, Alois Fertl

I have a M98-8K PLUS Magcubic TV-Box based on the Allwinner H618 SOC.
On board is a Sp6330 wifi/bt module that requires a 32kHz clock to
operate correctly. Without this change the clock from the SOC is
~29kHz and BT module does not start up. The patch enables the Internal
OSC Clock Auto Calibration of the H616/H618 which than provides the
necessary 32kHz and the BT module initializes successfully.
Add a flag and set it for H6 AND H616. The H618 is the same as H616
regarding rtc.

Signed-off-by: Alois Fertl <a.fertl@t-online.de>
---

v1->v2
- add flag and activate for H6 AND H616

v2->v3
- correct findings from review

I was hoping to get some feedback regarding the situation on H6,
where an external 32k crystal can be present.
From what I understand from the H6 manual there should be no
conflict as one can select which souce will drive the output.
Should certainly be tested but I don`t have H6 hardware.

 drivers/rtc/rtc-sun6i.c | 17 ++++++++++++++++-

 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index e0b85a0d5645..20e81ccdef29 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -42,6 +42,11 @@
 
 #define SUN6I_LOSC_CLK_PRESCAL			0x0008
 
+#define SUN6I_LOSC_CLK_AUTO_CAL			0x000c
+#define SUN6I_LOSC_CLK_AUTO_CAL_16MS		BIT(2)
+#define SUN6I_LOSC_CLK_AUTO_CAL_ENABLE		BIT(1)
+#define SUN6I_LOSC_CLK_AUTO_CAL_SEL_CAL		BIT(0)
+
 /* RTC */
 #define SUN6I_RTC_YMD				0x0010
 #define SUN6I_RTC_HMS				0x0014
@@ -126,7 +131,6 @@
  *     registers (R40, H6)
  *   - SYS power domain controls (R40)
  *   - DCXO controls (H6)
- *   - RC oscillator calibration (H6)
  *
  * These functions are not covered by this driver.
  */
@@ -138,6 +142,7 @@ struct sun6i_rtc_clk_data {
 	unsigned int has_losc_en : 1;
 	unsigned int has_auto_swt : 1;
 	unsigned int no_ext_losc : 1;
+	unsigned int has_auto_cal : 1;
 };
 
 #define RTC_LINEAR_DAY	BIT(0)
@@ -268,6 +273,14 @@ static void __init sun6i_rtc_clk_init(struct device_node *node,
 	}
 	writel(reg, rtc->base + SUN6I_LOSC_CTRL);
 
+	if (rtc->data->has_auto_cal) {
+		/* Enable internal OSC clock auto calibration */
+		reg = SUN6I_LOSC_CLK_AUTO_CAL_16MS |
+			SUN6I_LOSC_CLK_AUTO_CAL_ENABLE |
+			SUN6I_LOSC_CLK_AUTO_CAL_SEL_CAL;
+		writel(reg, rtc->base + SUN6I_LOSC_CLK_AUTO_CAL);
+	}
+
 	/* Yes, I know, this is ugly. */
 	sun6i_rtc = rtc;
 
@@ -380,6 +393,7 @@ static const struct sun6i_rtc_clk_data sun50i_h6_rtc_data = {
 	.has_out_clk = 1,
 	.has_losc_en = 1,
 	.has_auto_swt = 1,
+	.has_auto_cal = 1,
 };
 
 static void __init sun50i_h6_rtc_clk_init(struct device_node *node)
@@ -395,6 +409,7 @@ static const struct sun6i_rtc_clk_data sun50i_h616_rtc_data = {
 	.has_prescaler = 1,
 	.has_out_clk = 1,
 	.no_ext_losc = 1,
+	.has_auto_cal = 1,
 };
 
 static void __init sun50i_h616_rtc_clk_init(struct device_node *node)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/1] drivers/rtc: rtc-sun6i: AutoCal Internal OSC Clock
  2024-05-02 18:07 [PATCH v3 1/1] drivers/rtc: rtc-sun6i: AutoCal Internal OSC Clock Alois Fertl
@ 2024-05-03 11:02 ` Ryan Walklin
  2024-05-03 11:06   ` Ryan Walklin
  2024-05-03 11:27   ` Andre Przywara
  2024-05-03 11:13 ` Andre Przywara
  1 sibling, 2 replies; 6+ messages in thread
From: Ryan Walklin @ 2024-05-03 11:02 UTC (permalink / raw)
  To: Alois Fertl, a.zummo
  Cc: alexandre.belloni, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
	linux-rtc, linux-sunxi, linux-kernel

Hi Alois,

On Fri, 3 May 2024, at 6:07 AM, Alois Fertl wrote:
> I have a M98-8K PLUS Magcubic TV-Box based on the Allwinner H618 SOC.
> On board is a Sp6330 wifi/bt module that requires a 32kHz clock to
> operate correctly. Without this change the clock from the SOC is
> ~29kHz and BT module does not start up. The patch enables the Internal
> OSC Clock Auto Calibration of the H616/H618 which than provides the
> necessary 32kHz and the BT module initializes successfully.
> Add a flag and set it for H6 AND H616. The H618 is the same as H616
> regarding rtc.
>
> Signed-off-by: Alois Fertl <a.fertl@t-online.de>
> ---
>
> v1->v2
> - add flag and activate for H6 AND H616
>
> v2->v3
> - correct findings from review
>
> I was hoping to get some feedback regarding the situation on H6,
> where an external 32k crystal can be present.
> From what I understand from the H6 manual there should be no
> conflict as one can select which souce will drive the output.
> Should certainly be tested but I don`t have H6 hardware.
>
>  drivers/rtc/rtc-sun6i.c | 17 ++++++++++++++++-
>
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> index e0b85a0d5645..20e81ccdef29 100644
> --- a/drivers/rtc/rtc-sun6i.c
> +++ b/drivers/rtc/rtc-sun6i.c
> @@ -42,6 +42,11 @@
> 
>  #define SUN6I_LOSC_CLK_PRESCAL			0x0008
> 
> +#define SUN6I_LOSC_CLK_AUTO_CAL			0x000c
> +#define SUN6I_LOSC_CLK_AUTO_CAL_16MS		BIT(2)
> +#define SUN6I_LOSC_CLK_AUTO_CAL_ENABLE		BIT(1)
> +#define SUN6I_LOSC_CLK_AUTO_CAL_SEL_CAL		BIT(0)
> +
>  /* RTC */
>  #define SUN6I_RTC_YMD				0x0010
>  #define SUN6I_RTC_HMS				0x0014
> @@ -126,7 +131,6 @@
>   *     registers (R40, H6)
>   *   - SYS power domain controls (R40)
>   *   - DCXO controls (H6)
> - *   - RC oscillator calibration (H6)
>   *
>   * These functions are not covered by this driver.
>   */
> @@ -138,6 +142,7 @@ struct sun6i_rtc_clk_data {
>  	unsigned int has_losc_en : 1;
>  	unsigned int has_auto_swt : 1;
>  	unsigned int no_ext_losc : 1;
> +	unsigned int has_auto_cal : 1;
>  };
> 
>  #define RTC_LINEAR_DAY	BIT(0)
> @@ -268,6 +273,14 @@ static void __init sun6i_rtc_clk_init(struct 
> device_node *node,
>  	}
>  	writel(reg, rtc->base + SUN6I_LOSC_CTRL);
> 
> +	if (rtc->data->has_auto_cal) {
> +		/* Enable internal OSC clock auto calibration */
> +		reg = SUN6I_LOSC_CLK_AUTO_CAL_16MS |
> +			SUN6I_LOSC_CLK_AUTO_CAL_ENABLE |
> +			SUN6I_LOSC_CLK_AUTO_CAL_SEL_CAL;
> +		writel(reg, rtc->base + SUN6I_LOSC_CLK_AUTO_CAL);
> +	}
> +
>  	/* Yes, I know, this is ugly. */
>  	sun6i_rtc = rtc;
> 
> @@ -380,6 +393,7 @@ static const struct sun6i_rtc_clk_data 
> sun50i_h6_rtc_data = {
>  	.has_out_clk = 1,
>  	.has_losc_en = 1,
>  	.has_auto_swt = 1,
> +	.has_auto_cal = 1,
>  };
> 
>  static void __init sun50i_h6_rtc_clk_init(struct device_node *node)

I tried to test this with my H700 board but this struct is not actually in any mainline kernel and looks like it is from an earlier version of the H616 RTC changes which was never actually merged? Talked to Andre on IRC and he thinks the H6 and H616 RTC clocks *should* be equivalent.

> @@ -395,6 +409,7 @@ static const struct sun6i_rtc_clk_data 
> sun50i_h616_rtc_data = {
>  	.has_prescaler = 1,
>  	.has_out_clk = 1,
>  	.no_ext_losc = 1,
> +	.has_auto_cal = 1,
>  };
> 
>  static void __init sun50i_h616_rtc_clk_init(struct device_node *node)
> -- 
> 2.39.2

Regards,

Ryan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/1] drivers/rtc: rtc-sun6i: AutoCal Internal OSC Clock
  2024-05-03 11:02 ` Ryan Walklin
@ 2024-05-03 11:06   ` Ryan Walklin
  2024-05-03 11:27   ` Andre Przywara
  1 sibling, 0 replies; 6+ messages in thread
From: Ryan Walklin @ 2024-05-03 11:06 UTC (permalink / raw)
  To: Alois Fertl, a.zummo
  Cc: alexandre.belloni, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
	linux-rtc, linux-sunxi, linux-kernel


>> 
>>  static void __init sun50i_h616_rtc_clk_init(struct device_node *node)

Sorry, this struct. The H6 struct is present. 

>
> Ryan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/1] drivers/rtc: rtc-sun6i: AutoCal Internal OSC Clock
  2024-05-02 18:07 [PATCH v3 1/1] drivers/rtc: rtc-sun6i: AutoCal Internal OSC Clock Alois Fertl
  2024-05-03 11:02 ` Ryan Walklin
@ 2024-05-03 11:13 ` Andre Przywara
  2024-05-05 13:27   ` Alois Fertl
  1 sibling, 1 reply; 6+ messages in thread
From: Andre Przywara @ 2024-05-03 11:13 UTC (permalink / raw)
  To: Alois Fertl
  Cc: a.zummo, alexandre.belloni, wens, jernej.skrabec, samuel,
	linux-rtc, linux-sunxi, linux-kernel, Ryan Walklin

On Thu,  2 May 2024 20:07:36 +0200
Alois Fertl <a.fertl@t-online.de> wrote:

Hi Alois,

many thanks for taking care and sending a patch! I think this problem is
plaguing some people.

> I have a M98-8K PLUS Magcubic TV-Box based on the Allwinner H618 SOC.
> On board is a Sp6330 wifi/bt module that requires a 32kHz clock to
> operate correctly. Without this change the clock from the SOC is
> ~29kHz and BT module does not start up. The patch enables the Internal
> OSC Clock Auto Calibration of the H616/H618 which than provides the
> necessary 32kHz and the BT module initializes successfully.
> Add a flag and set it for H6 AND H616. The H618 is the same as H616
> regarding rtc.

(many thanks to Ryan for the head up on this)

So what tree is this patch based on? It certainly is not mainline, is it?
Mainline never supported the H616 RTC clocks via the RTC driver, this was
only through the new driver in the clk
directory, in drivers/clk/sunxi-ng/ccu-sun6i-rtc.c:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a93720329d4d2
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d91612d7f01aca

Please note that the H6 RTC clocks were intended to be handled via the new
driver as well, but this part was reverted:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=60d9f050da63b

Please only send patches based on the mainline tree.

I doesn't look that hard to adjust the patch to mainline, though to cover
the H6 is well this would require this code in both drivers. So when we
want to address the H6 as well, it might make sense to solve the problem
that triggered the revert first, to only have that change only in one
file. 

> Signed-off-by: Alois Fertl <a.fertl@t-online.de>
> ---
> 
> v1->v2
> - add flag and activate for H6 AND H616
> 
> v2->v3
> - correct findings from review
> 
> I was hoping to get some feedback regarding the situation on H6,
> where an external 32k crystal can be present.
> From what I understand from the H6 manual there should be no
> conflict as one can select which souce will drive the output.
> Should certainly be tested but I don`t have H6 hardware.

I think I should have H6 boards both with and without an external crystal
oscillator, so can check the situation there, but only next week.

>  drivers/rtc/rtc-sun6i.c | 17 ++++++++++++++++-
> 
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> index e0b85a0d5645..20e81ccdef29 100644
> --- a/drivers/rtc/rtc-sun6i.c
> +++ b/drivers/rtc/rtc-sun6i.c
> @@ -42,6 +42,11 @@
>  
>  #define SUN6I_LOSC_CLK_PRESCAL			0x0008
>  
> +#define SUN6I_LOSC_CLK_AUTO_CAL			0x000c
> +#define SUN6I_LOSC_CLK_AUTO_CAL_16MS		BIT(2)
> +#define SUN6I_LOSC_CLK_AUTO_CAL_ENABLE		BIT(1)
> +#define SUN6I_LOSC_CLK_AUTO_CAL_SEL_CAL		BIT(0)
> +
>  /* RTC */
>  #define SUN6I_RTC_YMD				0x0010
>  #define SUN6I_RTC_HMS				0x0014
> @@ -126,7 +131,6 @@
>   *     registers (R40, H6)
>   *   - SYS power domain controls (R40)
>   *   - DCXO controls (H6)
> - *   - RC oscillator calibration (H6)
>   *
>   * These functions are not covered by this driver.
>   */
> @@ -138,6 +142,7 @@ struct sun6i_rtc_clk_data {
>  	unsigned int has_losc_en : 1;
>  	unsigned int has_auto_swt : 1;
>  	unsigned int no_ext_losc : 1;
> +	unsigned int has_auto_cal : 1;
>  };
>  
>  #define RTC_LINEAR_DAY	BIT(0)
> @@ -268,6 +273,14 @@ static void __init sun6i_rtc_clk_init(struct device_node *node,
>  	}
>  	writel(reg, rtc->base + SUN6I_LOSC_CTRL);
>  
> +	if (rtc->data->has_auto_cal) {
> +		/* Enable internal OSC clock auto calibration */
> +		reg = SUN6I_LOSC_CLK_AUTO_CAL_16MS |
> +			SUN6I_LOSC_CLK_AUTO_CAL_ENABLE |
> +			SUN6I_LOSC_CLK_AUTO_CAL_SEL_CAL;
> +		writel(reg, rtc->base + SUN6I_LOSC_CLK_AUTO_CAL);
> +	}
> +
>  	/* Yes, I know, this is ugly. */
>  	sun6i_rtc = rtc;
>  
> @@ -380,6 +393,7 @@ static const struct sun6i_rtc_clk_data sun50i_h6_rtc_data = {
>  	.has_out_clk = 1,
>  	.has_losc_en = 1,
>  	.has_auto_swt = 1,
> +	.has_auto_cal = 1,
>  };
>  
>  static void __init sun50i_h6_rtc_clk_init(struct device_node *node)
> @@ -395,6 +409,7 @@ static const struct sun6i_rtc_clk_data sun50i_h616_rtc_data = {

For the records: this whole struct does not exist in mainline.

Cheers,
Andre

>  	.has_prescaler = 1,
>  	.has_out_clk = 1,
>  	.no_ext_losc = 1,
> +	.has_auto_cal = 1,
>  };
>  
>  static void __init sun50i_h616_rtc_clk_init(struct device_node *node)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/1] drivers/rtc: rtc-sun6i: AutoCal Internal OSC Clock
  2024-05-03 11:02 ` Ryan Walklin
  2024-05-03 11:06   ` Ryan Walklin
@ 2024-05-03 11:27   ` Andre Przywara
  1 sibling, 0 replies; 6+ messages in thread
From: Andre Przywara @ 2024-05-03 11:27 UTC (permalink / raw)
  To: Ryan Walklin
  Cc: Alois Fertl, a.zummo, alexandre.belloni, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, linux-rtc, linux-sunxi,
	linux-kernel

On Fri, 03 May 2024 23:02:13 +1200
"Ryan Walklin" <ryan@testtoast.com> wrote:

Hi,

> On Fri, 3 May 2024, at 6:07 AM, Alois Fertl wrote:
> > I have a M98-8K PLUS Magcubic TV-Box based on the Allwinner H618 SOC.
> > On board is a Sp6330 wifi/bt module that requires a 32kHz clock to
> > operate correctly. Without this change the clock from the SOC is
> > ~29kHz and BT module does not start up. The patch enables the Internal
> > OSC Clock Auto Calibration of the H616/H618 which than provides the
> > necessary 32kHz and the BT module initializes successfully.
> > Add a flag and set it for H6 AND H616. The H618 is the same as H616
> > regarding rtc.
> >
> > Signed-off-by: Alois Fertl <a.fertl@t-online.de>
> > ---
> >
> > v1->v2
> > - add flag and activate for H6 AND H616
> >
> > v2->v3
> > - correct findings from review
> >
> > I was hoping to get some feedback regarding the situation on H6,
> > where an external 32k crystal can be present.
> > From what I understand from the H6 manual there should be no
> > conflict as one can select which souce will drive the output.
> > Should certainly be tested but I don`t have H6 hardware.
> >
> >  drivers/rtc/rtc-sun6i.c | 17 ++++++++++++++++-
> >
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> > index e0b85a0d5645..20e81ccdef29 100644
> > --- a/drivers/rtc/rtc-sun6i.c
> > +++ b/drivers/rtc/rtc-sun6i.c
> > @@ -42,6 +42,11 @@
> > 
> >  #define SUN6I_LOSC_CLK_PRESCAL			0x0008
> > 
> > +#define SUN6I_LOSC_CLK_AUTO_CAL			0x000c
> > +#define SUN6I_LOSC_CLK_AUTO_CAL_16MS		BIT(2)
> > +#define SUN6I_LOSC_CLK_AUTO_CAL_ENABLE		BIT(1)
> > +#define SUN6I_LOSC_CLK_AUTO_CAL_SEL_CAL		BIT(0)
> > +
> >  /* RTC */
> >  #define SUN6I_RTC_YMD				0x0010
> >  #define SUN6I_RTC_HMS				0x0014
> > @@ -126,7 +131,6 @@
> >   *     registers (R40, H6)
> >   *   - SYS power domain controls (R40)
> >   *   - DCXO controls (H6)
> > - *   - RC oscillator calibration (H6)
> >   *
> >   * These functions are not covered by this driver.
> >   */
> > @@ -138,6 +142,7 @@ struct sun6i_rtc_clk_data {
> >  	unsigned int has_losc_en : 1;
> >  	unsigned int has_auto_swt : 1;
> >  	unsigned int no_ext_losc : 1;
> > +	unsigned int has_auto_cal : 1;
> >  };
> > 
> >  #define RTC_LINEAR_DAY	BIT(0)
> > @@ -268,6 +273,14 @@ static void __init sun6i_rtc_clk_init(struct 
> > device_node *node,
> >  	}
> >  	writel(reg, rtc->base + SUN6I_LOSC_CTRL);
> > 
> > +	if (rtc->data->has_auto_cal) {
> > +		/* Enable internal OSC clock auto calibration */
> > +		reg = SUN6I_LOSC_CLK_AUTO_CAL_16MS |
> > +			SUN6I_LOSC_CLK_AUTO_CAL_ENABLE |
> > +			SUN6I_LOSC_CLK_AUTO_CAL_SEL_CAL;
> > +		writel(reg, rtc->base + SUN6I_LOSC_CLK_AUTO_CAL);
> > +	}
> > +
> >  	/* Yes, I know, this is ugly. */
> >  	sun6i_rtc = rtc;
> > 
> > @@ -380,6 +393,7 @@ static const struct sun6i_rtc_clk_data 
> > sun50i_h6_rtc_data = {
> >  	.has_out_clk = 1,
> >  	.has_losc_en = 1,
> >  	.has_auto_swt = 1,
> > +	.has_auto_cal = 1,
> >  };
> > 
> >  static void __init sun50i_h6_rtc_clk_init(struct device_node *node)  
> 
> I tried to test this with my H700 board but this struct is not actually in any mainline kernel and looks like it is from an earlier version of the H616 RTC changes which was never actually merged?

Yes, thanks for the heads up, Ryan. Is this some Armbian branch, by any
chance?

> Talked to Andre on IRC and he thinks the H6 and H616 RTC clocks *should*
> be equivalent.

Yeah, that was my first naive idea, but it's actually not true, and the
increasing complexity of the RTC *clocks* were the trigger for the new
driver. That doesn't mean that the IOSC calibration routine cannot be the
same between the H6 and H616, though, but as it stands that would need to
live in two drivers (see my other email).

Cheers,
Andre

> 
> > @@ -395,6 +409,7 @@ static const struct sun6i_rtc_clk_data 
> > sun50i_h616_rtc_data = {
> >  	.has_prescaler = 1,
> >  	.has_out_clk = 1,
> >  	.no_ext_losc = 1,
> > +	.has_auto_cal = 1,
> >  };
> > 
> >  static void __init sun50i_h616_rtc_clk_init(struct device_node *node)
> > -- 
> > 2.39.2  
> 
> Regards,
> 
> Ryan
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/1] drivers/rtc: rtc-sun6i: AutoCal Internal OSC Clock
  2024-05-03 11:13 ` Andre Przywara
@ 2024-05-05 13:27   ` Alois Fertl
  0 siblings, 0 replies; 6+ messages in thread
From: Alois Fertl @ 2024-05-05 13:27 UTC (permalink / raw)
  To: Andre Przywara
  Cc: a.zummo, alexandre.belloni, wens, jernej.skrabec, samuel,
	linux-rtc, linux-sunxi, linux-kernel, Ryan Walklin

On Fri, 2024-05-03 at 12:13 +0100, Andre Przywara wrote:

Hi Andre and Ryan, thanks for responding, and sorry.
Andre's reply was somehow lost and I just found it on the
archive mirror. So hopefully my reply is correct.

And it is my first journey to do a patch.

> On Thu,  2 May 2024 20:07:36 +0200
> Alois Fertl <a.fertl@t-online.de> wrote:
> 
> Hi Alois,
> 
> many thanks for taking care and sending a patch! I think this problem
> is
> plaguing some people.
> 
> > I have a M98-8K PLUS Magcubic TV-Box based on the Allwinner H618
> > SOC.
> > On board is a Sp6330 wifi/bt module that requires a 32kHz clock to
> > operate correctly. Without this change the clock from the SOC is
> > ~29kHz and BT module does not start up. The patch enables the
> > Internal
> > OSC Clock Auto Calibration of the H616/H618 which than provides the
> > necessary 32kHz and the BT module initializes successfully.
> > Add a flag and set it for H6 AND H616. The H618 is the same as H616
> > regarding rtc.
> 
> (many thanks to Ryan for the head up on this)
> 
> So what tree is this patch based on? It certainly is not mainline, is
> it?
I'm running armbian based on linux-kernel-worktree/6.7__sunxi64__arm64 
which has one other rtc/rtc-sun6i.c patch applied before.
 "[PATCH] drv:rtc:sun6i: support RTCs without external LOSCs"

> Mainline never supported the H616 RTC clocks via the RTC driver, this
> was
> only through the new driver in the clk
> directory, in drivers/clk/sunxi-ng/ccu-sun6i-rtc.c:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a93720329d4d2
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d91612d7f01aca
I have found some code in for calibration in the sunxi-ng directory
that was never called and I doubt that it gives working calibration.
Should a kernel then use exclusively use the one or the other driver?

> 
> Please note that the H6 RTC clocks were intended to be handled via
> the new
> driver as well, but this part was reverted:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=60d9f050da63b
> 
> Please only send patches based on the mainline tree.
> 
> I doesn't look that hard to adjust the patch to mainline, though to
> cover
> the H6 is well this would require this code in both drivers. So when
> we
> want to address the H6 as well, it might make sense to solve the
> problem
> that triggered the revert first, to only have that change only in one
> file. 
Yes, the code would be useful for H6 when no external crystal is
present.

> 
> > Signed-off-by: Alois Fertl <a.fertl@t-online.de>
> > ---
> > 
> > v1->v2
> > - add flag and activate for H6 AND H616
> > 
> > v2->v3
> > - correct findings from review
> > 
> > I was hoping to get some feedback regarding the situation on H6,
> > where an external 32k crystal can be present.
> > From what I understand from the H6 manual there should be no
> > conflict as one can select which souce will drive the output.
> > Should certainly be tested but I don`t have H6 hardware.
> 
> I think I should have H6 boards both with and without an external
> crystal
> oscillator, so can check the situation there, but only next week.
> 
> >  drivers/rtc/rtc-sun6i.c | 17 ++++++++++++++++-
> > 
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> > index e0b85a0d5645..20e81ccdef29 100644
> > --- a/drivers/rtc/rtc-sun6i.c
> > +++ b/drivers/rtc/rtc-sun6i.c
> > @@ -42,6 +42,11 @@
> >  
> >  #define SUN6I_LOSC_CLK_PRESCAL                 0x0008
> >  
> > +#define SUN6I_LOSC_CLK_AUTO_CAL                        0x000c
> > +#define SUN6I_LOSC_CLK_AUTO_CAL_16MS           BIT(2)
> > +#define SUN6I_LOSC_CLK_AUTO_CAL_ENABLE         BIT(1)
> > +#define SUN6I_LOSC_CLK_AUTO_CAL_SEL_CAL                BIT(0)
> > +
> >  /* RTC */
> >  #define SUN6I_RTC_YMD                          0x0010
> >  #define SUN6I_RTC_HMS                          0x0014
> > @@ -126,7 +131,6 @@
> >   *     registers (R40, H6)
> >   *   - SYS power domain controls (R40)
> >   *   - DCXO controls (H6)
> > - *   - RC oscillator calibration (H6)
> >   *
> >   * These functions are not covered by this driver.
> >   */
> > @@ -138,6 +142,7 @@ struct sun6i_rtc_clk_data {
> >         unsigned int has_losc_en : 1;
> >         unsigned int has_auto_swt : 1;
> >         unsigned int no_ext_losc : 1;
> > +       unsigned int has_auto_cal : 1;
> >  };
> >  
> >  #define RTC_LINEAR_DAY BIT(0)
> > @@ -268,6 +273,14 @@ static void __init sun6i_rtc_clk_init(struct
> > device_node *node,
> >         }
> >         writel(reg, rtc->base + SUN6I_LOSC_CTRL);
> >  
> > +       if (rtc->data->has_auto_cal) {
> > +               /* Enable internal OSC clock auto calibration */
> > +               reg = SUN6I_LOSC_CLK_AUTO_CAL_16MS |
> > +                       SUN6I_LOSC_CLK_AUTO_CAL_ENABLE |
> > +                       SUN6I_LOSC_CLK_AUTO_CAL_SEL_CAL;
> > +               writel(reg, rtc->base + SUN6I_LOSC_CLK_AUTO_CAL);
> > +       }
> > +
> >         /* Yes, I know, this is ugly. */
> >         sun6i_rtc = rtc;
> >  
> > @@ -380,6 +393,7 @@ static const struct sun6i_rtc_clk_data
> > sun50i_h6_rtc_data = {
> >         .has_out_clk = 1,
> >         .has_losc_en = 1,
> >         .has_auto_swt = 1,
> > +       .has_auto_cal = 1,
> >  };
> >  
> >  static void __init sun50i_h6_rtc_clk_init(struct device_node
> > *node)
> > @@ -395,6 +409,7 @@ static const struct sun6i_rtc_clk_data
> > sun50i_h616_rtc_data = {
> 
> For the records: this whole struct does not exist in mainline.
> 
> Cheers,
> Andre
> 
> >         .has_prescaler = 1,
> >         .has_out_clk = 1,
> >         .no_ext_losc = 1,
> > +       .has_auto_cal = 1,
> >  };
> >  
> >  static void __init sun50i_h616_rtc_clk_init(struct device_node
> > *node)
> 
Cheers,
Alois

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-05-05 13:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-02 18:07 [PATCH v3 1/1] drivers/rtc: rtc-sun6i: AutoCal Internal OSC Clock Alois Fertl
2024-05-03 11:02 ` Ryan Walklin
2024-05-03 11:06   ` Ryan Walklin
2024-05-03 11:27   ` Andre Przywara
2024-05-03 11:13 ` Andre Przywara
2024-05-05 13:27   ` Alois Fertl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).