drivers/watchdog/orion_wdt.c | 64 +++++++----------------------------- 1 file changed, 12 insertions(+), 52 deletions(-)
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.
Switching to devm in these functions is fine as the proper
platform_device is passed in.
Assisted-by: Claude:Sonnet-4.6
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/watchdog/orion_wdt.c | 64 +++++++-----------------------------
1 file changed, 12 insertions(+), 52 deletions(-)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index a92701ff2653..85e9877de952 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -80,14 +80,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;
@@ -98,14 +93,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 */
atomic_io_modify(dev->reg + TIMER_CTRL,
@@ -121,14 +111,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);
+ if (dev->clk) {
atomic_io_modify(dev->reg + TIMER_CTRL,
WDT_AXP_FIXED_ENABLE_BIT,
WDT_AXP_FIXED_ENABLE_BIT);
@@ -138,16 +125,10 @@ static int armada375_wdt_clock_init(struct platform_device *pdev,
}
/* Mandatory fallback for proper devicetree backward compatibility */
- 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;
- }
-
atomic_io_modify(dev->reg + TIMER_CTRL,
WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT),
WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT));
@@ -162,14 +143,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;
@@ -612,7 +588,7 @@ static int orion_wdt_probe(struct platform_device *pdev)
pdev->name, dev);
if (ret < 0) {
dev_err(&pdev->dev, "failed to request IRQ\n");
- goto disable_clk;
+ return ret;
}
}
@@ -624,34 +600,19 @@ static int orion_wdt_probe(struct platform_device *pdev)
0, pdev->name, dev);
if (ret < 0) {
dev_err(&pdev->dev, "failed to request IRQ\n");
- goto disable_clk;
+ return ret;
}
}
watchdog_set_nowayout(&dev->wdt, nowayout);
- ret = watchdog_register_device(&dev->wdt);
+ ret = devm_watchdog_register_device(&pdev->dev, &dev->wdt);
if (ret)
- goto disable_clk;
+ return ret;
pr_info("Initial timeout %d sec%s\n",
dev->wdt.timeout, nowayout ? ", nowayout" : "");
return 0;
-
-disable_clk:
- clk_disable_unprepare(dev->clk);
- clk_put(dev->clk);
- return ret;
-}
-
-static void orion_wdt_remove(struct platform_device *pdev)
-{
- struct watchdog_device *wdt_dev = platform_get_drvdata(pdev);
- struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
-
- watchdog_unregister_device(wdt_dev);
- clk_disable_unprepare(dev->clk);
- clk_put(dev->clk);
}
static void orion_wdt_shutdown(struct platform_device *pdev)
@@ -662,7 +623,6 @@ static void orion_wdt_shutdown(struct platform_device *pdev)
static struct platform_driver orion_wdt_driver = {
.probe = orion_wdt_probe,
- .remove = orion_wdt_remove,
.shutdown = orion_wdt_shutdown,
.driver = {
.name = "orion_wdt",
--
2.54.0
Hi Rosen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on groeck-staging/hwmon]
[also build test WARNING on groeck-staging/hwmon-next groeck-staging/watchdog groeck-staging/watchdog-next linus/master v7.1-rc4 next-20260519]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Rosen-Penev/watchdog-orion_wdt-Use-devm-APIs-for-clock-and-watchdog-management/20260520-054408
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon
patch link: https://lore.kernel.org/r/20260519214229.16656-1-rosenp%40gmail.com
patch subject: [PATCH] watchdog: orion_wdt: Use devm APIs for clock and watchdog management
config: arm-dove_defconfig (https://download.01.org/0day-ci/archive/20260520/202605201246.gIDxpcj6-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260520/202605201246.gIDxpcj6-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605201246.gIDxpcj6-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/watchdog/orion_wdt.c: In function 'orion_wdt_clock_init':
>> drivers/watchdog/orion_wdt.c:82:13: warning: unused variable 'ret' [-Wunused-variable]
82 | int ret;
| ^~~
drivers/watchdog/orion_wdt.c: In function 'armada370_wdt_clock_init':
drivers/watchdog/orion_wdt.c:95:13: warning: unused variable 'ret' [-Wunused-variable]
95 | int ret;
| ^~~
drivers/watchdog/orion_wdt.c: In function 'armada375_wdt_clock_init':
drivers/watchdog/orion_wdt.c:113:13: warning: unused variable 'ret' [-Wunused-variable]
113 | int ret;
| ^~~
drivers/watchdog/orion_wdt.c: In function 'armadaxp_wdt_clock_init':
drivers/watchdog/orion_wdt.c:144:13: warning: unused variable 'ret' [-Wunused-variable]
144 | int ret;
| ^~~
vim +/ret +82 drivers/watchdog/orion_wdt.c
22ac92322c83334 drivers/watchdog/orion5x_wdt.c Sylver Bruneau 2008-06-26 78
1924227bcda1d18 drivers/watchdog/orion_wdt.c Ezequiel Garcia 2014-02-10 79 static int orion_wdt_clock_init(struct platform_device *pdev,
1924227bcda1d18 drivers/watchdog/orion_wdt.c Ezequiel Garcia 2014-02-10 80 struct orion_watchdog *dev)
df6707b21904950 drivers/watchdog/orion5x_wdt.c Thomas Reitmayr 2009-02-20 81 {
1924227bcda1d18 drivers/watchdog/orion_wdt.c Ezequiel Garcia 2014-02-10 @82 int ret;
df6707b21904950 drivers/watchdog/orion5x_wdt.c Thomas Reitmayr 2009-02-20 83
df5a672cef35a9e drivers/watchdog/orion_wdt.c Rosen Penev 2026-05-19 84 dev->clk = devm_clk_get_enabled(&pdev->dev, NULL);
1924227bcda1d18 drivers/watchdog/orion_wdt.c Ezequiel Garcia 2014-02-10 85 if (IS_ERR(dev->clk))
1924227bcda1d18 drivers/watchdog/orion_wdt.c Ezequiel Garcia 2014-02-10 86 return PTR_ERR(dev->clk);
df6707b21904950 drivers/watchdog/orion5x_wdt.c Thomas Reitmayr 2009-02-20 87
463f96e0cdacce5 drivers/watchdog/orion_wdt.c Ezequiel Garcia 2014-02-10 88 dev->clk_rate = clk_get_rate(dev->clk);
0dd6e4847ed8a42 drivers/watchdog/orion_wdt.c Axel Lin 2012-03-26 89 return 0;
df6707b21904950 drivers/watchdog/orion5x_wdt.c Thomas Reitmayr 2009-02-20 90 }
df6707b21904950 drivers/watchdog/orion5x_wdt.c Thomas Reitmayr 2009-02-20 91
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Tue, May 19, 2026 at 02:42:29PM -0700, Rosen Penev wrote:
> 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.
>
> Switching to devm in these functions is fine as the proper
> platform_device is passed in.
>
> Assisted-by: Claude:Sonnet-4.6
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
drivers/watchdog/orion_wdt.c: In function 'orion_wdt_clock_init':
drivers/watchdog/orion_wdt.c:82:13: error: unused variable 'ret' [-Werror=unused-variable]
82 | int ret;
| ^~~
drivers/watchdog/orion_wdt.c: In function 'armada370_wdt_clock_init':
drivers/watchdog/orion_wdt.c:95:13: error: unused variable 'ret' [-Werror=unused-variable]
95 | int ret;
| ^~~
drivers/watchdog/orion_wdt.c: In function 'armada375_wdt_clock_init':
drivers/watchdog/orion_wdt.c:113:13: error: unused variable 'ret' [-Werror=unused-variable]
113 | int ret;
| ^~~
drivers/watchdog/orion_wdt.c: In function 'armadaxp_wdt_clock_init':
drivers/watchdog/orion_wdt.c:144:13: error: unused variable 'ret' [-Werror=unused-variable]
144 | int ret;
On top of that, the APIs are not 1:1 replaceable.
I am so tired of this. Please stop. I am no longer going to accept any "cleanup"
patches.
Guenter
On Tue, May 19, 2026 at 3:34 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On Tue, May 19, 2026 at 02:42:29PM -0700, Rosen Penev wrote: > > 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. > > > > Switching to devm in these functions is fine as the proper > > platform_device is passed in. > > > > Assisted-by: Claude:Sonnet-4.6 > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > > drivers/watchdog/orion_wdt.c: In function 'orion_wdt_clock_init': > drivers/watchdog/orion_wdt.c:82:13: error: unused variable 'ret' [-Werror=unused-variable] > 82 | int ret; > | ^~~ > drivers/watchdog/orion_wdt.c: In function 'armada370_wdt_clock_init': > drivers/watchdog/orion_wdt.c:95:13: error: unused variable 'ret' [-Werror=unused-variable] > 95 | int ret; > | ^~~ > drivers/watchdog/orion_wdt.c: In function 'armada375_wdt_clock_init': > drivers/watchdog/orion_wdt.c:113:13: error: unused variable 'ret' [-Werror=unused-variable] > 113 | int ret; > | ^~~ > drivers/watchdog/orion_wdt.c: In function 'armadaxp_wdt_clock_init': > drivers/watchdog/orion_wdt.c:144:13: error: unused variable 'ret' [-Werror=unused-variable] > 144 | int ret; > > On top of that, the APIs are not 1:1 replaceable. > > I am so tired of this. Please stop. I am no longer going to accept any "cleanup" > patches. If I don't, the bot yells at me. If I do, you yell at me. It seems I am at an impasse. > > Guenter
On 5/19/26 15:44, Rosen Penev wrote: > On Tue, May 19, 2026 at 3:34 PM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On Tue, May 19, 2026 at 02:42:29PM -0700, Rosen Penev wrote: >>> 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. >>> >>> Switching to devm in these functions is fine as the proper >>> platform_device is passed in. >>> >>> Assisted-by: Claude:Sonnet-4.6 >>> Signed-off-by: Rosen Penev <rosenp@gmail.com> >> >> drivers/watchdog/orion_wdt.c: In function 'orion_wdt_clock_init': >> drivers/watchdog/orion_wdt.c:82:13: error: unused variable 'ret' [-Werror=unused-variable] >> 82 | int ret; >> | ^~~ >> drivers/watchdog/orion_wdt.c: In function 'armada370_wdt_clock_init': >> drivers/watchdog/orion_wdt.c:95:13: error: unused variable 'ret' [-Werror=unused-variable] >> 95 | int ret; >> | ^~~ >> drivers/watchdog/orion_wdt.c: In function 'armada375_wdt_clock_init': >> drivers/watchdog/orion_wdt.c:113:13: error: unused variable 'ret' [-Werror=unused-variable] >> 113 | int ret; >> | ^~~ >> drivers/watchdog/orion_wdt.c: In function 'armadaxp_wdt_clock_init': >> drivers/watchdog/orion_wdt.c:144:13: error: unused variable 'ret' [-Werror=unused-variable] >> 144 | int ret; >> >> On top of that, the APIs are not 1:1 replaceable. >> >> I am so tired of this. Please stop. I am no longer going to accept any "cleanup" >> patches. > If I don't, the bot yells at me. If I do, you yell at me. It seems I > am at an impasse. The above is the compiler yelling at you. Guenter
On Tue, May 19, 2026 at 4:10 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 5/19/26 15:44, Rosen Penev wrote: > > On Tue, May 19, 2026 at 3:34 PM Guenter Roeck <linux@roeck-us.net> wrote: > >> > >> On Tue, May 19, 2026 at 02:42:29PM -0700, Rosen Penev wrote: > >>> 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. > >>> > >>> Switching to devm in these functions is fine as the proper > >>> platform_device is passed in. > >>> > >>> Assisted-by: Claude:Sonnet-4.6 > >>> Signed-off-by: Rosen Penev <rosenp@gmail.com> > >> > >> drivers/watchdog/orion_wdt.c: In function 'orion_wdt_clock_init': > >> drivers/watchdog/orion_wdt.c:82:13: error: unused variable 'ret' [-Werror=unused-variable] > >> 82 | int ret; > >> | ^~~ > >> drivers/watchdog/orion_wdt.c: In function 'armada370_wdt_clock_init': > >> drivers/watchdog/orion_wdt.c:95:13: error: unused variable 'ret' [-Werror=unused-variable] > >> 95 | int ret; > >> | ^~~ > >> drivers/watchdog/orion_wdt.c: In function 'armada375_wdt_clock_init': > >> drivers/watchdog/orion_wdt.c:113:13: error: unused variable 'ret' [-Werror=unused-variable] > >> 113 | int ret; > >> | ^~~ > >> drivers/watchdog/orion_wdt.c: In function 'armadaxp_wdt_clock_init': > >> drivers/watchdog/orion_wdt.c:144:13: error: unused variable 'ret' [-Werror=unused-variable] > >> 144 | int ret; > >> > >> On top of that, the APIs are not 1:1 replaceable. > >> > >> I am so tired of this. Please stop. I am no longer going to accept any "cleanup" > >> patches. > > If I don't, the bot yells at me. If I do, you yell at me. It seems I > > am at an impasse. > > The above is the compiler yelling at you. No I mean my watchdog: orion: Use of_device_get_match_data() patch resulted in the AI complaining about clock code that I didn't touch. The above errors are me compiling for powerpc when there's a depend on ARM. > Guenter >
On 5/19/26 16:19, Rosen Penev wrote: > On Tue, May 19, 2026 at 4:10 PM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 5/19/26 15:44, Rosen Penev wrote: >>> On Tue, May 19, 2026 at 3:34 PM Guenter Roeck <linux@roeck-us.net> wrote: >>>> >>>> On Tue, May 19, 2026 at 02:42:29PM -0700, Rosen Penev wrote: >>>>> 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. >>>>> >>>>> Switching to devm in these functions is fine as the proper >>>>> platform_device is passed in. >>>>> >>>>> Assisted-by: Claude:Sonnet-4.6 >>>>> Signed-off-by: Rosen Penev <rosenp@gmail.com> >>>> >>>> drivers/watchdog/orion_wdt.c: In function 'orion_wdt_clock_init': >>>> drivers/watchdog/orion_wdt.c:82:13: error: unused variable 'ret' [-Werror=unused-variable] >>>> 82 | int ret; >>>> | ^~~ >>>> drivers/watchdog/orion_wdt.c: In function 'armada370_wdt_clock_init': >>>> drivers/watchdog/orion_wdt.c:95:13: error: unused variable 'ret' [-Werror=unused-variable] >>>> 95 | int ret; >>>> | ^~~ >>>> drivers/watchdog/orion_wdt.c: In function 'armada375_wdt_clock_init': >>>> drivers/watchdog/orion_wdt.c:113:13: error: unused variable 'ret' [-Werror=unused-variable] >>>> 113 | int ret; >>>> | ^~~ >>>> drivers/watchdog/orion_wdt.c: In function 'armadaxp_wdt_clock_init': >>>> drivers/watchdog/orion_wdt.c:144:13: error: unused variable 'ret' [-Werror=unused-variable] >>>> 144 | int ret; >>>> >>>> On top of that, the APIs are not 1:1 replaceable. >>>> >>>> I am so tired of this. Please stop. I am no longer going to accept any "cleanup" >>>> patches. >>> If I don't, the bot yells at me. If I do, you yell at me. It seems I >>> am at an impasse. >> >> The above is the compiler yelling at you. > No I mean my watchdog: orion: Use of_device_get_match_data() patch > resulted in the AI complaining about clock code that I didn't touch. > Problems with your patch: - It was not identified as bug fix - It explains what was changed, but not the underlying reason. "so the clock lifecycle is managed automatically" is not an explanation and does not explain _why_ the change is necessary. - It has no Reported-by: tag - It does not compile - The APIs are not 1:1 replaceable. At the very least, replacing of_clk_get_by_name with devm_clk_get_optional_enabled() in one function and with devm_clk_get_enabled() in another is very suspicious and asks for a detailed explanation. - Given that you tried to compile it on ppc very much suggests that it wasn't tested, and the limited test status was not mentioned in the patch (not that testing would be easy, given the different means used to get the clock). In such a situation it is much better to respond to the agent e-mail and tell it (and me) that you understand that there is an unrelated problem, but that you don't have the knowledge/understanding of the driver and its requirements to submit a fix. Guenter
© 2016 - 2026 Red Hat, Inc.