Re: [PATCH/RFC] iio: gp2ap002a00f: Add a driver for the device.

[Jacek Anaszewski <j.anaszewski@samsung.com>]

On 06/26/2013 04:49 PM, Jacek Anaszewski wrote:
> On 06/22/2013 01:32 PM, Jonathan Cameron wrote:
>> Sorry, due to some local email weirdness I don't seem to have an original
>> copy of this email, so have grabbed it from marc.info.
>> This may mess up the thread handling for others.
>>
>>> Add a new driver for the ambient light/proximity sensor
>>> device. The driver exposes three channels: light_clear
>>> light_ir and proximity. It also supports high and low
>>> ambient light threshold event and proximity detection
>>> event.
>>>
>> Just to check, are you aware of the drive in drivers/input/misc?
>
> Yes I am aware of it. I even got influenced by it to the extent
> that I mistakenly adopted its name for my driver, which should be
> gp2ap020a00f. The name will be corrected in the second version
> of the patch. The gp2ap002a00f device seems not to have too much
> in common with gp2ap020a00f.
>
>> A few comments from a somewhat superficial review below.
>>
> [...]
>>> +static const struct iio_chan_spec gp2ap002a00f_channels[] = {
>>> +	{
>>> +		.type = IIO_LIGHT,
>>> +		.channel2 = IIO_MOD_LIGHT_CLEAR,
>>> +		.modified = 1,
>>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> Just to confirm, you are providing no means at all of converting
>> these channels to any form of standard unit?
>> Without the datasheet I can't tell if this is because no conversion
>> is provided.
>
> Yes, this is intentional.
>
> [...]
>>> +	err = iio_triggered_buffer_setup(indio_dev, NULL,
>>> +		&gp2ap002a00f_trigger_handler,&gp2ap002a00f_buffer_setup_ops);
>>> +
>> So this is a single irq line for events and dataready?
>> I can't seem to find a decent datasheet online for this part which is
>> always annoying!
>
> The documentation I have for my disposal is confidential.
> Yes, there is common interrupt line for events and data ready.
> When threshold registers are filled with a value greater than 0
> the interrupts are considered enabled.

I messed up the things - events are considered enabled then.
Interrupts are being generated when converted value exceeds
threshold register value, and thus no interrupt is being
generated when converted value is 0 as in this case it
can't exceed threshold value.
Sorry for making confusion.

Thanks,
Jacek