* [PATCH v4 0/1] Compatible with NCT3015Y-R and NCT3018Y-R
@ 2023-08-16 1:25 ` Mia Lin
0 siblings, 0 replies; 10+ messages in thread
From: Mia Lin @ 2023-08-16 1:25 UTC (permalink / raw)
To: avifishman70, tmaimon77, tali.perry1, venture, yuenn,
benjaminfair, a.zummo, alexandre.belloni, KWLIU, JJLIU0, KFLIN,
mylin1
Cc: openbmc, linux-rtc, linux-kernel, Mia Lin
Changes since version 4:
Add an introduction bewteen NCT3015Y-R and NCT3018Y-R.
Restore the initial value of err in nct3018y_rtc_set_time().
Refine error messages to pinpoint the correct location.
Changes since version 3:
Remove the comparison between DT compatible and chip data.
Changes since version 2:
Add DT compatible to check the chip matches or not.
Changes since version 1:
rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
Mia Lin (1):
rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
drivers/rtc/rtc-nct3018y.c | 122 +++++++++++++++++++++++++------------
1 file changed, 83 insertions(+), 39 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 0/1] Compatible with NCT3015Y-R and NCT3018Y-R
@ 2023-08-16 1:25 ` Mia Lin
0 siblings, 0 replies; 10+ messages in thread
From: Mia Lin @ 2023-08-16 1:25 UTC (permalink / raw)
To: avifishman70, tmaimon77, tali.perry1, venture, yuenn,
benjaminfair, a.zummo, alexandre.belloni, KWLIU, JJLIU0, KFLIN,
mylin1
Cc: linux-rtc, openbmc, Mia Lin, linux-kernel
Changes since version 4:
Add an introduction bewteen NCT3015Y-R and NCT3018Y-R.
Restore the initial value of err in nct3018y_rtc_set_time().
Refine error messages to pinpoint the correct location.
Changes since version 3:
Remove the comparison between DT compatible and chip data.
Changes since version 2:
Add DT compatible to check the chip matches or not.
Changes since version 1:
rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
Mia Lin (1):
rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
drivers/rtc/rtc-nct3018y.c | 122 +++++++++++++++++++++++++------------
1 file changed, 83 insertions(+), 39 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
2023-08-16 1:25 ` Mia Lin
@ 2023-08-16 1:25 ` Mia Lin
-1 siblings, 0 replies; 10+ messages in thread
From: Mia Lin @ 2023-08-16 1:25 UTC (permalink / raw)
To: avifishman70, tmaimon77, tali.perry1, venture, yuenn,
benjaminfair, a.zummo, alexandre.belloni, KWLIU, JJLIU0, KFLIN,
mylin1
Cc: openbmc, linux-rtc, linux-kernel, Mia Lin
The NCT3015Y-R and NCT3018Y-R use the same datasheet
but have different topologies as follows.
- Topology (Only 1st i2c can set TWO bit and HF bit)
In NCT3015Y-R,
rtc 1st i2c is connected to a host CPU
rtc 2nd i2c is connected to a BMC
In NCT3018Y-R,
rtc 1st i2c is connected to a BMC
rtc 2nd i2c is connected to a host CPU
In order to be compatible with NCT3015Y-R and NCT3018Y-R,
- In probe,
If part number is NCT3018Y-R, only set HF bit to 24-Hour format.
Else, do nothing
- In set_time,
If part number is NCT3018Y-R && TWO bit is 0,
change TWO bit to 1, and restore TWO bit after updating time.
- Refine error messages to pinpoint the correct location.
Signed-off-by: Mia Lin <mimi05633@gmail.com>
---
drivers/rtc/rtc-nct3018y.c | 122 +++++++++++++++++++++++++------------
1 file changed, 83 insertions(+), 39 deletions(-)
diff --git a/drivers/rtc/rtc-nct3018y.c b/drivers/rtc/rtc-nct3018y.c
index a4e3f924837e..1a674e105682 100644
--- a/drivers/rtc/rtc-nct3018y.c
+++ b/drivers/rtc/rtc-nct3018y.c
@@ -23,6 +23,7 @@
#define NCT3018Y_REG_CTRL 0x0A /* timer control */
#define NCT3018Y_REG_ST 0x0B /* status */
#define NCT3018Y_REG_CLKO 0x0C /* clock out */
+#define NCT3018Y_REG_PART 0x21 /* part info */
#define NCT3018Y_BIT_AF BIT(7)
#define NCT3018Y_BIT_ST BIT(7)
@@ -37,6 +38,7 @@
#define NCT3018Y_REG_BAT_MASK 0x07
#define NCT3018Y_REG_CLKO_F_MASK 0x03 /* frequenc mask */
#define NCT3018Y_REG_CLKO_CKE 0x80 /* clock out enabled */
+#define NCT3018Y_REG_PART_NCT3018Y 0x02
struct nct3018y {
struct rtc_device *rtc;
@@ -50,12 +52,12 @@ static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on)
{
int err, flags;
- dev_dbg(&client->dev, "%s:on:%d\n", __func__, on);
+ dev_dbg(&client->dev, "%s: on=%d.\n", __func__, on);
- flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
if (flags < 0) {
dev_dbg(&client->dev,
- "Failed to read NCT3018Y_REG_CTRL\n");
+ "%s: Failed to read ctrl reg.\n", __func__);
return flags;
}
@@ -67,21 +69,21 @@ static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on)
flags |= NCT3018Y_BIT_CIE;
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_CTRL, flags);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_CTRL\n");
+ dev_dbg(&client->dev, "%s: Unable to write ctrl reg.\n", __func__);
return err;
}
- flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_ST);
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_ST);
if (flags < 0) {
dev_dbg(&client->dev,
- "Failed to read NCT3018Y_REG_ST\n");
+ "%s: Failed to read status reg.\n", __func__);
return flags;
}
flags &= ~(NCT3018Y_BIT_AF);
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_ST, flags);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_ST\n");
+ dev_dbg(&client->dev, "%s: Unable to write status reg.\n", __func__);
return err;
}
@@ -94,23 +96,22 @@ static int nct3018y_get_alarm_mode(struct i2c_client *client, unsigned char *ala
int flags;
if (alarm_enable) {
- dev_dbg(&client->dev, "%s:NCT3018Y_REG_CTRL\n", __func__);
- flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
if (flags < 0)
return flags;
*alarm_enable = flags & NCT3018Y_BIT_AIE;
}
if (alarm_flag) {
- dev_dbg(&client->dev, "%s:NCT3018Y_REG_ST\n", __func__);
- flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_ST);
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_ST);
if (flags < 0)
return flags;
*alarm_flag = flags & NCT3018Y_BIT_AF;
}
- dev_dbg(&client->dev, "%s:alarm_enable:%x alarm_flag:%x\n",
- __func__, *alarm_enable, *alarm_flag);
+ if (alarm_enable && alarm_flag)
+ dev_dbg(&client->dev, "%s: alarm_enable=%x, alarm_flag=%x.\n",
+ __func__, *alarm_enable, *alarm_flag);
return 0;
}
@@ -123,17 +124,17 @@ static irqreturn_t nct3018y_irq(int irq, void *dev_id)
unsigned char alarm_flag;
unsigned char alarm_enable;
- dev_dbg(&client->dev, "%s:irq:%d\n", __func__, irq);
+ dev_dbg(&client->dev, "%s: irq=%d.\n", __func__, irq);
err = nct3018y_get_alarm_mode(nct3018y->client, &alarm_enable, &alarm_flag);
if (err)
return IRQ_NONE;
if (alarm_flag) {
- dev_dbg(&client->dev, "%s:alarm flag:%x\n",
+ dev_dbg(&client->dev, "%s: alarm flag=%x.\n",
__func__, alarm_flag);
rtc_update_irq(nct3018y->rtc, 1, RTC_IRQF | RTC_AF);
nct3018y_set_alarm_mode(nct3018y->client, 0);
- dev_dbg(&client->dev, "%s:IRQ_HANDLED\n", __func__);
+ dev_dbg(&client->dev, "%s: IRQ_HANDLED.\n", __func__);
return IRQ_HANDLED;
}
@@ -155,7 +156,7 @@ static int nct3018y_rtc_read_time(struct device *dev, struct rtc_time *tm)
return err;
if (!buf[0]) {
- dev_dbg(&client->dev, " voltage <=1.7, date/time is not reliable.\n");
+ dev_dbg(&client->dev, "%s: voltage <=1.7, date/time is not reliable.\n", __func__);
return -EINVAL;
}
@@ -178,26 +179,50 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm)
{
struct i2c_client *client = to_i2c_client(dev);
unsigned char buf[4] = {0};
- int err;
+ int err, part_num, flags;
+ int restore_flags = 0;
+
+ part_num = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART);
+ if (part_num < 0) {
+ dev_dbg(&client->dev, "%s: Failed to read part info reg.\n", __func__);
+ return part_num;
+ }
+
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
+ if (flags < 0) {
+ dev_dbg(&client->dev, "%s: Failed to read ctrl reg.\n", __func__);
+ return flags;
+ }
+
+ /* Check and set TWO bit */
+ if ((part_num & NCT3018Y_REG_PART_NCT3018Y) && !(flags & NCT3018Y_BIT_TWO)) {
+ restore_flags = 1;
+ flags |= NCT3018Y_BIT_TWO;
+ err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_CTRL, flags);
+ if (err < 0) {
+ dev_dbg(&client->dev, "%s: Unable to write ctrl reg.\n", __func__);
+ return err;
+ }
+ }
buf[0] = bin2bcd(tm->tm_sec);
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_SC, buf[0]);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_SC\n");
+ dev_dbg(&client->dev, "%s: Unable to write seconds reg.\n", __func__);
return err;
}
buf[0] = bin2bcd(tm->tm_min);
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_MN, buf[0]);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_MN\n");
+ dev_dbg(&client->dev, "%s: Unable to write minutes reg.\n", __func__);
return err;
}
buf[0] = bin2bcd(tm->tm_hour);
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_HR, buf[0]);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_HR\n");
+ dev_dbg(&client->dev, "%s: Unable to write hour reg.\n", __func__);
return err;
}
@@ -208,10 +233,22 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm)
err = i2c_smbus_write_i2c_block_data(client, NCT3018Y_REG_DW,
sizeof(buf), buf);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write for day and mon and year\n");
+ dev_dbg(&client->dev, "%s: Unable to write for day and mon and year.\n", __func__);
return -EIO;
}
+ /* Restore TWO bit */
+ if (restore_flags) {
+ if (part_num & NCT3018Y_REG_PART_NCT3018Y)
+ flags &= ~NCT3018Y_BIT_TWO;
+
+ err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_CTRL, flags);
+ if (err < 0) {
+ dev_dbg(&client->dev, "%s: Unable to write ctrl reg.\n", __func__);
+ return err;
+ }
+ }
+
return err;
}
@@ -224,11 +261,11 @@ static int nct3018y_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm)
err = i2c_smbus_read_i2c_block_data(client, NCT3018Y_REG_SCA,
sizeof(buf), buf);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to read date\n");
+ dev_dbg(&client->dev, "%s: Unable to read date.\n", __func__);
return -EIO;
}
- dev_dbg(&client->dev, "%s: raw data is sec=%02x, min=%02x hr=%02x\n",
+ dev_dbg(&client->dev, "%s: raw data is sec=%02x, min=%02x, hr=%02x.\n",
__func__, buf[0], buf[2], buf[4]);
tm->time.tm_sec = bcd2bin(buf[0] & 0x7F);
@@ -239,7 +276,7 @@ static int nct3018y_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm)
if (err < 0)
return err;
- dev_dbg(&client->dev, "%s:s=%d m=%d, hr=%d, enabled=%d, pending=%d\n",
+ dev_dbg(&client->dev, "%s: s=%d m=%d, hr=%d, enabled=%d, pending=%d.\n",
__func__, tm->time.tm_sec, tm->time.tm_min,
tm->time.tm_hour, tm->enabled, tm->pending);
@@ -251,25 +288,25 @@ static int nct3018y_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)
struct i2c_client *client = to_i2c_client(dev);
int err;
- dev_dbg(dev, "%s, sec=%d, min=%d hour=%d tm->enabled:%d\n",
+ dev_dbg(dev, "%s: sec=%d, min=%d, hour=%d, tm->enabled=%d.\n",
__func__, tm->time.tm_sec, tm->time.tm_min, tm->time.tm_hour,
tm->enabled);
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_SCA, bin2bcd(tm->time.tm_sec));
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_SCA\n");
+ dev_dbg(&client->dev, "%s: Unable to write seconds alarm reg.\n", __func__);
return err;
}
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_MNA, bin2bcd(tm->time.tm_min));
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_MNA\n");
+ dev_dbg(&client->dev, "Unable to write minutes alarm reg.\n");
return err;
}
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_HRA, bin2bcd(tm->time.tm_hour));
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_HRA\n");
+ dev_dbg(&client->dev, "%s: Unable to write hour alarm reg.\n", __func__);
return err;
}
@@ -278,7 +315,7 @@ static int nct3018y_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)
static int nct3018y_irq_enable(struct device *dev, unsigned int enabled)
{
- dev_dbg(dev, "%s: alarm enable=%d\n", __func__, enabled);
+ dev_dbg(dev, "%s: alarm enable=%d.\n", __func__, enabled);
return nct3018y_set_alarm_mode(to_i2c_client(dev), enabled);
}
@@ -473,23 +510,29 @@ static int nct3018y_probe(struct i2c_client *client)
flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
if (flags < 0) {
- dev_dbg(&client->dev, "%s: read error\n", __func__);
+ dev_dbg(&client->dev, "%s: Failed to read ctrl reg.\n", __func__);
return flags;
} else if (flags & NCT3018Y_BIT_TWO) {
- dev_dbg(&client->dev, "%s: NCT3018Y_BIT_TWO is set\n", __func__);
+ dev_dbg(&client->dev, "%s: TWO bit is set.\n", __func__);
}
- flags = NCT3018Y_BIT_TWO;
- err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_CTRL, flags);
- if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_CTRL\n");
- return err;
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART);
+ if (flags < 0) {
+ dev_dbg(&client->dev, "%s: Failed to read part info reg.\n", __func__);
+ return flags;
+ } else if (flags & NCT3018Y_REG_PART_NCT3018Y) {
+ flags = NCT3018Y_BIT_HF;
+ err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_CTRL, flags);
+ if (err < 0) {
+ dev_dbg(&client->dev, "%s: Unable to write ctrl reg.\n", __func__);
+ return err;
+ }
}
flags = 0;
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_ST, flags);
if (err < 0) {
- dev_dbg(&client->dev, "%s: write error\n", __func__);
+ dev_dbg(&client->dev, "%s: Failed to clear status reg.\n", __func__);
return err;
}
@@ -507,7 +550,8 @@ static int nct3018y_probe(struct i2c_client *client)
IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
"nct3018y", client);
if (err) {
- dev_dbg(&client->dev, "unable to request IRQ %d\n", client->irq);
+ dev_dbg(&client->dev, "%s: Unable to request IRQ %d.\n",
+ __func__, client->irq);
return err;
}
} else {
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
@ 2023-08-16 1:25 ` Mia Lin
0 siblings, 0 replies; 10+ messages in thread
From: Mia Lin @ 2023-08-16 1:25 UTC (permalink / raw)
To: avifishman70, tmaimon77, tali.perry1, venture, yuenn,
benjaminfair, a.zummo, alexandre.belloni, KWLIU, JJLIU0, KFLIN,
mylin1
Cc: linux-rtc, openbmc, Mia Lin, linux-kernel
The NCT3015Y-R and NCT3018Y-R use the same datasheet
but have different topologies as follows.
- Topology (Only 1st i2c can set TWO bit and HF bit)
In NCT3015Y-R,
rtc 1st i2c is connected to a host CPU
rtc 2nd i2c is connected to a BMC
In NCT3018Y-R,
rtc 1st i2c is connected to a BMC
rtc 2nd i2c is connected to a host CPU
In order to be compatible with NCT3015Y-R and NCT3018Y-R,
- In probe,
If part number is NCT3018Y-R, only set HF bit to 24-Hour format.
Else, do nothing
- In set_time,
If part number is NCT3018Y-R && TWO bit is 0,
change TWO bit to 1, and restore TWO bit after updating time.
- Refine error messages to pinpoint the correct location.
Signed-off-by: Mia Lin <mimi05633@gmail.com>
---
drivers/rtc/rtc-nct3018y.c | 122 +++++++++++++++++++++++++------------
1 file changed, 83 insertions(+), 39 deletions(-)
diff --git a/drivers/rtc/rtc-nct3018y.c b/drivers/rtc/rtc-nct3018y.c
index a4e3f924837e..1a674e105682 100644
--- a/drivers/rtc/rtc-nct3018y.c
+++ b/drivers/rtc/rtc-nct3018y.c
@@ -23,6 +23,7 @@
#define NCT3018Y_REG_CTRL 0x0A /* timer control */
#define NCT3018Y_REG_ST 0x0B /* status */
#define NCT3018Y_REG_CLKO 0x0C /* clock out */
+#define NCT3018Y_REG_PART 0x21 /* part info */
#define NCT3018Y_BIT_AF BIT(7)
#define NCT3018Y_BIT_ST BIT(7)
@@ -37,6 +38,7 @@
#define NCT3018Y_REG_BAT_MASK 0x07
#define NCT3018Y_REG_CLKO_F_MASK 0x03 /* frequenc mask */
#define NCT3018Y_REG_CLKO_CKE 0x80 /* clock out enabled */
+#define NCT3018Y_REG_PART_NCT3018Y 0x02
struct nct3018y {
struct rtc_device *rtc;
@@ -50,12 +52,12 @@ static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on)
{
int err, flags;
- dev_dbg(&client->dev, "%s:on:%d\n", __func__, on);
+ dev_dbg(&client->dev, "%s: on=%d.\n", __func__, on);
- flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
if (flags < 0) {
dev_dbg(&client->dev,
- "Failed to read NCT3018Y_REG_CTRL\n");
+ "%s: Failed to read ctrl reg.\n", __func__);
return flags;
}
@@ -67,21 +69,21 @@ static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on)
flags |= NCT3018Y_BIT_CIE;
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_CTRL, flags);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_CTRL\n");
+ dev_dbg(&client->dev, "%s: Unable to write ctrl reg.\n", __func__);
return err;
}
- flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_ST);
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_ST);
if (flags < 0) {
dev_dbg(&client->dev,
- "Failed to read NCT3018Y_REG_ST\n");
+ "%s: Failed to read status reg.\n", __func__);
return flags;
}
flags &= ~(NCT3018Y_BIT_AF);
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_ST, flags);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_ST\n");
+ dev_dbg(&client->dev, "%s: Unable to write status reg.\n", __func__);
return err;
}
@@ -94,23 +96,22 @@ static int nct3018y_get_alarm_mode(struct i2c_client *client, unsigned char *ala
int flags;
if (alarm_enable) {
- dev_dbg(&client->dev, "%s:NCT3018Y_REG_CTRL\n", __func__);
- flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
if (flags < 0)
return flags;
*alarm_enable = flags & NCT3018Y_BIT_AIE;
}
if (alarm_flag) {
- dev_dbg(&client->dev, "%s:NCT3018Y_REG_ST\n", __func__);
- flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_ST);
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_ST);
if (flags < 0)
return flags;
*alarm_flag = flags & NCT3018Y_BIT_AF;
}
- dev_dbg(&client->dev, "%s:alarm_enable:%x alarm_flag:%x\n",
- __func__, *alarm_enable, *alarm_flag);
+ if (alarm_enable && alarm_flag)
+ dev_dbg(&client->dev, "%s: alarm_enable=%x, alarm_flag=%x.\n",
+ __func__, *alarm_enable, *alarm_flag);
return 0;
}
@@ -123,17 +124,17 @@ static irqreturn_t nct3018y_irq(int irq, void *dev_id)
unsigned char alarm_flag;
unsigned char alarm_enable;
- dev_dbg(&client->dev, "%s:irq:%d\n", __func__, irq);
+ dev_dbg(&client->dev, "%s: irq=%d.\n", __func__, irq);
err = nct3018y_get_alarm_mode(nct3018y->client, &alarm_enable, &alarm_flag);
if (err)
return IRQ_NONE;
if (alarm_flag) {
- dev_dbg(&client->dev, "%s:alarm flag:%x\n",
+ dev_dbg(&client->dev, "%s: alarm flag=%x.\n",
__func__, alarm_flag);
rtc_update_irq(nct3018y->rtc, 1, RTC_IRQF | RTC_AF);
nct3018y_set_alarm_mode(nct3018y->client, 0);
- dev_dbg(&client->dev, "%s:IRQ_HANDLED\n", __func__);
+ dev_dbg(&client->dev, "%s: IRQ_HANDLED.\n", __func__);
return IRQ_HANDLED;
}
@@ -155,7 +156,7 @@ static int nct3018y_rtc_read_time(struct device *dev, struct rtc_time *tm)
return err;
if (!buf[0]) {
- dev_dbg(&client->dev, " voltage <=1.7, date/time is not reliable.\n");
+ dev_dbg(&client->dev, "%s: voltage <=1.7, date/time is not reliable.\n", __func__);
return -EINVAL;
}
@@ -178,26 +179,50 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm)
{
struct i2c_client *client = to_i2c_client(dev);
unsigned char buf[4] = {0};
- int err;
+ int err, part_num, flags;
+ int restore_flags = 0;
+
+ part_num = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART);
+ if (part_num < 0) {
+ dev_dbg(&client->dev, "%s: Failed to read part info reg.\n", __func__);
+ return part_num;
+ }
+
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
+ if (flags < 0) {
+ dev_dbg(&client->dev, "%s: Failed to read ctrl reg.\n", __func__);
+ return flags;
+ }
+
+ /* Check and set TWO bit */
+ if ((part_num & NCT3018Y_REG_PART_NCT3018Y) && !(flags & NCT3018Y_BIT_TWO)) {
+ restore_flags = 1;
+ flags |= NCT3018Y_BIT_TWO;
+ err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_CTRL, flags);
+ if (err < 0) {
+ dev_dbg(&client->dev, "%s: Unable to write ctrl reg.\n", __func__);
+ return err;
+ }
+ }
buf[0] = bin2bcd(tm->tm_sec);
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_SC, buf[0]);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_SC\n");
+ dev_dbg(&client->dev, "%s: Unable to write seconds reg.\n", __func__);
return err;
}
buf[0] = bin2bcd(tm->tm_min);
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_MN, buf[0]);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_MN\n");
+ dev_dbg(&client->dev, "%s: Unable to write minutes reg.\n", __func__);
return err;
}
buf[0] = bin2bcd(tm->tm_hour);
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_HR, buf[0]);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_HR\n");
+ dev_dbg(&client->dev, "%s: Unable to write hour reg.\n", __func__);
return err;
}
@@ -208,10 +233,22 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm)
err = i2c_smbus_write_i2c_block_data(client, NCT3018Y_REG_DW,
sizeof(buf), buf);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write for day and mon and year\n");
+ dev_dbg(&client->dev, "%s: Unable to write for day and mon and year.\n", __func__);
return -EIO;
}
+ /* Restore TWO bit */
+ if (restore_flags) {
+ if (part_num & NCT3018Y_REG_PART_NCT3018Y)
+ flags &= ~NCT3018Y_BIT_TWO;
+
+ err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_CTRL, flags);
+ if (err < 0) {
+ dev_dbg(&client->dev, "%s: Unable to write ctrl reg.\n", __func__);
+ return err;
+ }
+ }
+
return err;
}
@@ -224,11 +261,11 @@ static int nct3018y_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm)
err = i2c_smbus_read_i2c_block_data(client, NCT3018Y_REG_SCA,
sizeof(buf), buf);
if (err < 0) {
- dev_dbg(&client->dev, "Unable to read date\n");
+ dev_dbg(&client->dev, "%s: Unable to read date.\n", __func__);
return -EIO;
}
- dev_dbg(&client->dev, "%s: raw data is sec=%02x, min=%02x hr=%02x\n",
+ dev_dbg(&client->dev, "%s: raw data is sec=%02x, min=%02x, hr=%02x.\n",
__func__, buf[0], buf[2], buf[4]);
tm->time.tm_sec = bcd2bin(buf[0] & 0x7F);
@@ -239,7 +276,7 @@ static int nct3018y_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm)
if (err < 0)
return err;
- dev_dbg(&client->dev, "%s:s=%d m=%d, hr=%d, enabled=%d, pending=%d\n",
+ dev_dbg(&client->dev, "%s: s=%d m=%d, hr=%d, enabled=%d, pending=%d.\n",
__func__, tm->time.tm_sec, tm->time.tm_min,
tm->time.tm_hour, tm->enabled, tm->pending);
@@ -251,25 +288,25 @@ static int nct3018y_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)
struct i2c_client *client = to_i2c_client(dev);
int err;
- dev_dbg(dev, "%s, sec=%d, min=%d hour=%d tm->enabled:%d\n",
+ dev_dbg(dev, "%s: sec=%d, min=%d, hour=%d, tm->enabled=%d.\n",
__func__, tm->time.tm_sec, tm->time.tm_min, tm->time.tm_hour,
tm->enabled);
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_SCA, bin2bcd(tm->time.tm_sec));
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_SCA\n");
+ dev_dbg(&client->dev, "%s: Unable to write seconds alarm reg.\n", __func__);
return err;
}
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_MNA, bin2bcd(tm->time.tm_min));
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_MNA\n");
+ dev_dbg(&client->dev, "Unable to write minutes alarm reg.\n");
return err;
}
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_HRA, bin2bcd(tm->time.tm_hour));
if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_HRA\n");
+ dev_dbg(&client->dev, "%s: Unable to write hour alarm reg.\n", __func__);
return err;
}
@@ -278,7 +315,7 @@ static int nct3018y_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)
static int nct3018y_irq_enable(struct device *dev, unsigned int enabled)
{
- dev_dbg(dev, "%s: alarm enable=%d\n", __func__, enabled);
+ dev_dbg(dev, "%s: alarm enable=%d.\n", __func__, enabled);
return nct3018y_set_alarm_mode(to_i2c_client(dev), enabled);
}
@@ -473,23 +510,29 @@ static int nct3018y_probe(struct i2c_client *client)
flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_CTRL);
if (flags < 0) {
- dev_dbg(&client->dev, "%s: read error\n", __func__);
+ dev_dbg(&client->dev, "%s: Failed to read ctrl reg.\n", __func__);
return flags;
} else if (flags & NCT3018Y_BIT_TWO) {
- dev_dbg(&client->dev, "%s: NCT3018Y_BIT_TWO is set\n", __func__);
+ dev_dbg(&client->dev, "%s: TWO bit is set.\n", __func__);
}
- flags = NCT3018Y_BIT_TWO;
- err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_CTRL, flags);
- if (err < 0) {
- dev_dbg(&client->dev, "Unable to write NCT3018Y_REG_CTRL\n");
- return err;
+ flags = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART);
+ if (flags < 0) {
+ dev_dbg(&client->dev, "%s: Failed to read part info reg.\n", __func__);
+ return flags;
+ } else if (flags & NCT3018Y_REG_PART_NCT3018Y) {
+ flags = NCT3018Y_BIT_HF;
+ err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_CTRL, flags);
+ if (err < 0) {
+ dev_dbg(&client->dev, "%s: Unable to write ctrl reg.\n", __func__);
+ return err;
+ }
}
flags = 0;
err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_ST, flags);
if (err < 0) {
- dev_dbg(&client->dev, "%s: write error\n", __func__);
+ dev_dbg(&client->dev, "%s: Failed to clear status reg.\n", __func__);
return err;
}
@@ -507,7 +550,8 @@ static int nct3018y_probe(struct i2c_client *client)
IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
"nct3018y", client);
if (err) {
- dev_dbg(&client->dev, "unable to request IRQ %d\n", client->irq);
+ dev_dbg(&client->dev, "%s: Unable to request IRQ %d.\n",
+ __func__, client->irq);
return err;
}
} else {
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
2023-08-16 1:25 ` Mia Lin
@ 2023-08-23 22:12 ` Alexandre Belloni
-1 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2023-08-23 22:12 UTC (permalink / raw)
To: Mia Lin
Cc: avifishman70, tmaimon77, tali.perry1, venture, yuenn,
benjaminfair, a.zummo, KWLIU, JJLIU0, KFLIN, mylin1, openbmc,
linux-rtc, linux-kernel
Hello,
On 16/08/2023 09:25:40+0800, Mia Lin wrote:
> - dev_dbg(&client->dev, "%s:alarm_enable:%x alarm_flag:%x\n",
> - __func__, *alarm_enable, *alarm_flag);
> + if (alarm_enable && alarm_flag)
I don't really see the point of conditionally displaying this debug
message.
> + dev_dbg(&client->dev, "%s: alarm_enable=%x, alarm_flag=%x.\n",
> + __func__, *alarm_enable, *alarm_flag);
>
> return 0;
> }
> @@ -123,17 +124,17 @@ static irqreturn_t nct3018y_irq(int irq, void *dev_id)
> unsigned char alarm_flag;
> unsigned char alarm_enable;
>
> - dev_dbg(&client->dev, "%s:irq:%d\n", __func__, irq);
> + dev_dbg(&client->dev, "%s: irq=%d.\n", __func__, irq);
You have many of those changes where you only add a space, I feel like
this is a matter of taste and this makes it more difficult than
necessary to read your patch.
> err = nct3018y_get_alarm_mode(nct3018y->client, &alarm_enable, &alarm_flag);
> if (err)
> return IRQ_NONE;
>
> if (alarm_flag) {
> - dev_dbg(&client->dev, "%s:alarm flag:%x\n",
> + dev_dbg(&client->dev, "%s: alarm flag=%x.\n",
> __func__, alarm_flag);
> rtc_update_irq(nct3018y->rtc, 1, RTC_IRQF | RTC_AF);
> nct3018y_set_alarm_mode(nct3018y->client, 0);
> - dev_dbg(&client->dev, "%s:IRQ_HANDLED\n", __func__);
> + dev_dbg(&client->dev, "%s: IRQ_HANDLED.\n", __func__);
> return IRQ_HANDLED;
> }
>
> @@ -155,7 +156,7 @@ static int nct3018y_rtc_read_time(struct device *dev, struct rtc_time *tm)
> return err;
>
> if (!buf[0]) {
> - dev_dbg(&client->dev, " voltage <=1.7, date/time is not reliable.\n");
> + dev_dbg(&client->dev, "%s: voltage <=1.7, date/time is not reliable.\n", __func__);
> return -EINVAL;
> }
>
> @@ -178,26 +179,50 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm)
> {
> struct i2c_client *client = to_i2c_client(dev);
> unsigned char buf[4] = {0};
> - int err;
> + int err, part_num, flags;
> + int restore_flags = 0;
> +
> + part_num = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART);
Do you really have to check the part number every time you set the time?
I don't expect it to change once read in probe.
> + if (part_num < 0) {
> + dev_dbg(&client->dev, "%s: Failed to read part info reg.\n", __func__);
> + return part_num;
> + }
> +
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
@ 2023-08-23 22:12 ` Alexandre Belloni
0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2023-08-23 22:12 UTC (permalink / raw)
To: Mia Lin
Cc: linux-rtc, a.zummo, mylin1, benjaminfair, KWLIU, avifishman70,
venture, openbmc, JJLIU0, linux-kernel, tali.perry1, KFLIN,
tmaimon77
Hello,
On 16/08/2023 09:25:40+0800, Mia Lin wrote:
> - dev_dbg(&client->dev, "%s:alarm_enable:%x alarm_flag:%x\n",
> - __func__, *alarm_enable, *alarm_flag);
> + if (alarm_enable && alarm_flag)
I don't really see the point of conditionally displaying this debug
message.
> + dev_dbg(&client->dev, "%s: alarm_enable=%x, alarm_flag=%x.\n",
> + __func__, *alarm_enable, *alarm_flag);
>
> return 0;
> }
> @@ -123,17 +124,17 @@ static irqreturn_t nct3018y_irq(int irq, void *dev_id)
> unsigned char alarm_flag;
> unsigned char alarm_enable;
>
> - dev_dbg(&client->dev, "%s:irq:%d\n", __func__, irq);
> + dev_dbg(&client->dev, "%s: irq=%d.\n", __func__, irq);
You have many of those changes where you only add a space, I feel like
this is a matter of taste and this makes it more difficult than
necessary to read your patch.
> err = nct3018y_get_alarm_mode(nct3018y->client, &alarm_enable, &alarm_flag);
> if (err)
> return IRQ_NONE;
>
> if (alarm_flag) {
> - dev_dbg(&client->dev, "%s:alarm flag:%x\n",
> + dev_dbg(&client->dev, "%s: alarm flag=%x.\n",
> __func__, alarm_flag);
> rtc_update_irq(nct3018y->rtc, 1, RTC_IRQF | RTC_AF);
> nct3018y_set_alarm_mode(nct3018y->client, 0);
> - dev_dbg(&client->dev, "%s:IRQ_HANDLED\n", __func__);
> + dev_dbg(&client->dev, "%s: IRQ_HANDLED.\n", __func__);
> return IRQ_HANDLED;
> }
>
> @@ -155,7 +156,7 @@ static int nct3018y_rtc_read_time(struct device *dev, struct rtc_time *tm)
> return err;
>
> if (!buf[0]) {
> - dev_dbg(&client->dev, " voltage <=1.7, date/time is not reliable.\n");
> + dev_dbg(&client->dev, "%s: voltage <=1.7, date/time is not reliable.\n", __func__);
> return -EINVAL;
> }
>
> @@ -178,26 +179,50 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm)
> {
> struct i2c_client *client = to_i2c_client(dev);
> unsigned char buf[4] = {0};
> - int err;
> + int err, part_num, flags;
> + int restore_flags = 0;
> +
> + part_num = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART);
Do you really have to check the part number every time you set the time?
I don't expect it to change once read in probe.
> + if (part_num < 0) {
> + dev_dbg(&client->dev, "%s: Failed to read part info reg.\n", __func__);
> + return part_num;
> + }
> +
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
2023-08-23 22:12 ` Alexandre Belloni
(?)
@ 2023-08-29 13:35 ` Minying Lin
2023-08-29 14:22 ` Alexandre Belloni
-1 siblings, 1 reply; 10+ messages in thread
From: Minying Lin @ 2023-08-29 13:35 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-rtc, a.zummo, mylin1, benjaminfair, KWLIU, avifishman70,
venture, openbmc, JJLIU0, linux-kernel, tali.perry1, KFLIN,
tmaimon77
[-- Attachment #1: Type: text/plain, Size: 3494 bytes --]
Dear Alexandre,
Thanks for your comments.
The replies are as follow.
Thanks.
Best regard,
Mia
Alexandre Belloni <alexandre.belloni@bootlin.com> 於 2023年8月24日 星期四寫道：
> Hello,
>
> On 16/08/2023 09:25:40+0800, Mia Lin wrote:
> > - dev_dbg(&client->dev, "%s:alarm_enable:%x alarm_flag:%x\n",
> > - __func__, *alarm_enable, *alarm_flag);
> > + if (alarm_enable && alarm_flag)
>
> I don't really see the point of conditionally displaying this debug
> message.
>
[Mia] I will remove it.
>
> > + dev_dbg(&client->dev, "%s: alarm_enable=%x,
> alarm_flag=%x.\n",
> > + __func__, *alarm_enable, *alarm_flag);
> >
> > return 0;
> > }
> > @@ -123,17 +124,17 @@ static irqreturn_t nct3018y_irq(int irq, void
> *dev_id)
> > unsigned char alarm_flag;
> > unsigned char alarm_enable;
> >
> > - dev_dbg(&client->dev, "%s:irq:%d\n", __func__, irq);
> > + dev_dbg(&client->dev, "%s: irq=%d.\n", __func__, irq);
>
> You have many of those changes where you only add a space, I feel like
> this is a matter of taste and this makes it more difficult than
> necessary to read your patch.
>
[Mia] Sorry for the ambiguity. I will remove those unnecessary changes.
>
> > err = nct3018y_get_alarm_mode(nct3018y->client, &alarm_enable,
> &alarm_flag);
> > if (err)
> > return IRQ_NONE;
> >
> > if (alarm_flag) {
> > - dev_dbg(&client->dev, "%s:alarm flag:%x\n",
> > + dev_dbg(&client->dev, "%s: alarm flag=%x.\n",
> > __func__, alarm_flag);
> > rtc_update_irq(nct3018y->rtc, 1, RTC_IRQF | RTC_AF);
> > nct3018y_set_alarm_mode(nct3018y->client, 0);
> > - dev_dbg(&client->dev, "%s:IRQ_HANDLED\n", __func__);
> > + dev_dbg(&client->dev, "%s: IRQ_HANDLED.\n", __func__);
> > return IRQ_HANDLED;
> > }
> >
> > @@ -155,7 +156,7 @@ static int nct3018y_rtc_read_time(struct device
> *dev, struct rtc_time *tm)
> > return err;
> >
> > if (!buf[0]) {
> > - dev_dbg(&client->dev, " voltage <=1.7, date/time is not
> reliable.\n");
> > + dev_dbg(&client->dev, "%s: voltage <=1.7, date/time is not
> reliable.\n", __func__);
> > return -EINVAL;
> > }
> >
> > @@ -178,26 +179,50 @@ static int nct3018y_rtc_set_time(struct device
> *dev, struct rtc_time *tm)
> > {
> > struct i2c_client *client = to_i2c_client(dev);
> > unsigned char buf[4] = {0};
> > - int err;
> > + int err, part_num, flags;
> > + int restore_flags = 0;
> > +
> > + part_num = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART);
>
> Do you really have to check the part number every time you set the time?
> I don't expect it to change once read in probe.
>
[Mia] Due to the 3018Y's topology, we need to set the TWO bit first to
obtain the write time capability, but the 3015Y does not have this problem.
Therefore, we use part number & TWO bit to determine whether we need to set
the TWO bit first before set time.
>
> > + if (part_num < 0) {
> > + dev_dbg(&client->dev, "%s: Failed to read part info
> reg.\n", __func__);
> > + return part_num;
> > + }
> > +
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
[-- Attachment #2: Type: text/html, Size: 5047 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
2023-08-29 13:35 ` Minying Lin
@ 2023-08-29 14:22 ` Alexandre Belloni
0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2023-08-29 14:22 UTC (permalink / raw)
To: Minying Lin
Cc: avifishman70, tmaimon77, tali.perry1, venture, yuenn,
benjaminfair, a.zummo, KWLIU, JJLIU0, KFLIN, mylin1, openbmc,
linux-rtc, linux-kernel
On 29/08/2023 21:35:36+0800, Minying Lin wrote:
> > Do you really have to check the part number every time you set the time?
> > I don't expect it to change once read in probe.
> >
> [Mia] Due to the 3018Y's topology, we need to set the TWO bit first to
> obtain the write time capability, but the 3015Y does not have this problem.
> Therefore, we use part number & TWO bit to determine whether we need to set
> the TWO bit first before set time.
>
Sure but why don't you store the info somewhere instead of reading it
from the RTC every time?
> >
> > > + if (part_num < 0) {
> > > + dev_dbg(&client->dev, "%s: Failed to read part info
> > reg.\n", __func__);
> > > + return part_num;
> > > + }
> > > +
> >
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> >
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
@ 2023-08-29 14:22 ` Alexandre Belloni
0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2023-08-29 14:22 UTC (permalink / raw)
To: Minying Lin
Cc: linux-rtc, a.zummo, mylin1, benjaminfair, KWLIU, avifishman70,
venture, openbmc, JJLIU0, linux-kernel, tali.perry1, KFLIN,
tmaimon77
On 29/08/2023 21:35:36+0800, Minying Lin wrote:
> > Do you really have to check the part number every time you set the time?
> > I don't expect it to change once read in probe.
> >
> [Mia] Due to the 3018Y's topology, we need to set the TWO bit first to
> obtain the write time capability, but the 3015Y does not have this problem.
> Therefore, we use part number & TWO bit to determine whether we need to set
> the TWO bit first before set time.
>
Sure but why don't you store the info somewhere instead of reading it
from the RTC every time?
> >
> > > + if (part_num < 0) {
> > > + dev_dbg(&client->dev, "%s: Failed to read part info
> > reg.\n", __func__);
> > > + return part_num;
> > > + }
> > > +
> >
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> >
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
2023-08-29 14:22 ` Alexandre Belloni
(?)
@ 2023-08-31 0:57 ` Mining Lin
-1 siblings, 0 replies; 10+ messages in thread
From: Mining Lin @ 2023-08-31 0:57 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-rtc, a.zummo, mylin1, benjaminfair, KWLIU, avifishman70,
venture, openbmc, JJLIU0, linux-kernel, tali.perry1, KFLIN,
tmaimon77
[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]
Dear Alexandre,
Thanks for your comments.
I will store the part number to global parameter in probe and use it to check before setting time.
Thanks.
Best regard,
Mia
Sent from my iPhone
> On Aug 29, 2023, at 10:22 PM, Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
>
> On 29/08/2023 21:35:36+0800, Minying Lin wrote:
>>> Do you really have to check the part number every time you set the time?
>>> I don't expect it to change once read in probe.
>>>
>> [Mia] Due to the 3018Y's topology, we need to set the TWO bit first to
>> obtain the write time capability, but the 3015Y does not have this problem.
>> Therefore, we use part number & TWO bit to determine whether we need to set
>> the TWO bit first before set time.
>>
>
> Sure but why don't you store the info somewhere instead of reading it
> from the RTC every time?
>
>>>
>>>> + if (part_num < 0) {
>>>> + dev_dbg(&client->dev, "%s: Failed to read part info
>>> reg.\n", __func__);
>>>> + return part_num;
>>>> + }
>>>> +
>>>
>>> --
>>> Alexandre Belloni, co-owner and COO, Bootlin
>>> Embedded Linux and Kernel engineering
>>> https://bootlin.com
>>>
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
[-- Attachment #2: Type: text/html, Size: 5128 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-08-31 0:58 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-16 1:25 [PATCH v4 0/1] Compatible with NCT3015Y-R and NCT3018Y-R Mia Lin
2023-08-16 1:25 ` Mia Lin
2023-08-16 1:25 ` [PATCH v4 1/1] rtc: nuvoton: " Mia Lin
2023-08-16 1:25 ` Mia Lin
2023-08-23 22:12 ` Alexandre Belloni
2023-08-23 22:12 ` Alexandre Belloni
2023-08-29 13:35 ` Minying Lin
2023-08-29 14:22 ` Alexandre Belloni
2023-08-29 14:22 ` Alexandre Belloni
2023-08-31 0:57 ` Mining Lin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.