From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D7406C4708A for ; Tue, 25 May 2021 21:18:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B870D61423 for ; Tue, 25 May 2021 21:18:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232678AbhEYVTv (ORCPT ); Tue, 25 May 2021 17:19:51 -0400 Received: from relay2-d.mail.gandi.net ([217.70.183.194]:43703 "EHLO relay2-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232526AbhEYVTu (ORCPT ); Tue, 25 May 2021 17:19:50 -0400 Received: (Authenticated sender: alexandre.belloni@bootlin.com) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id D359C40003; Tue, 25 May 2021 21:18:17 +0000 (UTC) Date: Tue, 25 May 2021 23:18:17 +0200 From: Alexandre Belloni To: "qxj511mail@gmail.com" Cc: Nobuhiro Iwamatsu , "a.zummo" , linux-rtc , linux-kernel , =?utf-8?B?6YKx5pmT6YeR?= Subject: Re: Re: [PATCH] rtc: rs5c372: Fix read the time from RTC is illegal When reading time from an uninitialized RTC chip, The value may be illegal Message-ID: References: <20210520033156.23209-1-qxj511mail@gmail.com> <202105251924130320028@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <202105251924130320028@gmail.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 25/05/2021 19:24:14+0800, qxj511mail@gmail.com wrote: > Please briefly describe the patch contained in the email to the subject. > And, please write a description of the patch in the text > ----> The legal days are 1 to 31 and the legal month is less than or equal to 12. > But for an uninitialized RTC chip, the time is random. > Days and month may be zero, leading to RTC_valid_TM failed > If the time on the RTC is invalid, then rtc_valid_tm has to fail, there is nothing to fix here. > add description && fix build error, so I modefied my code : > > Signed-off-by: qiuxiaojin > --- > drivers/rtc/rtc-rs5c372.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c > index 3bd6eaa0dcf6..cb3f90983778 100644 > --- a/drivers/rtc/rtc-rs5c372.c > +++ b/drivers/rtc/rtc-rs5c372.c > @@ -128,6 +128,9 @@ struct rs5c372 { > char *regs; > }; > > + > +static int rs5c372_rtc_set_time(struct device *dev, struct rtc_time *tm); > + > static int rs5c_get_regs(struct rs5c372 *rs5c) > { > struct i2c_client *client = rs5c->client; > @@ -212,6 +215,7 @@ static int rs5c372_rtc_read_time(struct device *dev, struct rtc_time *tm) > struct rs5c372 *rs5c = i2c_get_clientdata(client); > int status = rs5c_get_regs(rs5c); > unsigned char ctrl2 = rs5c->regs[RS5C_REG_CTRL2]; > + int flags_utime = 0; > > if (status < 0) > return status; > @@ -239,12 +243,28 @@ static int rs5c372_rtc_read_time(struct device *dev, struct rtc_time *tm) > tm->tm_wday = bcd2bin(rs5c->regs[RS5C372_REG_WDAY] & 0x07); > tm->tm_mday = bcd2bin(rs5c->regs[RS5C372_REG_DAY] & 0x3f); > > + /* The value read from the register may be zero, which is an illegal value */ > + if (tm->tm_mday < 1) { > + flags_utime++; > + tm->tm_mday = 1; > + } > + > /* tm->tm_mon is zero-based */ > tm->tm_mon = bcd2bin(rs5c->regs[RS5C372_REG_MONTH] & 0x1f) - 1; > > + if (tm->tm_mon < 0) { > + /* avoid illegal month */ > + flags_utime++; > + tm->tm_mon = 0; > + } > + > /* year is 1900 + tm->tm_year */ > tm->tm_year = bcd2bin(rs5c->regs[RS5C372_REG_YEAR]) + 100; > > + /* update legal time */ > + if (flags_utime > 0) > + rs5c372_rtc_set_time(dev, tm); > + > dev_dbg(&client->dev, "%s: tm is secs=%d, mins=%d, hours=%d, " > "mday=%d, mon=%d, year=%d, wday=%d\n", > __func__, > -- > 2.29.0 > > > > qxj511mail@gmail.com > > From: Nobuhiro Iwamatsu > Date: 2021-05-24 12:40 > To: qxj511mail > CC: Alessandro Zummo; Alexandre Belloni; linux-rtc; Linux Kernel Mailing List; qiuxiaojin > Subject: Re: [PATCH] rtc: rs5c372: Fix read the time from RTC is illegal When reading time from an uninitialized RTC chip, The value may be illegal > Hi, > > 2021年5月20日(木) 12:32 : > > > > From: qiuxiaojin > > Please briefly describe the patch contained in the email to the subject. > And, please write a description of the patch in the text > > > > > Signed-off-by: qiuxiaojin > > --- > > drivers/rtc/rtc-rs5c372.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c > > index 3bd6eaa0dcf6..ce61e15d5f3a 100644 > > --- a/drivers/rtc/rtc-rs5c372.c > > +++ b/drivers/rtc/rtc-rs5c372.c > > @@ -212,6 +212,7 @@ static int rs5c372_rtc_read_time(struct device *dev, struct rtc_time *tm) > > struct rs5c372 *rs5c = i2c_get_clientdata(client); > > int status = rs5c_get_regs(rs5c); > > unsigned char ctrl2 = rs5c->regs[RS5C_REG_CTRL2]; > > + int flags_utime = 0; > > > > if (status < 0) > > return status; > > @@ -239,12 +240,27 @@ static int rs5c372_rtc_read_time(struct device *dev, struct rtc_time *tm) > > tm->tm_wday = bcd2bin(rs5c->regs[RS5C372_REG_WDAY] & 0x07); > > tm->tm_mday = bcd2bin(rs5c->regs[RS5C372_REG_DAY] & 0x3f); > > > > + if (tm->tm_mday < 1) { > > + // The value read from the register may be zero, which is an illegal value > > Please use C89 style commet (/* */). > > > + flags_utime = flags_utime + 1; > > I like using ++ (flags_utime++). > > > + tm->tm_mday = 1; > > + } > > + > > /* tm->tm_mon is zero-based */ > > tm->tm_mon = bcd2bin(rs5c->regs[RS5C372_REG_MONTH] & 0x1f) - 1; > > > > + if (tm->tm_mon < 0) { > > + flags_utime = flags_utime + 1; > > + tm->tm_mday = 0; > > + } > > + > > /* year is 1900 + tm->tm_year */ > > tm->tm_year = bcd2bin(rs5c->regs[RS5C372_REG_YEAR]) + 100; > > > > + if (flags_utime > 0) { > > {} is unnecessary. > > > + rs5c372_rtc_set_time(dev, tm); > > A build error will occur because there is no declaration of > rs5c372_rtc_set_time. > Please make sure this can be compiled. > > > + } > > + > > dev_dbg(&client->dev, "%s: tm is secs=%d, mins=%d, hours=%d, " > > "mday=%d, mon=%d, year=%d, wday=%d\n", > > __func__, > > -- > > 2.29.0 > > > > > -- > Nobuhiro Iwamatsu > iwamatsu at {nigauri.org / debian.org} > GPG ID: 40AD1FA6 -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com