>
> Hi,
>
> On 01/12/2020 16:47:46+0800, Biwen Li wrote:
> > From: Biwen Li <biwen.li@xxxxxxx>
> >
> > - clear the flag TSF1 before enabling interrupt generation
> > - properly set flag WD_CD for rtc chips(pcf2129, pca2129)
> >
>
> This change has to be a separate patch.
Sure, np. Will separate the patch in v2.
>
> > Signed-off-by: Biwen Li <biwen.li@xxxxxxx>
> > ---
> > drivers/rtc/rtc-pcf2127.c | 21 ++++++++++++++++++++-
> > 1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
> > index 07a5630ec841..0a45e2512258 100644
> > --- a/drivers/rtc/rtc-pcf2127.c
> > +++ b/drivers/rtc/rtc-pcf2127.c
> > @@ -601,6 +601,10 @@ static int pcf2127_probe(struct device *dev, struct
> regmap *regmap,
> > * Watchdog timer enabled and reset pin /RST activated when timed out.
> > * Select 1Hz clock source for watchdog timer.
> > * Note: Countdown timer disabled and not available.
> > + * For pca2129, pcf2129, only bit[7] is for Symbol WD_CD
> > + * of register watchdg_tim_ctl. The bit[6] is labeled
> > + * as T. Bits labeled as T must always be written with
> > + * logic 0.
> > */
> > ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_WD_CTL,
> > PCF2127_BIT_WD_CTL_CD1 |
> > @@ -608,7 +612,7 @@ static int pcf2127_probe(struct device *dev, struct
> regmap *regmap,
> > PCF2127_BIT_WD_CTL_TF1 |
> > PCF2127_BIT_WD_CTL_TF0,
> > PCF2127_BIT_WD_CTL_CD1 |
> > - PCF2127_BIT_WD_CTL_CD0 |
> > + has_nvmem ? (PCF2127_BIT_WD_CTL_CD0) : (0) |
>
> I don't like that because has_nvmem has nothing to do with
> PCF2127_BIT_WD_CTL_CD0 and nothing guarantees that we won't ever get an
> RTC without RST but with NVRAM and that willprobbly be overlooked.
Okay, got it. Will correct it in v2.
>
> > PCF2127_BIT_WD_CTL_TF1);
> > if (ret) {
> > dev_err(dev, "%s: watchdog config (wd_ctl) failed\n", __func__); @@
> > -659,6 +663,21 @@ static int pcf2127_probe(struct device *dev, struct
> regmap *regmap,
> > return ret;
> > }
> >
> > + /*
> > + * Clear TSF1 field of ctrl1 register to clear interrupt
> > + * before enabling interrupt generation when
> > + * timestamp flag set. Unless the flag TSF1 won't
> > + * be cleared and the interrupt(INT pin) is
> > + * triggered continueously.
> > + */
> > + ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL1,
> > + PCF2127_BIT_CTRL1_TSF1,
> > + 0);
> > + if (ret) {
> > + dev_err(dev, "%s: control and status register 1 (ctrl1) failed, ret =
> 0x%x\n",
> > + __func__, ret);
> > + return ret;
> > + }
>
> Doing that means ignoring timestamps taken while the system is offline.
> It also doesn't fully solve the issue because you are not clearing TSF2 here and
> also it never gets cleared by the driver later on so I guess you will get the
> interrupt storm once a timestamp is taken.
Okay, got it. Thanks. Will clear TSF2 flag in v2.
>
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
