> +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 _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors