Re: [V2 1/2] i2c: brcmstb: Add Broadcom settop SoC i2c controller driver

[Ray Jui <rjui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>]

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
>