[1/2] Documentation: sample averaging for imx6ul_tsc
diff mbox

Message ID 1480232698-23075-1-git-send-email-guy.shapiro@mobi-wize.com
State Under Review
Headers show

Commit Message

Guy Shapiro Nov. 27, 2016, 7:44 a.m. UTC
The i.MX6UL internal touchscreen controller contains an option to
average upon samples. This feature reduces noise from the produced
touch locations.

This patch introduces a new device tree optional property for this
feature. It provides control over the amount of averaged samples per
touch event.

The property was inspired by a similar property on the
"brcm,iproc-touchscreen" binding.

Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
---
 .../devicetree/bindings/input/touchscreen/imx6ul_tsc.txt          | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Fabio Estevam Nov. 27, 2016, 12:38 p.m. UTC | #1
On Sun, Nov 27, 2016 at 5:44 AM, Guy Shapiro <guy.shapiro@mobi-wize.com> wrote:
> The i.MX6UL internal touchscreen controller contains an option to
> average upon samples. This feature reduces noise from the produced
> touch locations.
>
> This patch introduces a new device tree optional property for this
> feature. It provides control over the amount of averaged samples per
> touch event.
>
> The property was inspired by a similar property on the
> "brcm,iproc-touchscreen" binding.
>
> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
--
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
Rob Herring Dec. 1, 2016, 4:13 p.m. UTC | #2
On Sun, Nov 27, 2016 at 09:44:57AM +0200, Guy Shapiro wrote:
> The i.MX6UL internal touchscreen controller contains an option to
> average upon samples. This feature reduces noise from the produced
> touch locations.
> 
> This patch introduces a new device tree optional property for this
> feature. It provides control over the amount of averaged samples per
> touch event.
> 
> The property was inspired by a similar property on the
> "brcm,iproc-touchscreen" binding.
> 
> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
> ---
>  .../devicetree/bindings/input/touchscreen/imx6ul_tsc.txt          | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> index 853dff9..a66069f 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> @@ -17,6 +17,13 @@ Optional properties:
>    This value depends on the touch screen.
>  - pre-charge-time: the touch screen need some time to precharge.
>    This value depends on the touch screen.
> +- average-samples: Number of data samples which are averaged for each read.
> +	Valid values 0-4
> +	0 =  1 sample
> +	1 =  4 samples
> +	2 =  8 samples
> +	3 = 16 samples
> +	4 = 32 samples

Either this needs a vendor prefix or should be documented as a generic 
property. In the latter case, you should use actual number of samples 
(1-32) for the values.

>  
>  Example:
>  	tsc: tsc@02040000 {
> @@ -32,5 +39,6 @@ Example:
>  		xnur-gpio = <&gpio1 3 GPIO_ACTIVE_LOW>;
>  		measure-delay-time = <0xfff>;
>  		pre-charge-time = <0xffff>;
> +		average-samples = <4>;
>  		status = "okay";
>  	};
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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
Guy Shapiro Dec. 8, 2016, 3:15 p.m. UTC | #3
On 01/12/2016 18:13, Rob Herring wrote:
> On Sun, Nov 27, 2016 at 09:44:57AM +0200, Guy Shapiro wrote:
>> The i.MX6UL internal touchscreen controller contains an option to
>> average upon samples. This feature reduces noise from the produced
>> touch locations.
>>
>> This patch introduces a new device tree optional property for this
>> feature. It provides control over the amount of averaged samples per
>> touch event.
>>
>> The property was inspired by a similar property on the
>> "brcm,iproc-touchscreen" binding.
>>
>> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
>> ---
>>   .../devicetree/bindings/input/touchscreen/imx6ul_tsc.txt          | 8
> ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git
> a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
> b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>> index 853dff9..a66069f 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>> @@ -17,6 +17,13 @@ Optional properties:
>>     This value depends on the touch screen.
>>   - pre-charge-time: the touch screen need some time to precharge.
>>     This value depends on the touch screen.
>> +- average-samples: Number of data samples which are averaged for each
> read.
>> +	Valid values 0-4
>> +	0 =  1 sample
>> +	1 =  4 samples
>> +	2 =  8 samples
>> +	3 = 16 samples
>> +	4 = 32 samples
> Either this needs a vendor prefix or should be documented as a generic
> property. In the latter case, you should use actual number of samples
> (1-32) for the values.
In the term "generic property", do you mean to document it on 
bindings/input/touchscreen/touchscreen.txt ?
If so, should I add the "touchscreen-" prefix like all the other 
properties in that file?

Grepping bindings/input/touchscreen/, I found two other device drivers 
that implement a
similar property - "ti-tsc-adc" and "brcm,iproc-touchscreen" (The 
latter, BTW, uses a non
vendor prefixed property name).

Do we want to move from per-vendor properties to a generic one?
If we do, should we deprecate the existing vendor specific properties?
--
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
Rob Herring Dec. 8, 2016, 3:45 p.m. UTC | #4
On Thu, Dec 8, 2016 at 9:15 AM, Guy Shapiro <guy.shapiro@mobi-wize.com> wrote:
> On 01/12/2016 18:13, Rob Herring wrote:
>>
>> On Sun, Nov 27, 2016 at 09:44:57AM +0200, Guy Shapiro wrote:
>>>
>>> The i.MX6UL internal touchscreen controller contains an option to
>>> average upon samples. This feature reduces noise from the produced
>>> touch locations.
>>>
>>> This patch introduces a new device tree optional property for this
>>> feature. It provides control over the amount of averaged samples per
>>> touch event.
>>>
>>> The property was inspired by a similar property on the
>>> "brcm,iproc-touchscreen" binding.
>>>
>>> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
>>> ---
>>>   .../devicetree/bindings/input/touchscreen/imx6ul_tsc.txt          | 8
>>
>> ++++++++
>>>
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git
>>
>> a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>> b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>>>
>>> index 853dff9..a66069f 100644
>>> --- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>>> +++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
>>> @@ -17,6 +17,13 @@ Optional properties:
>>>     This value depends on the touch screen.
>>>   - pre-charge-time: the touch screen need some time to precharge.
>>>     This value depends on the touch screen.
>>> +- average-samples: Number of data samples which are averaged for each
>>
>> read.
>>>
>>> +       Valid values 0-4
>>> +       0 =  1 sample
>>> +       1 =  4 samples
>>> +       2 =  8 samples
>>> +       3 = 16 samples
>>> +       4 = 32 samples
>>
>> Either this needs a vendor prefix or should be documented as a generic
>> property. In the latter case, you should use actual number of samples
>> (1-32) for the values.
>
> In the term "generic property", do you mean to document it on
> bindings/input/touchscreen/touchscreen.txt ?

Yes.

> If so, should I add the "touchscreen-" prefix like all the other properties
> in that file?

Yes.

> Grepping bindings/input/touchscreen/, I found two other device drivers that
> implement a
> similar property - "ti-tsc-adc" and "brcm,iproc-touchscreen" (The latter,
> BTW, uses a non
> vendor prefixed property name).

Unfortunately, the brcm one doesn't look directly usable.

> Do we want to move from per-vendor properties to a generic one?

No. Those are set already. I just don't want to get more vendor properties.

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

Patch
diff mbox

diff --git a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
index 853dff9..a66069f 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
@@ -17,6 +17,13 @@  Optional properties:
   This value depends on the touch screen.
 - pre-charge-time: the touch screen need some time to precharge.
   This value depends on the touch screen.
+- average-samples: Number of data samples which are averaged for each read.
+	Valid values 0-4
+	0 =  1 sample
+	1 =  4 samples
+	2 =  8 samples
+	3 = 16 samples
+	4 = 32 samples
 
 Example:
 	tsc: tsc@02040000 {
@@ -32,5 +39,6 @@  Example:
 		xnur-gpio = <&gpio1 3 GPIO_ACTIVE_LOW>;
 		measure-delay-time = <0xfff>;
 		pre-charge-time = <0xffff>;
+		average-samples = <4>;
 		status = "okay";
 	};