> +static unsigned long dog_mask; /* Watcodgs that are in use */
>
> Please fix (spelling) the comment after dog_mask.
Oops
> + err = misc_register(&watchdog_miscdev);
> + if (err == 0)
> + old_wdd = watchdog;
> + else {
> + pr_err("%s: cannot register miscdev on minor=%d (err=%d
> + watchdog->info->identity, WATCHDOG_MINOR, err);
> + pr_err("%s: probably a legacy watchdog module is presen
> + watchdog->info->identity);
> + watchdog_release_id(watchdog);
> + err = watchdog_assign_id(watchdog, 1);
> + if (err < 0)
> + return err;
> + }
> + }
>
> You may want to check for err == -EBUSY in the fallback instead of assuming
> the problem is a legacy watchdog driver
Yes that is probably wise
> + err = watchdog_add_device(watchdog);
> + }
>
> You do this even for watchdog0 / for the watchdog we've also registered as
> misc_dev. I guess this is intentional and you want the watchdog to be
> available as both misc,130 as well as as MAJOR(dog_devt),0 ?
Yes - so that it appears in the watchdog class in sysfs for example.
> Other then these few remarks it looks fine. If you can resend it
> with my remarks addressed I'll add a Reviewed-By.
Ok I'll sort those next week.
Thanks
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
