Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 9 Mar 2022 22:29:43 +0100
From:   Alexandre Belloni <alexandre.belloni@bootlin.com>
To:     Hanna Hawa <hhhawa@amazon.com>
Cc:     a.zummo@towertech.it, linux-rtc@vger.kernel.org,
        linux-kernel@vger.kernel.org, dwmw@amazon.co.uk, benh@amazon.com,
        eduval@amazon.com, ronenk@amazon.com, talel@amazon.com,
        jonnyc@amazon.com, hanochu@amazon.com, farbere@amazon.com
Subject: Re: [RFC PATCH 1/1] rtc: bq32k: update years calculation according
 century bit
Message-ID: <YikcR25H8x1ambO9@piout.net>
References: <20220207070156.19373-1-hhhawa@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220207070156.19373-1-hhhawa@amazon.com>
Precedence: bulk

Hello,

On 07/02/2022 09:01:56+0200, Hanna Hawa wrote:
> tm_year filed hold the number of years since 1900, in case the century
> was changed the driver will return invalid year, as it will not
> increment the years field by 200.
> 
> This change update the years calculation in bq32k_rtc_read_time() and
> bq32k_rtc_write_time(). By increasing the years by 100 always, and only
> if the century enable bit (BQ32K_CENT_EN) is set and century bit
> (BQ32K_CENT) is cleared will increase again by 100 to represent the next
> century.
> 

I'm not sure I get the issue, currently, the driver considers that
BQ32K_CENT not set is 19YY and BQ32K_CENT set is 20YY and what is done
seems fine to me. Can you elaborate on what you are trying to fix?

> Signed-off-by: Hanna Hawa <hhhawa@amazon.com>
> ---
>  drivers/rtc/rtc-bq32k.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c
> index 2235c968842d..09795dd2728b 100644
> --- a/drivers/rtc/rtc-bq32k.c
> +++ b/drivers/rtc/rtc-bq32k.c
> @@ -108,8 +108,20 @@ static int bq32k_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	tm->tm_mday = bcd2bin(regs.date);
>  	tm->tm_wday = bcd2bin(regs.day) - 1;
>  	tm->tm_mon = bcd2bin(regs.month) - 1;
> -	tm->tm_year = bcd2bin(regs.years) +
> -				((regs.cent_hours & BQ32K_CENT) ? 100 : 0);
> +	/*
> +	 * tm_year is number of years since 1900. Need to increase the years by
> +	 * 100 always assuming we are on 20YY and not 19YY.
> +	 */
> +	tm->tm_year = bcd2bin(regs.years) + 100;
> +
> +	/*
> +	 * If the century enable bit (BQ32K_CENT_EN) is set, and century bit
> +	 * (BQ32K_CENT) is cleared, that means we are on the next century, which
> +	 * required to increase by 100.
> +	 */
> +	if ((regs.cent_hours & BQ32K_CENT_EN) &&
> +	    !(regs.cent_hours & BQ32K_CENT))
> +		tm->tm_year += 100;
>  
>  	return 0;
>  }
> @@ -117,6 +129,7 @@ static int bq32k_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  static int bq32k_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  {
>  	struct bq32k_regs regs;
> +	int year;
>  
>  	regs.seconds = bin2bcd(tm->tm_sec);
>  	regs.minutes = bin2bcd(tm->tm_min);
> @@ -125,11 +138,15 @@ static int bq32k_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	regs.date = bin2bcd(tm->tm_mday);
>  	regs.month = bin2bcd(tm->tm_mon + 1);
>  
> -	if (tm->tm_year >= 100) {
> +	/* Assume we are on 20YY and not 19YY */
> +	year = tm->tm_year - 100;
> +
> +	if (year < 100) {
>  		regs.cent_hours |= BQ32K_CENT;
> -		regs.years = bin2bcd(tm->tm_year - 100);
> -	} else
> -		regs.years = bin2bcd(tm->tm_year);
> +		regs.years = bin2bcd(year);
> +	} else {
> +		regs.years = bin2bcd(year - 100);
> +	}
>  
>  	return bq32k_write(dev, &regs, 0, sizeof(regs));
>  }
> -- 
> 2.17.1
> 

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