Re: [PATCH V3] iio: gyro: Add itg3200

[Lars-Peter Clausen <lars@metafoo.de>]

On 01/31/2013 07:37 PM, Manuel Stahl wrote:
> Am Donnerstag, 31. Januar 2013, 16:21:26 schrieb Lars-Peter Clausen:
>> On 01/31/2013 01:17 PM, Manuel Stahl wrote:
>>> Signed-off-by: Manuel Stahl <manuel.stahl@iis.fraunhofer.de>
>>
>> Having at least a short commit message wouldn't hurt. E.g. something along
>> the lines of: "This patch adds support for the InvenSense itg3200. The
>> itg3200 is a three-axis gyro and ..."
> 
> OK, no problem.
>  
>> I get one build warning:
>> 	CC      drivers/iio/gyro/itg3200_core.o
>> 	drivers/iio/gyro/itg3200_core.c:302: warning: initialization from
>> 	incompatible pointer type
> 
> [...]
> 
>>> +static ssize_t itg3200_read_raw(struct iio_dev *indio_dev,
>>
>> This triggers the build warning, the return type should be int.
>>
>>> +		const struct iio_chan_spec *chan,
>>> +		int *val, int *val2, long info)
>>> +{
>>> +	ssize_t ret = 0;
>>
>> What I meant with the comment in my previous mail was make ret int.
> 
> Sorry, I should do more exhaustive tests...
> 
> [...]
> 
>>> +static int itg3200_probe(struct i2c_client *client,
>>> +		const struct i2c_device_id *id)
>>> +{
>>> +	int ret;
>>> +	struct itg3200 *st;
>>> +	struct iio_dev *indio_dev;
>>> +	const unsigned long avail_scan_masks[] = { 0xffffffff, 0x0 };
>>
>> This needs to be static. Right now it is on the stack and well gone after
>> probe has been run.
> 
> Is it OK to make it static inside itg3200_probe() or should it be global?
> 

Does not really matter, but I guess I'd make it global (and add the itg3200
prefix to the variable's name).

- Lars