Compatible with NCT3015Y-R and NCT3018Y-R
Subsystems (2)
v5 cover Compatible with NCT3015Y-R and NCT3018Y-R
Changes since version 5:
Add global parameter to store part number.
Remove unnecessary changes for easier reading.
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 | 87 ++++++++++++++++++++++++++++---------- 1 file changed, 64 insertions(+), 23 deletions(-)
Signature · 1 line
2.17.1
PATCH v5 1/1 rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
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.
--- drivers/rtc/rtc-nct3018y.c | 87 ++++++++++++++++++++++++++++---------- 1 file changed, 64 insertions(+), 23 deletions(-)
drivers/rtc/rtc-nct3018y.c +64 -23
@@ -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 0x02struct nct3018y {struct rtc_device *rtc;
@@ -46,6 +48,8 @@ struct nct3018y {#endif }; +static int part_num; +static int nct3018y_set_alarm_mode (struct i2c_client *client,bool on) {int err, flags;
@@ -55,7 +59,7 @@ static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on) 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 +71,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 );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; }
@@ -155,7 +159,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 +182,44 @@ 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, flags; +int restore_flags =0 ; + + 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 +230,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,7 +258,7 @@ 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 ; }
@@ -257,19 +291,19 @@ static int nct3018y_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm) 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,"%s: Unable to write minutes alarm reg.\n" , __func__);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; }
@@ -473,23 +507,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; + 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; + }else if (part_num &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 +547,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 {
Signature · 1 line
2.17.1
Alexandre Belloni
maintainerRe: [PATCH v5 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
Quoted 44 lines · 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
- Inset_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 <[email protected]>--- drivers/rtc/rtc-nct3018y.c | 87 ++++++++++++++++++++++++++++---------- 1 file changed, 64 insertions(+), 23 deletions(-)drivers/rtc/rtc-nct3018y.c +2
-0@@ -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 0x02struct nct3018y {struct rtc_device *rtc;
drivers/rtc/rtc-nct3018y.c struct nct3018y { · line 52
Quoted 5 lines
drivers/rtc/rtc-nct3018y.c +2
-0@@ -46,6 +48,8 @@ struct nct3018y {#endif }; +static int part_num; +
This must be part of struct nct3018y.
drivers/rtc/rtc-nct3018y.c static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on) · line 62
Quoted 9 lines
drivers/rtc/rtc-nct3018y.c +1
-1static int nct3018y_set_alarm_mode (struct i2c_client *client,bool on) {int err, flags;@@ -55,7 +59,7 @@ static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on) 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__);
If you really insist on this change, what about:
#define pr_fmt(fmt) "%s: " fmt, __func__
Quoted 193 lines
drivers/rtc/rtc-nct3018y.c +59
-22return flags; }@@ -67,21 +71,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 );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; }@@ -155,7 +159,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 +182,44 @@ 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, flags; +int restore_flags =0 ; + + 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 +230,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,7 +258,7 @@ 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 ; }@@ -257,19 +291,19 @@ static int nct3018y_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm) 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,"%s: Unable to write minutes alarm reg.\n" , __func__);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; }@@ -473,23 +507,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; + 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; + }else if (part_num &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 +547,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 {Signature · 1 line
2.17.1
Signature · 3 lines
Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v5 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
Dear Alexandre,
Thanks for your comments.
The replies are as follow.
Thanks.
Best regard,
Mia
Quoted 51 lines · Alexandre Belloni
Quoted 47 lines · 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
- Inset_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 <[email protected]>---
drivers/rtc/rtc-nct3018y.c| 87 ++++++++++++++++++++++++++++----------1 file changed, 64 insertions(+), 23 deletions(-)drivers/rtc/rtc-nct3018y.c +2
-0@@ -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 0x02struct nct3018y {struct rtc_device *rtc;@@ -46,6 +48,8 @@ struct nct3018y {#endif };
Quoted 4 lines
drivers/rtc/rtc-nct3018y.c struct nct3018y { · line 52
Quoted 2 lines
drivers/rtc/rtc-nct3018y.c +2
-0+static int part_num; +This must be part of struct nct3018y.
[Mia] I will move the part number parameter to struct nct3018y.
Quoted 13 lines
drivers/rtc/rtc-nct3018y.c static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on) · line 62
Quoted 10 lines
drivers/rtc/rtc-nct3018y.c +1
-1static int nct3018y_set_alarm_mode (struct i2c_client *client,bool on) {int err, flags;@@ -55,7 +59,7 @@ static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on) 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__);If you really insist on this change, what about:
Quoted 1 line
#define
pr_fmt(fmt)"%s: " fmt,__func__
[Mia] Do you mean to replace dev_dbg with pr_debug? If yes, for consistency, I'm going to refine all messages via pr_debug. Thank you for your suggestion.
Quoted 198 lines
Quoted 193 lines
drivers/rtc/rtc-nct3018y.c +59
-22return flags; }@@ -67,21 +71,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 );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; }@@ -155,7 +159,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 +182,44 @@ 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, flags; +int restore_flags =0 ; + + 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 +230,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,7 +258,7 @@ 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 ; }@@ -257,19 +291,19 @@ static int nct3018y_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm) 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,"%s: Unable to write minutes alarm reg.\n" , __func__);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; }@@ -473,23 +507,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; + 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; + }else if (part_num &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 +547,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 {Signature · 1 line
2.17.1Signature · 3 lines
Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Alexandre Belloni
maintainerRe: [PATCH v5 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
Quoted 12 lines · Mining Lin
Quoted 12 lines
drivers/rtc/rtc-nct3018y.c static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on) · line 62
Quoted 9 lines
static int
nct3018y_set_alarm_mode(struct i2c_client *client, bool on){ int err, flags;drivers/rtc/rtc-nct3018y.c +1
-1@@ -55,7 +59,7 @@ static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on) 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__);If you really insist on this change, what about:
Quoted 2 lines
Quoted 1 line
#define
pr_fmt(fmt)"%s: " fmt,__func__[Mia] Do you mean to replace
dev_dbgwithpr_debug? If yes, for consistency, I'm going to refine all messages viapr_debug. Thank you for your suggestion.
No, I mean that instead of adding __func__ to all the messages just define pr_fmt, this should achieve what you want.
Signature · 3 lines
Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v5 1/1] rtc: nuvoton: Compatible with NCT3015Y-R and NCT3018Y-R
Dear Alexandre,
Thanks for your comments.
The replies are as follow.
Thanks.
Best regard,
Mia
Sent from my iPhone
Quoted 15 lines · Alexandre Belloni
Quoted 12 lines · Mining Lin
Quoted 12 lines
drivers/rtc/rtc-nct3018y.c static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on) · line 62
Quoted 9 lines
static int
nct3018y_set_alarm_mode(struct i2c_client *client, bool on){
int err, flags;drivers/rtc/rtc-nct3018y.c +1
-1@@ -55,7 +59,7 @@ static int nct3018y_set_alarm_mode(struct i2c_client *client, bool on) 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__);If you really insist on this change, what about:
Quoted 5 lines
Quoted 2 lines
Quoted 1 line
#define
pr_fmt(fmt)"%s: " fmt,__func__[Mia] Do you mean to replace
dev_dbgwithpr_debug? If yes, for consistency, I'm going to refine all messages viapr_debug. Thank you for your suggestion.No, I mean that instead of adding
__func__to all the messages just definepr_fmt, this should achieve what you want.
[Mia] I'd like to confirm that you mean that I just need to define pr_fmt so that dev_dbg can show function name in the log? I try to define pr_fmt and the function name is shown in the log by pr_*(e.g. pr_debug) but not by dev_*(e.g. dev_dbg). And I also add #define dev_fmt pr_fmt
but build error.
Quoted 4 lines
Signature · 3 lines
Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com