[PATCH 1/2] Documentation: sample averaging for imx6ul_tsc

[PATCH 1/2] Documentation: sample averaging for imx6ul_tsc

[Guy Shapiro]

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
 
 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

[PATCH 2/2] input: touchscreen: sample averaging for imx6ul_tsc

[Guy Shapiro]

The i.MX6UL internal touchscreen controller contains an option to
average upon samples. This feature reduces noise from the produced
touch locations.

This patch adds sample averaging support to the imx6ul_tsc device
driver.

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

diff --git a/drivers/input/touchscreen/imx6ul_tsc.c b/drivers/input/touchscreen/imx6ul_tsc.c
index 8275267..31724d9 100644
--- a/drivers/input/touchscreen/imx6ul_tsc.c
+++ b/drivers/input/touchscreen/imx6ul_tsc.c
@@ -25,6 +25,7 @@
 /* ADC configuration registers field define */
 #define ADC_AIEN		(0x1 << 7)
 #define ADC_CONV_DISABLE	0x1F
+#define ADC_AVGE		(0x1 << 5)
 #define ADC_CAL			(0x1 << 7)
 #define ADC_CALF		0x2
 #define ADC_12BIT_MODE		(0x2 << 2)
@@ -32,6 +33,7 @@
 #define ADC_CLK_DIV_8		(0x03 << 5)
 #define ADC_SHORT_SAMPLE_MODE	(0x0 << 4)
 #define ADC_HARDWARE_TRIGGER	(0x1 << 13)
+#define ADC_AVGS_SHIFT		14
 #define SELECT_CHANNEL_4	0x04
 #define SELECT_CHANNEL_1	0x01
 #define DISABLE_CONVERSION_INT	(0x0 << 7)
@@ -86,6 +88,7 @@ struct imx6ul_tsc {
 
 	int measure_delay_time;
 	int pre_charge_time;
+	int average_samples;
 
 	struct completion completion;
 };
@@ -107,6 +110,8 @@ static int imx6ul_adc_init(struct imx6ul_tsc *tsc)
 	adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
 	adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
 	adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
+	if (tsc->average_samples)
+		adc_cfg |= (tsc->average_samples - 1) << ADC_AVGS_SHIFT;
 	adc_cfg &= ~ADC_HARDWARE_TRIGGER;
 	writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
 
@@ -118,6 +123,8 @@ static int imx6ul_adc_init(struct imx6ul_tsc *tsc)
 	/* start ADC calibration */
 	adc_gc = readl(tsc->adc_regs + REG_ADC_GC);
 	adc_gc |= ADC_CAL;
+	if (tsc->average_samples)
+		adc_gc |= ADC_AVGE;
 	writel(adc_gc, tsc->adc_regs + REG_ADC_GC);
 
 	timeout = wait_for_completion_timeout
@@ -450,6 +457,16 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
 	if (err)
 		tsc->pre_charge_time = 0xfff;
 
+	err = of_property_read_u32(np, "average-samples",
+				   &tsc->average_samples);
+	if (err)
+		tsc->average_samples = 0;
+	if (tsc->average_samples > 4) {
+		dev_err(&pdev->dev, "average-samples (%u) must be [0-4]\n",
+			tsc->average_samples);
+		return -EINVAL;
+	}
+
 	err = input_register_device(tsc->input);
 	if (err) {
 		dev_err(&pdev->dev,
-- 
2.1.4

Re: [PATCH 2/2] input: touchscreen: sample averaging for imx6ul_tsc

[Fabio Estevam]

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 adds sample averaging support to the imx6ul_tsc device
> driver.
>
> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Re: [PATCH 2/2] input: touchscreen: sample averaging for imx6ul_tsc

[Dmitry Torokhov]

On Sun, Nov 27, 2016 at 10:39:10AM -0200, Fabio Estevam wrote:
> 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 adds sample averaging support to the imx6ul_tsc device
> > driver.
> >
> > Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
> 
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Folded the binding patch into this one and applied, thank you.

-- 
Dmitry

Re: [PATCH 1/2] Documentation: sample averaging for imx6ul_tsc

[Fabio Estevam]

On Sun, Nov 27, 2016 at 5:44 AM, Guy Shapiro <guy.shapiro-2HKgp+mgmS5l57MIdRCFDg@public.gmane.org> 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-2HKgp+mgmS5l57MIdRCFDg@public.gmane.org>

Reviewed-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Documentation: sample averaging for imx6ul_tsc

[Rob Herring]

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-2HKgp+mgmS5l57MIdRCFDg@public.gmane.org>
> ---
>  .../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-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Documentation: sample averaging for imx6ul_tsc

[Guy Shapiro]

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?

Re: [PATCH 1/2] Documentation: sample averaging for imx6ul_tsc

[Rob Herring]

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