| Message ID | 20221215150214.1109074-3-hugo@hugovil.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:adf:e747:0:0:0:0:0 with SMTP id c7csp433533wrn;
Thu, 15 Dec 2022 07:40:55 -0800 (PST)
X-Google-Smtp-Source:
AA0mqf7s7j26ysI4RRDJEjVrtmuygfmh7XQNxJZ/NqxyDnySyk2amdx+ZczyZl2t93lcEEEnhp/h
X-Received: by 2002:a05:6a20:9596:b0:aa:ac2:93c7 with SMTP id
iu22-20020a056a20959600b000aa0ac293c7mr35501977pzb.54.1671118854859;
Thu, 15 Dec 2022 07:40:54 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1671118854; cv=none;
d=google.com; s=arc-20160816;
b=ISYKY1GvcGz5LiKmFNPQkRfZeR0ivDzrmAsK2K4L6lGaOX1xHB5DRHoX1bXZgzpNVQ
gttaCcfcWcC2+/hHWQsRETFwlIntzc+uWpHSeNBYq1qA8FoiDO6J/EB6niO2flUmZgBy
ZS3GhQUCR6SOc6kvj/iiBJKzxWbjGQxkEhQStXbB8y7ZU4b9PtqK3ou/pcc7gOiK8oYu
UP8MYt+VovCLwRcpOuzARfN3eRuOw2F8Ae4Fos4iL3zrG69sMtpu8hI8Nlp8y9mQdO2I
lFBQjg9esAZHQ6gQHOF53UWRr+J071j1Sp54dz01YcqeWaxiaCTDmoKeQv9mgLbOFjwP
gF/w==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=list-id:precedence:subject:content-transfer-encoding:mime-version
:references:in-reply-to:message-id:date:cc:to:from:dkim-signature;
bh=DUk52/OBG9O2b+mwH7NQ1X4tP0YNoZQ+so1GRMEfi1E=;
b=FI9cQArEOtwHPkO/gFJDTFbgxz2ckxjYlflxIojvZuorTCxO8C1hjjWtJQZWmROcnR
4/1d78Tr5fF8C/H2HZejB5Obdy7/KxQCPK8SQsAeLDr4pStTBPHIpRARbCtWDalMBsMF
MquSH6ZcUUMt/t/qdCbJ//9WKa5Kkgp6PjDqYFTbdCxmGGBygkVzkU5soNml2NkO/AtH
tRVtwqX3yYCYtU8kyGhyWevY2WS0YyTgt/fZC0pBd04YP8wqTzJKztBGnljzWKfgrfUe
xFdx6hW5Dju3ITtR6TJy0hT1JeAiB5/vIneSnhH0QeV8eAojhmc1RPJrBvc1LoaZikU+
b+KQ==
ARC-Authentication-Results: i=1; mx.google.com;
dkim=fail header.i=@hugovil.com header.s=x header.b=qwPJXTaq;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
n5-20020a654cc5000000b00478d5e8c4f5si2818542pgt.383.2022.12.15.07.40.39;
Thu, 15 Dec 2022 07:40:54 -0800 (PST)
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;
Authentication-Results: mx.google.com;
dkim=fail header.i=@hugovil.com header.s=x header.b=qwPJXTaq;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S230258AbiLOPUB (ORCPT <rfc822;jeantsuru.cumc.mandola@gmail.com>
+ 99 others); Thu, 15 Dec 2022 10:20:01 -0500
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48140 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S230228AbiLOPTZ (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Thu, 15 Dec 2022 10:19:25 -0500
Received: from mail.hugovil.com (mail.hugovil.com [162.243.120.170])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D82CF31DD5;
Thu, 15 Dec 2022 07:18:35 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=hugovil.com
; s=x; h=Subject:Content-Transfer-Encoding:MIME-Version:References:
In-Reply-To:Message-Id:Date:Cc:To:From:Sender:Reply-To:Content-Type:
Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender:
Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:
List-Subscribe:List-Post:List-Owner:List-Archive;
bh=DUk52/OBG9O2b+mwH7NQ1X4tP0YNoZQ+so1GRMEfi1E=;
b=qwPJXTaqJTga6gu/CpIuhowGGY
UMYkvlM5UKC2M8hqHEbhBzJO6KLw80kIQCtAUTHwNV5bKYjq5yHy5Y9tQC+RbrVhNnbEK1+xMqSXc
rvyV8TA22Ac/rVt/XH3qjsJxqDVxR7nakH29tksgitFBWmVl2trxBKa+xnN8qoz/KA48=;
Received: from modemcable168.174-80-70.mc.videotron.ca ([70.80.174.168]:48102
helo=pettiford.lan)
by mail.hugovil.com with esmtpa (Exim 4.92)
(envelope-from <hugo@hugovil.com>)
id 1p5pm9-0000EC-QT; Thu, 15 Dec 2022 10:03:58 -0500
From: Hugo Villeneuve <hugo@hugovil.com>
To: a.zummo@towertech.it, alexandre.belloni@bootlin.com,
robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org
Cc: linux-rtc@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, hugo@hugovil.com,
Hugo Villeneuve <hvilleneuve@dimonoff.com>
Date: Thu, 15 Dec 2022 10:02:03 -0500
Message-Id: <20221215150214.1109074-3-hugo@hugovil.com>
X-Mailer: git-send-email 2.30.2
In-Reply-To: <20221215150214.1109074-1-hugo@hugovil.com>
References: <20221215150214.1109074-1-hugo@hugovil.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SA-Exim-Connect-IP: 70.80.174.168
X-SA-Exim-Mail-From: hugo@hugovil.com
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
X-Spam-Level:
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_PASS,SPF_PASS
autolearn=ham autolearn_force=no version=3.4.6
Subject: [PATCH v3 02/14] rtc: pcf2127: adapt for time/date registers at any
offset
X-SA-Exim-Version: 4.2.1 (built Wed, 08 May 2019 21:11:16 +0000)
X-SA-Exim-Scanned: Yes (on mail.hugovil.com)
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1752295124440773425?=
X-GMAIL-MSGID: =?utf-8?q?1752295124440773425?=
|
| Series |
rtc: pcf2127: add PCF2131 driver
|
|
Commit Message
Hugo Villeneuve
Dec. 15, 2022, 3:02 p.m. UTC
From: Hugo Villeneuve <hvilleneuve@dimonoff.com> This will simplify the implementation of new variants into this driver. Some variants (PCF2131) have a 100th seconds register. This register is currently not supported in this driver. Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> --- drivers/rtc/rtc-pcf2127.c | 68 ++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 29 deletions(-)
Comments
Den tor. 15. dec. 2022 kl. 16.20 skrev Hugo Villeneuve <hugo@hugovil.com>: > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > This will simplify the implementation of new variants into this driver. > > Some variants (PCF2131) have a 100th seconds register. This register is > currently not supported in this driver. > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > --- > drivers/rtc/rtc-pcf2127.c | 68 ++++++++++++++++++++++----------------- > 1 file changed, 39 insertions(+), 29 deletions(-) > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > index b9a5d47a439f..fb0caacaabee 100644 > --- a/drivers/rtc/rtc-pcf2127.c > +++ b/drivers/rtc/rtc-pcf2127.c > @@ -44,14 +44,17 @@ > #define PCF2127_BIT_CTRL3_BF BIT(3) > #define PCF2127_BIT_CTRL3_BTSE BIT(4) > /* Time and date registers */ > -#define PCF2127_REG_SC 0x03 > +#define PCF2127_REG_TIME_DATE_BASE 0x03 > +/* Time and date registers offsets (starting from base register) */ > +#define PCF2127_OFFSET_TD_SC 0 > +#define PCF2127_OFFSET_TD_MN 1 > +#define PCF2127_OFFSET_TD_HR 2 > +#define PCF2127_OFFSET_TD_DM 3 > +#define PCF2127_OFFSET_TD_DW 4 > +#define PCF2127_OFFSET_TD_MO 5 > +#define PCF2127_OFFSET_TD_YR 6 > +/* Time and date registers bits */ > #define PCF2127_BIT_SC_OSF BIT(7) > -#define PCF2127_REG_MN 0x04 > -#define PCF2127_REG_HR 0x05 > -#define PCF2127_REG_DM 0x06 > -#define PCF2127_REG_DW 0x07 > -#define PCF2127_REG_MO 0x08 > -#define PCF2127_REG_YR 0x09 > /* Alarm registers */ > #define PCF2127_REG_ALARM_SC 0x0A > #define PCF2127_REG_ALARM_MN 0x0B > @@ -106,6 +109,7 @@ struct pcf21xx_config { > int max_register; > unsigned int has_nvmem:1; > unsigned int has_bit_wd_ctl_cd0:1; > + u8 regs_td_base; /* Time/data base registers. */ There is only one base register, so no need to add s. I think td is an odd short, so maybe u8 reg_time_base. /Bruno > }; > > struct pcf2127 { > @@ -125,27 +129,31 @@ struct pcf2127 { > static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > { > struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > - unsigned char buf[10]; > + unsigned char buf[7]; > + unsigned int ctrl3; > int ret; > > /* > * Avoid reading CTRL2 register as it causes WD_VAL register > * value to reset to 0 which means watchdog is stopped. > */ > - ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_CTRL3, > - (buf + PCF2127_REG_CTRL3), > - ARRAY_SIZE(buf) - PCF2127_REG_CTRL3); > - if (ret) { > - dev_err(dev, "%s: read error\n", __func__); > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &ctrl3); > + if (ret) > return ret; > - } > > - if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF) > + if (ctrl3 & PCF2127_BIT_CTRL3_BLF) > dev_info(dev, > "low voltage detected, check/replace RTC battery.\n"); > > + ret = regmap_bulk_read(pcf2127->regmap, pcf2127->cfg->regs_td_base, > + buf, sizeof(buf)); > + if (ret) { > + dev_err(dev, "%s: read error\n", __func__); > + return ret; > + } > + > /* Clock integrity is not guaranteed when OSF flag is set. */ > - if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) { > + if (buf[PCF2127_OFFSET_TD_SC] & PCF2127_BIT_SC_OSF) { > /* > * no need clear the flag here, > * it will be cleared once the new date is saved > @@ -158,18 +166,18 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > dev_dbg(dev, > "%s: raw data is cr3=%02x, sec=%02x, min=%02x, hr=%02x, " > "mday=%02x, wday=%02x, mon=%02x, year=%02x\n", > - __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC], > - buf[PCF2127_REG_MN], buf[PCF2127_REG_HR], > - buf[PCF2127_REG_DM], buf[PCF2127_REG_DW], > - buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]); > - > - tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F); > - tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F); > - tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F); /* rtc hr 0-23 */ > - tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F); > - tm->tm_wday = buf[PCF2127_REG_DW] & 0x07; > - tm->tm_mon = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > - tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]); > + __func__, ctrl3, buf[PCF2127_OFFSET_TD_SC], > + buf[PCF2127_OFFSET_TD_MN], buf[PCF2127_OFFSET_TD_HR], > + buf[PCF2127_OFFSET_TD_DM], buf[PCF2127_OFFSET_TD_DW], > + buf[PCF2127_OFFSET_TD_MO], buf[PCF2127_OFFSET_TD_YR]); > + > + tm->tm_sec = bcd2bin(buf[PCF2127_OFFSET_TD_SC] & 0x7F); > + tm->tm_min = bcd2bin(buf[PCF2127_OFFSET_TD_MN] & 0x7F); > + tm->tm_hour = bcd2bin(buf[PCF2127_OFFSET_TD_HR] & 0x3F); /* rtc hr 0-23 */ > + tm->tm_mday = bcd2bin(buf[PCF2127_OFFSET_TD_DM] & 0x3F); > + tm->tm_wday = buf[PCF2127_OFFSET_TD_DW] & 0x07; > + tm->tm_mon = bcd2bin(buf[PCF2127_OFFSET_TD_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > + tm->tm_year = bcd2bin(buf[PCF2127_OFFSET_TD_YR]); > tm->tm_year += 100; > > dev_dbg(dev, "%s: tm is secs=%d, mins=%d, hours=%d, " > @@ -207,7 +215,7 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm) > buf[i++] = bin2bcd(tm->tm_year - 100); > > /* write register's data */ > - err = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_SC, buf, i); > + err = regmap_bulk_write(pcf2127->regmap, pcf2127->cfg->regs_td_base, buf, i); > if (err) { > dev_err(dev, > "%s: err=%d", __func__, err); > @@ -650,11 +658,13 @@ static struct pcf21xx_config pcf21xx_cfg[] = { > .max_register = 0x1d, > .has_nvmem = 1, > .has_bit_wd_ctl_cd0 = 1, > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > }, > [PCF2129] = { > .max_register = 0x19, > .has_nvmem = 0, > .has_bit_wd_ctl_cd0 = 0, > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > }, > }; > > -- > 2.30.2 >
On Mon, 19 Dec 2022 10:34:08 +0100 Bruno Thomsen <bruno.thomsen@gmail.com> wrote: > Den tor. 15. dec. 2022 kl. 16.20 skrev Hugo Villeneuve <hugo@hugovil.com>: > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > This will simplify the implementation of new variants into this driver. > > > > Some variants (PCF2131) have a 100th seconds register. This register is > > currently not supported in this driver. > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > --- > > drivers/rtc/rtc-pcf2127.c | 68 ++++++++++++++++++++++----------------- > > 1 file changed, 39 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > > index b9a5d47a439f..fb0caacaabee 100644 > > --- a/drivers/rtc/rtc-pcf2127.c > > +++ b/drivers/rtc/rtc-pcf2127.c > > @@ -44,14 +44,17 @@ > > #define PCF2127_BIT_CTRL3_BF BIT(3) > > #define PCF2127_BIT_CTRL3_BTSE BIT(4) > > /* Time and date registers */ > > -#define PCF2127_REG_SC 0x03 > > +#define PCF2127_REG_TIME_DATE_BASE 0x03 > > +/* Time and date registers offsets (starting from base register) */ > > +#define PCF2127_OFFSET_TD_SC 0 > > +#define PCF2127_OFFSET_TD_MN 1 > > +#define PCF2127_OFFSET_TD_HR 2 > > +#define PCF2127_OFFSET_TD_DM 3 > > +#define PCF2127_OFFSET_TD_DW 4 > > +#define PCF2127_OFFSET_TD_MO 5 > > +#define PCF2127_OFFSET_TD_YR 6 > > +/* Time and date registers bits */ > > #define PCF2127_BIT_SC_OSF BIT(7) > > -#define PCF2127_REG_MN 0x04 > > -#define PCF2127_REG_HR 0x05 > > -#define PCF2127_REG_DM 0x06 > > -#define PCF2127_REG_DW 0x07 > > -#define PCF2127_REG_MO 0x08 > > -#define PCF2127_REG_YR 0x09 > > /* Alarm registers */ > > #define PCF2127_REG_ALARM_SC 0x0A > > #define PCF2127_REG_ALARM_MN 0x0B > > @@ -106,6 +109,7 @@ struct pcf21xx_config { > > int max_register; > > unsigned int has_nvmem:1; > > unsigned int has_bit_wd_ctl_cd0:1; > > + u8 regs_td_base; /* Time/data base registers. */ > > There is only one base register, so no need to add s. > I think td is an odd short, so maybe u8 reg_time_base. > > /Bruno Hi, it makes sense, I will fix it in V4. I will also fix the comment "Time/data" -> "Time/date". Hugo. > > }; > > > > struct pcf2127 { > > @@ -125,27 +129,31 @@ struct pcf2127 { > > static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > > { > > struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > > - unsigned char buf[10]; > > + unsigned char buf[7]; > > + unsigned int ctrl3; > > int ret; > > > > /* > > * Avoid reading CTRL2 register as it causes WD_VAL register > > * value to reset to 0 which means watchdog is stopped. > > */ > > - ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_CTRL3, > > - (buf + PCF2127_REG_CTRL3), > > - ARRAY_SIZE(buf) - PCF2127_REG_CTRL3); > > - if (ret) { > > - dev_err(dev, "%s: read error\n", __func__); > > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &ctrl3); > > + if (ret) > > return ret; > > - } > > > > - if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF) > > + if (ctrl3 & PCF2127_BIT_CTRL3_BLF) > > dev_info(dev, > > "low voltage detected, check/replace RTC battery.\n"); > > > > + ret = regmap_bulk_read(pcf2127->regmap, pcf2127->cfg->regs_td_base, > > + buf, sizeof(buf)); > > + if (ret) { > > + dev_err(dev, "%s: read error\n", __func__); > > + return ret; > > + } > > + > > /* Clock integrity is not guaranteed when OSF flag is set. */ > > - if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) { > > + if (buf[PCF2127_OFFSET_TD_SC] & PCF2127_BIT_SC_OSF) { > > /* > > * no need clear the flag here, > > * it will be cleared once the new date is saved > > @@ -158,18 +166,18 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > > dev_dbg(dev, > > "%s: raw data is cr3=%02x, sec=%02x, min=%02x, hr=%02x, " > > "mday=%02x, wday=%02x, mon=%02x, year=%02x\n", > > - __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC], > > - buf[PCF2127_REG_MN], buf[PCF2127_REG_HR], > > - buf[PCF2127_REG_DM], buf[PCF2127_REG_DW], > > - buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]); > > - > > - tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F); > > - tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F); > > - tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F); /* rtc hr 0-23 */ > > - tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F); > > - tm->tm_wday = buf[PCF2127_REG_DW] & 0x07; > > - tm->tm_mon = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > > - tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]); > > + __func__, ctrl3, buf[PCF2127_OFFSET_TD_SC], > > + buf[PCF2127_OFFSET_TD_MN], buf[PCF2127_OFFSET_TD_HR], > > + buf[PCF2127_OFFSET_TD_DM], buf[PCF2127_OFFSET_TD_DW], > > + buf[PCF2127_OFFSET_TD_MO], buf[PCF2127_OFFSET_TD_YR]); > > + > > + tm->tm_sec = bcd2bin(buf[PCF2127_OFFSET_TD_SC] & 0x7F); > > + tm->tm_min = bcd2bin(buf[PCF2127_OFFSET_TD_MN] & 0x7F); > > + tm->tm_hour = bcd2bin(buf[PCF2127_OFFSET_TD_HR] & 0x3F); /* rtc hr 0-23 */ > > + tm->tm_mday = bcd2bin(buf[PCF2127_OFFSET_TD_DM] & 0x3F); > > + tm->tm_wday = buf[PCF2127_OFFSET_TD_DW] & 0x07; > > + tm->tm_mon = bcd2bin(buf[PCF2127_OFFSET_TD_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > > + tm->tm_year = bcd2bin(buf[PCF2127_OFFSET_TD_YR]); > > tm->tm_year += 100; > > > > dev_dbg(dev, "%s: tm is secs=%d, mins=%d, hours=%d, " > > @@ -207,7 +215,7 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm) > > buf[i++] = bin2bcd(tm->tm_year - 100); > > > > /* write register's data */ > > - err = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_SC, buf, i); > > + err = regmap_bulk_write(pcf2127->regmap, pcf2127->cfg->regs_td_base, buf, i); > > if (err) { > > dev_err(dev, > > "%s: err=%d", __func__, err); > > @@ -650,11 +658,13 @@ static struct pcf21xx_config pcf21xx_cfg[] = { > > .max_register = 0x1d, > > .has_nvmem = 1, > > .has_bit_wd_ctl_cd0 = 1, > > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > > }, > > [PCF2129] = { > > .max_register = 0x19, > > .has_nvmem = 0, > > .has_bit_wd_ctl_cd0 = 0, > > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > > }, > > }; > > > > -- > > 2.30.2 > > >
Den man. 19. dec. 2022 kl. 10.34 skrev Bruno Thomsen <bruno.thomsen@gmail.com>: > > Den tor. 15. dec. 2022 kl. 16.20 skrev Hugo Villeneuve <hugo@hugovil.com>: > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > This will simplify the implementation of new variants into this driver. > > > > Some variants (PCF2131) have a 100th seconds register. This register is > > currently not supported in this driver. > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > --- > > drivers/rtc/rtc-pcf2127.c | 68 ++++++++++++++++++++++----------------- > > 1 file changed, 39 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > > index b9a5d47a439f..fb0caacaabee 100644 > > --- a/drivers/rtc/rtc-pcf2127.c > > +++ b/drivers/rtc/rtc-pcf2127.c > > @@ -44,14 +44,17 @@ > > #define PCF2127_BIT_CTRL3_BF BIT(3) > > #define PCF2127_BIT_CTRL3_BTSE BIT(4) > > /* Time and date registers */ > > -#define PCF2127_REG_SC 0x03 > > +#define PCF2127_REG_TIME_DATE_BASE 0x03 > > +/* Time and date registers offsets (starting from base register) */ > > +#define PCF2127_OFFSET_TD_SC 0 Maybe do the same change for register defines as the pc721xx_config struct parameter. This will also match what you did in patch 3 with alarm registers. - PCF2127_REG_TIME_DATE_BASE + PCF2127_REG_TIME_BASE - PCF2127_OFFSET_TD_SC + PCF2127_OFFSET_TIME_SC etc. > > +#define PCF2127_OFFSET_TD_MN 1 > > +#define PCF2127_OFFSET_TD_HR 2 > > +#define PCF2127_OFFSET_TD_DM 3 > > +#define PCF2127_OFFSET_TD_DW 4 > > +#define PCF2127_OFFSET_TD_MO 5 > > +#define PCF2127_OFFSET_TD_YR 6 > > +/* Time and date registers bits */ > > #define PCF2127_BIT_SC_OSF BIT(7) > > -#define PCF2127_REG_MN 0x04 > > -#define PCF2127_REG_HR 0x05 > > -#define PCF2127_REG_DM 0x06 > > -#define PCF2127_REG_DW 0x07 > > -#define PCF2127_REG_MO 0x08 > > -#define PCF2127_REG_YR 0x09 > > /* Alarm registers */ > > #define PCF2127_REG_ALARM_SC 0x0A > > #define PCF2127_REG_ALARM_MN 0x0B > > @@ -106,6 +109,7 @@ struct pcf21xx_config { > > int max_register; > > unsigned int has_nvmem:1; > > unsigned int has_bit_wd_ctl_cd0:1; > > + u8 regs_td_base; /* Time/data base registers. */ > > There is only one base register, so no need to add s. > I think td is an odd short, so maybe u8 reg_time_base. > > /Bruno > > > }; > > > > struct pcf2127 { > > @@ -125,27 +129,31 @@ struct pcf2127 { > > static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > > { > > struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > > - unsigned char buf[10]; > > + unsigned char buf[7]; > > + unsigned int ctrl3; > > int ret; > > > > /* > > * Avoid reading CTRL2 register as it causes WD_VAL register > > * value to reset to 0 which means watchdog is stopped. > > */ > > - ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_CTRL3, > > - (buf + PCF2127_REG_CTRL3), > > - ARRAY_SIZE(buf) - PCF2127_REG_CTRL3); > > - if (ret) { > > - dev_err(dev, "%s: read error\n", __func__); > > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &ctrl3); > > + if (ret) > > return ret; > > - } > > > > - if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF) > > + if (ctrl3 & PCF2127_BIT_CTRL3_BLF) > > dev_info(dev, > > "low voltage detected, check/replace RTC battery.\n"); > > > > + ret = regmap_bulk_read(pcf2127->regmap, pcf2127->cfg->regs_td_base, > > + buf, sizeof(buf)); > > + if (ret) { > > + dev_err(dev, "%s: read error\n", __func__); > > + return ret; > > + } > > + > > /* Clock integrity is not guaranteed when OSF flag is set. */ > > - if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) { > > + if (buf[PCF2127_OFFSET_TD_SC] & PCF2127_BIT_SC_OSF) { > > /* > > * no need clear the flag here, > > * it will be cleared once the new date is saved > > @@ -158,18 +166,18 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > > dev_dbg(dev, > > "%s: raw data is cr3=%02x, sec=%02x, min=%02x, hr=%02x, " > > "mday=%02x, wday=%02x, mon=%02x, year=%02x\n", > > - __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC], > > - buf[PCF2127_REG_MN], buf[PCF2127_REG_HR], > > - buf[PCF2127_REG_DM], buf[PCF2127_REG_DW], > > - buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]); > > - > > - tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F); > > - tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F); > > - tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F); /* rtc hr 0-23 */ > > - tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F); > > - tm->tm_wday = buf[PCF2127_REG_DW] & 0x07; > > - tm->tm_mon = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > > - tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]); > > + __func__, ctrl3, buf[PCF2127_OFFSET_TD_SC], > > + buf[PCF2127_OFFSET_TD_MN], buf[PCF2127_OFFSET_TD_HR], > > + buf[PCF2127_OFFSET_TD_DM], buf[PCF2127_OFFSET_TD_DW], > > + buf[PCF2127_OFFSET_TD_MO], buf[PCF2127_OFFSET_TD_YR]); > > + > > + tm->tm_sec = bcd2bin(buf[PCF2127_OFFSET_TD_SC] & 0x7F); > > + tm->tm_min = bcd2bin(buf[PCF2127_OFFSET_TD_MN] & 0x7F); > > + tm->tm_hour = bcd2bin(buf[PCF2127_OFFSET_TD_HR] & 0x3F); /* rtc hr 0-23 */ > > + tm->tm_mday = bcd2bin(buf[PCF2127_OFFSET_TD_DM] & 0x3F); > > + tm->tm_wday = buf[PCF2127_OFFSET_TD_DW] & 0x07; > > + tm->tm_mon = bcd2bin(buf[PCF2127_OFFSET_TD_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > > + tm->tm_year = bcd2bin(buf[PCF2127_OFFSET_TD_YR]); > > tm->tm_year += 100; > > > > dev_dbg(dev, "%s: tm is secs=%d, mins=%d, hours=%d, " > > @@ -207,7 +215,7 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm) > > buf[i++] = bin2bcd(tm->tm_year - 100); > > > > /* write register's data */ > > - err = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_SC, buf, i); > > + err = regmap_bulk_write(pcf2127->regmap, pcf2127->cfg->regs_td_base, buf, i); > > if (err) { > > dev_err(dev, > > "%s: err=%d", __func__, err); > > @@ -650,11 +658,13 @@ static struct pcf21xx_config pcf21xx_cfg[] = { > > .max_register = 0x1d, > > .has_nvmem = 1, > > .has_bit_wd_ctl_cd0 = 1, > > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > > }, > > [PCF2129] = { > > .max_register = 0x19, > > .has_nvmem = 0, > > .has_bit_wd_ctl_cd0 = 0, > > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > > }, > > }; > > > > -- > > 2.30.2 > >
On 15/12/2022 10:02:03-0500, Hugo Villeneuve wrote: > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > This will simplify the implementation of new variants into this driver. > > Some variants (PCF2131) have a 100th seconds register. This register is > currently not supported in this driver. > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > --- > drivers/rtc/rtc-pcf2127.c | 68 ++++++++++++++++++++++----------------- > 1 file changed, 39 insertions(+), 29 deletions(-) > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > index b9a5d47a439f..fb0caacaabee 100644 > --- a/drivers/rtc/rtc-pcf2127.c > +++ b/drivers/rtc/rtc-pcf2127.c > @@ -44,14 +44,17 @@ > #define PCF2127_BIT_CTRL3_BF BIT(3) > #define PCF2127_BIT_CTRL3_BTSE BIT(4) > /* Time and date registers */ > -#define PCF2127_REG_SC 0x03 > +#define PCF2127_REG_TIME_DATE_BASE 0x03 > +/* Time and date registers offsets (starting from base register) */ > +#define PCF2127_OFFSET_TD_SC 0 > +#define PCF2127_OFFSET_TD_MN 1 > +#define PCF2127_OFFSET_TD_HR 2 > +#define PCF2127_OFFSET_TD_DM 3 > +#define PCF2127_OFFSET_TD_DW 4 > +#define PCF2127_OFFSET_TD_MO 5 > +#define PCF2127_OFFSET_TD_YR 6 Same comment as for the alarms, I would simply remove the defines as they don't really carry any useful information. > +/* Time and date registers bits */ > #define PCF2127_BIT_SC_OSF BIT(7) > -#define PCF2127_REG_MN 0x04 > -#define PCF2127_REG_HR 0x05 > -#define PCF2127_REG_DM 0x06 > -#define PCF2127_REG_DW 0x07 > -#define PCF2127_REG_MO 0x08 > -#define PCF2127_REG_YR 0x09 > /* Alarm registers */ > #define PCF2127_REG_ALARM_SC 0x0A > #define PCF2127_REG_ALARM_MN 0x0B > @@ -106,6 +109,7 @@ struct pcf21xx_config { > int max_register; > unsigned int has_nvmem:1; > unsigned int has_bit_wd_ctl_cd0:1; > + u8 regs_td_base; /* Time/data base registers. */ > }; > > struct pcf2127 { > @@ -125,27 +129,31 @@ struct pcf2127 { > static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > { > struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > - unsigned char buf[10]; > + unsigned char buf[7]; > + unsigned int ctrl3; > int ret; > > /* > * Avoid reading CTRL2 register as it causes WD_VAL register > * value to reset to 0 which means watchdog is stopped. > */ > - ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_CTRL3, > - (buf + PCF2127_REG_CTRL3), > - ARRAY_SIZE(buf) - PCF2127_REG_CTRL3); > - if (ret) { > - dev_err(dev, "%s: read error\n", __func__); > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &ctrl3); > + if (ret) > return ret; > - } > > - if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF) > + if (ctrl3 & PCF2127_BIT_CTRL3_BLF) > dev_info(dev, > "low voltage detected, check/replace RTC battery.\n"); > > + ret = regmap_bulk_read(pcf2127->regmap, pcf2127->cfg->regs_td_base, > + buf, sizeof(buf)); > + if (ret) { > + dev_err(dev, "%s: read error\n", __func__); > + return ret; > + } > + > /* Clock integrity is not guaranteed when OSF flag is set. */ > - if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) { > + if (buf[PCF2127_OFFSET_TD_SC] & PCF2127_BIT_SC_OSF) { > /* > * no need clear the flag here, > * it will be cleared once the new date is saved > @@ -158,18 +166,18 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > dev_dbg(dev, > "%s: raw data is cr3=%02x, sec=%02x, min=%02x, hr=%02x, " > "mday=%02x, wday=%02x, mon=%02x, year=%02x\n", > - __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC], > - buf[PCF2127_REG_MN], buf[PCF2127_REG_HR], > - buf[PCF2127_REG_DM], buf[PCF2127_REG_DW], > - buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]); > - > - tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F); > - tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F); > - tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F); /* rtc hr 0-23 */ > - tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F); > - tm->tm_wday = buf[PCF2127_REG_DW] & 0x07; > - tm->tm_mon = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > - tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]); > + __func__, ctrl3, buf[PCF2127_OFFSET_TD_SC], > + buf[PCF2127_OFFSET_TD_MN], buf[PCF2127_OFFSET_TD_HR], > + buf[PCF2127_OFFSET_TD_DM], buf[PCF2127_OFFSET_TD_DW], > + buf[PCF2127_OFFSET_TD_MO], buf[PCF2127_OFFSET_TD_YR]); > + > + tm->tm_sec = bcd2bin(buf[PCF2127_OFFSET_TD_SC] & 0x7F); > + tm->tm_min = bcd2bin(buf[PCF2127_OFFSET_TD_MN] & 0x7F); > + tm->tm_hour = bcd2bin(buf[PCF2127_OFFSET_TD_HR] & 0x3F); /* rtc hr 0-23 */ You can drop the comment > + tm->tm_mday = bcd2bin(buf[PCF2127_OFFSET_TD_DM] & 0x3F); > + tm->tm_wday = buf[PCF2127_OFFSET_TD_DW] & 0x07; > + tm->tm_mon = bcd2bin(buf[PCF2127_OFFSET_TD_MO] & 0x1F) - 1; /* rtc mn 1-12 */ This comment too. > + tm->tm_year = bcd2bin(buf[PCF2127_OFFSET_TD_YR]); > tm->tm_year += 100; > > dev_dbg(dev, "%s: tm is secs=%d, mins=%d, hours=%d, " > @@ -207,7 +215,7 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm) > buf[i++] = bin2bcd(tm->tm_year - 100); > > /* write register's data */ > - err = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_SC, buf, i); > + err = regmap_bulk_write(pcf2127->regmap, pcf2127->cfg->regs_td_base, buf, i); > if (err) { > dev_err(dev, > "%s: err=%d", __func__, err); > @@ -650,11 +658,13 @@ static struct pcf21xx_config pcf21xx_cfg[] = { > .max_register = 0x1d, > .has_nvmem = 1, > .has_bit_wd_ctl_cd0 = 1, > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > }, > [PCF2129] = { > .max_register = 0x19, > .has_nvmem = 0, > .has_bit_wd_ctl_cd0 = 0, > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > }, > }; > > -- > 2.30.2 >
On Fri, 20 Jan 2023 19:47:53 +0100 Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > On 15/12/2022 10:02:03-0500, Hugo Villeneuve wrote: > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > This will simplify the implementation of new variants into this driver. > > > > Some variants (PCF2131) have a 100th seconds register. This register is > > currently not supported in this driver. > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > --- > > drivers/rtc/rtc-pcf2127.c | 68 ++++++++++++++++++++++----------------- > > 1 file changed, 39 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > > index b9a5d47a439f..fb0caacaabee 100644 > > --- a/drivers/rtc/rtc-pcf2127.c > > +++ b/drivers/rtc/rtc-pcf2127.c > > @@ -44,14 +44,17 @@ > > #define PCF2127_BIT_CTRL3_BF BIT(3) > > #define PCF2127_BIT_CTRL3_BTSE BIT(4) > > /* Time and date registers */ > > -#define PCF2127_REG_SC 0x03 > > +#define PCF2127_REG_TIME_DATE_BASE 0x03 > > +/* Time and date registers offsets (starting from base register) */ > > +#define PCF2127_OFFSET_TD_SC 0 > > +#define PCF2127_OFFSET_TD_MN 1 > > +#define PCF2127_OFFSET_TD_HR 2 > > +#define PCF2127_OFFSET_TD_DM 3 > > +#define PCF2127_OFFSET_TD_DW 4 > > +#define PCF2127_OFFSET_TD_MO 5 > > +#define PCF2127_OFFSET_TD_YR 6 > > Same comment as for the alarms, I would simply remove the defines as > they don't really carry any useful information. Note that if I remove them, the patch for pcf2127_rtc_read_time() would look like this: /* Clock integrity is not guaranteed when OSF flag is set. */ - if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) { + if (buf[0] & PCF2127_BIT_SC_OSF) { ... - __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC], - buf[PCF2127_REG_MN], buf[PCF2127_REG_HR], - buf[PCF2127_REG_DM], buf[PCF2127_REG_DW], - buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]); - - tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F); - tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F); - tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F); /* rtc hr 0-23 */ - tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F); - tm->tm_wday = buf[PCF2127_REG_DW] & 0x07; - tm->tm_mon = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1; /* rtc mn 1-12 */ - tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]); + __func__, ctrl3, buf[0], + buf[1], buf[2], + buf[3], buf[4], + buf[5], buf[PCF2127_OFFSET_TD_YR]); + + tm->tm_sec = bcd2bin(buf[0] & 0x7F); + tm->tm_min = bcd2bin(buf[1] & 0x7F); + tm->tm_hour = bcd2bin(buf[2] & 0x3F); /* rtc hr 0-23 */ + tm->tm_mday = bcd2bin(buf[3] & 0x3F); + tm->tm_wday = buf[4] & 0x07; + tm->tm_mon = bcd2bin(buf[5] & 0x1F) - 1; /* rtc mn 1-12 */ + tm->tm_year = bcd2bin(buf[6]); Do you still want to remove the defines then? > > +/* Time and date registers bits */ > > #define PCF2127_BIT_SC_OSF BIT(7) > > -#define PCF2127_REG_MN 0x04 > > -#define PCF2127_REG_HR 0x05 > > -#define PCF2127_REG_DM 0x06 > > -#define PCF2127_REG_DW 0x07 > > -#define PCF2127_REG_MO 0x08 > > -#define PCF2127_REG_YR 0x09 > > /* Alarm registers */ > > #define PCF2127_REG_ALARM_SC 0x0A > > #define PCF2127_REG_ALARM_MN 0x0B > > @@ -106,6 +109,7 @@ struct pcf21xx_config { > > int max_register; > > unsigned int has_nvmem:1; > > unsigned int has_bit_wd_ctl_cd0:1; > > + u8 regs_td_base; /* Time/data base registers. */ > > }; > > > > struct pcf2127 { > > @@ -125,27 +129,31 @@ struct pcf2127 { > > static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > > { > > struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > > - unsigned char buf[10]; > > + unsigned char buf[7]; > > + unsigned int ctrl3; > > int ret; > > > > /* > > * Avoid reading CTRL2 register as it causes WD_VAL register > > * value to reset to 0 which means watchdog is stopped. > > */ > > - ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_CTRL3, > > - (buf + PCF2127_REG_CTRL3), > > - ARRAY_SIZE(buf) - PCF2127_REG_CTRL3); > > - if (ret) { > > - dev_err(dev, "%s: read error\n", __func__); > > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &ctrl3); > > + if (ret) > > return ret; > > - } > > > > - if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF) > > + if (ctrl3 & PCF2127_BIT_CTRL3_BLF) > > dev_info(dev, > > "low voltage detected, check/replace RTC battery.\n"); > > > > + ret = regmap_bulk_read(pcf2127->regmap, pcf2127->cfg->regs_td_base, > > + buf, sizeof(buf)); > > + if (ret) { > > + dev_err(dev, "%s: read error\n", __func__); > > + return ret; > > + } > > + > > /* Clock integrity is not guaranteed when OSF flag is set. */ > > - if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) { > > + if (buf[PCF2127_OFFSET_TD_SC] & PCF2127_BIT_SC_OSF) { > > /* > > * no need clear the flag here, > > * it will be cleared once the new date is saved > > @@ -158,18 +166,18 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > > dev_dbg(dev, > > "%s: raw data is cr3=%02x, sec=%02x, min=%02x, hr=%02x, " > > "mday=%02x, wday=%02x, mon=%02x, year=%02x\n", > > - __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC], > > - buf[PCF2127_REG_MN], buf[PCF2127_REG_HR], > > - buf[PCF2127_REG_DM], buf[PCF2127_REG_DW], > > - buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]); > > - > > - tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F); > > - tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F); > > - tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F); /* rtc hr 0-23 */ > > - tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F); > > - tm->tm_wday = buf[PCF2127_REG_DW] & 0x07; > > - tm->tm_mon = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > > - tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]); > > + __func__, ctrl3, buf[PCF2127_OFFSET_TD_SC], > > + buf[PCF2127_OFFSET_TD_MN], buf[PCF2127_OFFSET_TD_HR], > > + buf[PCF2127_OFFSET_TD_DM], buf[PCF2127_OFFSET_TD_DW], > > + buf[PCF2127_OFFSET_TD_MO], buf[PCF2127_OFFSET_TD_YR]); > > + > > + tm->tm_sec = bcd2bin(buf[PCF2127_OFFSET_TD_SC] & 0x7F); > > + tm->tm_min = bcd2bin(buf[PCF2127_OFFSET_TD_MN] & 0x7F); > > + tm->tm_hour = bcd2bin(buf[PCF2127_OFFSET_TD_HR] & 0x3F); /* rtc hr 0-23 */ > > You can drop the comment Done > > > + tm->tm_mday = bcd2bin(buf[PCF2127_OFFSET_TD_DM] & 0x3F); > > + tm->tm_wday = buf[PCF2127_OFFSET_TD_DW] & 0x07; > > + tm->tm_mon = bcd2bin(buf[PCF2127_OFFSET_TD_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > > This comment too. Done > > > + tm->tm_year = bcd2bin(buf[PCF2127_OFFSET_TD_YR]); > > tm->tm_year += 100; > > > > dev_dbg(dev, "%s: tm is secs=%d, mins=%d, hours=%d, " > > @@ -207,7 +215,7 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm) > > buf[i++] = bin2bcd(tm->tm_year - 100); > > > > /* write register's data */ > > - err = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_SC, buf, i); > > + err = regmap_bulk_write(pcf2127->regmap, pcf2127->cfg->regs_td_base, buf, i); > > if (err) { > > dev_err(dev, > > "%s: err=%d", __func__, err); > > @@ -650,11 +658,13 @@ static struct pcf21xx_config pcf21xx_cfg[] = { > > .max_register = 0x1d, > > .has_nvmem = 1, > > .has_bit_wd_ctl_cd0 = 1, > > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > > }, > > [PCF2129] = { > > .max_register = 0x19, > > .has_nvmem = 0, > > .has_bit_wd_ctl_cd0 = 0, > > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > > }, > > }; > > > > -- > > 2.30.2 > > > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com >
On 23/01/2023 10:54:57-0500, Hugo Villeneuve wrote: > On Fri, 20 Jan 2023 19:47:53 +0100 > Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > > > On 15/12/2022 10:02:03-0500, Hugo Villeneuve wrote: > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > > > This will simplify the implementation of new variants into this driver. > > > > > > Some variants (PCF2131) have a 100th seconds register. This register is > > > currently not supported in this driver. > > > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > --- > > > drivers/rtc/rtc-pcf2127.c | 68 ++++++++++++++++++++++----------------- > > > 1 file changed, 39 insertions(+), 29 deletions(-) > > > > > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > > > index b9a5d47a439f..fb0caacaabee 100644 > > > --- a/drivers/rtc/rtc-pcf2127.c > > > +++ b/drivers/rtc/rtc-pcf2127.c > > > @@ -44,14 +44,17 @@ > > > #define PCF2127_BIT_CTRL3_BF BIT(3) > > > #define PCF2127_BIT_CTRL3_BTSE BIT(4) > > > /* Time and date registers */ > > > -#define PCF2127_REG_SC 0x03 > > > +#define PCF2127_REG_TIME_DATE_BASE 0x03 > > > +/* Time and date registers offsets (starting from base register) */ > > > +#define PCF2127_OFFSET_TD_SC 0 > > > +#define PCF2127_OFFSET_TD_MN 1 > > > +#define PCF2127_OFFSET_TD_HR 2 > > > +#define PCF2127_OFFSET_TD_DM 3 > > > +#define PCF2127_OFFSET_TD_DW 4 > > > +#define PCF2127_OFFSET_TD_MO 5 > > > +#define PCF2127_OFFSET_TD_YR 6 > > > > Same comment as for the alarms, I would simply remove the defines as > > they don't really carry any useful information. > > Note that if I remove them, the patch for pcf2127_rtc_read_time() would look like this: > > /* Clock integrity is not guaranteed when OSF flag is set. */ > - if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) { > + if (buf[0] & PCF2127_BIT_SC_OSF) { > ... > - __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC], > - buf[PCF2127_REG_MN], buf[PCF2127_REG_HR], > - buf[PCF2127_REG_DM], buf[PCF2127_REG_DW], > - buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]); > - > - tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F); > - tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F); > - tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F); /* rtc hr 0-23 */ > - tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F); > - tm->tm_wday = buf[PCF2127_REG_DW] & 0x07; > - tm->tm_mon = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > - tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]); > + __func__, ctrl3, buf[0], > + buf[1], buf[2], > + buf[3], buf[4], > + buf[5], buf[PCF2127_OFFSET_TD_YR]); > + > + tm->tm_sec = bcd2bin(buf[0] & 0x7F); > + tm->tm_min = bcd2bin(buf[1] & 0x7F); > + tm->tm_hour = bcd2bin(buf[2] & 0x3F); /* rtc hr 0-23 */ > + tm->tm_mday = bcd2bin(buf[3] & 0x3F); > + tm->tm_wday = buf[4] & 0x07; > + tm->tm_mon = bcd2bin(buf[5] & 0x1F) - 1; /* rtc mn 1-12 */ > + tm->tm_year = bcd2bin(buf[6]); > > Do you still want to remove the defines then? This is fine, yes. > > > > > +/* Time and date registers bits */ > > > #define PCF2127_BIT_SC_OSF BIT(7) > > > -#define PCF2127_REG_MN 0x04 > > > -#define PCF2127_REG_HR 0x05 > > > -#define PCF2127_REG_DM 0x06 > > > -#define PCF2127_REG_DW 0x07 > > > -#define PCF2127_REG_MO 0x08 > > > -#define PCF2127_REG_YR 0x09 > > > /* Alarm registers */ > > > #define PCF2127_REG_ALARM_SC 0x0A > > > #define PCF2127_REG_ALARM_MN 0x0B > > > @@ -106,6 +109,7 @@ struct pcf21xx_config { > > > int max_register; > > > unsigned int has_nvmem:1; > > > unsigned int has_bit_wd_ctl_cd0:1; > > > + u8 regs_td_base; /* Time/data base registers. */ > > > }; > > > > > > struct pcf2127 { > > > @@ -125,27 +129,31 @@ struct pcf2127 { > > > static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > > > { > > > struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > > > - unsigned char buf[10]; > > > + unsigned char buf[7]; > > > + unsigned int ctrl3; > > > int ret; > > > > > > /* > > > * Avoid reading CTRL2 register as it causes WD_VAL register > > > * value to reset to 0 which means watchdog is stopped. > > > */ > > > - ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_CTRL3, > > > - (buf + PCF2127_REG_CTRL3), > > > - ARRAY_SIZE(buf) - PCF2127_REG_CTRL3); > > > - if (ret) { > > > - dev_err(dev, "%s: read error\n", __func__); > > > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &ctrl3); > > > + if (ret) > > > return ret; > > > - } > > > > > > - if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF) > > > + if (ctrl3 & PCF2127_BIT_CTRL3_BLF) > > > dev_info(dev, > > > "low voltage detected, check/replace RTC battery.\n"); > > > > > > + ret = regmap_bulk_read(pcf2127->regmap, pcf2127->cfg->regs_td_base, > > > + buf, sizeof(buf)); > > > + if (ret) { > > > + dev_err(dev, "%s: read error\n", __func__); > > > + return ret; > > > + } > > > + > > > /* Clock integrity is not guaranteed when OSF flag is set. */ > > > - if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) { > > > + if (buf[PCF2127_OFFSET_TD_SC] & PCF2127_BIT_SC_OSF) { > > > /* > > > * no need clear the flag here, > > > * it will be cleared once the new date is saved > > > @@ -158,18 +166,18 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) > > > dev_dbg(dev, > > > "%s: raw data is cr3=%02x, sec=%02x, min=%02x, hr=%02x, " > > > "mday=%02x, wday=%02x, mon=%02x, year=%02x\n", > > > - __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC], > > > - buf[PCF2127_REG_MN], buf[PCF2127_REG_HR], > > > - buf[PCF2127_REG_DM], buf[PCF2127_REG_DW], > > > - buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]); > > > - > > > - tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F); > > > - tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F); > > > - tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F); /* rtc hr 0-23 */ > > > - tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F); > > > - tm->tm_wday = buf[PCF2127_REG_DW] & 0x07; > > > - tm->tm_mon = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > > > - tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]); > > > + __func__, ctrl3, buf[PCF2127_OFFSET_TD_SC], > > > + buf[PCF2127_OFFSET_TD_MN], buf[PCF2127_OFFSET_TD_HR], > > > + buf[PCF2127_OFFSET_TD_DM], buf[PCF2127_OFFSET_TD_DW], > > > + buf[PCF2127_OFFSET_TD_MO], buf[PCF2127_OFFSET_TD_YR]); > > > + > > > + tm->tm_sec = bcd2bin(buf[PCF2127_OFFSET_TD_SC] & 0x7F); > > > + tm->tm_min = bcd2bin(buf[PCF2127_OFFSET_TD_MN] & 0x7F); > > > + tm->tm_hour = bcd2bin(buf[PCF2127_OFFSET_TD_HR] & 0x3F); /* rtc hr 0-23 */ > > > > You can drop the comment > > Done > > > > > > + tm->tm_mday = bcd2bin(buf[PCF2127_OFFSET_TD_DM] & 0x3F); > > > + tm->tm_wday = buf[PCF2127_OFFSET_TD_DW] & 0x07; > > > + tm->tm_mon = bcd2bin(buf[PCF2127_OFFSET_TD_MO] & 0x1F) - 1; /* rtc mn 1-12 */ > > > > This comment too. > > Done > > > > > > + tm->tm_year = bcd2bin(buf[PCF2127_OFFSET_TD_YR]); > > > tm->tm_year += 100; > > > > > > dev_dbg(dev, "%s: tm is secs=%d, mins=%d, hours=%d, " > > > @@ -207,7 +215,7 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm) > > > buf[i++] = bin2bcd(tm->tm_year - 100); > > > > > > /* write register's data */ > > > - err = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_SC, buf, i); > > > + err = regmap_bulk_write(pcf2127->regmap, pcf2127->cfg->regs_td_base, buf, i); > > > if (err) { > > > dev_err(dev, > > > "%s: err=%d", __func__, err); > > > @@ -650,11 +658,13 @@ static struct pcf21xx_config pcf21xx_cfg[] = { > > > .max_register = 0x1d, > > > .has_nvmem = 1, > > > .has_bit_wd_ctl_cd0 = 1, > > > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > > > }, > > > [PCF2129] = { > > > .max_register = 0x19, > > > .has_nvmem = 0, > > > .has_bit_wd_ctl_cd0 = 0, > > > + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, > > > }, > > > }; > > > > > > -- > > > 2.30.2 > > > > > > > -- > > Alexandre Belloni, co-owner and COO, Bootlin > > Embedded Linux and Kernel engineering > > https://bootlin.com > > > > > -- > Hugo Villeneuve <hugo@hugovil.com>
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c index b9a5d47a439f..fb0caacaabee 100644 --- a/drivers/rtc/rtc-pcf2127.c +++ b/drivers/rtc/rtc-pcf2127.c @@ -44,14 +44,17 @@ #define PCF2127_BIT_CTRL3_BF BIT(3) #define PCF2127_BIT_CTRL3_BTSE BIT(4) /* Time and date registers */ -#define PCF2127_REG_SC 0x03 +#define PCF2127_REG_TIME_DATE_BASE 0x03 +/* Time and date registers offsets (starting from base register) */ +#define PCF2127_OFFSET_TD_SC 0 +#define PCF2127_OFFSET_TD_MN 1 +#define PCF2127_OFFSET_TD_HR 2 +#define PCF2127_OFFSET_TD_DM 3 +#define PCF2127_OFFSET_TD_DW 4 +#define PCF2127_OFFSET_TD_MO 5 +#define PCF2127_OFFSET_TD_YR 6 +/* Time and date registers bits */ #define PCF2127_BIT_SC_OSF BIT(7) -#define PCF2127_REG_MN 0x04 -#define PCF2127_REG_HR 0x05 -#define PCF2127_REG_DM 0x06 -#define PCF2127_REG_DW 0x07 -#define PCF2127_REG_MO 0x08 -#define PCF2127_REG_YR 0x09 /* Alarm registers */ #define PCF2127_REG_ALARM_SC 0x0A #define PCF2127_REG_ALARM_MN 0x0B @@ -106,6 +109,7 @@ struct pcf21xx_config { int max_register; unsigned int has_nvmem:1; unsigned int has_bit_wd_ctl_cd0:1; + u8 regs_td_base; /* Time/data base registers. */ }; struct pcf2127 { @@ -125,27 +129,31 @@ struct pcf2127 { static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) { struct pcf2127 *pcf2127 = dev_get_drvdata(dev); - unsigned char buf[10]; + unsigned char buf[7]; + unsigned int ctrl3; int ret; /* * Avoid reading CTRL2 register as it causes WD_VAL register * value to reset to 0 which means watchdog is stopped. */ - ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_CTRL3, - (buf + PCF2127_REG_CTRL3), - ARRAY_SIZE(buf) - PCF2127_REG_CTRL3); - if (ret) { - dev_err(dev, "%s: read error\n", __func__); + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &ctrl3); + if (ret) return ret; - } - if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF) + if (ctrl3 & PCF2127_BIT_CTRL3_BLF) dev_info(dev, "low voltage detected, check/replace RTC battery.\n"); + ret = regmap_bulk_read(pcf2127->regmap, pcf2127->cfg->regs_td_base, + buf, sizeof(buf)); + if (ret) { + dev_err(dev, "%s: read error\n", __func__); + return ret; + } + /* Clock integrity is not guaranteed when OSF flag is set. */ - if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) { + if (buf[PCF2127_OFFSET_TD_SC] & PCF2127_BIT_SC_OSF) { /* * no need clear the flag here, * it will be cleared once the new date is saved @@ -158,18 +166,18 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm) dev_dbg(dev, "%s: raw data is cr3=%02x, sec=%02x, min=%02x, hr=%02x, " "mday=%02x, wday=%02x, mon=%02x, year=%02x\n", - __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC], - buf[PCF2127_REG_MN], buf[PCF2127_REG_HR], - buf[PCF2127_REG_DM], buf[PCF2127_REG_DW], - buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]); - - tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F); - tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F); - tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F); /* rtc hr 0-23 */ - tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F); - tm->tm_wday = buf[PCF2127_REG_DW] & 0x07; - tm->tm_mon = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1; /* rtc mn 1-12 */ - tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]); + __func__, ctrl3, buf[PCF2127_OFFSET_TD_SC], + buf[PCF2127_OFFSET_TD_MN], buf[PCF2127_OFFSET_TD_HR], + buf[PCF2127_OFFSET_TD_DM], buf[PCF2127_OFFSET_TD_DW], + buf[PCF2127_OFFSET_TD_MO], buf[PCF2127_OFFSET_TD_YR]); + + tm->tm_sec = bcd2bin(buf[PCF2127_OFFSET_TD_SC] & 0x7F); + tm->tm_min = bcd2bin(buf[PCF2127_OFFSET_TD_MN] & 0x7F); + tm->tm_hour = bcd2bin(buf[PCF2127_OFFSET_TD_HR] & 0x3F); /* rtc hr 0-23 */ + tm->tm_mday = bcd2bin(buf[PCF2127_OFFSET_TD_DM] & 0x3F); + tm->tm_wday = buf[PCF2127_OFFSET_TD_DW] & 0x07; + tm->tm_mon = bcd2bin(buf[PCF2127_OFFSET_TD_MO] & 0x1F) - 1; /* rtc mn 1-12 */ + tm->tm_year = bcd2bin(buf[PCF2127_OFFSET_TD_YR]); tm->tm_year += 100; dev_dbg(dev, "%s: tm is secs=%d, mins=%d, hours=%d, " @@ -207,7 +215,7 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm) buf[i++] = bin2bcd(tm->tm_year - 100); /* write register's data */ - err = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_SC, buf, i); + err = regmap_bulk_write(pcf2127->regmap, pcf2127->cfg->regs_td_base, buf, i); if (err) { dev_err(dev, "%s: err=%d", __func__, err); @@ -650,11 +658,13 @@ static struct pcf21xx_config pcf21xx_cfg[] = { .max_register = 0x1d, .has_nvmem = 1, .has_bit_wd_ctl_cd0 = 1, + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, }, [PCF2129] = { .max_register = 0x19, .has_nvmem = 0, .has_bit_wd_ctl_cd0 = 0, + .regs_td_base = PCF2127_REG_TIME_DATE_BASE, }, };