Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221215150214.1109074-1-hugo@hugovil.com> <20221215150214.1109074-12-hugo@hugovil.com>
In-Reply-To: <20221215150214.1109074-12-hugo@hugovil.com>
From:   Bruno Thomsen <bruno.thomsen@gmail.com>
Date:   Sat, 7 Jan 2023 19:44:23 +0100
Message-ID: <CAH+2xPBD4ezWPzMj4YzF63duWnw3_af6KkC7eq1EWS=5F_5NGw@mail.gmail.com>
Subject: Re: [PATCH v3 11/14] rtc: pcf2127: adapt time/date registers write
 sequence for PCF2131
To:     Hugo Villeneuve <hugo@hugovil.com>
Cc:     a.zummo@towertech.it, alexandre.belloni@bootlin.com,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        linux-rtc@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Hugo Villeneuve <hvilleneuve@dimonoff.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Den tor. 15. dec. 2022 kl. 16.19 skrev Hugo Villeneuve <hugo@hugovil.com>:
>
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
>
> The sequence for updating the time/date registers is slightly
> different between PCF2127/29 and PCF2131.
>
> For PCF2127/29, during write operations, the time counting
> circuits (memory locations 03h through 09h) are automatically blocked.
>
> For PCF2131, time/date registers write access requires setting the
> STOP bit and sending the clear prescaler instruction (CPR). STOP then
> needs to be released once write operation is completed.
>
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  drivers/rtc/rtc-pcf2127.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
> index e4b78b9c03f9..11fbdab6bf01 100644
> --- a/drivers/rtc/rtc-pcf2127.c
> +++ b/drivers/rtc/rtc-pcf2127.c
> @@ -39,6 +39,7 @@
>  #define PCF2127_REG_CTRL1              0x00
>  #define PCF2127_BIT_CTRL1_POR_OVRD             BIT(3)
>  #define PCF2127_BIT_CTRL1_TSF1                 BIT(4)
> +#define PCF2127_BIT_CTRL1_STOP                 BIT(5)
>  /* Control register 2 */
>  #define PCF2127_REG_CTRL2              0x01
>  #define PCF2127_BIT_CTRL2_AIE                  BIT(1)
> @@ -70,6 +71,7 @@
>  #define PCF2131_REG_SR_RESET           0x05
>  #define PCF2131_SR_RESET_READ_PATTERN  0b00100100 /* Fixed pattern. */
>  #define PCF2131_SR_RESET_RESET_CMD     0x2C /* SR is bit 3. */
> +#define PCF2131_SR_RESET_CPR_CMD       0xA4 /* CPR is bit 7. */

Replace 0xA4 with (BIT(2) | BIT(5) | BIT(7)) or
(PCF2131_SR_RESET_READ_PATTERN | BIT(7))

>  /* Time and date registers */
>  #define PCF2127_REG_TIME_DATE_BASE     0x03
>  #define PCF2131_REG_TIME_DATE_BASE     0x07 /* Register 0x06 is 100th seconds,
> @@ -307,7 +309,31 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm)
>         /* year */
>         buf[i++] = bin2bcd(tm->tm_year - 100);
>
> -       /* write register's data */
> +       /* Write access to time registers:
> +        * PCF2127/29: no special action required.
> +        * PCF2131:    requires setting the STOP bit. STOP bit needs to
> +        *             be cleared after time registers are updated.
> +        *             It is also recommended to set CPR bit, although
> +        *             write access will work without it.
> +        */
> +       if (pcf2127->cfg->has_reset_reg) {
> +               err = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL1,
> +                                        PCF2127_BIT_CTRL1_STOP,
> +                                        PCF2127_BIT_CTRL1_STOP);
> +               if (err) {
> +                       dev_err(dev, "setting STOP bit failed\n");
> +                       return err;
> +               }
> +
> +               err = regmap_write(pcf2127->regmap, pcf2127->cfg->reg_reset,
> +                                  PCF2131_SR_RESET_CPR_CMD);
> +               if (err) {
> +                       dev_err(dev, "sending CPR cmd failed\n");
> +                       return err;
> +               }
> +       }
> +
> +       /* write time register's data */
>         err = regmap_bulk_write(pcf2127->regmap, pcf2127->cfg->regs_td_base, buf, i);
>         if (err) {
>                 dev_err(dev,
> @@ -315,6 +341,16 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm)
>                 return err;
>         }
>
> +       if (pcf2127->cfg->has_reset_reg) {
> +               /* Clear STOP bit (PCF2131 only) after write is completed. */
> +               err = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL1,
> +                                        PCF2127_BIT_CTRL1_STOP, 0);
> +               if (err) {
> +                       dev_err(dev, "clearing STOP bit failed\n");
> +                       return err;
> +               }
> +       }
> +
>         return 0;
>  }
>
> --
> 2.30.2
>