Mark Brown wrote: > On Thu, Jul 16, 2009 at 11:18:02AM +0000, Jonathan Cameron wrote: >>> --- /dev/null >>> +++ b/drivers/hwmon/wm8350-hwmon.c >>> @@ -0,0 +1,154 @@ >> ... >>> +#include <linux/delay.h> >>> +#include <linux/platform_device.h> > >> Why input-polldev? or for that matter delay.h, mutex.h, dmi.h >> Can't immediately see any use of things from them, but I >> haven't build tested without to check. > > TBH I templated the headers off another driver and never rechecked them > - obviously I've got far too much stuff in there. > >>> + val = wm8350_read_auxadc(wm8350, channel, 0, 0) >>> + * WM8350_AUX_COEFF / 1000; > >> I'd normally be a bit dubious about the lack of error handling here, >> but having taken a look in at the wm8350-core it doesn't look like any >> errors that occur can get through anyway (now whether they would >> ideally be passed up to here is a different question!) > > Yeah, and if it were being changed it'd probably be easier to merge the > driver and then do cross-subsystem patches fixing up the readback > function and its users in one fell swoop. Realistically if the AUXADC > starts failing inaccurate supply voltage readings are probably the least > of your worries, though. Indeed. Not exactly high priority work. Certainly something to do at a later date if anyone is feeling tidy ;) > >> The rest looks fine to me. Get rid of (or justify) the headers >> and you have my ack. > > I'll prune the headers next time I submit. Great, Jonathan