Re: [PATCH 1/3] watchdog_dev: Add support for having more then 1 watchdog

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> +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


[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux