All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Add DT support for tps6507x touchscreen
@ 2017-03-09 14:31 yegorslists
  2017-03-09 14:31 ` [PATCH v2 1/3] tps6507x-ts: update to devm_* API yegorslists
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: yegorslists @ 2017-03-09 14:31 UTC (permalink / raw)
  To: linux-input
  Cc: dmitry.torokhov, lee.jones, robh, mark.rutland, andrej.skvortzov,
	nsekhar, khilman, Yegor Yefremov

From: Yegor Yefremov <yegorslists@googlemail.com>

This is an attempt to revive DT support for TI tps6507x that was started
in 2013 [1], [2].

Changes since V2:
 - use devm_input_allocate_polled_device and remove tps6507x_ts_remove
 - device_property_read_* API to get poll_interval and min_pressure
 - remove id values from DT and make them static

Current changes:
 - remove tsc node
 - add /bits/ 16 to for 16-bit values
 - rework bindings description, i.e. all properties are optional

Histor of original changes:

Changes since V3:
 - Rebased on top of Dmitry's changes
 - Removed error handling for optional DT properties

Changes since V2:
 - Updated tps6507x documentation.
 - Removed unnecessary code.

Changes since V1:
 - Updated tps6507x documentation.
 - Updated commit message.
 - return proper error value in absence platform and DT data
   for touchscreen.

[1] https://www.spinics.net/lists/devicetree/msg09388.html
[2] https://patchwork.kernel.org/patch/2324441/

Yegor Yefremov (3):
  tps6507x-ts: update to devm_* API
  tps6507x-ts: add DT support
  tps6507x-ts: add DT bindings description

 Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++-
 drivers/input/touchscreen/tps6507x-ts.c            | 71 ++++++----------------
 2 files changed, 31 insertions(+), 56 deletions(-)

-- 
2.1.4


^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH v2 1/3] tps6507x-ts: update to devm_* API
  2017-03-09 14:31 [PATCH v2 0/3] Add DT support for tps6507x touchscreen yegorslists
@ 2017-03-09 14:31 ` yegorslists
  2017-03-11  0:29   ` Dmitry Torokhov
  2017-03-09 14:31 ` [PATCH v2 2/3] tps6507x-ts: add DT support yegorslists
  2017-03-09 14:31 ` [PATCH v2 3/3] tps6507x-ts: add DT bindings description yegorslists
  2 siblings, 1 reply; 15+ messages in thread
From: yegorslists @ 2017-03-09 14:31 UTC (permalink / raw)
  To: linux-input
  Cc: dmitry.torokhov, lee.jones, robh, mark.rutland, andrej.skvortzov,
	nsekhar, khilman, Yegor Yefremov

From: Yegor Yefremov <yegorslists@googlemail.com>

Update the code to use devm_* API so that driver
core will manage resources.

Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
 drivers/input/touchscreen/tps6507x-ts.c | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/input/touchscreen/tps6507x-ts.c b/drivers/input/touchscreen/tps6507x-ts.c
index a340bfc..fbaa2f68 100644
--- a/drivers/input/touchscreen/tps6507x-ts.c
+++ b/drivers/input/touchscreen/tps6507x-ts.c
@@ -226,7 +226,7 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
 	 */
 	init_data = tps_board->tps6507x_ts_init_data;
 
-	tsc = kzalloc(sizeof(struct tps6507x_ts), GFP_KERNEL);
+	tsc = devm_kzalloc(&pdev->dev, sizeof(struct tps6507x_ts), GFP_KERNEL);
 	if (!tsc) {
 		dev_err(tps6507x_dev->dev, "failed to allocate driver data\n");
 		return -ENOMEM;
@@ -240,11 +240,10 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
 	snprintf(tsc->phys, sizeof(tsc->phys),
 		 "%s/input0", dev_name(tsc->dev));
 
-	poll_dev = input_allocate_polled_device();
+	poll_dev = devm_input_allocate_polled_device(&pdev->dev);
 	if (!poll_dev) {
 		dev_err(tsc->dev, "Failed to allocate polled input device.\n");
-		error = -ENOMEM;
-		goto err_free_mem;
+		return -ENOMEM;
 	}
 
 	tsc->poll_dev = poll_dev;
@@ -274,34 +273,15 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
 
 	error = tps6507x_adc_standby(tsc);
 	if (error)
-		goto err_free_polled_dev;
+		return error;
 
 	error = input_register_polled_device(poll_dev);
 	if (error)
-		goto err_free_polled_dev;
+		return error;
 
 	platform_set_drvdata(pdev, tsc);
 
 	return 0;
-
-err_free_polled_dev:
-	input_free_polled_device(poll_dev);
-err_free_mem:
-	kfree(tsc);
-	return error;
-}
-
-static int tps6507x_ts_remove(struct platform_device *pdev)
-{
-	struct tps6507x_ts *tsc = platform_get_drvdata(pdev);
-	struct input_polled_dev *poll_dev = tsc->poll_dev;
-
-	input_unregister_polled_device(poll_dev);
-	input_free_polled_device(poll_dev);
-
-	kfree(tsc);
-
-	return 0;
 }
 
 static struct platform_driver tps6507x_ts_driver = {
@@ -309,7 +289,6 @@ static struct platform_driver tps6507x_ts_driver = {
 		.name = "tps6507x-ts",
 	},
 	.probe = tps6507x_ts_probe,
-	.remove = tps6507x_ts_remove,
 };
 module_platform_driver(tps6507x_ts_driver);
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH v2 2/3] tps6507x-ts: add DT support
  2017-03-09 14:31 [PATCH v2 0/3] Add DT support for tps6507x touchscreen yegorslists
  2017-03-09 14:31 ` [PATCH v2 1/3] tps6507x-ts: update to devm_* API yegorslists
@ 2017-03-09 14:31 ` yegorslists
  2017-03-09 15:02   ` Yegor Yefremov
  2017-03-09 14:31 ` [PATCH v2 3/3] tps6507x-ts: add DT bindings description yegorslists
  2 siblings, 1 reply; 15+ messages in thread
From: yegorslists @ 2017-03-09 14:31 UTC (permalink / raw)
  To: linux-input
  Cc: dmitry.torokhov, lee.jones, robh, mark.rutland, andrej.skvortzov,
	nsekhar, khilman, Yegor Yefremov

From: Yegor Yefremov <yegorslists@googlemail.com>

This patch adds support for DT to tps6507x-ts driver.

Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
 drivers/input/touchscreen/tps6507x-ts.c | 40 ++++++++++-----------------------
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/input/touchscreen/tps6507x-ts.c b/drivers/input/touchscreen/tps6507x-ts.c
index fbaa2f68..502ed0c 100644
--- a/drivers/input/touchscreen/tps6507x-ts.c
+++ b/drivers/input/touchscreen/tps6507x-ts.c
@@ -22,6 +22,8 @@
 #include <linux/mfd/tps6507x.h>
 #include <linux/input/tps6507x-ts.h>
 #include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #define TSC_DEFAULT_POLL_PERIOD 30 /* ms */
 #define TPS_DEFAULT_MIN_PRESSURE 0x30
@@ -202,30 +204,11 @@ static void tps6507x_ts_poll(struct input_polled_dev *poll_dev)
 static int tps6507x_ts_probe(struct platform_device *pdev)
 {
 	struct tps6507x_dev *tps6507x_dev = dev_get_drvdata(pdev->dev.parent);
-	const struct tps6507x_board *tps_board;
-	const struct touchscreen_init_data *init_data;
 	struct tps6507x_ts *tsc;
 	struct input_polled_dev *poll_dev;
 	struct input_dev *input_dev;
 	int error;
 
-	/*
-	 * tps_board points to pmic related constants
-	 * coming from the board-evm file.
-	 */
-	tps_board = dev_get_platdata(tps6507x_dev->dev);
-	if (!tps_board) {
-		dev_err(tps6507x_dev->dev,
-			"Could not find tps6507x platform data\n");
-		return -ENODEV;
-	}
-
-	/*
-	 * init_data points to array of regulator_init structures
-	 * coming from the board-evm file.
-	 */
-	init_data = tps_board->tps6507x_ts_init_data;
-
 	tsc = devm_kzalloc(&pdev->dev, sizeof(struct tps6507x_ts), GFP_KERNEL);
 	if (!tsc) {
 		dev_err(tps6507x_dev->dev, "failed to allocate driver data\n");
@@ -234,8 +217,6 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
 
 	tsc->mfd = tps6507x_dev;
 	tsc->dev = tps6507x_dev->dev;
-	tsc->min_pressure = init_data ?
-			init_data->min_pressure : TPS_DEFAULT_MIN_PRESSURE;
 
 	snprintf(tsc->phys, sizeof(tsc->phys),
 		 "%s/input0", dev_name(tsc->dev));
@@ -250,8 +231,6 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
 
 	poll_dev->private = tsc;
 	poll_dev->poll = tps6507x_ts_poll;
-	poll_dev->poll_interval = init_data ?
-			init_data->poll_period : TSC_DEFAULT_POLL_PERIOD;
 
 	input_dev = poll_dev->input;
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
@@ -265,11 +244,16 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
 	input_dev->phys = tsc->phys;
 	input_dev->dev.parent = tsc->dev;
 	input_dev->id.bustype = BUS_I2C;
-	if (init_data) {
-		input_dev->id.vendor = init_data->vendor;
-		input_dev->id.product = init_data->product;
-		input_dev->id.version = init_data->version;
-	}
+	input_dev->id.vendor = 0;
+	input_dev->id.product = 65070;
+	input_dev->id.version = 0x100;
+
+	tsc->poll_dev->poll_interval = TSC_DEFAULT_POLL_PERIOD;
+	tsc->min_pressure = TPS_DEFAULT_MIN_PRESSURE;
+	device_property_read_u32_array(tsc->dev, "ti,poll-period",
+				       &tsc->poll_dev->poll_interval, 1);
+	device_property_read_u16_array(tsc->dev, "ti,min-pressure",
+				       &tsc->min_pressure, 1);
 
 	error = tps6507x_adc_standby(tsc);
 	if (error)
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH v2 3/3] tps6507x-ts: add DT bindings description
  2017-03-09 14:31 [PATCH v2 0/3] Add DT support for tps6507x touchscreen yegorslists
  2017-03-09 14:31 ` [PATCH v2 1/3] tps6507x-ts: update to devm_* API yegorslists
  2017-03-09 14:31 ` [PATCH v2 2/3] tps6507x-ts: add DT support yegorslists
@ 2017-03-09 14:31 ` yegorslists
  2017-03-09 14:49   ` Rob Herring
  2 siblings, 1 reply; 15+ messages in thread
From: yegorslists @ 2017-03-09 14:31 UTC (permalink / raw)
  To: linux-input
  Cc: dmitry.torokhov, lee.jones, robh, mark.rutland, andrej.skvortzov,
	nsekhar, khilman, Yegor Yefremov

From: Yegor Yefremov <yegorslists@googlemail.com>

Provide description for following properties:

- ti,poll-period
- ti,min-pressure

Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
 Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
index 8fffa3c..8875662 100644
--- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
+++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
@@ -1,4 +1,8 @@
-TPS6507x Power Management Integrated Circuit
+TPS6507x Multifunctional Device.
+
+Features provided by TPS6507x:
+        1. Power Management Integrated Circuit.
+        2. Touch-Screen.
 
 Required properties:
 - compatible: "ti,tps6507x"
@@ -30,6 +34,12 @@ Regulator Optional properties:
 			1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
   If this property is not defined, it defaults to 0 (not enabled).
 
+Touchscreen Optional properties:
+- ti,poll-period: Time at which touch input is getting sampled in ms.
+		  Default value: 30 ms.
+- ti,min-pressure: Minimum pressure value to trigger touch.
+		   Default value: 0x30.
+
 Example:
 
 	pmu: tps6507x@48 {
@@ -40,6 +50,9 @@ Example:
 		vindcdc3-supply = <...>;
 		vinldo1_2-supply = <...>;
 
+		ti,poll-period = <30>;
+		ti,min-pressure = <0x30>;
+
 		regulators {
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -87,5 +100,4 @@ Example:
 				regulator-boot-on;
 			};
 		};
-
 	};
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] tps6507x-ts: add DT bindings description
  2017-03-09 14:31 ` [PATCH v2 3/3] tps6507x-ts: add DT bindings description yegorslists
@ 2017-03-09 14:49   ` Rob Herring
  2017-03-09 15:18     ` Yegor Yefremov
  0 siblings, 1 reply; 15+ messages in thread
From: Rob Herring @ 2017-03-09 14:49 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: linux-input, Dmitry Torokhov, Lee Jones, Mark Rutland,
	Andrey Skvortsov, Sekhar Nori, Kevin Hilman

On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>

This needs to go to DT list.

>
> Provide description for following properties:
>
> - ti,poll-period
> - ti,min-pressure
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> index 8fffa3c..8875662 100644
> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> @@ -1,4 +1,8 @@
> -TPS6507x Power Management Integrated Circuit
> +TPS6507x Multifunctional Device.
> +
> +Features provided by TPS6507x:
> +        1. Power Management Integrated Circuit.
> +        2. Touch-Screen.
>
>  Required properties:
>  - compatible: "ti,tps6507x"
> @@ -30,6 +34,12 @@ Regulator Optional properties:
>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>    If this property is not defined, it defaults to 0 (not enabled).
>
> +Touchscreen Optional properties:
> +- ti,poll-period: Time at which touch input is getting sampled in ms.
> +                 Default value: 30 ms.

Isn't there a standard property for this?

> +- ti,min-pressure: Minimum pressure value to trigger touch.
> +                  Default value: 0x30.
> +
>  Example:
>
>         pmu: tps6507x@48 {
> @@ -40,6 +50,9 @@ Example:
>                 vindcdc3-supply = <...>;
>                 vinldo1_2-supply = <...>;
>
> +               ti,poll-period = <30>;
> +               ti,min-pressure = <0x30>;
> +
>                 regulators {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> @@ -87,5 +100,4 @@ Example:
>                                 regulator-boot-on;
>                         };
>                 };
> -
>         };
> --
> 2.1.4
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/3] tps6507x-ts: add DT support
  2017-03-09 14:31 ` [PATCH v2 2/3] tps6507x-ts: add DT support yegorslists
@ 2017-03-09 15:02   ` Yegor Yefremov
  0 siblings, 0 replies; 15+ messages in thread
From: Yegor Yefremov @ 2017-03-09 15:02 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Lee Jones, Rob Herring, mark.rutland,
	Andrey Skvortsov, Nori, Sekhar, Kevin Hilman, Yegor Yefremov

Hi Dmitry, Sekhar,

On Thu, Mar 9, 2017 at 3:31 PM,  <yegorslists@googlemail.com> wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
>
> This patch adds support for DT to tps6507x-ts driver.
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
>  drivers/input/touchscreen/tps6507x-ts.c | 40 ++++++++++-----------------------
>  1 file changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/input/touchscreen/tps6507x-ts.c b/drivers/input/touchscreen/tps6507x-ts.c
> index fbaa2f68..502ed0c 100644
> --- a/drivers/input/touchscreen/tps6507x-ts.c
> +++ b/drivers/input/touchscreen/tps6507x-ts.c
> @@ -22,6 +22,8 @@
>  #include <linux/mfd/tps6507x.h>
>  #include <linux/input/tps6507x-ts.h>
>  #include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>
>  #define TSC_DEFAULT_POLL_PERIOD 30 /* ms */
>  #define TPS_DEFAULT_MIN_PRESSURE 0x30
> @@ -202,30 +204,11 @@ static void tps6507x_ts_poll(struct input_polled_dev *poll_dev)
>  static int tps6507x_ts_probe(struct platform_device *pdev)
>  {
>         struct tps6507x_dev *tps6507x_dev = dev_get_drvdata(pdev->dev.parent);
> -       const struct tps6507x_board *tps_board;
> -       const struct touchscreen_init_data *init_data;
>         struct tps6507x_ts *tsc;
>         struct input_polled_dev *poll_dev;
>         struct input_dev *input_dev;
>         int error;
>
> -       /*
> -        * tps_board points to pmic related constants
> -        * coming from the board-evm file.
> -        */
> -       tps_board = dev_get_platdata(tps6507x_dev->dev);
> -       if (!tps_board) {
> -               dev_err(tps6507x_dev->dev,
> -                       "Could not find tps6507x platform data\n");
> -               return -ENODEV;
> -       }
> -
> -       /*
> -        * init_data points to array of regulator_init structures
> -        * coming from the board-evm file.
> -        */
> -       init_data = tps_board->tps6507x_ts_init_data;
> -
>         tsc = devm_kzalloc(&pdev->dev, sizeof(struct tps6507x_ts), GFP_KERNEL);
>         if (!tsc) {
>                 dev_err(tps6507x_dev->dev, "failed to allocate driver data\n");
> @@ -234,8 +217,6 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
>
>         tsc->mfd = tps6507x_dev;
>         tsc->dev = tps6507x_dev->dev;
> -       tsc->min_pressure = init_data ?
> -                       init_data->min_pressure : TPS_DEFAULT_MIN_PRESSURE;
>
>         snprintf(tsc->phys, sizeof(tsc->phys),
>                  "%s/input0", dev_name(tsc->dev));
> @@ -250,8 +231,6 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
>
>         poll_dev->private = tsc;
>         poll_dev->poll = tps6507x_ts_poll;
> -       poll_dev->poll_interval = init_data ?
> -                       init_data->poll_period : TSC_DEFAULT_POLL_PERIOD;
>
>         input_dev = poll_dev->input;
>         input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> @@ -265,11 +244,16 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
>         input_dev->phys = tsc->phys;
>         input_dev->dev.parent = tsc->dev;
>         input_dev->id.bustype = BUS_I2C;
> -       if (init_data) {
> -               input_dev->id.vendor = init_data->vendor;
> -               input_dev->id.product = init_data->product;
> -               input_dev->id.version = init_data->version;
> -       }
> +       input_dev->id.vendor = 0;
> +       input_dev->id.product = 65070;
> +       input_dev->id.version = 0x100;

I've left the data as is to preserve current behavior until we decide
on what exact IDs to take.

> +       tsc->poll_dev->poll_interval = TSC_DEFAULT_POLL_PERIOD;
> +       tsc->min_pressure = TPS_DEFAULT_MIN_PRESSURE;
> +       device_property_read_u32_array(tsc->dev, "ti,poll-period",
> +                                      &tsc->poll_dev->poll_interval, 1);
> +       device_property_read_u16_array(tsc->dev, "ti,min-pressure",
> +                                      &tsc->min_pressure, 1);

This is working for DT, but I have problems with converting
arch/arm/mach-davinci/board-da850-evm.c

tps is a i2c device. How do I obtain struct device *dev from i2c_board?

Yegor

>         error = tps6507x_adc_standby(tsc);
>         if (error)
> --
> 2.1.4
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] tps6507x-ts: add DT bindings description
  2017-03-09 14:49   ` Rob Herring
@ 2017-03-09 15:18     ` Yegor Yefremov
  2017-03-09 17:55       ` Dmitry Torokhov
  0 siblings, 1 reply; 15+ messages in thread
From: Yegor Yefremov @ 2017-03-09 15:18 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-input, Dmitry Torokhov, Lee Jones, Mark Rutland,
	Andrey Skvortsov, Sekhar Nori, Kevin Hilman

On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
>> From: Yegor Yefremov <yegorslists@googlemail.com>
>
> This needs to go to DT list.

Will do.

>>
>> Provide description for following properties:
>>
>> - ti,poll-period
>> - ti,min-pressure
>>
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> index 8fffa3c..8875662 100644
>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> @@ -1,4 +1,8 @@
>> -TPS6507x Power Management Integrated Circuit
>> +TPS6507x Multifunctional Device.
>> +
>> +Features provided by TPS6507x:
>> +        1. Power Management Integrated Circuit.
>> +        2. Touch-Screen.
>>
>>  Required properties:
>>  - compatible: "ti,tps6507x"
>> @@ -30,6 +34,12 @@ Regulator Optional properties:
>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>>    If this property is not defined, it defaults to 0 (not enabled).
>>
>> +Touchscreen Optional properties:
>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
>> +                 Default value: 30 ms.
>
> Isn't there a standard property for this?

Such a value will be already used in [1]. I've looked at [2], but it
doesn't have such a field.

[1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
[2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

>> +- ti,min-pressure: Minimum pressure value to trigger touch.
>> +                  Default value: 0x30.
>> +
>>  Example:
>>
>>         pmu: tps6507x@48 {
>> @@ -40,6 +50,9 @@ Example:
>>                 vindcdc3-supply = <...>;
>>                 vinldo1_2-supply = <...>;
>>
>> +               ti,poll-period = <30>;
>> +               ti,min-pressure = <0x30>;
>> +
>>                 regulators {
>>                         #address-cells = <1>;
>>                         #size-cells = <0>;
>> @@ -87,5 +100,4 @@ Example:
>>                                 regulator-boot-on;
>>                         };
>>                 };
>> -
>>         };
>> --
>> 2.1.4
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] tps6507x-ts: add DT bindings description
  2017-03-09 15:18     ` Yegor Yefremov
@ 2017-03-09 17:55       ` Dmitry Torokhov
  2017-03-10  6:41         ` Sekhar Nori
  2017-03-10 16:26         ` Rob Herring
  0 siblings, 2 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2017-03-09 17:55 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: Rob Herring, linux-input, Lee Jones, Mark Rutland,
	Andrey Skvortsov, Sekhar Nori, Kevin Hilman

On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
> > On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
> >> From: Yegor Yefremov <yegorslists@googlemail.com>
> >
> > This needs to go to DT list.
> 
> Will do.
> 
> >>
> >> Provide description for following properties:
> >>
> >> - ti,poll-period
> >> - ti,min-pressure
> >>
> >> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> >> ---
> >>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
> >>  1 file changed, 14 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> index 8fffa3c..8875662 100644
> >> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> @@ -1,4 +1,8 @@
> >> -TPS6507x Power Management Integrated Circuit
> >> +TPS6507x Multifunctional Device.
> >> +
> >> +Features provided by TPS6507x:
> >> +        1. Power Management Integrated Circuit.
> >> +        2. Touch-Screen.
> >>
> >>  Required properties:
> >>  - compatible: "ti,tps6507x"
> >> @@ -30,6 +34,12 @@ Regulator Optional properties:
> >>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
> >>    If this property is not defined, it defaults to 0 (not enabled).
> >>
> >> +Touchscreen Optional properties:
> >> +- ti,poll-period: Time at which touch input is getting sampled in ms.
> >> +                 Default value: 30 ms.
> >
> > Isn't there a standard property for this?
> 
> Such a value will be already used in [1]. I've looked at [2], but it
> doesn't have such a field.
> 
> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

I think the bigger question (since we are adding this to DT I assume it
is not longer "toy" driver) can we convert it to a proper
interrupt-driven scheme? Polling for real shipping devices is not
efficient, I do not think we should even try to allow this mode in DTS
bindings (note that tsc2007 use is different: it allows to specify
sampling interval once touch is detected).

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] tps6507x-ts: add DT bindings description
  2017-03-09 17:55       ` Dmitry Torokhov
@ 2017-03-10  6:41         ` Sekhar Nori
  2017-03-10 18:00           ` Dmitry Torokhov
  2017-03-10 16:26         ` Rob Herring
  1 sibling, 1 reply; 15+ messages in thread
From: Sekhar Nori @ 2017-03-10  6:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Yegor Yefremov
  Cc: Rob Herring, linux-input, Lee Jones, Mark Rutland,
	Andrey Skvortsov, Kevin Hilman

On Thursday 09 March 2017 11:25 PM, Dmitry Torokhov wrote:
> On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
>> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
>>> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
>>>> From: Yegor Yefremov <yegorslists@googlemail.com>
>>>
>>> This needs to go to DT list.
>>
>> Will do.
>>
>>>>
>>>> Provide description for following properties:
>>>>
>>>> - ti,poll-period
>>>> - ti,min-pressure
>>>>
>>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>>>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>> index 8fffa3c..8875662 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>> @@ -1,4 +1,8 @@
>>>> -TPS6507x Power Management Integrated Circuit
>>>> +TPS6507x Multifunctional Device.
>>>> +
>>>> +Features provided by TPS6507x:
>>>> +        1. Power Management Integrated Circuit.
>>>> +        2. Touch-Screen.
>>>>
>>>>  Required properties:
>>>>  - compatible: "ti,tps6507x"
>>>> @@ -30,6 +34,12 @@ Regulator Optional properties:
>>>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>>>>    If this property is not defined, it defaults to 0 (not enabled).
>>>>
>>>> +Touchscreen Optional properties:
>>>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
>>>> +                 Default value: 30 ms.
>>>
>>> Isn't there a standard property for this?
>>
>> Such a value will be already used in [1]. I've looked at [2], but it
>> doesn't have such a field.
>>
>> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
>> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> 
> I think the bigger question (since we are adding this to DT I assume it
> is not longer "toy" driver) can we convert it to a proper
> interrupt-driven scheme? Polling for real shipping devices is not
> efficient, I do not think we should even try to allow this mode in DTS
> bindings (note that tsc2007 use is different: it allows to specify
> sampling interval once touch is detected).

Unfortunately, the only current user of tps6507x in kernel (da850-evm)
does not have the touch interrupt connected to the processor. So if we
dont support polled mode in when using device-tree, da850-evm.dts wont
be able to use this driver.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] tps6507x-ts: add DT bindings description
  2017-03-09 17:55       ` Dmitry Torokhov
  2017-03-10  6:41         ` Sekhar Nori
@ 2017-03-10 16:26         ` Rob Herring
  1 sibling, 0 replies; 15+ messages in thread
From: Rob Herring @ 2017-03-10 16:26 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Yegor Yefremov, linux-input, Lee Jones, Mark Rutland,
	Andrey Skvortsov, Sekhar Nori, Kevin Hilman

On Thu, Mar 9, 2017 at 11:55 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
>> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
>> > On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
>> >> From: Yegor Yefremov <yegorslists@googlemail.com>
>> >
>> > This needs to go to DT list.
>>
>> Will do.
>>
>> >>
>> >> Provide description for following properties:
>> >>
>> >> - ti,poll-period
>> >> - ti,min-pressure
>> >>
>> >> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> >> ---
>> >>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>> >>  1 file changed, 14 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >> index 8fffa3c..8875662 100644
>> >> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >> @@ -1,4 +1,8 @@
>> >> -TPS6507x Power Management Integrated Circuit
>> >> +TPS6507x Multifunctional Device.
>> >> +
>> >> +Features provided by TPS6507x:
>> >> +        1. Power Management Integrated Circuit.
>> >> +        2. Touch-Screen.
>> >>
>> >>  Required properties:
>> >>  - compatible: "ti,tps6507x"
>> >> @@ -30,6 +34,12 @@ Regulator Optional properties:
>> >>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>> >>    If this property is not defined, it defaults to 0 (not enabled).
>> >>
>> >> +Touchscreen Optional properties:
>> >> +- ti,poll-period: Time at which touch input is getting sampled in ms.
>> >> +                 Default value: 30 ms.
>> >
>> > Isn't there a standard property for this?
>>
>> Such a value will be already used in [1]. I've looked at [2], but it
>> doesn't have such a field.
>>
>> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
>> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>
> I think the bigger question (since we are adding this to DT I assume it
> is not longer "toy" driver) can we convert it to a proper
> interrupt-driven scheme? Polling for real shipping devices is not
> efficient, I do not think we should even try to allow this mode in DTS
> bindings (note that tsc2007 use is different: it allows to specify
> sampling interval once touch is detected).

I guess I was assuming that polling period corresponded to sampling
period or rate. The latter makes more sense for a binding and I would
think is fairly common whether the h/w has functionality or the driver
has to implement the sampling rate in s/w.

Rob

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] tps6507x-ts: add DT bindings description
  2017-03-10  6:41         ` Sekhar Nori
@ 2017-03-10 18:00           ` Dmitry Torokhov
  2017-03-10 19:54             ` Yegor Yefremov
  0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2017-03-10 18:00 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Yegor Yefremov, Rob Herring, linux-input, Lee Jones,
	Mark Rutland, Andrey Skvortsov, Kevin Hilman

On Fri, Mar 10, 2017 at 12:11:08PM +0530, Sekhar Nori wrote:
> On Thursday 09 March 2017 11:25 PM, Dmitry Torokhov wrote:
> > On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
> >> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
> >>> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
> >>>> From: Yegor Yefremov <yegorslists@googlemail.com>
> >>>
> >>> This needs to go to DT list.
> >>
> >> Will do.
> >>
> >>>>
> >>>> Provide description for following properties:
> >>>>
> >>>> - ti,poll-period
> >>>> - ti,min-pressure
> >>>>
> >>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
> >>>>  1 file changed, 14 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >>>> index 8fffa3c..8875662 100644
> >>>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >>>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >>>> @@ -1,4 +1,8 @@
> >>>> -TPS6507x Power Management Integrated Circuit
> >>>> +TPS6507x Multifunctional Device.
> >>>> +
> >>>> +Features provided by TPS6507x:
> >>>> +        1. Power Management Integrated Circuit.
> >>>> +        2. Touch-Screen.
> >>>>
> >>>>  Required properties:
> >>>>  - compatible: "ti,tps6507x"
> >>>> @@ -30,6 +34,12 @@ Regulator Optional properties:
> >>>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
> >>>>    If this property is not defined, it defaults to 0 (not enabled).
> >>>>
> >>>> +Touchscreen Optional properties:
> >>>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
> >>>> +                 Default value: 30 ms.
> >>>
> >>> Isn't there a standard property for this?
> >>
> >> Such a value will be already used in [1]. I've looked at [2], but it
> >> doesn't have such a field.
> >>
> >> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> >> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> > 
> > I think the bigger question (since we are adding this to DT I assume it
> > is not longer "toy" driver) can we convert it to a proper
> > interrupt-driven scheme? Polling for real shipping devices is not
> > efficient, I do not think we should even try to allow this mode in DTS
> > bindings (note that tsc2007 use is different: it allows to specify
> > sampling interval once touch is detected).
> 
> Unfortunately, the only current user of tps6507x in kernel (da850-evm)
> does not have the touch interrupt connected to the processor. So if we
> dont support polled mode in when using device-tree, da850-evm.dts wont
> be able to use this driver.

Do we care? Right now the driver is unusable for any practical
application I think as it burns CPU cycles by constantly polling the
touch controller.

Yegor, your conversion to device tree: is it done because you have a
real product using this?

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] tps6507x-ts: add DT bindings description
  2017-03-10 18:00           ` Dmitry Torokhov
@ 2017-03-10 19:54             ` Yegor Yefremov
  2017-03-10 20:08               ` Dmitry Torokhov
  0 siblings, 1 reply; 15+ messages in thread
From: Yegor Yefremov @ 2017-03-10 19:54 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Sekhar Nori, Rob Herring, linux-input, Lee Jones, Mark Rutland,
	Andrey Skvortsov, Kevin Hilman

On Fri, Mar 10, 2017 at 7:00 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Mar 10, 2017 at 12:11:08PM +0530, Sekhar Nori wrote:
>> On Thursday 09 March 2017 11:25 PM, Dmitry Torokhov wrote:
>> > On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
>> >> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
>> >>> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
>> >>>> From: Yegor Yefremov <yegorslists@googlemail.com>
>> >>>
>> >>> This needs to go to DT list.
>> >>
>> >> Will do.
>> >>
>> >>>>
>> >>>> Provide description for following properties:
>> >>>>
>> >>>> - ti,poll-period
>> >>>> - ti,min-pressure
>> >>>>
>> >>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> >>>> ---
>> >>>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>> >>>>  1 file changed, 14 insertions(+), 2 deletions(-)
>> >>>>
>> >>>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >>>> index 8fffa3c..8875662 100644
>> >>>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >>>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >>>> @@ -1,4 +1,8 @@
>> >>>> -TPS6507x Power Management Integrated Circuit
>> >>>> +TPS6507x Multifunctional Device.
>> >>>> +
>> >>>> +Features provided by TPS6507x:
>> >>>> +        1. Power Management Integrated Circuit.
>> >>>> +        2. Touch-Screen.
>> >>>>
>> >>>>  Required properties:
>> >>>>  - compatible: "ti,tps6507x"
>> >>>> @@ -30,6 +34,12 @@ Regulator Optional properties:
>> >>>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>> >>>>    If this property is not defined, it defaults to 0 (not enabled).
>> >>>>
>> >>>> +Touchscreen Optional properties:
>> >>>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
>> >>>> +                 Default value: 30 ms.
>> >>>
>> >>> Isn't there a standard property for this?
>> >>
>> >> Such a value will be already used in [1]. I've looked at [2], but it
>> >> doesn't have such a field.
>> >>
>> >> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
>> >> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>> >
>> > I think the bigger question (since we are adding this to DT I assume it
>> > is not longer "toy" driver) can we convert it to a proper
>> > interrupt-driven scheme? Polling for real shipping devices is not
>> > efficient, I do not think we should even try to allow this mode in DTS
>> > bindings (note that tsc2007 use is different: it allows to specify
>> > sampling interval once touch is detected).
>>
>> Unfortunately, the only current user of tps6507x in kernel (da850-evm)
>> does not have the touch interrupt connected to the processor. So if we
>> dont support polled mode in when using device-tree, da850-evm.dts wont
>> be able to use this driver.
>
> Do we care? Right now the driver is unusable for any practical
> application I think as it burns CPU cycles by constantly polling the
> touch controller.
>
> Yegor, your conversion to device tree: is it done because you have a
> real product using this?

Yes. It is VS-860 device http://www.visionsystems.de/produkte/vs-860.html

It even seems to have interrupt pin connected. So we have to support both modes.

Yegor

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] tps6507x-ts: add DT bindings description
  2017-03-10 19:54             ` Yegor Yefremov
@ 2017-03-10 20:08               ` Dmitry Torokhov
  2017-03-13 14:15                 ` Sekhar Nori
  0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2017-03-10 20:08 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: Sekhar Nori, Rob Herring, linux-input, Lee Jones, Mark Rutland,
	Andrey Skvortsov, Kevin Hilman

On Fri, Mar 10, 2017 at 08:54:51PM +0100, Yegor Yefremov wrote:
> On Fri, Mar 10, 2017 at 7:00 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Fri, Mar 10, 2017 at 12:11:08PM +0530, Sekhar Nori wrote:
> >> On Thursday 09 March 2017 11:25 PM, Dmitry Torokhov wrote:
> >> > On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
> >> >> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
> >> >>> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
> >> >>>> From: Yegor Yefremov <yegorslists@googlemail.com>
> >> >>>
> >> >>> This needs to go to DT list.
> >> >>
> >> >> Will do.
> >> >>
> >> >>>>
> >> >>>> Provide description for following properties:
> >> >>>>
> >> >>>> - ti,poll-period
> >> >>>> - ti,min-pressure
> >> >>>>
> >> >>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> >> >>>> ---
> >> >>>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
> >> >>>>  1 file changed, 14 insertions(+), 2 deletions(-)
> >> >>>>
> >> >>>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> >>>> index 8fffa3c..8875662 100644
> >> >>>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> >>>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> >>>> @@ -1,4 +1,8 @@
> >> >>>> -TPS6507x Power Management Integrated Circuit
> >> >>>> +TPS6507x Multifunctional Device.
> >> >>>> +
> >> >>>> +Features provided by TPS6507x:
> >> >>>> +        1. Power Management Integrated Circuit.
> >> >>>> +        2. Touch-Screen.
> >> >>>>
> >> >>>>  Required properties:
> >> >>>>  - compatible: "ti,tps6507x"
> >> >>>> @@ -30,6 +34,12 @@ Regulator Optional properties:
> >> >>>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
> >> >>>>    If this property is not defined, it defaults to 0 (not enabled).
> >> >>>>
> >> >>>> +Touchscreen Optional properties:
> >> >>>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
> >> >>>> +                 Default value: 30 ms.
> >> >>>
> >> >>> Isn't there a standard property for this?
> >> >>
> >> >> Such a value will be already used in [1]. I've looked at [2], but it
> >> >> doesn't have such a field.
> >> >>
> >> >> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> >> >> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> >> >
> >> > I think the bigger question (since we are adding this to DT I assume it
> >> > is not longer "toy" driver) can we convert it to a proper
> >> > interrupt-driven scheme? Polling for real shipping devices is not
> >> > efficient, I do not think we should even try to allow this mode in DTS
> >> > bindings (note that tsc2007 use is different: it allows to specify
> >> > sampling interval once touch is detected).
> >>
> >> Unfortunately, the only current user of tps6507x in kernel (da850-evm)
> >> does not have the touch interrupt connected to the processor. So if we
> >> dont support polled mode in when using device-tree, da850-evm.dts wont
> >> be able to use this driver.
> >
> > Do we care? Right now the driver is unusable for any practical
> > application I think as it burns CPU cycles by constantly polling the
> > touch controller.
> >
> > Yegor, your conversion to device tree: is it done because you have a
> > real product using this?
> 
> Yes. It is VS-860 device http://www.visionsystems.de/produkte/vs-860.html
> 
> It even seems to have interrupt pin connected. So we have to support both modes.

Well, I'd rather supported interrupt only. Polling is a toy mode.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] tps6507x-ts: update to devm_* API
  2017-03-09 14:31 ` [PATCH v2 1/3] tps6507x-ts: update to devm_* API yegorslists
@ 2017-03-11  0:29   ` Dmitry Torokhov
  0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2017-03-11  0:29 UTC (permalink / raw)
  To: yegorslists
  Cc: linux-input, lee.jones, robh, mark.rutland, andrej.skvortzov,
	nsekhar, khilman

On Thu, Mar 09, 2017 at 03:31:07PM +0100, yegorslists@googlemail.com wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
> 
> Update the code to use devm_* API so that driver
> core will manage resources.
> 
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>


Applied, thank you. We can continue discussing DT conversion, but this
patch is good as is.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] tps6507x-ts: add DT bindings description
  2017-03-10 20:08               ` Dmitry Torokhov
@ 2017-03-13 14:15                 ` Sekhar Nori
  0 siblings, 0 replies; 15+ messages in thread
From: Sekhar Nori @ 2017-03-13 14:15 UTC (permalink / raw)
  To: Dmitry Torokhov, Yegor Yefremov
  Cc: Rob Herring, linux-input, Lee Jones, Mark Rutland,
	Andrey Skvortsov, Kevin Hilman

Hi Dmitry,

On Saturday 11 March 2017 01:38 AM, Dmitry Torokhov wrote:
> On Fri, Mar 10, 2017 at 08:54:51PM +0100, Yegor Yefremov wrote:
>> On Fri, Mar 10, 2017 at 7:00 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>>> On Fri, Mar 10, 2017 at 12:11:08PM +0530, Sekhar Nori wrote:
>>>> On Thursday 09 March 2017 11:25 PM, Dmitry Torokhov wrote:
>>>>> On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
>>>>>> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
>>>>>>> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
>>>>>>>> From: Yegor Yefremov <yegorslists@googlemail.com>
>>>>>>>
>>>>>>> This needs to go to DT list.
>>>>>>
>>>>>> Will do.
>>>>>>
>>>>>>>>
>>>>>>>> Provide description for following properties:
>>>>>>>>
>>>>>>>> - ti,poll-period
>>>>>>>> - ti,min-pressure
>>>>>>>>
>>>>>>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>>>>>>>> ---
>>>>>>>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>>>>>>>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>>>>>> index 8fffa3c..8875662 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>>>>>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>>>>>> @@ -1,4 +1,8 @@
>>>>>>>> -TPS6507x Power Management Integrated Circuit
>>>>>>>> +TPS6507x Multifunctional Device.
>>>>>>>> +
>>>>>>>> +Features provided by TPS6507x:
>>>>>>>> +        1. Power Management Integrated Circuit.
>>>>>>>> +        2. Touch-Screen.
>>>>>>>>
>>>>>>>>  Required properties:
>>>>>>>>  - compatible: "ti,tps6507x"
>>>>>>>> @@ -30,6 +34,12 @@ Regulator Optional properties:
>>>>>>>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>>>>>>>>    If this property is not defined, it defaults to 0 (not enabled).
>>>>>>>>
>>>>>>>> +Touchscreen Optional properties:
>>>>>>>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
>>>>>>>> +                 Default value: 30 ms.
>>>>>>>
>>>>>>> Isn't there a standard property for this?
>>>>>>
>>>>>> Such a value will be already used in [1]. I've looked at [2], but it
>>>>>> doesn't have such a field.
>>>>>>
>>>>>> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
>>>>>> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>>>>>
>>>>> I think the bigger question (since we are adding this to DT I assume it
>>>>> is not longer "toy" driver) can we convert it to a proper
>>>>> interrupt-driven scheme? Polling for real shipping devices is not
>>>>> efficient, I do not think we should even try to allow this mode in DTS
>>>>> bindings (note that tsc2007 use is different: it allows to specify
>>>>> sampling interval once touch is detected).
>>>>
>>>> Unfortunately, the only current user of tps6507x in kernel (da850-evm)
>>>> does not have the touch interrupt connected to the processor. So if we
>>>> dont support polled mode in when using device-tree, da850-evm.dts wont
>>>> be able to use this driver.
>>>
>>> Do we care? Right now the driver is unusable for any practical
>>> application I think as it burns CPU cycles by constantly polling the
>>> touch controller.
>>>
>>> Yegor, your conversion to device tree: is it done because you have a
>>> real product using this?
>>
>> Yes. It is VS-860 device http://www.visionsystems.de/produkte/vs-860.html
>>
>> It even seems to have interrupt pin connected. So we have to support both modes.
> 
> Well, I'd rather supported interrupt only. Polling is a toy mode.

On my DA850 EVM, running

$ evtest /dev/input/event0  > /dev/null &

and monitoring cpu load using top while continuously touching the screen
takes the CPU to about 90% idle. This is against 98% idle with no touch.
This running on ARM9 at 300Mhz. The processor can do about 450Mhz.

So, while I agree polling mode is not ideal, its not completely useless
as well. It will help the EVM to move to DT completely. Also, this
should not affect any core infrastructure or even interrupt driven users
of this driver.

Please do reconsider the stance on interrupt-only DT mode.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-03-13 14:15 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-09 14:31 [PATCH v2 0/3] Add DT support for tps6507x touchscreen yegorslists
2017-03-09 14:31 ` [PATCH v2 1/3] tps6507x-ts: update to devm_* API yegorslists
2017-03-11  0:29   ` Dmitry Torokhov
2017-03-09 14:31 ` [PATCH v2 2/3] tps6507x-ts: add DT support yegorslists
2017-03-09 15:02   ` Yegor Yefremov
2017-03-09 14:31 ` [PATCH v2 3/3] tps6507x-ts: add DT bindings description yegorslists
2017-03-09 14:49   ` Rob Herring
2017-03-09 15:18     ` Yegor Yefremov
2017-03-09 17:55       ` Dmitry Torokhov
2017-03-10  6:41         ` Sekhar Nori
2017-03-10 18:00           ` Dmitry Torokhov
2017-03-10 19:54             ` Yegor Yefremov
2017-03-10 20:08               ` Dmitry Torokhov
2017-03-13 14:15                 ` Sekhar Nori
2017-03-10 16:26         ` Rob Herring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.