| Message ID | 20230913013719.8342-2-mimi05633@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Compatible with NCT3015Y-R and NCT3018Y-R | expand |
On 13/09/2023 09:37:19+0800, Mia Lin wrote: > 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. > > Signed-off-by: Mia Lin <mimi05633@gmail.com> > --- > drivers/rtc/rtc-nct3018y.c | 52 +++++++++++++++++++++++++++++++++----- > 1 file changed, 46 insertions(+), 6 deletions(-) > > diff --git a/drivers/rtc/rtc-nct3018y.c b/drivers/rtc/rtc-nct3018y.c > index ed4e606be8e5..b006b58e15e2 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,10 +38,12 @@ > #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; > struct i2c_client *client; > + int part_num; > #ifdef CONFIG_COMMON_CLK > struct clk_hw clkout_hw; > #endif > @@ -177,8 +180,27 @@ static int nct3018y_rtc_read_time(struct device *dev, struct rtc_time *tm) > static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm) > { > struct i2c_client *client = to_i2c_client(dev); > + struct nct3018y *nct3018y = dev_get_drvdata(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, "Failed to read NCT3018Y_REG_CTRL.\n"); > + return flags; > + } > + > + /* Check and set TWO bit */ > + if ((nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); > + return err; > + } > + } > > buf[0] = bin2bcd(tm->tm_sec); > err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_SC, buf[0]); > @@ -212,6 +234,18 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm) > return -EIO; > } > > + /* Restore TWO bit */ > + if (restore_flags) { > + if (nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); > + return err; > + } > + } > + > return err; > } > > @@ -479,11 +513,17 @@ static int nct3018y_probe(struct i2c_client *client) > dev_dbg(&client->dev, "%s: NCT3018Y_BIT_TWO 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; > + nct3018y->part_num = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART); > + if (nct3018y->part_num < 0) { > + dev_dbg(&client->dev, "Failed to read NCT3018Y_REG_PART.\n"); > + return nct3018y->part_num; > + } else if (nct3018y->part_num & NCT3018Y_REG_PART_NCT3018Y) { This is a weird way to check as this will accept any value of NCT3018Y_REG_PART as long as bit 1 is set, is this really what you want? > + flags = NCT3018Y_BIT_HF; > + 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 = 0; > -- > 2.17.1 >
On 04/11/2023 00:16:40+0100, Alexandre Belloni wrote: > On 13/09/2023 09:37:19+0800, Mia Lin wrote: > > 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. > > > > Signed-off-by: Mia Lin <mimi05633@gmail.com> > > --- I forgot to add, please include a changelog here, this will make my reviews easier (and faster). > > drivers/rtc/rtc-nct3018y.c | 52 +++++++++++++++++++++++++++++++++----- > > 1 file changed, 46 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/rtc/rtc-nct3018y.c b/drivers/rtc/rtc-nct3018y.c > > index ed4e606be8e5..b006b58e15e2 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,10 +38,12 @@ > > #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; > > struct i2c_client *client; > > + int part_num; > > #ifdef CONFIG_COMMON_CLK > > struct clk_hw clkout_hw; > > #endif > > @@ -177,8 +180,27 @@ static int nct3018y_rtc_read_time(struct device *dev, struct rtc_time *tm) > > static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm) > > { > > struct i2c_client *client = to_i2c_client(dev); > > + struct nct3018y *nct3018y = dev_get_drvdata(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, "Failed to read NCT3018Y_REG_CTRL.\n"); > > + return flags; > > + } > > + > > + /* Check and set TWO bit */ > > + if ((nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); > > + return err; > > + } > > + } > > > > buf[0] = bin2bcd(tm->tm_sec); > > err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_SC, buf[0]); > > @@ -212,6 +234,18 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm) > > return -EIO; > > } > > > > + /* Restore TWO bit */ > > + if (restore_flags) { > > + if (nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); > > + return err; > > + } > > + } > > + > > return err; > > } > > > > @@ -479,11 +513,17 @@ static int nct3018y_probe(struct i2c_client *client) > > dev_dbg(&client->dev, "%s: NCT3018Y_BIT_TWO 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; > > + nct3018y->part_num = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART); > > + if (nct3018y->part_num < 0) { > > + dev_dbg(&client->dev, "Failed to read NCT3018Y_REG_PART.\n"); > > + return nct3018y->part_num; > > + } else if (nct3018y->part_num & NCT3018Y_REG_PART_NCT3018Y) { > > This is a weird way to check as this will accept any value of > NCT3018Y_REG_PART as long as bit 1 is set, is this really what you want? > > > + flags = NCT3018Y_BIT_HF; > > + 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 = 0; > > -- > > 2.17.1 > > > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Dear Alexandre, Thanks for your comments. The replies are as follow. Thanks. Best regard, Mia Sent from my iPhone > On Nov 4, 2023, at 7:16 AM, Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > > On 13/09/2023 09:37:19+0800, Mia Lin wrote: >> 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. >> >> Signed-off-by: Mia Lin <mimi05633@gmail.com> >> --- >> drivers/rtc/rtc-nct3018y.c | 52 +++++++++++++++++++++++++++++++++----- >> 1 file changed, 46 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/rtc/rtc-nct3018y.c b/drivers/rtc/rtc-nct3018y.c >> index ed4e606be8e5..b006b58e15e2 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,10 +38,12 @@ >> #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; >> struct i2c_client *client; >> + int part_num; >> #ifdef CONFIG_COMMON_CLK >> struct clk_hw clkout_hw; >> #endif >> @@ -177,8 +180,27 @@ static int nct3018y_rtc_read_time(struct device *dev, struct rtc_time *tm) >> static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm) >> { >> struct i2c_client *client = to_i2c_client(dev); >> + struct nct3018y *nct3018y = dev_get_drvdata(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, "Failed to read NCT3018Y_REG_CTRL.\n"); >> + return flags; >> + } >> + >> + /* Check and set TWO bit */ >> + if ((nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); >> + return err; >> + } >> + } >> >> buf[0] = bin2bcd(tm->tm_sec); >> err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_SC, buf[0]); >> @@ -212,6 +234,18 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm) >> return -EIO; >> } >> >> + /* Restore TWO bit */ >> + if (restore_flags) { >> + if (nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); >> + return err; >> + } >> + } >> + >> return err; >> } >> >> @@ -479,11 +513,17 @@ static int nct3018y_probe(struct i2c_client *client) >> dev_dbg(&client->dev, "%s: NCT3018Y_BIT_TWO 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; >> + nct3018y->part_num = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART); >> + if (nct3018y->part_num < 0) { >> + dev_dbg(&client->dev, "Failed to read NCT3018Y_REG_PART.\n"); >> + return nct3018y->part_num; >> + } else if (nct3018y->part_num & NCT3018Y_REG_PART_NCT3018Y) { > > This is a weird way to check as this will accept any value of > NCT3018Y_REG_PART as long as bit 1 is set, is this really what you want? ［Mia] Thanks for your comments. I’ll fix it. > >> + flags = NCT3018Y_BIT_HF; >> + 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 = 0; >> -- >> 2.17.1 >> > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Dear Alexandre, I will add change log next patch. Thanks. Best regard, Mia Sent from my iPhone > On Nov 4, 2023, at 7:17 AM, Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > > On 04/11/2023 00:16:40+0100, Alexandre Belloni wrote: >>> On 13/09/2023 09:37:19+0800, Mia Lin wrote: >>> 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. >>> >>> Signed-off-by: Mia Lin <mimi05633@gmail.com> >>> --- > > I forgot to add, please include a changelog here, this will make my > reviews easier (and faster). > >>> drivers/rtc/rtc-nct3018y.c | 52 +++++++++++++++++++++++++++++++++----- >>> 1 file changed, 46 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/rtc/rtc-nct3018y.c b/drivers/rtc/rtc-nct3018y.c >>> index ed4e606be8e5..b006b58e15e2 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,10 +38,12 @@ >>> #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; >>> struct i2c_client *client; >>> + int part_num; >>> #ifdef CONFIG_COMMON_CLK >>> struct clk_hw clkout_hw; >>> #endif >>> @@ -177,8 +180,27 @@ static int nct3018y_rtc_read_time(struct device *dev, struct rtc_time *tm) >>> static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm) >>> { >>> struct i2c_client *client = to_i2c_client(dev); >>> + struct nct3018y *nct3018y = dev_get_drvdata(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, "Failed to read NCT3018Y_REG_CTRL.\n"); >>> + return flags; >>> + } >>> + >>> + /* Check and set TWO bit */ >>> + if ((nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); >>> + return err; >>> + } >>> + } >>> >>> buf[0] = bin2bcd(tm->tm_sec); >>> err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_SC, buf[0]); >>> @@ -212,6 +234,18 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm) >>> return -EIO; >>> } >>> >>> + /* Restore TWO bit */ >>> + if (restore_flags) { >>> + if (nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); >>> + return err; >>> + } >>> + } >>> + >>> return err; >>> } >>> >>> @@ -479,11 +513,17 @@ static int nct3018y_probe(struct i2c_client *client) >>> dev_dbg(&client->dev, "%s: NCT3018Y_BIT_TWO 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; >>> + nct3018y->part_num = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART); >>> + if (nct3018y->part_num < 0) { >>> + dev_dbg(&client->dev, "Failed to read NCT3018Y_REG_PART.\n"); >>> + return nct3018y->part_num; >>> + } else if (nct3018y->part_num & NCT3018Y_REG_PART_NCT3018Y) { >> >> This is a weird way to check as this will accept any value of >> NCT3018Y_REG_PART as long as bit 1 is set, is this really what you want? >> >>> + flags = NCT3018Y_BIT_HF; >>> + 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 = 0; >>> -- >>> 2.17.1 >>> >> >> -- >> 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
diff --git a/drivers/rtc/rtc-nct3018y.c b/drivers/rtc/rtc-nct3018y.c index ed4e606be8e5..b006b58e15e2 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,10 +38,12 @@ #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; struct i2c_client *client; + int part_num; #ifdef CONFIG_COMMON_CLK struct clk_hw clkout_hw; #endif @@ -177,8 +180,27 @@ static int nct3018y_rtc_read_time(struct device *dev, struct rtc_time *tm) static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm) { struct i2c_client *client = to_i2c_client(dev); + struct nct3018y *nct3018y = dev_get_drvdata(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, "Failed to read NCT3018Y_REG_CTRL.\n"); + return flags; + } + + /* Check and set TWO bit */ + if ((nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); + return err; + } + } buf[0] = bin2bcd(tm->tm_sec); err = i2c_smbus_write_byte_data(client, NCT3018Y_REG_SC, buf[0]); @@ -212,6 +234,18 @@ static int nct3018y_rtc_set_time(struct device *dev, struct rtc_time *tm) return -EIO; } + /* Restore TWO bit */ + if (restore_flags) { + if (nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); + return err; + } + } + return err; } @@ -479,11 +513,17 @@ static int nct3018y_probe(struct i2c_client *client) dev_dbg(&client->dev, "%s: NCT3018Y_BIT_TWO 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; + nct3018y->part_num = i2c_smbus_read_byte_data(client, NCT3018Y_REG_PART); + if (nct3018y->part_num < 0) { + dev_dbg(&client->dev, "Failed to read NCT3018Y_REG_PART.\n"); + return nct3018y->part_num; + } else if (nct3018y->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, "Unable to write NCT3018Y_REG_CTRL.\n"); + return err; + } } flags = 0;
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. Signed-off-by: Mia Lin <mimi05633@gmail.com> --- drivers/rtc/rtc-nct3018y.c | 52 +++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 6 deletions(-)