[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231212100920.wq26mylvn5qsms6w@uda0497581>
Date: Tue, 12 Dec 2023 15:39:20 +0530
From: Manorit Chawdhry <m-chawdhry@...com>
To: Vignesh Raghavendra <vigneshr@...com>
CC: Wim Van Sebroeck <wim@...ux-watchdog.org>,
Guenter Roeck <linux@...ck-us.net>,
Tero Kristo <kristo@...nel.org>,
<linux-watchdog@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <afd@...com>
Subject: Re: [PATCH v2] watchdog: rti_wdt: Drop RPM count when unused
Hi Vignesh,
On 09:46-20231122, Vignesh Raghavendra wrote:
> Do a RPM put if watchdog is not already started during probe and re
> enable it in watchdog start.
>
> On K3 SoCs, watchdogs and their corresponding CPUs are under same PD, so
> if the reference count of unused watchdogs aren't dropped, it will lead
> to CPU hotplug failures as Device Management firmware won't allow to
> turn off the PD due to dangling reference count.
>
> Fixes: 2d63908bdbfb ("watchdog: Add K3 RTI watchdog support")
> Signed-off-by: Vignesh Raghavendra <vigneshr@...com>
> ---
>
> v2:
> * Drop 1/2 (will be posted along with runtime_pm callbacks)
> * Use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() which
> takes care of err handling
>
> v1: lore.kernel.org/r/20231110100726.2930218-1-vigneshr@...com
>
> drivers/watchdog/rti_wdt.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
> index 8e1be7ba0103..9215793a1c81 100644
> --- a/drivers/watchdog/rti_wdt.c
> +++ b/drivers/watchdog/rti_wdt.c
> @@ -77,6 +77,11 @@ static int rti_wdt_start(struct watchdog_device *wdd)
> {
> u32 timer_margin;
> struct rti_wdt_device *wdt = watchdog_get_drvdata(wdd);
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(wdd->parent);
> + if (ret)
> + return ret;
>
> /* set timeout period */
> timer_margin = (u64)wdd->timeout * wdt->freq;
> @@ -343,6 +348,9 @@ static int rti_wdt_probe(struct platform_device *pdev)
> if (last_ping)
> watchdog_set_last_hw_keepalive(wdd, last_ping);
>
> + if (!watchdog_hw_running(wdd))
> + pm_runtime_put_sync(&pdev->dev);
> +
> return 0;
>
> err_iomap:
> @@ -357,7 +365,10 @@ static void rti_wdt_remove(struct platform_device *pdev)
> struct rti_wdt_device *wdt = platform_get_drvdata(pdev);
>
> watchdog_unregister_device(&wdt->wdd);
> - pm_runtime_put(&pdev->dev);
> +
> + if (!pm_runtime_suspended(&pdev->dev))
> + pm_runtime_put(&pdev->dev);
> +
> pm_runtime_disable(&pdev->dev);
> }
>
Tested the following on j721s2 evm.
https://gist.github.com/manorit2001/05a85582fea670d39cc3cfb2df1e5328
Tested-by: Manorit Chawdhry <m-chawdhry@...com>
Regards,
Manorit
> --
> 2.42.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Powered by blists - more mailing lists