Re: [PATCH] watchdog: orion_wdt: Use devm APIs for clock and watchdog management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The local variable `ret` is declared but never used in multiple clock initialization functions (`orion_wdt_clock_init`, `armada370_wdt_clock_init`, `armada375_wdt_clock_init`, `armadaxp_wdt_clock_init`).
--

commit 2de00d567f4d73cf286aa9c7120abab320758a10
Author: Rosen Penev <rosenp@gmail.com>

watchdog: orion_wdt: Use devm APIs for clock and watchdog management

Replace clk_get/clk_prepare_enable/clk_put with devm_clk_get_enabled
and devm_clk_get_optional_enabled so the clock lifecycle is managed
automatically. Switch to devm_watchdog_register_device to eliminate
the manual remove callback and the disable_clk error path.

> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 0e145f762f6f2..b81ba2607c6d5 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -81,14 +81,9 @@ static int orion_wdt_clock_init(struct platform_device *pdev,
>  {
>  	int ret;
>  
> -	dev->clk = clk_get(&pdev->dev, NULL);
> +	dev->clk = devm_clk_get_enabled(&pdev->dev, NULL);
>  	if (IS_ERR(dev->clk))
>  		return PTR_ERR(dev->clk);
> -	ret = clk_prepare_enable(dev->clk);
> -	if (ret) {
> -		clk_put(dev->clk);
> -		return ret;
> -	}
>  
>  	dev->clk_rate = clk_get_rate(dev->clk);
>  	return 0;

[Severity: Low]
This isn't a bug, but with the removal of clk_prepare_enable(), the local
variable ret is no longer used in this function. Should the declaration be
removed to prevent compiler warnings?

[ ... ]

> @@ -99,14 +94,9 @@ static int armada370_wdt_clock_init(struct platform_device *pdev,
>  {
>  	int ret;
>  
> -	dev->clk = clk_get(&pdev->dev, NULL);
> +	dev->clk = devm_clk_get_enabled(&pdev->dev, NULL);
>  	if (IS_ERR(dev->clk))
>  		return PTR_ERR(dev->clk);
> -	ret = clk_prepare_enable(dev->clk);
> -	if (ret) {
> -		clk_put(dev->clk);
> -		return ret;
> -	}
>  
>  	/* Setup watchdog input clock */

[Severity: Low]
This isn't a bug, but ret is also left unused in armada370_wdt_clock_init()
after these changes.

[ ... ]

> @@ -122,14 +112,11 @@ static int armada375_wdt_clock_init(struct platform_device *pdev,
>  {
>  	int ret;
>  
> -	dev->clk = of_clk_get_by_name(pdev->dev.of_node, "fixed");
> -	if (!IS_ERR(dev->clk)) {
> -		ret = clk_prepare_enable(dev->clk);
> -		if (ret) {
> -			clk_put(dev->clk);
> -			return ret;
> -		}
> +	dev->clk = devm_clk_get_optional_enabled(&pdev->dev, "fixed");
> +	if (IS_ERR(dev->clk))
> +		return PTR_ERR(dev->clk);

[Severity: Low]
This isn't a bug, but ret is also left unused in armada375_wdt_clock_init()
after these changes.

[ ... ]

> @@ -163,14 +144,9 @@ static int armadaxp_wdt_clock_init(struct platform_device *pdev,
>  	int ret;
>  	u32 val;
>  
> -	dev->clk = of_clk_get_by_name(pdev->dev.of_node, "fixed");
> +	dev->clk = devm_clk_get_enabled(&pdev->dev, "fixed");
>  	if (IS_ERR(dev->clk))
>  		return PTR_ERR(dev->clk);
> -	ret = clk_prepare_enable(dev->clk);
> -	if (ret) {
> -		clk_put(dev->clk);
> -		return ret;
> -	}
>  
>  	/* Fix the wdt and timer1 clock frequency to 25MHz */
>  	val = WDT_AXP_FIXED_ENABLE_BIT | TIMER1_FIXED_ENABLE_BIT;

[Severity: Low]
This isn't a bug, but ret is also left unused in armadaxp_wdt_clock_init()
after these changes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519214229.16656-1-rosenp@gmail.com?part=1