* [PATCH 0/3] rtc: pcf2127: add alarm support
@ 2020-06-07 17:06 liambeguin
2020-06-07 17:06 ` [PATCH 1/3] rtc: pcf2127: add pca2129 device id liambeguin
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: liambeguin @ 2020-06-07 17:06 UTC (permalink / raw)
To: a.zummo, alexandre.belloni, linux-rtc
From: Liam Beguin <lvb@xiphos.com>
The board used to test this series has the interrupt line of the RTC
connected to a circuit controlling the power of the board.
An event on the interrupt line while the board is off will power it on.
This was tested on a PCA2129, with:
$ date "2010-10-10 10:10"
Sun Oct 10 10:10:00 UTC 2010
$ /usr/sbin/rtcwake -u -d /dev/rtc0 -s10 --mode off
[ ... ]
$ # power on after 10 seconds
Liam Beguin (3):
rtc: pcf2127: add pca2129 device id
rtc: pcf2127: add alarm support
rtc: pcf2127: reset alarm interrupt at power on
drivers/rtc/rtc-pcf2127.c | 131 +++++++++++++++++++++++++++++++++++++-
1 file changed, 128 insertions(+), 3 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] rtc: pcf2127: add pca2129 device id 2020-06-07 17:06 [PATCH 0/3] rtc: pcf2127: add alarm support liambeguin @ 2020-06-07 17:06 ` liambeguin 2020-06-09 20:48 ` Bruno Thomsen 2020-06-07 17:06 ` [PATCH 2/3] rtc: pcf2127: add alarm support liambeguin 2020-06-07 17:06 ` [PATCH 3/3] rtc: pcf2127: reset alarm interrupt at power on liambeguin 2 siblings, 1 reply; 11+ messages in thread From: liambeguin @ 2020-06-07 17:06 UTC (permalink / raw) To: a.zummo, alexandre.belloni, linux-rtc From: Liam Beguin <lvb@xiphos.com> From: Liam Beguin <lvb@xiphos.com> The PCA2129 is the automotive grade version of the PCF2129. add it to the list of compatibles. Signed-off-by: Liam Beguin <lvb@xiphos.com> --- drivers/rtc/rtc-pcf2127.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c index 4e50d6768f13..396a1144a213 100644 --- a/drivers/rtc/rtc-pcf2127.c +++ b/drivers/rtc/rtc-pcf2127.c @@ -546,6 +546,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, static const struct of_device_id pcf2127_of_match[] = { { .compatible = "nxp,pcf2127" }, { .compatible = "nxp,pcf2129" }, + { .compatible = "nxp,pca2129" }, {} }; MODULE_DEVICE_TABLE(of, pcf2127_of_match); @@ -656,6 +657,7 @@ static int pcf2127_i2c_probe(struct i2c_client *client, static const struct i2c_device_id pcf2127_i2c_id[] = { { "pcf2127", 1 }, { "pcf2129", 0 }, + { "pca2129", 0 }, { } }; MODULE_DEVICE_TABLE(i2c, pcf2127_i2c_id); @@ -720,6 +722,7 @@ static int pcf2127_spi_probe(struct spi_device *spi) static const struct spi_device_id pcf2127_spi_id[] = { { "pcf2127", 1 }, { "pcf2129", 0 }, + { "pca2129", 0 }, { } }; MODULE_DEVICE_TABLE(spi, pcf2127_spi_id); -- 2.27.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] rtc: pcf2127: add pca2129 device id 2020-06-07 17:06 ` [PATCH 1/3] rtc: pcf2127: add pca2129 device id liambeguin @ 2020-06-09 20:48 ` Bruno Thomsen 0 siblings, 0 replies; 11+ messages in thread From: Bruno Thomsen @ 2020-06-09 20:48 UTC (permalink / raw) To: liambeguin; +Cc: Alessandro Zummo, Alexandre Belloni, linux-rtc Den søn. 7. jun. 2020 kl. 19.06 skrev <liambeguin@gmail.com>: > > From: Liam Beguin <lvb@xiphos.com> > > From: Liam Beguin <lvb@xiphos.com> > > The PCA2129 is the automotive grade version of the PCF2129. > add it to the list of compatibles. > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > --- > drivers/rtc/rtc-pcf2127.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > index 4e50d6768f13..396a1144a213 100644 > --- a/drivers/rtc/rtc-pcf2127.c > +++ b/drivers/rtc/rtc-pcf2127.c > @@ -546,6 +546,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > static const struct of_device_id pcf2127_of_match[] = { > { .compatible = "nxp,pcf2127" }, > { .compatible = "nxp,pcf2129" }, > + { .compatible = "nxp,pca2129" }, Remember to document new compatible string here: Documentation/devicetree/bindings/rtc/trivial-rtc.yaml /Bruno > {} > }; > MODULE_DEVICE_TABLE(of, pcf2127_of_match); > @@ -656,6 +657,7 @@ static int pcf2127_i2c_probe(struct i2c_client *client, > static const struct i2c_device_id pcf2127_i2c_id[] = { > { "pcf2127", 1 }, > { "pcf2129", 0 }, > + { "pca2129", 0 }, > { } > }; > MODULE_DEVICE_TABLE(i2c, pcf2127_i2c_id); > @@ -720,6 +722,7 @@ static int pcf2127_spi_probe(struct spi_device *spi) > static const struct spi_device_id pcf2127_spi_id[] = { > { "pcf2127", 1 }, > { "pcf2129", 0 }, > + { "pca2129", 0 }, > { } > }; > MODULE_DEVICE_TABLE(spi, pcf2127_spi_id); > -- > 2.27.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] rtc: pcf2127: add alarm support 2020-06-07 17:06 [PATCH 0/3] rtc: pcf2127: add alarm support liambeguin 2020-06-07 17:06 ` [PATCH 1/3] rtc: pcf2127: add pca2129 device id liambeguin @ 2020-06-07 17:06 ` liambeguin 2020-06-09 20:42 ` Bruno Thomsen 2020-06-09 21:05 ` Alexandre Belloni 2020-06-07 17:06 ` [PATCH 3/3] rtc: pcf2127: reset alarm interrupt at power on liambeguin 2 siblings, 2 replies; 11+ messages in thread From: liambeguin @ 2020-06-07 17:06 UTC (permalink / raw) To: a.zummo, alexandre.belloni, linux-rtc From: Liam Beguin <lvb@xiphos.com> From: Liam Beguin <lvb@xiphos.com> Add alarm support for the pcf2127 RTC chip family. Tested on pca2129. Signed-off-by: Liam Beguin <lvb@xiphos.com> --- drivers/rtc/rtc-pcf2127.c | 120 +++++++++++++++++++++++++++++++++++++- 1 file changed, 117 insertions(+), 3 deletions(-) diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c index 396a1144a213..3eeb085a7c72 100644 --- a/drivers/rtc/rtc-pcf2127.c +++ b/drivers/rtc/rtc-pcf2127.c @@ -28,7 +28,9 @@ #define PCF2127_BIT_CTRL1_TSF1 BIT(4) /* Control register 2 */ #define PCF2127_REG_CTRL2 0x01 +#define PCF2127_BIT_CTRL2_AIE BIT(1) #define PCF2127_BIT_CTRL2_TSIE BIT(2) +#define PCF2127_BIT_CTRL2_AF BIT(4) #define PCF2127_BIT_CTRL2_TSF2 BIT(5) /* Control register 3 */ #define PCF2127_REG_CTRL3 0x02 @@ -46,6 +48,12 @@ #define PCF2127_REG_DW 0x07 #define PCF2127_REG_MO 0x08 #define PCF2127_REG_YR 0x09 +/* Alarm registers */ +#define PCF2127_REG_ALARM_SC 0x0A +#define PCF2127_REG_ALARM_MN 0x0B +#define PCF2127_REG_ALARM_HR 0x0C +#define PCF2127_REG_ALARM_DM 0x0D +#define PCF2127_REG_ALARM_DW 0x0E /* Watchdog registers */ #define PCF2127_REG_WD_CTL 0x10 #define PCF2127_BIT_WD_CTL_TF0 BIT(0) @@ -185,6 +193,107 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm) return 0; } +static int pcf2127_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) +{ + struct pcf2127 *pcf2127 = dev_get_drvdata(dev); + unsigned int buf[5], ctrl2; + int ret; + + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2); + if (ret) { + dev_err(dev, "%s: ctrl2 read error\n", __func__); + return ret; + } + ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf, 5); + if (ret) { + dev_err(dev, "%s: alarm read error\n", __func__); + return ret; + } + + alrm->enabled = ctrl2 & PCF2127_BIT_CTRL2_AIE; + alrm->pending = ctrl2 & PCF2127_BIT_CTRL2_AF; + + alrm->time.tm_sec = bcd2bin(buf[0] & 0x7F); + alrm->time.tm_min = bcd2bin(buf[1] & 0x7F); + alrm->time.tm_hour = bcd2bin(buf[2] & 0x3F); + alrm->time.tm_mday = bcd2bin(buf[3] & 0x3F); + alrm->time.tm_wday = buf[4] & 0x07; + + dev_dbg(dev, "%s: alarm is %d:%d:%d, mday=%d, wday=%d\n", __func__, + alrm->time.tm_hour, alrm->time.tm_min, alrm->time.tm_sec, + alrm->time.tm_mday, alrm->time.tm_wday); + + return 0; +} + +static int pcf2127_rtc_alarm_irq_enable(struct device *dev, u32 enable) +{ + struct pcf2127 *pcf2127 = dev_get_drvdata(dev); + unsigned int ctrl2; + int ret; + + dev_dbg(dev, "%s: %s\n", __func__, enable ? "enable" : "disable"); + + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2); + if (ret) { + dev_err(dev, "%s: ctrl2 read error\n", __func__); + return ret; + } + + if (enable) + ret = regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2, + ctrl2 | PCF2127_BIT_CTRL2_AIE); + else + ret = regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2, + ctrl2 & ~PCF2127_BIT_CTRL2_AIE); + + if (ret) { + dev_err(dev, "%s: failed to enable alarm (%d)\n", __func__, + ret); + return ret; + } + + return 0; +} + +static int pcf2127_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) +{ + struct pcf2127 *pcf2127 = dev_get_drvdata(dev); + unsigned int ctrl2; + uint8_t buf[5]; + int ret; + + ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2, + PCF2127_BIT_CTRL2_AF, + (unsigned int)~PCF2127_BIT_CTRL2_AF); + if (ret) { + dev_err(dev, "%s: failed to clear alarm interrupt flag (%d)", + __func__, ret); + return ret; + } + + buf[0] = bin2bcd(alrm->time.tm_sec); + buf[1] = bin2bcd(alrm->time.tm_min); + buf[2] = bin2bcd(alrm->time.tm_hour); + buf[3] = bin2bcd(alrm->time.tm_mday); + buf[4] = (alrm->time.tm_wday & 0x07); + + dev_dbg(dev, "%s: alarm set for: %d:%d:%d, mday=%d, wday=%d\n", + __func__, alrm->time.tm_hour, alrm->time.tm_min, + alrm->time.tm_sec, alrm->time.tm_mday, alrm->time.tm_wday); + + ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf, 5); + if (ret) { + dev_err(dev, "%s: failed to write alarm registers (%d)", + __func__, ret); + return ret; + } + + pcf2127_rtc_alarm_irq_enable(dev, alrm->enabled); + + return 0; +} + #ifdef CONFIG_RTC_INTF_DEV static int pcf2127_rtc_ioctl(struct device *dev, unsigned int cmd, unsigned long arg) @@ -211,9 +320,12 @@ static int pcf2127_rtc_ioctl(struct device *dev, #endif static const struct rtc_class_ops pcf2127_rtc_ops = { - .ioctl = pcf2127_rtc_ioctl, - .read_time = pcf2127_rtc_read_time, - .set_time = pcf2127_rtc_set_time, + .ioctl = pcf2127_rtc_ioctl, + .read_time = pcf2127_rtc_read_time, + .set_time = pcf2127_rtc_set_time, + .read_alarm = pcf2127_rtc_read_alarm, + .set_alarm = pcf2127_rtc_set_alarm, + .alarm_irq_enable = pcf2127_rtc_alarm_irq_enable, }; static int pcf2127_nvmem_read(void *priv, unsigned int offset, @@ -434,6 +546,8 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, pcf2127->rtc->ops = &pcf2127_rtc_ops; + device_init_wakeup(dev, true); + pcf2127->wdd.parent = dev; pcf2127->wdd.info = &pcf2127_wdt_info; pcf2127->wdd.ops = &pcf2127_watchdog_ops; -- 2.27.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] rtc: pcf2127: add alarm support 2020-06-07 17:06 ` [PATCH 2/3] rtc: pcf2127: add alarm support liambeguin @ 2020-06-09 20:42 ` Bruno Thomsen 2020-06-10 15:38 ` Liam Beguin 2020-06-09 21:05 ` Alexandre Belloni 1 sibling, 1 reply; 11+ messages in thread From: Bruno Thomsen @ 2020-06-09 20:42 UTC (permalink / raw) To: liambeguin; +Cc: Alessandro Zummo, Alexandre Belloni, linux-rtc Hi Liam, See comments below. Den søn. 7. jun. 2020 kl. 19.06 skrev <liambeguin@gmail.com>: > > From: Liam Beguin <lvb@xiphos.com> > > From: Liam Beguin <lvb@xiphos.com> > > Add alarm support for the pcf2127 RTC chip family. > Tested on pca2129. > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > --- > drivers/rtc/rtc-pcf2127.c | 120 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 117 insertions(+), 3 deletions(-) > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > index 396a1144a213..3eeb085a7c72 100644 > --- a/drivers/rtc/rtc-pcf2127.c > +++ b/drivers/rtc/rtc-pcf2127.c > @@ -28,7 +28,9 @@ > #define PCF2127_BIT_CTRL1_TSF1 BIT(4) > /* Control register 2 */ > #define PCF2127_REG_CTRL2 0x01 > +#define PCF2127_BIT_CTRL2_AIE BIT(1) > #define PCF2127_BIT_CTRL2_TSIE BIT(2) > +#define PCF2127_BIT_CTRL2_AF BIT(4) > #define PCF2127_BIT_CTRL2_TSF2 BIT(5) > /* Control register 3 */ > #define PCF2127_REG_CTRL3 0x02 > @@ -46,6 +48,12 @@ > #define PCF2127_REG_DW 0x07 > #define PCF2127_REG_MO 0x08 > #define PCF2127_REG_YR 0x09 > +/* Alarm registers */ > +#define PCF2127_REG_ALARM_SC 0x0A > +#define PCF2127_REG_ALARM_MN 0x0B > +#define PCF2127_REG_ALARM_HR 0x0C > +#define PCF2127_REG_ALARM_DM 0x0D > +#define PCF2127_REG_ALARM_DW 0x0E > /* Watchdog registers */ > #define PCF2127_REG_WD_CTL 0x10 > #define PCF2127_BIT_WD_CTL_TF0 BIT(0) > @@ -185,6 +193,107 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm) > return 0; > } > > +static int pcf2127_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) > +{ > + struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > + unsigned int buf[5], ctrl2; > + int ret; > + > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2); > + if (ret) { > + dev_err(dev, "%s: ctrl2 read error\n", __func__); > + return ret; > + } Reading CTRL2 register causes watchdog to stop. Aways call pcf2127_wdt_active_ping() after CTRL2 access to ensure the watchdog is running if enabled. > + ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf, 5); Replace 5 with sizeof(buf). > + if (ret) { > + dev_err(dev, "%s: alarm read error\n", __func__); > + return ret; > + } > + > + alrm->enabled = ctrl2 & PCF2127_BIT_CTRL2_AIE; > + alrm->pending = ctrl2 & PCF2127_BIT_CTRL2_AF; > + > + alrm->time.tm_sec = bcd2bin(buf[0] & 0x7F); > + alrm->time.tm_min = bcd2bin(buf[1] & 0x7F); > + alrm->time.tm_hour = bcd2bin(buf[2] & 0x3F); > + alrm->time.tm_mday = bcd2bin(buf[3] & 0x3F); > + alrm->time.tm_wday = buf[4] & 0x07; > + > + dev_dbg(dev, "%s: alarm is %d:%d:%d, mday=%d, wday=%d\n", __func__, > + alrm->time.tm_hour, alrm->time.tm_min, alrm->time.tm_sec, > + alrm->time.tm_mday, alrm->time.tm_wday); > + > + return 0; > +} > + > +static int pcf2127_rtc_alarm_irq_enable(struct device *dev, u32 enable) > +{ > + struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > + unsigned int ctrl2; > + int ret; > + > + dev_dbg(dev, "%s: %s\n", __func__, enable ? "enable" : "disable"); Delete debug trace. > + > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2); > + if (ret) { > + dev_err(dev, "%s: ctrl2 read error\n", __func__); > + return ret; > + } > + > + if (enable) > + ret = regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2, > + ctrl2 | PCF2127_BIT_CTRL2_AIE); > + else > + ret = regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2, > + ctrl2 & ~PCF2127_BIT_CTRL2_AIE); > + > + if (ret) { > + dev_err(dev, "%s: failed to enable alarm (%d)\n", __func__, > + ret); > + return ret; > + } Replace regmap_read() and regmap_write() with a regmap_update_bits(). So something like: ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2, enabled ? PCF2127_REG_CTRL2 : 0); And remember to call pcf2127_wdt_active_ping(). > + > + return 0; > +} > + > +static int pcf2127_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) > +{ > + struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > + unsigned int ctrl2; > + uint8_t buf[5]; > + int ret; > + > + ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2, > + PCF2127_BIT_CTRL2_AF, > + (unsigned int)~PCF2127_BIT_CTRL2_AF); If you just want to clear the AF bit in CTRL2, just do: ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2, PCF2127_BIT_CTRL2_AF, 0); > + if (ret) { > + dev_err(dev, "%s: failed to clear alarm interrupt flag (%d)", > + __func__, ret); > + return ret; > + } > + > + buf[0] = bin2bcd(alrm->time.tm_sec); > + buf[1] = bin2bcd(alrm->time.tm_min); > + buf[2] = bin2bcd(alrm->time.tm_hour); > + buf[3] = bin2bcd(alrm->time.tm_mday); > + buf[4] = (alrm->time.tm_wday & 0x07); > + > + dev_dbg(dev, "%s: alarm set for: %d:%d:%d, mday=%d, wday=%d\n", > + __func__, alrm->time.tm_hour, alrm->time.tm_min, > + alrm->time.tm_sec, alrm->time.tm_mday, alrm->time.tm_wday); > + > + ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf, 5); Replace 5 with sizeof(buf). /Bruno > + if (ret) { > + dev_err(dev, "%s: failed to write alarm registers (%d)", > + __func__, ret); > + return ret; > + } > + > + pcf2127_rtc_alarm_irq_enable(dev, alrm->enabled); > + > + return 0; > +} > + > #ifdef CONFIG_RTC_INTF_DEV > static int pcf2127_rtc_ioctl(struct device *dev, > unsigned int cmd, unsigned long arg) > @@ -211,9 +320,12 @@ static int pcf2127_rtc_ioctl(struct device *dev, > #endif > > static const struct rtc_class_ops pcf2127_rtc_ops = { > - .ioctl = pcf2127_rtc_ioctl, > - .read_time = pcf2127_rtc_read_time, > - .set_time = pcf2127_rtc_set_time, > + .ioctl = pcf2127_rtc_ioctl, > + .read_time = pcf2127_rtc_read_time, > + .set_time = pcf2127_rtc_set_time, > + .read_alarm = pcf2127_rtc_read_alarm, > + .set_alarm = pcf2127_rtc_set_alarm, > + .alarm_irq_enable = pcf2127_rtc_alarm_irq_enable, > }; > > static int pcf2127_nvmem_read(void *priv, unsigned int offset, > @@ -434,6 +546,8 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > pcf2127->rtc->ops = &pcf2127_rtc_ops; > > + device_init_wakeup(dev, true); > + > pcf2127->wdd.parent = dev; > pcf2127->wdd.info = &pcf2127_wdt_info; > pcf2127->wdd.ops = &pcf2127_watchdog_ops; > -- > 2.27.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] rtc: pcf2127: add alarm support 2020-06-09 20:42 ` Bruno Thomsen @ 2020-06-10 15:38 ` Liam Beguin 0 siblings, 0 replies; 11+ messages in thread From: Liam Beguin @ 2020-06-10 15:38 UTC (permalink / raw) To: Bruno Thomsen; +Cc: Alessandro Zummo, Alexandre Belloni, linux-rtc Hi Bruno, Thanks for your comments, I've updated the patch and will send a v2 soon. On Tue Jun 9, 2020 at 10:42 PM Bruno Thomsen wrote: > Hi Liam, > > See comments below. > > Den søn. 7. jun. 2020 kl. 19.06 skrev <liambeguin@gmail.com>: > > > > From: Liam Beguin <lvb@xiphos.com> > > > > From: Liam Beguin <lvb@xiphos.com> > > > > Add alarm support for the pcf2127 RTC chip family. > > Tested on pca2129. > > > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > > --- > > drivers/rtc/rtc-pcf2127.c | 120 +++++++++++++++++++++++++++++++++++++- > > 1 file changed, 117 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > > index 396a1144a213..3eeb085a7c72 100644 > > --- a/drivers/rtc/rtc-pcf2127.c > > +++ b/drivers/rtc/rtc-pcf2127.c > > @@ -28,7 +28,9 @@ > > #define PCF2127_BIT_CTRL1_TSF1 BIT(4) > > /* Control register 2 */ > > #define PCF2127_REG_CTRL2 0x01 > > +#define PCF2127_BIT_CTRL2_AIE BIT(1) > > #define PCF2127_BIT_CTRL2_TSIE BIT(2) > > +#define PCF2127_BIT_CTRL2_AF BIT(4) > > #define PCF2127_BIT_CTRL2_TSF2 BIT(5) > > /* Control register 3 */ > > #define PCF2127_REG_CTRL3 0x02 > > @@ -46,6 +48,12 @@ > > #define PCF2127_REG_DW 0x07 > > #define PCF2127_REG_MO 0x08 > > #define PCF2127_REG_YR 0x09 > > +/* Alarm registers */ > > +#define PCF2127_REG_ALARM_SC 0x0A > > +#define PCF2127_REG_ALARM_MN 0x0B > > +#define PCF2127_REG_ALARM_HR 0x0C > > +#define PCF2127_REG_ALARM_DM 0x0D > > +#define PCF2127_REG_ALARM_DW 0x0E > > /* Watchdog registers */ > > #define PCF2127_REG_WD_CTL 0x10 > > #define PCF2127_BIT_WD_CTL_TF0 BIT(0) > > @@ -185,6 +193,107 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm) > > return 0; > > } > > > > +static int pcf2127_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) > > +{ > > + struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > > + unsigned int buf[5], ctrl2; > > + int ret; > > + > > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2); > > + if (ret) { > > + dev_err(dev, "%s: ctrl2 read error\n", __func__); > > + return ret; > > + } > > Reading CTRL2 register causes watchdog to stop. > > Aways call pcf2127_wdt_active_ping() after CTRL2 access to ensure the watchdog > is running if enabled. > > > + ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf, 5); > > Replace 5 with sizeof(buf). > > > + if (ret) { > > + dev_err(dev, "%s: alarm read error\n", __func__); > > + return ret; > > + } > > + > > + alrm->enabled = ctrl2 & PCF2127_BIT_CTRL2_AIE; > > + alrm->pending = ctrl2 & PCF2127_BIT_CTRL2_AF; > > + > > + alrm->time.tm_sec = bcd2bin(buf[0] & 0x7F); > > + alrm->time.tm_min = bcd2bin(buf[1] & 0x7F); > > + alrm->time.tm_hour = bcd2bin(buf[2] & 0x3F); > > + alrm->time.tm_mday = bcd2bin(buf[3] & 0x3F); > > + alrm->time.tm_wday = buf[4] & 0x07; > > + > > + dev_dbg(dev, "%s: alarm is %d:%d:%d, mday=%d, wday=%d\n", __func__, > > + alrm->time.tm_hour, alrm->time.tm_min, alrm->time.tm_sec, > > + alrm->time.tm_mday, alrm->time.tm_wday); > > + > > + return 0; > > +} > > + > > +static int pcf2127_rtc_alarm_irq_enable(struct device *dev, u32 enable) > > +{ > > + struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > > + unsigned int ctrl2; > > + int ret; > > + > > + dev_dbg(dev, "%s: %s\n", __func__, enable ? "enable" : "disable"); > > Delete debug trace. > > > + > > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2); > > + if (ret) { > > + dev_err(dev, "%s: ctrl2 read error\n", __func__); > > + return ret; > > + } > > + > > + if (enable) > > + ret = regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2, > > + ctrl2 | PCF2127_BIT_CTRL2_AIE); > > + else > > + ret = regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2, > > + ctrl2 & ~PCF2127_BIT_CTRL2_AIE); > > + > > + if (ret) { > > + dev_err(dev, "%s: failed to enable alarm (%d)\n", __func__, > > + ret); > > + return ret; > > + } > > Replace regmap_read() and regmap_write() with a regmap_update_bits(). > > So something like: > > ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2, > enabled ? PCF2127_REG_CTRL2 : 0); > > And remember to call pcf2127_wdt_active_ping(). > > > + > > + return 0; > > +} > > + > > +static int pcf2127_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) > > +{ > > + struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > > + unsigned int ctrl2; > > + uint8_t buf[5]; > > + int ret; > > + > > + ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2, > > + PCF2127_BIT_CTRL2_AF, > > + (unsigned int)~PCF2127_BIT_CTRL2_AF); > > If you just want to clear the AF bit in CTRL2, just do: > > ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2, > PCF2127_BIT_CTRL2_AF, 0); > > > + if (ret) { > > + dev_err(dev, "%s: failed to clear alarm interrupt flag (%d)", > > + __func__, ret); > > + return ret; > > + } > > + > > + buf[0] = bin2bcd(alrm->time.tm_sec); > > + buf[1] = bin2bcd(alrm->time.tm_min); > > + buf[2] = bin2bcd(alrm->time.tm_hour); > > + buf[3] = bin2bcd(alrm->time.tm_mday); > > + buf[4] = (alrm->time.tm_wday & 0x07); > > + > > + dev_dbg(dev, "%s: alarm set for: %d:%d:%d, mday=%d, wday=%d\n", > > + __func__, alrm->time.tm_hour, alrm->time.tm_min, > > + alrm->time.tm_sec, alrm->time.tm_mday, alrm->time.tm_wday); > > + > > + ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf, 5); > > Replace 5 with sizeof(buf). > > /Bruno > Thanks again for your time, Liam > > + if (ret) { > > + dev_err(dev, "%s: failed to write alarm registers (%d)", > > + __func__, ret); > > + return ret; > > + } > > + > > + pcf2127_rtc_alarm_irq_enable(dev, alrm->enabled); > > + > > + return 0; > > +} > > + > > #ifdef CONFIG_RTC_INTF_DEV > > static int pcf2127_rtc_ioctl(struct device *dev, > > unsigned int cmd, unsigned long arg) > > @@ -211,9 +320,12 @@ static int pcf2127_rtc_ioctl(struct device *dev, > > #endif > > > > static const struct rtc_class_ops pcf2127_rtc_ops = { > > - .ioctl = pcf2127_rtc_ioctl, > > - .read_time = pcf2127_rtc_read_time, > > - .set_time = pcf2127_rtc_set_time, > > + .ioctl = pcf2127_rtc_ioctl, > > + .read_time = pcf2127_rtc_read_time, > > + .set_time = pcf2127_rtc_set_time, > > + .read_alarm = pcf2127_rtc_read_alarm, > > + .set_alarm = pcf2127_rtc_set_alarm, > > + .alarm_irq_enable = pcf2127_rtc_alarm_irq_enable, > > }; > > > > static int pcf2127_nvmem_read(void *priv, unsigned int offset, > > @@ -434,6 +546,8 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > > > pcf2127->rtc->ops = &pcf2127_rtc_ops; > > > > + device_init_wakeup(dev, true); > > + > > pcf2127->wdd.parent = dev; > > pcf2127->wdd.info = &pcf2127_wdt_info; > > pcf2127->wdd.ops = &pcf2127_watchdog_ops; > > -- > > 2.27.0 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] rtc: pcf2127: add alarm support 2020-06-07 17:06 ` [PATCH 2/3] rtc: pcf2127: add alarm support liambeguin 2020-06-09 20:42 ` Bruno Thomsen @ 2020-06-09 21:05 ` Alexandre Belloni 2020-06-10 15:49 ` Liam Beguin 1 sibling, 1 reply; 11+ messages in thread From: Alexandre Belloni @ 2020-06-09 21:05 UTC (permalink / raw) To: liambeguin; +Cc: a.zummo, linux-rtc On 07/06/2020 13:06:09-0400, liambeguin@gmail.com wrote: > static int pcf2127_nvmem_read(void *priv, unsigned int offset, > @@ -434,6 +546,8 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > pcf2127->rtc->ops = &pcf2127_rtc_ops; > > + device_init_wakeup(dev, true); > + This can't be done unconditionally, You need to have been able to request an interrupt or the wakeup-source property needs to be present. The interrupt handler is also missing from the patch. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] rtc: pcf2127: add alarm support 2020-06-09 21:05 ` Alexandre Belloni @ 2020-06-10 15:49 ` Liam Beguin 2020-06-10 16:32 ` Alexandre Belloni 0 siblings, 1 reply; 11+ messages in thread From: Liam Beguin @ 2020-06-10 15:49 UTC (permalink / raw) To: Alexandre Belloni; +Cc: a.zummo, linux-rtc Hi Alexandre, On Tue Jun 9, 2020 at 11:05 PM Alexandre Belloni wrote: > On 07/06/2020 13:06:09-0400, liambeguin@gmail.com wrote: > > static int pcf2127_nvmem_read(void *priv, unsigned int offset, > > @@ -434,6 +546,8 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > > > pcf2127->rtc->ops = &pcf2127_rtc_ops; > > > > + device_init_wakeup(dev, true); > > + > > This can't be done unconditionally, You need to have been able to > request an interrupt or the wakeup-source property needs to be present. > > The interrupt handler is also missing from the patch. Like I tried to explain in the cover letter, the interrupt line isn't connected to the CPU on the board I'm using. I'd be glad to add the interrupt handler to this patch. Is there a way I can make it conditional? Thanks, Liam > > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] rtc: pcf2127: add alarm support 2020-06-10 15:49 ` Liam Beguin @ 2020-06-10 16:32 ` Alexandre Belloni 0 siblings, 0 replies; 11+ messages in thread From: Alexandre Belloni @ 2020-06-10 16:32 UTC (permalink / raw) To: Liam Beguin; +Cc: a.zummo, linux-rtc On 10/06/2020 11:49:06-0400, Liam Beguin wrote: > Hi Alexandre, > > On Tue Jun 9, 2020 at 11:05 PM Alexandre Belloni wrote: > > On 07/06/2020 13:06:09-0400, liambeguin@gmail.com wrote: > > > static int pcf2127_nvmem_read(void *priv, unsigned int offset, > > > @@ -434,6 +546,8 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > > > > > pcf2127->rtc->ops = &pcf2127_rtc_ops; > > > > > > + device_init_wakeup(dev, true); > > > + > > > > This can't be done unconditionally, You need to have been able to > > request an interrupt or the wakeup-source property needs to be present. > > > > The interrupt handler is also missing from the patch. > > Like I tried to explain in the cover letter, the interrupt line isn't > connected to the CPU on the board I'm using. > I'd be glad to add the interrupt handler to this patch. Is there a way I > can make it conditional? It is necessarily conditional, as it won't be used if no interrupt is provided. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] rtc: pcf2127: reset alarm interrupt at power on 2020-06-07 17:06 [PATCH 0/3] rtc: pcf2127: add alarm support liambeguin 2020-06-07 17:06 ` [PATCH 1/3] rtc: pcf2127: add pca2129 device id liambeguin 2020-06-07 17:06 ` [PATCH 2/3] rtc: pcf2127: add alarm support liambeguin @ 2020-06-07 17:06 ` liambeguin 2020-06-09 21:07 ` Alexandre Belloni 2 siblings, 1 reply; 11+ messages in thread From: liambeguin @ 2020-06-07 17:06 UTC (permalink / raw) To: a.zummo, alexandre.belloni, linux-rtc From: Liam Beguin <lvb@xiphos.com> From: Liam Beguin <lvb@xiphos.com> Signed-off-by: Liam Beguin <lvb@xiphos.com> --- drivers/rtc/rtc-pcf2127.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c index 3eeb085a7c72..f004a4030970 100644 --- a/drivers/rtc/rtc-pcf2127.c +++ b/drivers/rtc/rtc-pcf2127.c @@ -546,6 +546,14 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, pcf2127->rtc->ops = &pcf2127_rtc_ops; + ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2, + PCF2127_BIT_CTRL2_AIE, 0); + if (ret) { + dev_err(dev, "%s: failed to clear interrupt enable bit (%d)", + __func__, ret); + return ret; + } + device_init_wakeup(dev, true); pcf2127->wdd.parent = dev; -- 2.27.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] rtc: pcf2127: reset alarm interrupt at power on 2020-06-07 17:06 ` [PATCH 3/3] rtc: pcf2127: reset alarm interrupt at power on liambeguin @ 2020-06-09 21:07 ` Alexandre Belloni 0 siblings, 0 replies; 11+ messages in thread From: Alexandre Belloni @ 2020-06-09 21:07 UTC (permalink / raw) To: liambeguin; +Cc: a.zummo, linux-rtc On 07/06/2020 13:06:10-0400, liambeguin@gmail.com wrote: > From: Liam Beguin <lvb@xiphos.com> > > From: Liam Beguin <lvb@xiphos.com> Always include a commit message. > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > --- > drivers/rtc/rtc-pcf2127.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > index 3eeb085a7c72..f004a4030970 100644 > --- a/drivers/rtc/rtc-pcf2127.c > +++ b/drivers/rtc/rtc-pcf2127.c > @@ -546,6 +546,14 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > pcf2127->rtc->ops = &pcf2127_rtc_ops; > > + ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2, > + PCF2127_BIT_CTRL2_AIE, 0); > + if (ret) { > + dev_err(dev, "%s: failed to clear interrupt enable bit (%d)", > + __func__, ret); > + return ret; > + } > + The driver simply must not do that or alarms will be missed. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-06-10 16:32 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-07 17:06 [PATCH 0/3] rtc: pcf2127: add alarm support liambeguin 2020-06-07 17:06 ` [PATCH 1/3] rtc: pcf2127: add pca2129 device id liambeguin 2020-06-09 20:48 ` Bruno Thomsen 2020-06-07 17:06 ` [PATCH 2/3] rtc: pcf2127: add alarm support liambeguin 2020-06-09 20:42 ` Bruno Thomsen 2020-06-10 15:38 ` Liam Beguin 2020-06-09 21:05 ` Alexandre Belloni 2020-06-10 15:49 ` Liam Beguin 2020-06-10 16:32 ` Alexandre Belloni 2020-06-07 17:06 ` [PATCH 3/3] rtc: pcf2127: reset alarm interrupt at power on liambeguin 2020-06-09 21:07 ` Alexandre Belloni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).