devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Input: imx6ul_tsc - generalize the averaging property
@ 2016-12-11  7:06 Guy Shapiro
  2016-12-13 19:10 ` Rob Herring
  2016-12-13 19:54 ` Rob Herring
  0 siblings, 2 replies; 6+ messages in thread
From: Guy Shapiro @ 2016-12-11  7:06 UTC (permalink / raw)
  To: dmitry.torokhov, robh
  Cc: fabio.estevam, mark.rutland, Guy Shapiro, devicetree, haibo.chen,
	linux-input, linux-arm-kernel

Make the avarage-samples property a general touchscreen property
rather than imx6ul device specific.

Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
---
 .../bindings/input/touchscreen/imx6ul_tsc.txt      | 11 ++----
 .../bindings/input/touchscreen/touchscreen.txt     |  3 ++
 drivers/input/touchscreen/imx6ul_tsc.c             | 46 ++++++++++++++++------
 3 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
index a66069f..d4927c2 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
@@ -17,13 +17,8 @@ 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
+- touchscreen-average-samples: Number of data samples which are averaged for
+  each read. Valid values are 1, 4, 8, 16 and 32.
 
 Example:
 	tsc: tsc@02040000 {
@@ -39,6 +34,6 @@ Example:
 		xnur-gpio = <&gpio1 3 GPIO_ACTIVE_LOW>;
 		measure-delay-time = <0xfff>;
 		pre-charge-time = <0xffff>;
-		average-samples = <4>;
+		touchscreen-average-samples = <32>;
 		status = "okay";
 	};
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
index bccaa4e..537643e 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
@@ -14,6 +14,9 @@ Optional properties for Touchscreens:
  - touchscreen-fuzz-pressure	: pressure noise value of the absolute input
 				  device (arbitrary range dependent on the
 				  controller)
+ - touchscreen-average-samples : Number of data samples which are averaged
+				  for each read (valid values dependent on the
+				  controller)
  - touchscreen-inverted-x	: X axis is inverted (boolean)
  - touchscreen-inverted-y	: Y axis is inverted (boolean)
  - touchscreen-swapped-x-y	: X and Y axis are swapped (boolean)
diff --git a/drivers/input/touchscreen/imx6ul_tsc.c b/drivers/input/touchscreen/imx6ul_tsc.c
index d2a3912..58d1aa5 100644
--- a/drivers/input/touchscreen/imx6ul_tsc.c
+++ b/drivers/input/touchscreen/imx6ul_tsc.c
@@ -93,7 +93,8 @@ struct imx6ul_tsc {
 
 	u32 measure_delay_time;
 	u32 pre_charge_time;
-	u32 average_samples;
+	bool average_enable;
+	u32 average_select;
 
 	struct completion completion;
 };
@@ -117,9 +118,9 @@ static int imx6ul_adc_init(struct imx6ul_tsc *tsc)
 	adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
 	adc_cfg &= ~(ADC_CLK_DIV_MASK | ADC_SAMPLE_MODE_MASK);
 	adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
-	if (tsc->average_samples) {
+	if (tsc->average_enable) {
 		adc_cfg &= ~ADC_AVGS_MASK;
-		adc_cfg |= (tsc->average_samples - 1) << ADC_AVGS_SHIFT;
+		adc_cfg |= (tsc->average_select) << ADC_AVGS_SHIFT;
 	}
 	adc_cfg &= ~ADC_HARDWARE_TRIGGER;
 	writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
@@ -132,7 +133,7 @@ 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)
+	if (tsc->average_enable)
 		adc_gc |= ADC_AVGE;
 	writel(adc_gc, tsc->adc_regs + REG_ADC_GC);
 
@@ -362,6 +363,7 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
 	int err;
 	int tsc_irq;
 	int adc_irq;
+	u32 average_samples;
 
 	tsc = devm_kzalloc(&pdev->dev, sizeof(*tsc), GFP_KERNEL);
 	if (!tsc)
@@ -466,14 +468,36 @@ 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);
+	err = of_property_read_u32(np, "touchscreen-average-samples",
+				   &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);
+		average_samples = 1;
+
+	switch (average_samples) {
+	case 1:
+		tsc->average_enable = false;
+		tsc->average_select = 0; /* value unused; initialize anyway */
+		break;
+	case 4:
+		tsc->average_enable = true;
+		tsc->average_select = 0;
+		break;
+	case 8:
+		tsc->average_enable = true;
+		tsc->average_select = 1;
+		break;
+	case 16:
+		tsc->average_enable = true;
+		tsc->average_select = 2;
+		break;
+	case 32:
+		tsc->average_enable = true;
+		tsc->average_select = 3;
+		break;
+	default:
+		dev_err(&pdev->dev,
+			"touchscreen-average-samples (%u) must be 1, 4, 8, 16 or 32\n",
+			average_samples);
 		return -EINVAL;
 	}
 
-- 
2.1.4

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

* Re: [PATCH] Input: imx6ul_tsc - generalize the averaging property
  2016-12-11  7:06 [PATCH] Input: imx6ul_tsc - generalize the averaging property Guy Shapiro
@ 2016-12-13 19:10 ` Rob Herring
  2016-12-13 19:14   ` Dmitry Torokhov
  2016-12-13 19:54 ` Rob Herring
  1 sibling, 1 reply; 6+ messages in thread
From: Rob Herring @ 2016-12-13 19:10 UTC (permalink / raw)
  To: Guy Shapiro
  Cc: dmitry.torokhov, fabio.estevam, mark.rutland, devicetree,
	haibo.chen, linux-input, linux-arm-kernel

On Sun, Dec 11, 2016 at 09:06:43AM +0200, Guy Shapiro wrote:
> Make the avarage-samples property a general touchscreen property
> rather than imx6ul device specific.
> 
> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
> ---
>  .../bindings/input/touchscreen/imx6ul_tsc.txt      | 11 ++----
>  .../bindings/input/touchscreen/touchscreen.txt     |  3 ++
>  drivers/input/touchscreen/imx6ul_tsc.c             | 46 ++++++++++++++++------
>  3 files changed, 41 insertions(+), 19 deletions(-)

You can't just switch existing bindings as that breaks compatibility 
with old dtbs. The kernel driver would need to support both. Just 
introduce the new common property and use it for your device.

Rob

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

* Re: [PATCH] Input: imx6ul_tsc - generalize the averaging property
  2016-12-13 19:10 ` Rob Herring
@ 2016-12-13 19:14   ` Dmitry Torokhov
  2016-12-13 19:48     ` Rob Herring
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2016-12-13 19:14 UTC (permalink / raw)
  To: Rob Herring, Guy Shapiro
  Cc: fabio.estevam, mark.rutland, devicetree, haibo.chen, linux-input,
	linux-arm-kernel

On December 13, 2016 11:10:50 AM PST, Rob Herring <robh@kernel.org> wrote:
>On Sun, Dec 11, 2016 at 09:06:43AM +0200, Guy Shapiro wrote:
>> Make the avarage-samples property a general touchscreen property
>> rather than imx6ul device specific.
>> 
>> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
>> ---
>>  .../bindings/input/touchscreen/imx6ul_tsc.txt      | 11 ++----
>>  .../bindings/input/touchscreen/touchscreen.txt     |  3 ++
>>  drivers/input/touchscreen/imx6ul_tsc.c             | 46
>++++++++++++++++------
>>  3 files changed, 41 insertions(+), 19 deletions(-)
>
>You can't just switch existing bindings as that breaks compatibility 
>with old dtbs. The kernel driver would need to support both. Just 
>introduce the new common property and use it for your device.
>

The old binding is only in my tree at the moment, so I do not think there is compatibility concerns.

Are you OK with the new binding itself?


Thanks.

-- 
Dmitry

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

* Re: [PATCH] Input: imx6ul_tsc - generalize the averaging property
  2016-12-13 19:14   ` Dmitry Torokhov
@ 2016-12-13 19:48     ` Rob Herring
  0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2016-12-13 19:48 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Fabio Estevam, Mark Rutland, Guy Shapiro,
	devicetree@vger.kernel.org, Haibo Chen,
	linux-input@vger.kernel.org, linux-arm-kernel@lists.infradead.org

On Tue, Dec 13, 2016 at 1:14 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On December 13, 2016 11:10:50 AM PST, Rob Herring <robh@kernel.org> wrote:
>>On Sun, Dec 11, 2016 at 09:06:43AM +0200, Guy Shapiro wrote:
>>> Make the avarage-samples property a general touchscreen property
>>> rather than imx6ul device specific.
>>>
>>> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
>>> ---
>>>  .../bindings/input/touchscreen/imx6ul_tsc.txt      | 11 ++----
>>>  .../bindings/input/touchscreen/touchscreen.txt     |  3 ++
>>>  drivers/input/touchscreen/imx6ul_tsc.c             | 46
>>++++++++++++++++------
>>>  3 files changed, 41 insertions(+), 19 deletions(-)
>>
>>You can't just switch existing bindings as that breaks compatibility
>>with old dtbs. The kernel driver would need to support both. Just
>>introduce the new common property and use it for your device.
>>
>
> The old binding is only in my tree at the moment, so I do not think there is compatibility concerns.
>
> Are you OK with the new binding itself?

Ah, then yes. For the binding:

Acked-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH] Input: imx6ul_tsc - generalize the averaging property
  2016-12-11  7:06 [PATCH] Input: imx6ul_tsc - generalize the averaging property Guy Shapiro
  2016-12-13 19:10 ` Rob Herring
@ 2016-12-13 19:54 ` Rob Herring
  2016-12-14  7:09   ` Guy Shapiro
  1 sibling, 1 reply; 6+ messages in thread
From: Rob Herring @ 2016-12-13 19:54 UTC (permalink / raw)
  To: Guy Shapiro
  Cc: Fabio Estevam, Mark Rutland, devicetree@vger.kernel.org,
	Haibo Chen, Dmitry Torokhov, linux-input@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org

On Sun, Dec 11, 2016 at 1:06 AM, Guy Shapiro <guy.shapiro@mobi-wize.com> wrote:
> Make the avarage-samples property a general touchscreen property
> rather than imx6ul device specific.
>
> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
> ---
>  .../bindings/input/touchscreen/imx6ul_tsc.txt      | 11 ++----
>  .../bindings/input/touchscreen/touchscreen.txt     |  3 ++
>  drivers/input/touchscreen/imx6ul_tsc.c             | 46 ++++++++++++++++------
>  3 files changed, 41 insertions(+), 19 deletions(-)

[...]

> +       switch (average_samples) {
> +       case 1:
> +               tsc->average_enable = false;
> +               tsc->average_select = 0; /* value unused; initialize anyway */
> +               break;
> +       case 4:
> +               tsc->average_enable = true;
> +               tsc->average_select = 0;
> +               break;
> +       case 8:
> +               tsc->average_enable = true;
> +               tsc->average_select = 1;
> +               break;
> +       case 16:
> +               tsc->average_enable = true;
> +               tsc->average_select = 2;
> +               break;
> +       case 32:
> +               tsc->average_enable = true;
> +               tsc->average_select = 3;
> +               break;

This could be more efficiently written as

tsc->average_select = log2(average_samples) - 2;

Then enable if >=0.

Rob

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

* Re: [PATCH] Input: imx6ul_tsc - generalize the averaging property
  2016-12-13 19:54 ` Rob Herring
@ 2016-12-14  7:09   ` Guy Shapiro
  0 siblings, 0 replies; 6+ messages in thread
From: Guy Shapiro @ 2016-12-14  7:09 UTC (permalink / raw)
  To: Rob Herring
  Cc: Fabio Estevam, Mark Rutland, devicetree@vger.kernel.org,
	Haibo Chen, Dmitry Torokhov, linux-input@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org

On 13/12/2016 21:54, Rob Herring wrote:

> On Sun, Dec 11, 2016 at 1:06 AM, Guy Shapiro <guy.shapiro@mobi-wize.com> wrote:
>> Make the avarage-samples property a general touchscreen property
>> rather than imx6ul device specific.
>>
>> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
>> ---
>>   .../bindings/input/touchscreen/imx6ul_tsc.txt      | 11 ++----
>>   .../bindings/input/touchscreen/touchscreen.txt     |  3 ++
>>   drivers/input/touchscreen/imx6ul_tsc.c             | 46 ++++++++++++++++------
>>   3 files changed, 41 insertions(+), 19 deletions(-)
> [...]
>
>> +       switch (average_samples) {
>> +       case 1:
>> +               tsc->average_enable = false;
>> +               tsc->average_select = 0; /* value unused; initialize anyway */
>> +               break;
>> +       case 4:
>> +               tsc->average_enable = true;
>> +               tsc->average_select = 0;
>> +               break;
>> +       case 8:
>> +               tsc->average_enable = true;
>> +               tsc->average_select = 1;
>> +               break;
>> +       case 16:
>> +               tsc->average_enable = true;
>> +               tsc->average_select = 2;
>> +               break;
>> +       case 32:
>> +               tsc->average_enable = true;
>> +               tsc->average_select = 3;
>> +               break;
> This could be more efficiently written as
>
> tsc->average_select = log2(average_samples) - 2;
>
> Then enable if >=0.

Using '1' to indicate no averaging is more consistent then using '0'.
I think it is better to validate the values rather then round them.

What do you think about:
+       switch (average_samples) {
+       case 1:
+               tsc->average_enable = false;
+               tsc->average_select = 0; /* value unused; initialize 
anyway */
+               break;
+       case 4:
+       case 8:
+       case 16:
+       case 32:
+               tsc->average_enable = true;
+               tsc->average_select = ilog2(average_samples) - 2;
+               break;
+       default:
+               dev_err(&pdev->dev,
+                       "touchscreen-average-samples (%u) must be 1, 4, 
8, 16 or 32\n",
+                       average_samples);

Guy.

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

end of thread, other threads:[~2016-12-14  7:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-11  7:06 [PATCH] Input: imx6ul_tsc - generalize the averaging property Guy Shapiro
2016-12-13 19:10 ` Rob Herring
2016-12-13 19:14   ` Dmitry Torokhov
2016-12-13 19:48     ` Rob Herring
2016-12-13 19:54 ` Rob Herring
2016-12-14  7:09   ` Guy Shapiro

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