From: Alois Fertl <A.Fertl@t-online.de>
To: Andre Przywara <andre.przywara@arm.com>
Cc: a.zummo@towertech.it, alexandre.belloni@bootlin.com,
wens@csie.org, jernej.skrabec@gmail.com, samuel@sholland.org,
linux-rtc@vger.kernel.org, linux-sunxi@lists.linux.dev,
linux-kernel@vger.kernel.org, Ryan Walklin <ryan@testtoast.com>
Subject: Re: [PATCH v3 1/1] drivers/rtc: rtc-sun6i: AutoCal Internal OSC Clock
Date: Sun, 05 May 2024 15:27:05 +0200 [thread overview]
Message-ID: <6d872479ea873d25bd1f0d893439c6ceafd11327.camel@t-online.de> (raw)
In-Reply-To: <20240503121304.5fc6add5@donnerap.manchester.arm.com>
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
prev parent reply other threads:[~2024-05-05 13:37 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6d872479ea873d25bd1f0d893439c6ceafd11327.camel@t-online.de \
--to=a.fertl@t-online.de \
--cc=20240503121304.5fc6add5@donnerap.manchester.arm.com \
--cc=a.zummo@towertech.it \
--cc=alexandre.belloni@bootlin.com \
--cc=andre.przywara@arm.com \
--cc=jernej.skrabec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=ryan@testtoast.com \
--cc=samuel@sholland.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).