* [PATCH 1/3] watchdog: iTCO_wdt: use module_platform_device() macro @ 2020-11-17 15:22 Enrico Weigelt, metux IT consult 2020-11-17 15:22 ` [PATCH 2/3] watchdog: iTCO_wdt: use dev_*() instead of pr_*() for logging Enrico Weigelt, metux IT consult 2020-11-17 15:22 ` [PATCH 3/3] watchdog: iTCO_wdt: use pm_ptr() Enrico Weigelt, metux IT consult 0 siblings, 2 replies; 8+ messages in thread From: Enrico Weigelt, metux IT consult @ 2020-11-17 15:22 UTC (permalink / raw) To: linux-kernel; +Cc: wim, linux, linux-watchdog Reducing init boilerplate by using the module_platform_device macro. Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net> --- drivers/watchdog/iTCO_wdt.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c index a370a185a41c..f2ddc8fc71cd 100644 --- a/drivers/watchdog/iTCO_wdt.c +++ b/drivers/watchdog/iTCO_wdt.c @@ -651,21 +651,7 @@ static struct platform_driver iTCO_wdt_driver = { }, }; -static int __init iTCO_wdt_init_module(void) -{ - pr_info("Intel TCO WatchDog Timer Driver v%s\n", DRV_VERSION); - - return platform_driver_register(&iTCO_wdt_driver); -} - -static void __exit iTCO_wdt_cleanup_module(void) -{ - platform_driver_unregister(&iTCO_wdt_driver); - pr_info("Watchdog Module Unloaded\n"); -} - -module_init(iTCO_wdt_init_module); -module_exit(iTCO_wdt_cleanup_module); +module_platform_driver(iTCO_wdt_driver); MODULE_AUTHOR("Wim Van Sebroeck <wim@iguana.be>"); MODULE_DESCRIPTION("Intel TCO WatchDog Timer Driver"); -- 2.11.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] watchdog: iTCO_wdt: use dev_*() instead of pr_*() for logging 2020-11-17 15:22 [PATCH 1/3] watchdog: iTCO_wdt: use module_platform_device() macro Enrico Weigelt, metux IT consult @ 2020-11-17 15:22 ` Enrico Weigelt, metux IT consult 2021-06-09 20:32 ` Bjorn Helgaas 2020-11-17 15:22 ` [PATCH 3/3] watchdog: iTCO_wdt: use pm_ptr() Enrico Weigelt, metux IT consult 1 sibling, 1 reply; 8+ messages in thread From: Enrico Weigelt, metux IT consult @ 2020-11-17 15:22 UTC (permalink / raw) To: linux-kernel; +Cc: wim, linux, linux-watchdog For device log outputs, it's better to have device name / ID prefixed in all messages, so use the proper dev_*() functions here. Explicit message on module load/unload don't seem to be really helpful (we have other means to check which modules have been loaded), instead just add noise to the kernel log. So, removing them. Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net> --- drivers/watchdog/iTCO_wdt.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c index f2ddc8fc71cd..edc588a06ae6 100644 --- a/drivers/watchdog/iTCO_wdt.c +++ b/drivers/watchdog/iTCO_wdt.c @@ -40,8 +40,6 @@ * Includes, defines, variables, module parameters, ... */ -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt - /* Module and version information */ #define DRV_NAME "iTCO_wdt" #define DRV_VERSION "1.11" @@ -279,7 +277,7 @@ static int iTCO_wdt_start(struct watchdog_device *wd_dev) /* disable chipset's NO_REBOOT bit */ if (p->update_no_reboot_bit(p->no_reboot_priv, false)) { spin_unlock(&p->io_lock); - pr_err("failed to reset NO_REBOOT flag, reboot disabled by hardware/BIOS\n"); + dev_err(wd_dev->dev, "failed to reset NO_REBOOT flag, reboot disabled by hardware/BIOS\n"); return -EIO; } @@ -510,7 +508,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev) /* Check chipset's NO_REBOOT bit */ if (p->update_no_reboot_bit(p->no_reboot_priv, false) && iTCO_vendor_check_noreboot_on()) { - pr_info("unable to reset NO_REBOOT flag, device disabled by hardware/BIOS\n"); + dev_info(dev, "unable to reset NO_REBOOT flag, device disabled by hardware/BIOS\n"); return -ENODEV; /* Cannot reset NO_REBOOT bit */ } @@ -530,12 +528,12 @@ static int iTCO_wdt_probe(struct platform_device *pdev) if (!devm_request_region(dev, p->tco_res->start, resource_size(p->tco_res), pdev->name)) { - pr_err("I/O address 0x%04llx already in use, device disabled\n", + dev_err(dev, "I/O address 0x%04llx already in use, device disabled\n", (u64)TCOBASE(p)); return -EBUSY; } - pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n", + dev_info(dev, "Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n", pdata->name, pdata->version, (u64)TCOBASE(p)); /* Clear out the (probably old) status */ @@ -558,7 +556,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev) break; } - p->wddev.info = &ident, + p->wddev.info = &ident, p->wddev.ops = &iTCO_wdt_ops, p->wddev.bootstatus = 0; p->wddev.timeout = WATCHDOG_TIMEOUT; @@ -575,7 +573,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev) if not reset to the default */ if (iTCO_wdt_set_timeout(&p->wddev, heartbeat)) { iTCO_wdt_set_timeout(&p->wddev, WATCHDOG_TIMEOUT); - pr_info("timeout value out of range, using %d\n", + dev_info(dev, "timeout value out of range, using %d\n", WATCHDOG_TIMEOUT); } @@ -583,11 +581,11 @@ static int iTCO_wdt_probe(struct platform_device *pdev) watchdog_stop_on_unregister(&p->wddev); ret = devm_watchdog_register_device(dev, &p->wddev); if (ret != 0) { - pr_err("cannot register watchdog device (err=%d)\n", ret); + dev_err(dev, "cannot register watchdog device (err=%d)\n", ret); return ret; } - pr_info("initialized. heartbeat=%d sec (nowayout=%d)\n", + dev_info(dev, "initialized. heartbeat=%d sec (nowayout=%d)\n", heartbeat, nowayout); return 0; -- 2.11.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] watchdog: iTCO_wdt: use dev_*() instead of pr_*() for logging 2020-11-17 15:22 ` [PATCH 2/3] watchdog: iTCO_wdt: use dev_*() instead of pr_*() for logging Enrico Weigelt, metux IT consult @ 2021-06-09 20:32 ` Bjorn Helgaas 2021-06-09 23:54 ` Guenter Roeck 0 siblings, 1 reply; 8+ messages in thread From: Bjorn Helgaas @ 2021-06-09 20:32 UTC (permalink / raw) To: Enrico Weigelt, metux IT consult; +Cc: linux-kernel, wim, linux, linux-watchdog On Tue, Nov 17, 2020 at 04:22:13PM +0100, Enrico Weigelt, metux IT consult wrote: > For device log outputs, it's better to have device name / ID > prefixed in all messages, so use the proper dev_*() functions here. > > Explicit message on module load/unload don't seem to be really helpful > (we have other means to check which modules have been loaded), instead > just add noise to the kernel log. So, removing them. > > Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net> I like this patch a lot; thanks for doing it! It's merged upstream as c21172b3a73e ("watchdog: iTCO_wdt: use dev_*() instead of pr_*() for logging"). It looks like there are a couple more pr_err() uses, so I wondered if they were missed or skipped intentionally: if (p->smi_res) { /* The TCO logic uses the TCO_EN bit in the SMI_EN register */ if (!devm_request_region(dev, p->smi_res->start, resource_size(p->smi_res), pdev->name)) { pr_err("I/O address 0x%04llx already in use, device disabled\n", (u64)SMI_EN(p)); return -EBUSY; } } else if (iTCO_vendorsupport || turn_SMI_watchdog_clear_off >= p->iTCO_version) { pr_err("SMI I/O resource is missing\n"); return -ENODEV; } > --- > drivers/watchdog/iTCO_wdt.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c > index f2ddc8fc71cd..edc588a06ae6 100644 > --- a/drivers/watchdog/iTCO_wdt.c > +++ b/drivers/watchdog/iTCO_wdt.c > @@ -40,8 +40,6 @@ > * Includes, defines, variables, module parameters, ... > */ > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > - > /* Module and version information */ > #define DRV_NAME "iTCO_wdt" > #define DRV_VERSION "1.11" > @@ -279,7 +277,7 @@ static int iTCO_wdt_start(struct watchdog_device *wd_dev) > /* disable chipset's NO_REBOOT bit */ > if (p->update_no_reboot_bit(p->no_reboot_priv, false)) { > spin_unlock(&p->io_lock); > - pr_err("failed to reset NO_REBOOT flag, reboot disabled by hardware/BIOS\n"); > + dev_err(wd_dev->dev, "failed to reset NO_REBOOT flag, reboot disabled by hardware/BIOS\n"); > return -EIO; > } > > @@ -510,7 +508,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev) > /* Check chipset's NO_REBOOT bit */ > if (p->update_no_reboot_bit(p->no_reboot_priv, false) && > iTCO_vendor_check_noreboot_on()) { > - pr_info("unable to reset NO_REBOOT flag, device disabled by hardware/BIOS\n"); > + dev_info(dev, "unable to reset NO_REBOOT flag, device disabled by hardware/BIOS\n"); > return -ENODEV; /* Cannot reset NO_REBOOT bit */ > } > > @@ -530,12 +528,12 @@ static int iTCO_wdt_probe(struct platform_device *pdev) > if (!devm_request_region(dev, p->tco_res->start, > resource_size(p->tco_res), > pdev->name)) { > - pr_err("I/O address 0x%04llx already in use, device disabled\n", > + dev_err(dev, "I/O address 0x%04llx already in use, device disabled\n", > (u64)TCOBASE(p)); > return -EBUSY; > } > > - pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n", > + dev_info(dev, "Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n", > pdata->name, pdata->version, (u64)TCOBASE(p)); > > /* Clear out the (probably old) status */ > @@ -558,7 +556,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev) > break; > } > > - p->wddev.info = &ident, > + p->wddev.info = &ident, > p->wddev.ops = &iTCO_wdt_ops, > p->wddev.bootstatus = 0; > p->wddev.timeout = WATCHDOG_TIMEOUT; > @@ -575,7 +573,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev) > if not reset to the default */ > if (iTCO_wdt_set_timeout(&p->wddev, heartbeat)) { > iTCO_wdt_set_timeout(&p->wddev, WATCHDOG_TIMEOUT); > - pr_info("timeout value out of range, using %d\n", > + dev_info(dev, "timeout value out of range, using %d\n", > WATCHDOG_TIMEOUT); > } > > @@ -583,11 +581,11 @@ static int iTCO_wdt_probe(struct platform_device *pdev) > watchdog_stop_on_unregister(&p->wddev); > ret = devm_watchdog_register_device(dev, &p->wddev); > if (ret != 0) { > - pr_err("cannot register watchdog device (err=%d)\n", ret); > + dev_err(dev, "cannot register watchdog device (err=%d)\n", ret); > return ret; > } > > - pr_info("initialized. heartbeat=%d sec (nowayout=%d)\n", > + dev_info(dev, "initialized. heartbeat=%d sec (nowayout=%d)\n", > heartbeat, nowayout); > > return 0; > -- > 2.11.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] watchdog: iTCO_wdt: use dev_*() instead of pr_*() for logging 2021-06-09 20:32 ` Bjorn Helgaas @ 2021-06-09 23:54 ` Guenter Roeck 2021-06-14 17:55 ` Enrico Weigelt, metux IT consult 0 siblings, 1 reply; 8+ messages in thread From: Guenter Roeck @ 2021-06-09 23:54 UTC (permalink / raw) To: Bjorn Helgaas Cc: Enrico Weigelt, metux IT consult, linux-kernel, wim, linux-watchdog On Wed, Jun 09, 2021 at 03:32:29PM -0500, Bjorn Helgaas wrote: > On Tue, Nov 17, 2020 at 04:22:13PM +0100, Enrico Weigelt, metux IT consult wrote: > > For device log outputs, it's better to have device name / ID > > prefixed in all messages, so use the proper dev_*() functions here. > > > > Explicit message on module load/unload don't seem to be really helpful > > (we have other means to check which modules have been loaded), instead > > just add noise to the kernel log. So, removing them. > > > > Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net> > > I like this patch a lot; thanks for doing it! It's merged upstream as > c21172b3a73e ("watchdog: iTCO_wdt: use dev_*() instead of pr_*() for > logging"). > > It looks like there are a couple more pr_err() uses, so I wondered if > they were missed or skipped intentionally: > > if (p->smi_res) { > /* The TCO logic uses the TCO_EN bit in the SMI_EN register */ > if (!devm_request_region(dev, p->smi_res->start, > resource_size(p->smi_res), > pdev->name)) { > pr_err("I/O address 0x%04llx already in use, device disabled\n", > (u64)SMI_EN(p)); > return -EBUSY; > } > } else if (iTCO_vendorsupport || > turn_SMI_watchdog_clear_off >= p->iTCO_version) { > pr_err("SMI I/O resource is missing\n"); > return -ENODEV; > } > The above came in with a recent commit. I suspect they simply got lost, and they should be converted as well. Guenter ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] watchdog: iTCO_wdt: use dev_*() instead of pr_*() for logging 2021-06-09 23:54 ` Guenter Roeck @ 2021-06-14 17:55 ` Enrico Weigelt, metux IT consult 2021-06-15 14:51 ` Bjorn Helgaas 0 siblings, 1 reply; 8+ messages in thread From: Enrico Weigelt, metux IT consult @ 2021-06-14 17:55 UTC (permalink / raw) To: Guenter Roeck, Bjorn Helgaas Cc: Enrico Weigelt, metux IT consult, linux-kernel, wim, linux-watchdog On 10.06.21 01:54, Guenter Roeck wrote: > The above came in with a recent commit. I suspect they simply got lost, > and they should be converted as well. Since I might need to touch it again in near future, I'll do it in one of the next rounds. --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] watchdog: iTCO_wdt: use dev_*() instead of pr_*() for logging 2021-06-14 17:55 ` Enrico Weigelt, metux IT consult @ 2021-06-15 14:51 ` Bjorn Helgaas 0 siblings, 0 replies; 8+ messages in thread From: Bjorn Helgaas @ 2021-06-15 14:51 UTC (permalink / raw) To: Enrico Weigelt, metux IT consult Cc: Guenter Roeck, Enrico Weigelt, metux IT consult, linux-kernel, wim, linux-watchdog On Mon, Jun 14, 2021 at 07:55:24PM +0200, Enrico Weigelt, metux IT consult wrote: > On 10.06.21 01:54, Guenter Roeck wrote: > > > The above came in with a recent commit. I suspect they simply got lost, > > and they should be converted as well. > > Since I might need to touch it again in near future, I'll do it in > one of the next rounds. Thanks! ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] watchdog: iTCO_wdt: use pm_ptr() 2020-11-17 15:22 [PATCH 1/3] watchdog: iTCO_wdt: use module_platform_device() macro Enrico Weigelt, metux IT consult 2020-11-17 15:22 ` [PATCH 2/3] watchdog: iTCO_wdt: use dev_*() instead of pr_*() for logging Enrico Weigelt, metux IT consult @ 2020-11-17 15:22 ` Enrico Weigelt, metux IT consult 2020-11-30 22:17 ` Guenter Roeck 1 sibling, 1 reply; 8+ messages in thread From: Enrico Weigelt, metux IT consult @ 2020-11-17 15:22 UTC (permalink / raw) To: linux-kernel; +Cc: wim, linux, linux-watchdog Reduce a little bit of boilerplate by using pm_ptr(). Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net> --- drivers/watchdog/iTCO_wdt.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c index edc588a06ae6..660fc262fbc0 100644 --- a/drivers/watchdog/iTCO_wdt.c +++ b/drivers/watchdog/iTCO_wdt.c @@ -636,16 +636,13 @@ static const struct dev_pm_ops iTCO_wdt_pm = { .resume_noirq = iTCO_wdt_resume_noirq, }; -#define ITCO_WDT_PM_OPS (&iTCO_wdt_pm) -#else -#define ITCO_WDT_PM_OPS NULL #endif /* CONFIG_PM_SLEEP */ static struct platform_driver iTCO_wdt_driver = { .probe = iTCO_wdt_probe, .driver = { .name = DRV_NAME, - .pm = ITCO_WDT_PM_OPS, + .pm = pm_ptr(&iTCO_wdt_pm), }, }; -- 2.11.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] watchdog: iTCO_wdt: use pm_ptr() 2020-11-17 15:22 ` [PATCH 3/3] watchdog: iTCO_wdt: use pm_ptr() Enrico Weigelt, metux IT consult @ 2020-11-30 22:17 ` Guenter Roeck 0 siblings, 0 replies; 8+ messages in thread From: Guenter Roeck @ 2020-11-30 22:17 UTC (permalink / raw) To: Enrico Weigelt, metux IT consult; +Cc: linux-kernel, wim, linux-watchdog On Tue, Nov 17, 2020 at 04:22:14PM +0100, Enrico Weigelt, metux IT consult wrote: > Reduce a little bit of boilerplate by using pm_ptr(). > > Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net> > --- > drivers/watchdog/iTCO_wdt.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c > index edc588a06ae6..660fc262fbc0 100644 > --- a/drivers/watchdog/iTCO_wdt.c > +++ b/drivers/watchdog/iTCO_wdt.c > @@ -636,16 +636,13 @@ static const struct dev_pm_ops iTCO_wdt_pm = { > .resume_noirq = iTCO_wdt_resume_noirq, > }; > > -#define ITCO_WDT_PM_OPS (&iTCO_wdt_pm) > -#else > -#define ITCO_WDT_PM_OPS NULL > #endif /* CONFIG_PM_SLEEP */ > > static struct platform_driver iTCO_wdt_driver = { > .probe = iTCO_wdt_probe, > .driver = { > .name = DRV_NAME, > - .pm = ITCO_WDT_PM_OPS, > + .pm = pm_ptr(&iTCO_wdt_pm), Not sure if this works. The ifdef is for CONFIG_PM_SLEEP, but pm_ptr depends on CONFIG_PM. While PM_SLEEP selects PM, PM is also selected by other means. Other drivers using pm_ptr also use SIMPLE_DEV_PM_OPS() and __maybe_unused for the function declarations. This is not the case here. With this in mind, I'd rather not make this change without changing the rest of the code as well to ensure consistency. Thanks, Guenter > }, > }; > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-06-15 14:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-17 15:22 [PATCH 1/3] watchdog: iTCO_wdt: use module_platform_device() macro Enrico Weigelt, metux IT consult 2020-11-17 15:22 ` [PATCH 2/3] watchdog: iTCO_wdt: use dev_*() instead of pr_*() for logging Enrico Weigelt, metux IT consult 2021-06-09 20:32 ` Bjorn Helgaas 2021-06-09 23:54 ` Guenter Roeck 2021-06-14 17:55 ` Enrico Weigelt, metux IT consult 2021-06-15 14:51 ` Bjorn Helgaas 2020-11-17 15:22 ` [PATCH 3/3] watchdog: iTCO_wdt: use pm_ptr() Enrico Weigelt, metux IT consult 2020-11-30 22:17 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).