Message-ID: <55CA48A2.9050409@synaptics.com>
Date:	Tue, 11 Aug 2015 12:10:26 -0700
From:	Andrew Duggan <aduggan@...aptics.com>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
CC:	<linux-input@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	Benjamin Tissoires <benjamin.tissoires@...hat.com>,
	Christopher Heiny <cheiny@...aptics.com>,
	Stephen Chandler Paul <cpaul@...hat.com>
Subject: Re: [PATCH] Input: synaptics-rmi4: Add device tree support for RMI4
 I2C devices

Hi Dmitry,

On 08/09/2015 12:40 AM, Dmitry Torokhov wrote:
> Hi Andrew,
>
> On Fri, Aug 07, 2015 at 05:37:49PM -0700, Andrew Duggan wrote:
>> +Optional Properties:
>> +- syna,sensor-name: The string containing the name of the sensor.
>> +- syna,attn-gpio: The GPIO number used to assert attention.
>> +- syna,attn-polarity: The polarity of the attention GPIO.
>> +- syna,level-triggered: Set to 1 if attention GPIO is level triggered, 0 if
>> +			edge triggered.
> We already have generic bindings for i2c devices interrupt line, along
> with trigger type. We also have standard bindings for gpios, with the
> polarity. Interrupts are also parsed by the I2C core. There is no need
> to invent your own bindings.

In the current implementation rmi_driver expects an attention GPIO from 
the platform data and manages it internally. I just added those 
parameters to device tree. But, that essentially duplicates the generic 
I2C and device tree interrupt handling. I agree that rmi_driver should 
just use the irq provided in the I2C client and then we don't have to 
duplicate the GPIO handling and we can use the generic bindings.

One option is to check to see if there is an irq in the I2C client and 
skip the GPIO initialization if there is. I can provide a patch which 
does that and a v2 of this patch to use the generic bindings. But, this 
also brings up the question on whether or not we should still have the 
code for handling the GPIO in rmi_driver at all. The last time the GPIO 
code was discussed, Chris said it allowed for releasing the irq for 
certain diagnostic modes. But, there are cases when rmi_driver won't 
have access to the GPIO or when the device is directly connected to the 
IO_APIC (ie HID or SMBus devices) so the diagnostic mode won't be 
available for those devices. Also, I'm also not convinced that the 
diagnostic capabilities justify duplicating the GPIO handling. If most 
platforms are using device tree at this point I think it might be time 
to remove the GPIO code from rmi_driver and let the lower levels handle 
interrupts. Or, if there are major objections I can work around it and 
leave it as an option in the platform data for devices which still use 
board files.

http://marc.info/?l=linux-input&m=139155532321252&w=2

>> +- syna,poll-interval-ms: The interval in milliseconds to wait between reading
>> +			interrupts when the driver is polling.
>> +- syna,reset-delay-ms: The number of milliseconds to wait after resetting the
>> +			device.
>> +- syna,f01-*: Additional parameters specific to RMI4 function 1
>> +		see Documentation/devicetree/bindings/input/rmi4/rmi_f01.txt.
>> +- syna,f11-*: Additional parameters specific to RMI4 function 11
>> +		see Documentation/devicetree/bindings/input/rmi4/rmi_f11.txt.
> For function parameters I wonder if they should not be modelled as
> subnodes with "reg" representing function number.

I'll look in to doing this. Putting the function parameters into 
subnodes makes sense to me.

> Thanks.
>

Thanks,
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists