On 4/15/2015 12:50 AM, Wolfram Sang wrote: > Hi Ray, > > thanks for the review! I agree to all points except one minor thing: > >>>> +#include <linux/device.h> >>>> +#include <linux/kernel.h> >>>> +#include <linux/module.h> >>>> +#include <linux/sched.h> >>>> +#include <linux/i2c.h> >>>> +#include <linux/interrupt.h> >>>> +#include <linux/platform_device.h> >>>> +#include <linux/clk.h> >>>> +#include <linux/io.h> >>>> +#include <linux/clk.h> >>>> +#include <linux/slab.h> >>>> +#include <linux/version.h> >>>> +#include <linux/delay.h> >> >> Wolfram would prefer the includes to be sorted in alphabetic order > > Sidenote: And one can see why here: clk.h is included twice. > >>>> + adap->owner = THIS_MODULE; >> >> Not needed... > > This is needed. It is not needed to set it with the platform_driver > struct, but for the adapter created, this is needed. Okay that's really good to know. Thanks, Wolfram! > > Thanks again, > > Wolfram > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html