On Wed, Oct 11, 2017 at 06:46:31PM +0100, Radu Rendec wrote:
> Hello,
>
> In a project I'm working on we have a valid use case where we activate
> both the i6300esb and softdog watchdogs. We always activate i6300esb
> first (which uses the "legacy" watchdog API) and then softdog. This
> gets us two "error" level messages (coming from watchdog_cdev_register)
> although softdog falls back to the "new" API and registers its char
> device just fine.
>
> Since watchdog_cdev_register/watchdog_dev_register seem to be used only
> by watchdog_register_device and the latter always falls back to the
> "new" API, I'm thinking about lowering the log level of these messages
> when err is -EBUSY. Something along the lines of:
>
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -928,11 +928,14 @@ static int watchdog_cdev_register(struct
> watchdog_device *wdd, dev_t devno)
> watchdog_miscdev.parent = wdd->parent;
> err = misc_register(&watchdog_miscdev);
> if (err != 0) {
> - pr_err("%s: cannot register miscdev on minor=%d
> (err=%d).\n",
> - wdd->info->identity, WATCHDOG_MINOR, err);
> - if (err == -EBUSY)
> - pr_err("%s: a legacy watchdog module is
> probably present.\n",
> - wdd->info->identity);
> + if (err == -EBUSY) {
> + pr_info("%s: cannot register miscdev on
> minor=%d (err=%d).\n",
> + wdd->info->identity,
> WATCHDOG_MINOR, err);
> + pr_info("%s: a legacy watchdog module is
> probably present.\n",
> + wdd->info->identity);
> + } else
> + pr_err("%s: cannot register miscdev on
> minor=%d (err=%d).\n",
> + wdd->info->identity, WATCHDOG_MINOR,
> err);
> old_wd_data = NULL;
> kfree(wd_data);
> return err;
>
> Does this look like a good approach? If not, what would you recommend?
> In any case, I want to upstream the change, so better ask first :)
>
I would suggest to convert the offending driver to use the watchdog subsystem
(and along the line remove the restriction of only supporting a single
instance). You have the hardware, so that should be a straightforward change.
Thanks,
Guenter
