Re: [PATCH 2/4] rtc: s3c: Add time range

From: Sam Protsenko <semen.protsenko@linaro.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>,
	linux-rtc@vger.kernel.org,
	Linux Samsung SOC <linux-samsung-soc@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/4] rtc: s3c: Add time range
Date: Thu, 21 Oct 2021 22:48:51 +0300	[thread overview]
Message-ID: <CAPLW+4=RuoT016zHotKvrNNxB_bZt4VXhZRWkGuJs22XeOpcpA@mail.gmail.com> (raw)
In-Reply-To: <effeb83b-7923-7086-5b4f-36266015e137@canonical.com>

On Wed, 20 Oct 2021 at 09:29, Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 19/10/2021 21:12, Sam Protsenko wrote:
> > Krzysztof, do you have by chance the doc for different SoCs supported
> > by S3C RTC driver? I can implement proper values for min/max range for
> > each SoC, as Alexandre asked, by adding those to driver data. But I
> > need max year register value (100, 1000, etc) for each of those chips:
> >
> >   - "samsung,s3c2410-rtc"
> >   - "samsung,s3c2416-rtc"
> >   - "samsung,s3c2443-rtc"
> >   - "samsung,s3c6410-rtc"
> >   - "samsung,exynos3250-rtc"
> >
> > For example Exynos850 TRM states that BCDYEAR register has [11:0] bits
> > for holding the year value in BCD format, so it's 10^(12/4)=1000 years
> > max.
> >
>
> I think all S3C chips have only 8-bit wide year, so 2000-2099, while
> S5Pv210 and Exynos has 12-bit (1000 years). However I doubt there is big
> benefit of supporting more than 2100. :) If you still want, you would
> need to create the patch carefully because not many people can test it...
>

Guys,

After testing thoroughly, I can confirm that Alexandre is right about
leap years (Exynos850 RTC treats both 2000 and 2100 as leap years).
And it also overflows internally on 2159 year, limiting the actual
time range at 160 years. So I'll keep that range at 100 years for all
RTCs. As Krzysztof said, there is no practical reasons in trying to
increase it anyway. Will send v2 soon.

What I'm curious about is RTC testing. I've found this test suite:

    tools/testing/selftests/rtc/rtctest.c

But it doesn't seem to cover corner cases (like checking leap years,
which was discussed here). Just a thought: maybe it should be added
there, so everyone can benefit from that? For example, I know that in
Linaro we are running LKFT tests for different boards, so that might
theoretically reveal some bugs. Though I understand possible
implications: we probably don't know which ranges are supported in
driver that's being tested. Anyway, just saying.

>
> Best regards,
> Krzysztof