Re: [PATCH] rtc: s3c-rtc: Avoid using broken ALMYEAR register

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

Hello Marek,

On 13/11/2018 12:32:50+0100, Marek Szyprowski wrote:
> (RTC,ALM)YEAR registers of Exynos built-in RTC device contains 3 BCD
> characters. s3c-rtc driver uses only 2 lower of them and supports years
> from 2000..2099 range. The third BCD value is typically set to 0, but it
> looks that handling of it is broken in the hardware. It sometimes
> defaults to a random (even non-BCD) value. This is not an issue
> for handling RTCYEAR register, because bcd2bin() properly handles only
> 8bit values (2 BCD characters, the third one is skipped). The problem
> is however with ALMYEAR register and proper RTC alarm operation. When
> YEAREN bit is set for the configured alarm, RTC hardware triggers alarm
> only when ALMYEAR and RTCYEAR matches. This usually doesn't happen
> because of the random noise on the third BCD character.
> 

I'm probably missing something but can't you set that third BCD value to
0 in ALMYEAR?

> Fix this by simply skipping setting ALMYEAR register in alarm
> configuration. This workaround fixes broken alarm operation on Exynos
> built-in rtc device. My tests revealed that the issue happens on the
> following Exynos series: 3250, 4210, 4412, 5250 and 5410.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/rtc/rtc-s3c.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> index 75c8c5033e08..58e03ac3578b 100644
> --- a/drivers/rtc/rtc-s3c.c
> +++ b/drivers/rtc/rtc-s3c.c
> @@ -327,7 +327,6 @@ static int s3c_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	struct rtc_time *tm = &alrm->time;
>  	unsigned int alrm_en;
>  	int ret;
> -	int year = tm->tm_year - 100;
>  
>  	dev_dbg(dev, "s3c_rtc_setalarm: %d, %04d.%02d.%02d %02d:%02d:%02d\n",
>  		alrm->enabled,
> @@ -356,11 +355,6 @@ static int s3c_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  		writeb(bin2bcd(tm->tm_hour), info->base + S3C2410_ALMHOUR);
>  	}
>  
> -	if (year < 100 && year >= 0) {
> -		alrm_en |= S3C2410_RTCALM_YEAREN;
> -		writeb(bin2bcd(year), info->base + S3C2410_ALMYEAR);
> -	}
> -
>  	if (tm->tm_mon < 12 && tm->tm_mon >= 0) {
>  		alrm_en |= S3C2410_RTCALM_MONEN;
>  		writeb(bin2bcd(tm->tm_mon + 1), info->base + S3C2410_ALMMON);
> -- 
> 2.17.1
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com