| Message ID | 1563264921-42973-4-git-send-email-biju.das@bp.renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add RTC support | expand |
On Tue 2019-07-16 09:15:14, Biju Das wrote: > commit 188306ac9536ec47674ffa9dd330f69927679aeb upstream. > > As per 8.2.6 Setting and reading the time in RTC mode, first stop the clok, > then reset it before setting the date and time registers. Finally, start > the clock. > > This uses register address wrap around from 0x2f to 0x00 for > efficiency. How does wrap around work? AFAICT it is supposed to have ram at 0x40. Does it really provide increased efficiency (given regmap layer in between) and will such trick cause problems in future? If regmap is not aware of register mirrors it might get confused and provide stale values, for example... Best regards, Pavel
Hi Pavel, Thanks for the feedback. > -----Original Message----- > From: Pavel Machek <pavel@denx.de> > Sent: Tuesday, July 16, 2019 10:04 PM > To: Biju Das <biju.das@bp.renesas.com> > Cc: cip-dev@lists.cip-project.org > Subject: Re: [cip-dev] [PATCH 4.4.y-cip 03/10] rtc: pcf85363: set time > accurately > > On Tue 2019-07-16 09:15:14, Biju Das wrote: > > commit 188306ac9536ec47674ffa9dd330f69927679aeb upstream. > > > > As per 8.2.6 Setting and reading the time in RTC mode, first stop the > > clok, then reset it before setting the date and time registers. > > Finally, start the clock. > > > > This uses register address wrap around from 0x2f to 0x00 for > > efficiency. > > How does wrap around work? AFAICT it is supposed to have ram at 0x40. > Please see the document [1] and [2] section 8, that have the details related to wrap around. [1] https://www.nxp.com/docs/en/data-sheet/PCF85363A.pdf [2] https://www.nxp.com/docs/en/data-sheet/PCF85263A.pdf Regards, Biju > Does it really provide increased efficiency (given regmap layer in > between) and will such trick cause problems in future? If regmap is not > aware of register mirrors it might get confused and provide stale values, for > example... > > Best regards, > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) > http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Wed, 2019-07-17 at 07:47 +0000, Biju Das wrote: > Hi Pavel, > > Thanks for the feedback. > > > -----Original Message----- > > From: Pavel Machek <pavel@denx.de> > > Sent: Tuesday, July 16, 2019 10:04 PM > > To: Biju Das <biju.das@bp.renesas.com> > > Cc: cip-dev@lists.cip-project.org > > Subject: Re: [cip-dev] [PATCH 4.4.y-cip 03/10] rtc: pcf85363: set time > > accurately > > > > On Tue 2019-07-16 09:15:14, Biju Das wrote: > > > commit 188306ac9536ec47674ffa9dd330f69927679aeb upstream. > > > > > > As per 8.2.6 Setting and reading the time in RTC mode, first stop the > > > clok, then reset it before setting the date and time registers. > > > Finally, start the clock. > > > > > > This uses register address wrap around from 0x2f to 0x00 for > > > efficiency. > > > > How does wrap around work? AFAICT it is supposed to have ram at 0x40. > > > Please see the document [1] and [2] section 8, that have the details related to wrap around. > [1] https://www.nxp.com/docs/en/data-sheet/PCF85363A.pdf > [2] https://www.nxp.com/docs/en/data-sheet/PCF85263A.pdf Regardless of what the hardware does... > > Does it really provide increased efficiency (given regmap layer in > > between) and will such trick cause problems in future? If regmap is not > > aware of register mirrors it might get confused and provide stale values, for > > example... ...I think Pavel is right on this point. regmap doesn't seem to have any provision for register addresses wrapping around, and it looks like this causes a buffer overrun in the register cache. Ben.
diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c index 5fbc6df..dc57a6f 100644 --- a/drivers/rtc/rtc-pcf85363.c +++ b/drivers/rtc/rtc-pcf85363.c @@ -73,6 +73,10 @@ #define CTRL_RESETS 0x2f #define CTRL_RAM 0x40 +#define STOP_EN_STOP BIT(0) + +#define RESET_CPR 0xa4 + #define NVRAM_SIZE 0x40 static struct i2c_driver pcf85363_driver; @@ -115,8 +119,12 @@ static int pcf85363_rtc_read_time(struct device *dev, struct rtc_time *tm) static int pcf85363_rtc_set_time(struct device *dev, struct rtc_time *tm) { struct pcf85363 *pcf85363 = dev_get_drvdata(dev); - unsigned char buf[DT_YEARS + 1]; - int len = sizeof(buf); + unsigned char tmp[11]; + unsigned char *buf = &tmp[2]; + int ret; + + tmp[0] = STOP_EN_STOP; + tmp[1] = RESET_CPR; buf[DT_100THS] = 0; buf[DT_SECS] = bin2bcd(tm->tm_sec); @@ -127,8 +135,12 @@ static int pcf85363_rtc_set_time(struct device *dev, struct rtc_time *tm) buf[DT_MONTHS] = bin2bcd(tm->tm_mon + 1); buf[DT_YEARS] = bin2bcd(tm->tm_year % 100); - return regmap_bulk_write(pcf85363->regmap, DT_100THS, - buf, len); + ret = regmap_bulk_write(pcf85363->regmap, CTRL_STOP_EN, + tmp, sizeof(tmp)); + if (ret) + return ret; + + return regmap_write(pcf85363->regmap, CTRL_STOP_EN, 0); } static const struct rtc_class_ops rtc_ops = {