Re: [PATCH v7 4/9] drivers:input:tsc2007: add iio interface to read external ADC input and temperature

From: H. Nikolaus Schaller
Date: Sat Nov 12 2016 - 14:23:45 EST


> Am 12.11.2016 um 15:19 schrieb Jonathan Cameron <jic23@xxxxxxxxxx>:
>
> On 12/11/16 14:04, Jonathan Cameron wrote:
>> On 11/11/16 19:02, H. Nikolaus Schaller wrote:
>>> The tsc2007 chip not only has a resistive touch screen controller but
>>> also an external AUX adc imput which can be used for an ambient
>>> light sensor, battery voltage monitoring or any general purpose.
>>>
>>> Additionally it can measure the chip temperature.
>>>
>>> This extension provides an iio interface for these adc channels.
>>>
>>> Since it is not wasting much resources and is very straightforward,
>>> we simply provide all other adc channels as optional iio interfaces
>>> as weel. This can be used for debugging or special applications.
>>>
>>> This patch also splits the tsc2007 driver in several source files:
>>> tsc2007.h -- constants, structs and stubs
>>> tsc2007_core.c -- functional parts of the original driver
>>> tsc2007_iio.c -- the optional iio stuff
>>>
>>> Makefile magic allows to conditionally link the iio
>>> stuff if CONFIG_IIO=y in a way that it works with
>>> CONFIG_TOUCHSCREEN_TSC2007=m.
>>>
>>
>> I ran a quick build test and it blows up in a number of ways related to
>> the #ifdef CONFIG_IIO.
>>
>> I'm not entirely sure why but
>> #if IS_ENABLED(CONFIG_IIO) works fine.
> Ah, I'm being sleepy today. It's because I'm building IIO as a module
> and the symbol defined would therefore be CONFIG_IIO_MODULE.
>
> The IS_ENABLED macro takes care of both cases.

Ah, yes. I wasn't aware that CONFIG_IIO is tristate at all.
We have it as "y" and therefore didn't notice.

I will collect this fix for V8.

>>
>> Otherwise looks good to me
>>> Signed-off-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
>> Reviewed-by: Jonathan Cameron <jic23@xxxxxxxxxx>

Will add this as well.

Thanks for the detailled review!

BR,
Nikolaus