watchdog: Add watchdog driver for OCTEON SOCs.
in [Kernel]
|
Prev: MIPS: Add option to export uasm API.
Next: Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
From: Ralf Baechle on 23 Jul 2010 23:10 On Fri, Jul 23, 2010 at 06:41:47PM -0700, David Daney wrote: Wim, ok to merge this one through the MIPS tree? Ralf -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Sam Ravnborg on 24 Jul 2010 00:00 Hi David. In general a very nicely commented piece of code! A few nits... > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index 72f3e20..4e7179b 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -114,6 +114,8 @@ obj-$(CONFIG_PNX833X_WDT) += pnx833x_wdt.o > obj-$(CONFIG_SIBYTE_WDOG) += sb_wdog.o > obj-$(CONFIG_AR7_WDT) += ar7_wdt.o > obj-$(CONFIG_TXX9_WDT) += txx9wdt.o > +obj-$(CONFIG_OCTEON_WDT) += octeon-wdt.o > +octeon-wdt-objs := octeon-wdt-main.o octeon-wdt-nmi.o The use of foo-objs := ... is considered old-school. Use: octeon-wdt-y := octeon-wdt-main.o octeon-wdt-nmi.o It is the same functionality. > + > +#include <linux/bitops.h> > +#include <linux/cpu.h> > +#include <linux/cpumask.h> > +#include <linux/delay.h> > +#include <linux/fs.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.h> > +#include <linux/miscdevice.h> > +#include <linux/module.h> > +#include <linux/smp.h> > +#include <linux/string.h> > +#include <linux/watchdog.h> People have started to use the "inverse christmas tree" for include files. But you sort is alphabetically which is also good. The "inverse christmas tree" would look like this: #include <linux/miscdevice.h> #include <linux/interrupt.h> #include <linux/watchdog.h> #include <linux/cpumask.h> #include <linux/bitops.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/string.h> #include <linux/delay.h> #include <linux/cpu.h> #include <linux/smp.h> #include <linux/fs.h> Both styles are fine so pick what you like. > +module_param(heartbeat, int, 0444); module_param(heartbeat, int, S_IRUSR | S_IRGRP | S_IROTH); or even the shorter: module_param(heartbeat, int, S_IRUGO); This is a bit more descriptive - but still the same functionality. > +module_param(nowayout, int, 0444); ditto. > +void octeon_wdt_nmi_stage3(uint64_t reg[32]) > +{ > + uint64_t i; The kernel version of this type is "u64". We usually say that the stdint types belongs to userspace. But it is used in many places so no big deal. Note: you use stdint types in several places, but there is no need to repeat the comment. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Wim Van Sebroeck on 29 Jul 2010 14:10
Hi Ralf, Will review tomorrow, but if OK then it can indeed go via MIPS tree. Kind regards, Wim. > On Fri, Jul 23, 2010 at 06:41:47PM -0700, David Daney wrote: > > Wim, ok to merge this one through the MIPS tree? > > Ralf -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |