Re: [PATCH v2 2/2] rtc: mt6397: Fix mt6357 RTC year offset handling for hwclock commands

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 02/04/25 12:51, Alexandre Mergnat ha scritto:
> The mt6357 RTC was failing when using the `hwclock -r --verbose` command,
> despite reading correctly through sysfs. There is high chance for other
> platform to be impacted by the year offset handling issue.
> 
> The hardware RTC registers store years relative to 1968, but the driver
> wasn't consistently applying the offset when converting between
> hardware and Linux time representation.
> 
> This inconsistency caused alarm rollover failures during device
> registration, with the error "alarm rollover not handled -22" in the
> logs, causing hwclock commands to fail.
> 
> The ioctl interface used by the hwclock command requires proper time
> range validation that wasn't happening with the inconsistent year
> offsets.
> 
> Fixes the issue by applying the year offset in all operations:
>     - Adding (RTC_MIN_YEAR - RTC_BASE_YEAR) when reading from hardware
>     - Subtracting the same offset when writing to hardware
>     - Using the same logic for both regular time and alarm operations
> 
> With these changes, the hwclock command works correctly and time
> values are consistently handled across all interfaces.
> 
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
>   drivers/rtc/rtc-mt6397.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index 692c00ff544b2..ba52e225dc8fa 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -77,7 +77,8 @@ static int __mtk_rtc_read_time(struct mt6397_rtc *rtc,
>   	tm->tm_mday = data[RTC_OFFSET_DOM];
>   	tm->tm_wday = data[RTC_OFFSET_DOW];
>   	tm->tm_mon = data[RTC_OFFSET_MTH] & RTC_TC_MTH_MASK;
> -	tm->tm_year = data[RTC_OFFSET_YEAR];
> +	/* The RTC registers store years since 1968 (hardware's base year) */
> +	tm->tm_year = data[RTC_OFFSET_YEAR] + (RTC_MIN_YEAR - RTC_BASE_YEAR);

This patch received a NACK because of RTC_MIN_YEAR_OFFSET.

What you're doing here is avoiding to use the "RTC_MIN_YEAR_OFFSET" definition name
but otherwise doing the very same thing that was NACKed before.

Regards,
Angelo

>   
>   	ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_TC_SEC, sec);
>   exit:
> @@ -119,7 +120,8 @@ static int mtk_rtc_set_time(struct device *dev, struct rtc_time *tm)
>   	data[RTC_OFFSET_DOM] = tm->tm_mday;
>   	data[RTC_OFFSET_DOW] = tm->tm_wday;
>   	data[RTC_OFFSET_MTH] = tm->tm_mon;
> -	data[RTC_OFFSET_YEAR] = tm->tm_year;
> +	/* Convert from tm_year (years since 1900) to RTC register format (years since 1968) */
> +	data[RTC_OFFSET_YEAR] = tm->tm_year - (RTC_MIN_YEAR - RTC_BASE_YEAR);
>   
>   	mutex_lock(&rtc->lock);
>   	ret = regmap_bulk_write(rtc->regmap, rtc->addr_base + RTC_TC_SEC,
> @@ -165,8 +167,8 @@ static int mtk_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
>   	tm->tm_hour = data[RTC_OFFSET_HOUR] & RTC_AL_HOU_MASK;
>   	tm->tm_mday = data[RTC_OFFSET_DOM] & RTC_AL_DOM_MASK;
>   	tm->tm_mon = data[RTC_OFFSET_MTH] & RTC_AL_MTH_MASK;
> -	tm->tm_year = data[RTC_OFFSET_YEAR] & RTC_AL_YEA_MASK;
> -
> +	/* Apply the same offset conversion for alarm read */
> +	tm->tm_year = (data[RTC_OFFSET_YEAR] & RTC_AL_YEA_MASK) + (RTC_MIN_YEAR - RTC_BASE_YEAR);
>   	tm->tm_mon--;
>   
>   	return 0;
> @@ -200,8 +202,9 @@ static int mtk_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
>   				(tm->tm_mday & RTC_AL_DOM_MASK));
>   	data[RTC_OFFSET_MTH] = ((data[RTC_OFFSET_MTH] & ~(RTC_AL_MTH_MASK)) |
>   				(tm->tm_mon & RTC_AL_MTH_MASK));
> +	/* Convert alarm year using the same offset as in read/write time */
>   	data[RTC_OFFSET_YEAR] = ((data[RTC_OFFSET_YEAR] & ~(RTC_AL_YEA_MASK)) |
> -				(tm->tm_year & RTC_AL_YEA_MASK));
> +				((tm->tm_year - (RTC_MIN_YEAR - RTC_BASE_YEAR)) & RTC_AL_YEA_MASK));
>   
>   	if (alm->enabled) {
>   		ret = regmap_bulk_write(rtc->regmap,
>