[v2,0/3] watchdog: pm8916_wdt: Some minor improvements

Optimize the pm8916_wdt.c driver a bit by pinging the watchdog using a
write instead of read-modify-write. Also report the reboot reason to
userspace and (temporarily) ping the watchdog from the kernel if it was
already enabled by the bootloader.

---
Changes in v2: Improve error handling (suggested by Guenter)

Stephan Gerhold (3):
  watchdog: pm8916_wdt: Avoid read of write-only PET register
  watchdog: pm8916_wdt: Report reboot reason
  watchdog: pm8916_wdt: Handle watchdog enabled by bootloader

 drivers/watchdog/pm8916_wdt.c | 41 +++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

On Wed, 29 Jun 2022 at 10:48, Stephan Gerhold
<stephan.gerhold@kernkonzept.com> wrote:
>
> PMIC_WD_RESET_PET is a write-only register that is used to ping
> the watchdog. It does not make sense to use read-modify-write
> for it: a register read will never return anything but zero.
> (And actually even if it did we would still want to write again
> to ensure the watchdog is pinged.)
>
> Reduce the overhead for the watchdog ping slightly by using
> regmap_write() directly instead.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

> ---
> Changes in v2: Add Guenter's Reviewed-by
> ---
>  drivers/watchdog/pm8916_wdt.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/watchdog/pm8916_wdt.c b/drivers/watchdog/pm8916_wdt.c
> index 0937b8d33104..670cd79f4cf9 100644
> --- a/drivers/watchdog/pm8916_wdt.c
> +++ b/drivers/watchdog/pm8916_wdt.c
> @@ -58,9 +58,8 @@ static int pm8916_wdt_ping(struct watchdog_device *wdev)
>  {
>         struct pm8916_wdt *wdt = watchdog_get_drvdata(wdev);
>
> -       return regmap_update_bits(wdt->regmap,
> -                                 wdt->baseaddr + PON_PMIC_WD_RESET_PET,
> -                                 WATCHDOG_PET_BIT, WATCHDOG_PET_BIT);
> +       return regmap_write(wdt->regmap, wdt->baseaddr + PON_PMIC_WD_RESET_PET,
> +                           WATCHDOG_PET_BIT);
>  }
>
>  static int pm8916_wdt_configure_timers(struct watchdog_device *wdev)
> --
> 2.30.2
>

On Wed, 29 Jun 2022 at 10:48, Stephan Gerhold
<stephan.gerhold@kernkonzept.com> wrote:
>
> The PM8916 PMIC provides "power-off reason" (POFF_REASON) registers
> to allow detecting why the board was powered off or rebooted. This
> can be used to expose if a reset happened due to a watchdog timeout.
> The watchdog API also provides status bits for overtemperature and
> undervoltage which happen to be reported in the same PMIC register.
>
> Make this information available as part of the watchdog device
> so userspace can decide to handle the situation accordingly.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

> ---
> Changes in v2: Improve error handling (suggested by Guenter)
> ---
>  drivers/watchdog/pm8916_wdt.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/pm8916_wdt.c b/drivers/watchdog/pm8916_wdt.c
> index 670cd79f4cf9..49f1a5204526 100644
> --- a/drivers/watchdog/pm8916_wdt.c
> +++ b/drivers/watchdog/pm8916_wdt.c
> @@ -9,6 +9,12 @@
>  #include <linux/regmap.h>
>  #include <linux/watchdog.h>
>
> +#define PON_POFF_REASON1               0x0c
> +#define PON_POFF_REASON1_PMIC_WD       BIT(2)
> +#define PON_POFF_REASON2               0x0d
> +#define PON_POFF_REASON2_UVLO          BIT(5)
> +#define PON_POFF_REASON2_OTST3         BIT(6)
> +
>  #define PON_INT_RT_STS                 0x10
>  #define PMIC_WD_BARK_STS_BIT           BIT(6)
>
> @@ -110,12 +116,14 @@ static irqreturn_t pm8916_wdt_isr(int irq, void *arg)
>  }
>
>  static const struct watchdog_info pm8916_wdt_ident = {
> -       .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +       .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE |
> +                  WDIOF_OVERHEAT | WDIOF_CARDRESET | WDIOF_POWERUNDER,
>         .identity = "QCOM PM8916 PON WDT",
>  };
>
>  static const struct watchdog_info pm8916_wdt_pt_ident = {
>         .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE |
> +                  WDIOF_OVERHEAT | WDIOF_CARDRESET | WDIOF_POWERUNDER |
>                    WDIOF_PRETIMEOUT,
>         .identity = "QCOM PM8916 PON WDT",
>  };
> @@ -135,6 +143,7 @@ static int pm8916_wdt_probe(struct platform_device *pdev)
>         struct pm8916_wdt *wdt;
>         struct device *parent;
>         int err, irq;
> +       u8 poff[2];
>
>         wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
>         if (!wdt)
> @@ -175,6 +184,21 @@ static int pm8916_wdt_probe(struct platform_device *pdev)
>                 wdt->wdev.info = &pm8916_wdt_ident;
>         }
>
> +       err = regmap_bulk_read(wdt->regmap, wdt->baseaddr + PON_POFF_REASON1,
> +                              &poff, ARRAY_SIZE(poff));
> +       if (err) {
> +               dev_err(dev, "failed to read POFF reason: %d\n", err);
> +               return err;
> +       }
> +
> +       dev_dbg(dev, "POFF reason: %#x %#x\n", poff[0], poff[1]);
> +       if (poff[0] & PON_POFF_REASON1_PMIC_WD)
> +               wdt->wdev.bootstatus |= WDIOF_CARDRESET;
> +       if (poff[1] & PON_POFF_REASON2_UVLO)
> +               wdt->wdev.bootstatus |= WDIOF_POWERUNDER;
> +       if (poff[1] & PON_POFF_REASON2_OTST3)
> +               wdt->wdev.bootstatus |= WDIOF_OVERHEAT;
> +
>         /* Configure watchdog to hard-reset mode */
>         err = regmap_write(wdt->regmap,
>                            wdt->baseaddr + PON_PMIC_WD_RESET_S2_CTL,
> --
> 2.30.2
>