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@xxxxxxxxxxx> 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@xxxxxxxxxxx> > > --- > > > > 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
