Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Tue, 12 Dec 2023 15:39:20 +0530
From:   Manorit Chawdhry <m-chawdhry@ti.com>
To:     Vignesh Raghavendra <vigneshr@ti.com>
CC:     Wim Van Sebroeck <wim@linux-watchdog.org>,
        Guenter Roeck <linux@roeck-us.net>,
        Tero Kristo <kristo@kernel.org>,
        <linux-watchdog@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>, <afd@ti.com>
Subject: Re: [PATCH v2] watchdog: rti_wdt: Drop RPM count when unused
Message-ID: <20231212100920.wq26mylvn5qsms6w@uda0497581>
References: <20231122041642.2015936-1-vigneshr@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231122041642.2015936-1-vigneshr@ti.com>
Precedence: bulk

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@ti.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@ti.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@ti.com>

Regards,
Manorit

> -- 
> 2.42.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel