diff mbox

[v2,3/8] Input: synaptics-rmi4: Add dribble and palm gesture parameters to device tree

Message ID 1466813917-15841-1-git-send-email-aduggan@synaptics.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Duggan June 25, 2016, 12:18 a.m. UTC
Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
---
 .../devicetree/bindings/input/rmi4/rmi_2d_sensor.txt          |  4 ++++
 drivers/input/rmi4/rmi_2d_sensor.c                            | 11 +++++++++++
 2 files changed, 15 insertions(+)

Comments

Rob Herring (Arm) June 28, 2016, 8:56 p.m. UTC | #1
On Fri, Jun 24, 2016 at 05:18:37PM -0700, Andrew Duggan wrote:
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> ---
>  .../devicetree/bindings/input/rmi4/rmi_2d_sensor.txt          |  4 ++++
>  drivers/input/rmi4/rmi_2d_sensor.c                            | 11 +++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
> index f2c30c8..86cb8e3 100644
> --- a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
> +++ b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
> @@ -37,6 +37,10 @@ Optional Properties:
>  				disable reporing absolute position data.
>  - syna,rezero-wait-ms: Time in miliseconds to wait after issuing a rezero
>  				command.
> +- syna,dribble-enable: Enable reporting of dribble packets in the firmware.
> +- syna,dribble-disable: Disable reporting of dribble packets in the firmware.
> +- syna,palm-detect-enable: Enable reporting the palm detect gesture in the firmware.
> +- syna,palm-detect-disable: Disable reporting the palm detect gesture in the firmware.

No need for enable and disable. Make the common case be no property and 
the property is the opposite.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Duggan June 29, 2016, 12:44 a.m. UTC | #2
Hi Rob,

On 06/28/2016 01:56 PM, Rob Herring wrote:
> On Fri, Jun 24, 2016 at 05:18:37PM -0700, Andrew Duggan wrote:
>> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
>> ---
>>   .../devicetree/bindings/input/rmi4/rmi_2d_sensor.txt          |  4 ++++
>>   drivers/input/rmi4/rmi_2d_sensor.c                            | 11 +++++++++++
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>> index f2c30c8..86cb8e3 100644
>> --- a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>> +++ b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>> @@ -37,6 +37,10 @@ Optional Properties:
>>   				disable reporing absolute position data.
>>   - syna,rezero-wait-ms: Time in miliseconds to wait after issuing a rezero
>>   				command.
>> +- syna,dribble-enable: Enable reporting of dribble packets in the firmware.
>> +- syna,dribble-disable: Disable reporting of dribble packets in the firmware.
>> +- syna,palm-detect-enable: Enable reporting the palm detect gesture in the firmware.
>> +- syna,palm-detect-disable: Disable reporting the palm detect gesture in the firmware.
> No need for enable and disable. Make the common case be no property and
> the property is the opposite.

There are actually three states here. No property means to use the 
default value set in the firmware. The enable parameter means that the 
default should be overwritten and the feature should be explicitly 
enabled. Disable means the feature should be explicitly disabled. An 
earlier version of this patch had one parameter per feature which needed 
to be set to 0, 1, or 2 which correspond the the values of an enum in 
the driver. It was suggested that the current approach would be clearer 
then setting 0, 1, or 2.

Thanks for reviewing,
Andrew

> Rob

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nick Dyer July 4, 2016, 3 p.m. UTC | #3
Hi Andrew

On 29/06/2016 01:44, Andrew Duggan wrote:
> On 06/28/2016 01:56 PM, Rob Herring wrote:
>> On Fri, Jun 24, 2016 at 05:18:37PM -0700, Andrew Duggan wrote:
>>> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
>>> ---
>>>   .../devicetree/bindings/input/rmi4/rmi_2d_sensor.txt          |  4 ++++
>>>   drivers/input/rmi4/rmi_2d_sensor.c                            | 11
>>> +++++++++++
>>>   2 files changed, 15 insertions(+)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>>> b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>>> index f2c30c8..86cb8e3 100644
>>> --- a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>>> +++ b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>>> @@ -37,6 +37,10 @@ Optional Properties:
>>>                   disable reporing absolute position data.
>>>   - syna,rezero-wait-ms: Time in miliseconds to wait after issuing a rezero
>>>                   command.
>>> +- syna,dribble-enable: Enable reporting of dribble packets in the
>>> firmware.
>>> +- syna,dribble-disable: Disable reporting of dribble packets in the
>>> firmware.
>>> +- syna,palm-detect-enable: Enable reporting the palm detect gesture in
>>> the firmware.
>>> +- syna,palm-detect-disable: Disable reporting the palm detect gesture
>>> in the firmware.
>> No need for enable and disable. Make the common case be no property and
>> the property is the opposite.
> 
> There are actually three states here. No property means to use the default
> value set in the firmware. The enable parameter means that the default
> should be overwritten and the feature should be explicitly enabled. Disable
> means the feature should be explicitly disabled. An earlier version of this
> patch had one parameter per feature which needed to be set to 0, 1, or 2
> which correspond the the values of an enum in the driver. It was suggested
> that the current approach would be clearer then setting 0, 1, or 2.

I wonder if it might be worth mentioning explicitly in the devicetree
documentation that the firmware is being overridden - perhaps
"syna,dribble-force-disable"?

cheers

Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
index f2c30c8..86cb8e3 100644
--- a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
+++ b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
@@ -37,6 +37,10 @@  Optional Properties:
 				disable reporing absolute position data.
 - syna,rezero-wait-ms: Time in miliseconds to wait after issuing a rezero
 				command.
+- syna,dribble-enable: Enable reporting of dribble packets in the firmware.
+- syna,dribble-disable: Disable reporting of dribble packets in the firmware.
+- syna,palm-detect-enable: Enable reporting the palm detect gesture in the firmware.
+- syna,palm-detect-disable: Disable reporting the palm detect gesture in the firmware.
 
 
 Example of a RMI4 I2C device with F11:
diff --git a/drivers/input/rmi4/rmi_2d_sensor.c b/drivers/input/rmi4/rmi_2d_sensor.c
index e97bd7f..ee26e35 100644
--- a/drivers/input/rmi4/rmi_2d_sensor.c
+++ b/drivers/input/rmi4/rmi_2d_sensor.c
@@ -317,6 +317,17 @@  int rmi_2d_sensor_of_probe(struct device *dev,
 
 	pdata->rezero_wait = val;
 
+	if (of_property_read_bool(dev->of_node, "syna,dribble-enable"))
+		pdata->dribble = RMI_REG_STATE_ON;
+	else if (of_property_read_bool(dev->of_node, "syna,dribble-disable"))
+		pdata->dribble = RMI_REG_STATE_OFF;
+
+	if (of_property_read_bool(dev->of_node, "syna,palm-detect-enable"))
+		pdata->palm_detect = RMI_REG_STATE_ON;
+	else if (of_property_read_bool(dev->of_node,
+				"syna,palm-detect-disable"))
+		pdata->palm_detect = RMI_REG_STATE_OFF;
+
 	return 0;
 }
 #else