linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/7] Input: synaptics-rmi4: Add dribble and palm gesture parameters to devicetree
@ 2016-06-03 18:40 Andrew Duggan
  2016-06-03 18:57 ` Mark Rutland
  2016-06-08  7:39 ` Linus Walleij
  0 siblings, 2 replies; 4+ messages in thread
From: Andrew Duggan @ 2016-06-03 18:40 UTC (permalink / raw)
  To: linux-input, linux-kernel
  Cc: Andrew Duggan, Dmitry Torokhov, Linus Walleij, Jiri Kosina,
	Benjamin Tissoires, Vincent Huang, Nick Dyer, devicetree

Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
---
 .../devicetree/bindings/input/rmi4/rmi_2d_sensor.txt       |  5 +++++
 drivers/input/rmi4/rmi_2d_sensor.c                         | 14 ++++++++++++++
 2 files changed, 19 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..822df11 100644
--- a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
+++ b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
@@ -37,6 +37,11 @@ Optional Properties:
 				disable reporing absolute position data.
 - syna,rezero-wait-ms: Time in miliseconds to wait after issuing a rezero
 				command.
+- syna,dribble: Control reporting of dribble packets. Values are 0 for
+		default, 1 for explicitly disable, 2 for explicitly enable.
+- syna,palm_detect: Control reporting of the palm detect gesture. Values
+			are 0 for default, 1 for explicitly disable, 2 for
+			explicitly enable.
 
 
 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..c41db3b 100644
--- a/drivers/input/rmi4/rmi_2d_sensor.c
+++ b/drivers/input/rmi4/rmi_2d_sensor.c
@@ -317,6 +317,20 @@ int rmi_2d_sensor_of_probe(struct device *dev,
 
 	pdata->rezero_wait = val;
 
+	retval = rmi_of_property_read_u32(dev, &val, "syna,dribble",
+						1);
+	if (retval)
+		return retval;
+
+	pdata->dribble = val;
+
+	retval = rmi_of_property_read_u32(dev, &val, "syna,palm_detect",
+						1);
+	if (retval)
+		return retval;
+
+	pdata->palm_detect = val;
+
 	return 0;
 }
 #else
-- 
2.5.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/7] Input: synaptics-rmi4: Add dribble and palm gesture parameters to devicetree
  2016-06-03 18:40 [PATCH 3/7] Input: synaptics-rmi4: Add dribble and palm gesture parameters to devicetree Andrew Duggan
@ 2016-06-03 18:57 ` Mark Rutland
  2016-06-07 19:11   ` Andrew Duggan
  2016-06-08  7:39 ` Linus Walleij
  1 sibling, 1 reply; 4+ messages in thread
From: Mark Rutland @ 2016-06-03 18:57 UTC (permalink / raw)
  To: Andrew Duggan
  Cc: linux-input, linux-kernel, Dmitry Torokhov, Linus Walleij,
	Jiri Kosina, Benjamin Tissoires, Vincent Huang, Nick Dyer,
	devicetree

On Fri, Jun 03, 2016 at 11:40:28AM -0700, Andrew Duggan wrote:
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> ---
>  .../devicetree/bindings/input/rmi4/rmi_2d_sensor.txt       |  5 +++++
>  drivers/input/rmi4/rmi_2d_sensor.c                         | 14 ++++++++++++++
>  2 files changed, 19 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..822df11 100644
> --- a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
> +++ b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
> @@ -37,6 +37,11 @@ Optional Properties:
>  				disable reporing absolute position data.
>  - syna,rezero-wait-ms: Time in miliseconds to wait after issuing a rezero
>  				command.
> +- syna,dribble: Control reporting of dribble packets. Values are 0 for
> +		default, 1 for explicitly disable, 2 for explicitly enable.

This sounds like a driver option one might choose at runtime, not a
fixed hardware/integration property.

Why does this belong in the DT?

> +- syna,palm_detect: Control reporting of the palm detect gesture. Values
> +			are 0 for default, 1 for explicitly disable, 2 for
> +			explicitly enable.

Likewise, same question here.

Additionally, please use '-', not '_' in property names.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/7] Input: synaptics-rmi4: Add dribble and palm gesture parameters to devicetree
  2016-06-03 18:57 ` Mark Rutland
@ 2016-06-07 19:11   ` Andrew Duggan
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Duggan @ 2016-06-07 19:11 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-input, linux-kernel, Dmitry Torokhov, Linus Walleij,
	Jiri Kosina, Benjamin Tissoires, Vincent Huang, Nick Dyer,
	devicetree

Hi Mark,

Thanks for reviewing.

On 06/03/2016 11:57 AM, Mark Rutland wrote:
> On Fri, Jun 03, 2016 at 11:40:28AM -0700, Andrew Duggan wrote:
>> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
>> ---
>>   .../devicetree/bindings/input/rmi4/rmi_2d_sensor.txt       |  5 +++++
>>   drivers/input/rmi4/rmi_2d_sensor.c                         | 14 ++++++++++++++
>>   2 files changed, 19 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..822df11 100644
>> --- a/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>> +++ b/Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>> @@ -37,6 +37,11 @@ Optional Properties:
>>   				disable reporing absolute position data.
>>   - syna,rezero-wait-ms: Time in miliseconds to wait after issuing a rezero
>>   				command.
>> +- syna,dribble: Control reporting of dribble packets. Values are 0 for
>> +		default, 1 for explicitly disable, 2 for explicitly enable.
> This sounds like a driver option one might choose at runtime, not a
> fixed hardware/integration property.
>
> Why does this belong in the DT?

My intention is to enable or disable these properties on a platform 
basis. Dribble packets could be enabled on platforms which have issues 
reliably reporting data. Dribble packets report multiple finger lift 
events when a finger leaves the touchpad. The default will be to disable 
them, but if a platform does not reliably report data (examples we have 
observed are issues with the I2C controller) then the additional finger 
lift events could avoid fingers from becoming "stuck" when the host does 
not receive a finger lift event.

>> +- syna,palm_detect: Control reporting of the palm detect gesture. Values
>> +			are 0 for default, 1 for explicitly disable, 2 for
>> +			explicitly enable.
> Likewise, same question here.

Similarly, this property was added to disable this functionality on a 
particular platform where it was causing interference.

Since there properties are set in firmware which is generally configured 
per platform I went ahead and added them to the device tree bindings.

> Additionally, please use '-', not '_' in property names.

Sorry, Rob gave me similar feedback for previous properties. But, I must 
have forgotten when making this change. I can fix this if we decide to 
keep these properties in devicetree.

Thanks,
Andrew

> Thanks,
> Mark.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/7] Input: synaptics-rmi4: Add dribble and palm gesture parameters to devicetree
  2016-06-03 18:40 [PATCH 3/7] Input: synaptics-rmi4: Add dribble and palm gesture parameters to devicetree Andrew Duggan
  2016-06-03 18:57 ` Mark Rutland
@ 2016-06-08  7:39 ` Linus Walleij
  1 sibling, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2016-06-08  7:39 UTC (permalink / raw)
  To: Andrew Duggan
  Cc: Linux Input, linux-kernel, Dmitry Torokhov, Jiri Kosina,
	Benjamin Tissoires, Vincent Huang, Nick Dyer, devicetree

On Fri, Jun 3, 2016 at 8:40 PM, Andrew Duggan <aduggan@synaptics.com> wrote:

> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>

> +- syna,dribble: Control reporting of dribble packets. Values are 0 for
> +               default, 1 for explicitly disable, 2 for explicitly enable.
> +- syna,palm_detect: Control reporting of the palm detect gesture. Values
> +                       are 0 for default, 1 for explicitly disable, 2 for
> +                       explicitly enable.

I think these belong in the device tree, but you should add four bool
props instead:

syna,dribble-disable;
syna,dribble-enable;
syna,palm-detect-disable;
syna,palm-detect-enable;

Those are way easier for the human authoring the device tree to
cope with than the opaque parameters.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-06-08  7:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-03 18:40 [PATCH 3/7] Input: synaptics-rmi4: Add dribble and palm gesture parameters to devicetree Andrew Duggan
2016-06-03 18:57 ` Mark Rutland
2016-06-07 19:11   ` Andrew Duggan
2016-06-08  7:39 ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).