mbox series

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

Message ID 20220627111432.2625168-1-stephan.gerhold@kernkonzept.com
Headers show
Series watchdog: pm8916_wdt: Some minor improvements | expand

Message

Stephan Gerhold June 27, 2022, 11:14 a.m. UTC 
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.

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 | 36 +++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

Comments

Guenter Roeck June 27, 2022, 2:12 p.m. UTC | #1 
On 6/27/22 04:14, Stephan Gerhold wrote:
> The bootloader might already enable the watchdog to catch hangs
> during the boot process. In that case the kernel needs to ping
> the watchdog temporarily until userspace is fully started.
> 
> Add a check for this in the probe() function and set the WDOG_HW_RUNNING
> flag to make the watchdog core handle this automatically.
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>
> ---
>   drivers/watchdog/pm8916_wdt.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/watchdog/pm8916_wdt.c b/drivers/watchdog/pm8916_wdt.c
> index 3fc03fdae466..4ebd1d0cab85 100644
> --- a/drivers/watchdog/pm8916_wdt.c
> +++ b/drivers/watchdog/pm8916_wdt.c
> @@ -142,6 +142,7 @@ static int pm8916_wdt_probe(struct platform_device *pdev)
>   	struct device *dev = &pdev->dev;
>   	struct pm8916_wdt *wdt;
>   	struct device *parent;
> +	unsigned int val;
>   	int err, irq;
>   	u8 poff[2];
>   
> @@ -198,6 +199,11 @@ static int pm8916_wdt_probe(struct platform_device *pdev)
>   		dev_err(dev, "failed to read POFF reason: %d\n", err);
>   	}
>   
> +	err = regmap_read(wdt->regmap, wdt->baseaddr + PON_PMIC_WD_RESET_S2_CTL2,
> +			  &val);
> +	if (err == 0 && val & S2_RESET_EN_BIT)
> +		set_bit(WDOG_HW_RUNNING, &wdt->wdev.status);
> +

If reading the status returns an error, I'd assume that something is seriously
wrong. If it is ok to ignore such errors, explain why this is the case.

Thanks,
Guenter

>   	/* Configure watchdog to hard-reset mode */
>   	err = regmap_write(wdt->regmap,
>   			   wdt->baseaddr + PON_PMIC_WD_RESET_S2_CTL,