diff mbox

Input: sun4i-ts - allow to adjust some settings via device-tree properties.

Message ID 1426175150-27005-1-git-send-email-karme@karme.de
State Needs Review / ACK, archived
Headers show

Checks

Context Check Description
robh/checkpatch warning total: 1 errors, 0 warnings, 0 lines checked
robh/patch-applied success

Commit Message

Jens Thiele March 12, 2015, 3:45 p.m. UTC
This commit introduces two new optional device-tree properties:
"tp-sensitive-adjust": adjust sensitivity of pen down detection
"filter-type": select median and averaging filter

The previous fixed defaults, didn't work well for the Olimex
A13-LCD10TS (I have).

Signed-off-by: Jens Thiele <karme@karme.de>
---
 .../devicetree/bindings/input/touchscreen/sun4i.txt |   19 +++++++++++++++++--
 drivers/input/touchscreen/sun4i-ts.c                |   17 +++++++++++++----
 2 files changed, 30 insertions(+), 6 deletions(-)

Comments

Hans de Goede March 16, 2015, 10:25 a.m. UTC | #1
Hi,

On 12-03-15 16:45, Jens Thiele wrote:
> This commit introduces two new optional device-tree properties:
> "tp-sensitive-adjust": adjust sensitivity of pen down detection
> "filter-type": select median and averaging filter
>
> The previous fixed defaults, didn't work well for the Olimex
> A13-LCD10TS (I have).
>
> Signed-off-by: Jens Thiele <karme@karme.de>

Thanks for working on this, this patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

> ---
>   .../devicetree/bindings/input/touchscreen/sun4i.txt |   19 +++++++++++++++++--
>   drivers/input/touchscreen/sun4i-ts.c                |   17 +++++++++++++----
>   2 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> index 433332d..c93edfa 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> @@ -8,8 +8,20 @@ Required properties:
>    - #thermal-sensor-cells: shall be 0
>
>   Optional properties:
> - - allwinner,ts-attached: boolean indicating that an actual touchscreen is
> -			  attached to the controller
> + - allwinner,ts-attached	 : boolean indicating that an actual touchscreen
> +				   is attached to the controller
> + - allwinner,tp-sensitive-adjust : integer (4 bits)
> +				   adjust sensitivity of pen down detection
> +				   between 0 (least sensitive) and 15
> +				   (defaults to 15)
> + - allwinner,filter-type 	 : integer (2 bits)
> +   				   select median and averaging filter
> +				   samples used for median / averaging filter
> +				   0: 4/2
> +				   1: 5/3
> +				   2: 8/4
> +				   3: 16/8
> +				   (defaults to 1)
>
>   Example:
>
> @@ -19,4 +31,7 @@ Example:
>   		interrupts = <29>;
>   		allwinner,ts-attached;
>   		#thermal-sensor-cells = <0>;
> +		/* sensitive/noisy touch panel */
> +		allwinner,tp-sensitive-adjust = <0>;
> +		allwinner,filter-type = <3>;
>   	};
> diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
> index b93a28b..d1cf847 100644
> --- a/drivers/input/touchscreen/sun4i-ts.c
> +++ b/drivers/input/touchscreen/sun4i-ts.c
> @@ -30,6 +30,10 @@
>    * These kinds of heuristics are just asking for trouble (and don't belong
>    * in the kernel). So this driver offers straight forward, reliable single
>    * touch functionality only.
> + *
> + * s.a. A20 User Manual "1.15 TP" (Documentation/arm/sunxi/README)
> + * (looks like the description in the A20 User Manual v1.3 is better
> + * than the one in the A10 User Manual v.1.5)
>    */
>
>   #include <linux/err.h>
> @@ -246,6 +250,9 @@ static int sun4i_ts_probe(struct platform_device *pdev)
>   	int error;
>   	u32 reg;
>   	bool ts_attached;
> +	/* optional device-tree properties, use old fixed values as default */
> +	u32 tp_sensitive_adjust = 15;
> +	u32 filter_type = 1;
>
>   	ts = devm_kzalloc(dev, sizeof(struct sun4i_ts_data), GFP_KERNEL);
>   	if (!ts)
> @@ -313,14 +320,16 @@ static int sun4i_ts_probe(struct platform_device *pdev)
>   	       ts->base + TP_CTRL0);
>
>   	/*
> -	 * sensitive_adjust = 15 : max, which is not all that sensitive,
> +	 * tp_sensitive_adjust is an optional property
>   	 * tp_mode = 0 : only x and y coordinates, as we don't use dual touch
>   	 */
> -	writel(TP_SENSITIVE_ADJUST(15) | TP_MODE_SELECT(0),
> +	of_property_read_u32(np, "allwinner,tp-sensitive-adjust", &tp_sensitive_adjust);
> +	writel(TP_SENSITIVE_ADJUST(tp_sensitive_adjust) | TP_MODE_SELECT(0),
>   	       ts->base + TP_CTRL2);
>
> -	/* Enable median filter, type 1 : 5/3 */
> -	writel(FILTER_EN(1) | FILTER_TYPE(1), ts->base + TP_CTRL3);
> +	/* Enable median and averaging filter, optional property for filter type */
> +	of_property_read_u32(np, "allwinner,filter-type", &filter_type);
> +	writel(FILTER_EN(1) | FILTER_TYPE(filter_type), ts->base + TP_CTRL3);
>
>   	/* Enable temperature measurement, period 1953 (2 seconds) */
>   	writel(TEMP_ENABLE(1) | TEMP_PERIOD(1953), ts->base + TP_TPR);
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Ripard March 18, 2015, 10:37 a.m. UTC | #2
On Thu, Mar 12, 2015 at 04:45:50PM +0100, Jens Thiele wrote:
> This commit introduces two new optional device-tree properties:
> "tp-sensitive-adjust": adjust sensitivity of pen down detection
> "filter-type": select median and averaging filter
> 
> The previous fixed defaults, didn't work well for the Olimex
> A13-LCD10TS (I have).
> 
> Signed-off-by: Jens Thiele <karme@karme.de>
> ---
>  .../devicetree/bindings/input/touchscreen/sun4i.txt |   19 +++++++++++++++++--
>  drivers/input/touchscreen/sun4i-ts.c                |   17 +++++++++++++----
>  2 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> index 433332d..c93edfa 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> @@ -8,8 +8,20 @@ Required properties:
>   - #thermal-sensor-cells: shall be 0
>  
>  Optional properties:
> - - allwinner,ts-attached: boolean indicating that an actual touchscreen is
> -			  attached to the controller
> + - allwinner,ts-attached	 : boolean indicating that an actual touchscreen
> +				   is attached to the controller
> + - allwinner,tp-sensitive-adjust : integer (4 bits)
> +				   adjust sensitivity of pen down detection
> +				   between 0 (least sensitive) and 15
> +				   (defaults to 15)
> + - allwinner,filter-type 	 : integer (2 bits)
> +   				   select median and averaging filter
> +				   samples used for median / averaging filter
> +				   0: 4/2
> +				   1: 5/3
> +				   2: 8/4
> +				   3: 16/8
> +				   (defaults to 1)

You shouldn't rely on any bits length in your binding.

And this looks really more of a configuration option, and would be
better exposed through sysfs.

Maxime
Hans de Goede March 18, 2015, 11:01 a.m. UTC | #3
Hi,

On 18-03-15 11:37, Maxime Ripard wrote:
> On Thu, Mar 12, 2015 at 04:45:50PM +0100, Jens Thiele wrote:
>> This commit introduces two new optional device-tree properties:
>> "tp-sensitive-adjust": adjust sensitivity of pen down detection
>> "filter-type": select median and averaging filter
>>
>> The previous fixed defaults, didn't work well for the Olimex
>> A13-LCD10TS (I have).
>>
>> Signed-off-by: Jens Thiele <karme@karme.de>
>> ---
>>   .../devicetree/bindings/input/touchscreen/sun4i.txt |   19 +++++++++++++++++--
>>   drivers/input/touchscreen/sun4i-ts.c                |   17 +++++++++++++----
>>   2 files changed, 30 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
>> index 433332d..c93edfa 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
>> @@ -8,8 +8,20 @@ Required properties:
>>    - #thermal-sensor-cells: shall be 0
>>
>>   Optional properties:
>> - - allwinner,ts-attached: boolean indicating that an actual touchscreen is
>> -			  attached to the controller
>> + - allwinner,ts-attached	 : boolean indicating that an actual touchscreen
>> +				   is attached to the controller
>> + - allwinner,tp-sensitive-adjust : integer (4 bits)
>> +				   adjust sensitivity of pen down detection
>> +				   between 0 (least sensitive) and 15
>> +				   (defaults to 15)
>> + - allwinner,filter-type 	 : integer (2 bits)
>> +   				   select median and averaging filter
>> +				   samples used for median / averaging filter
>> +				   0: 4/2
>> +				   1: 5/3
>> +				   2: 8/4
>> +				   3: 16/8
>> +				   (defaults to 1)
>
> You shouldn't rely on any bits length in your binding.
>
> And this looks really more of a configuration option, and would be
> better exposed through sysfs.

Erm, no, this is to adjust for properties of the touchscreen, so this is
hardware configuration, and as such definitely belongs in the dts.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov March 23, 2015, 4:12 p.m. UTC | #4
On Wed, Mar 18, 2015 at 12:01:58PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 18-03-15 11:37, Maxime Ripard wrote:
> >On Thu, Mar 12, 2015 at 04:45:50PM +0100, Jens Thiele wrote:
> >>This commit introduces two new optional device-tree properties:
> >>"tp-sensitive-adjust": adjust sensitivity of pen down detection
> >>"filter-type": select median and averaging filter
> >>
> >>The previous fixed defaults, didn't work well for the Olimex
> >>A13-LCD10TS (I have).
> >>
> >>Signed-off-by: Jens Thiele <karme@karme.de>
> >>---
> >>  .../devicetree/bindings/input/touchscreen/sun4i.txt |   19 +++++++++++++++++--
> >>  drivers/input/touchscreen/sun4i-ts.c                |   17 +++++++++++++----
> >>  2 files changed, 30 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> >>index 433332d..c93edfa 100644
> >>--- a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> >>+++ b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> >>@@ -8,8 +8,20 @@ Required properties:
> >>   - #thermal-sensor-cells: shall be 0
> >>
> >>  Optional properties:
> >>- - allwinner,ts-attached: boolean indicating that an actual touchscreen is
> >>-			  attached to the controller
> >>+ - allwinner,ts-attached	 : boolean indicating that an actual touchscreen
> >>+				   is attached to the controller
> >>+ - allwinner,tp-sensitive-adjust : integer (4 bits)
> >>+				   adjust sensitivity of pen down detection
> >>+				   between 0 (least sensitive) and 15
> >>+				   (defaults to 15)
> >>+ - allwinner,filter-type 	 : integer (2 bits)
> >>+   				   select median and averaging filter
> >>+				   samples used for median / averaging filter
> >>+				   0: 4/2
> >>+				   1: 5/3
> >>+				   2: 8/4
> >>+				   3: 16/8
> >>+				   (defaults to 1)
> >
> >You shouldn't rely on any bits length in your binding.
> >
> >And this looks really more of a configuration option, and would be
> >better exposed through sysfs.
> 
> Erm, no, this is to adjust for properties of the touchscreen, so this is
> hardware configuration, and as such definitely belongs in the dts.

Applied, thank you.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
index 433332d..c93edfa 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
@@ -8,8 +8,20 @@  Required properties:
  - #thermal-sensor-cells: shall be 0
 
 Optional properties:
- - allwinner,ts-attached: boolean indicating that an actual touchscreen is
-			  attached to the controller
+ - allwinner,ts-attached	 : boolean indicating that an actual touchscreen
+				   is attached to the controller
+ - allwinner,tp-sensitive-adjust : integer (4 bits)
+				   adjust sensitivity of pen down detection
+				   between 0 (least sensitive) and 15
+				   (defaults to 15)
+ - allwinner,filter-type 	 : integer (2 bits)
+   				   select median and averaging filter
+				   samples used for median / averaging filter
+				   0: 4/2
+				   1: 5/3
+				   2: 8/4
+				   3: 16/8
+				   (defaults to 1)
 
 Example:
 
@@ -19,4 +31,7 @@  Example:
 		interrupts = <29>;
 		allwinner,ts-attached;
 		#thermal-sensor-cells = <0>;
+		/* sensitive/noisy touch panel */
+		allwinner,tp-sensitive-adjust = <0>;
+		allwinner,filter-type = <3>;
 	};
diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
index b93a28b..d1cf847 100644
--- a/drivers/input/touchscreen/sun4i-ts.c
+++ b/drivers/input/touchscreen/sun4i-ts.c
@@ -30,6 +30,10 @@ 
  * These kinds of heuristics are just asking for trouble (and don't belong
  * in the kernel). So this driver offers straight forward, reliable single
  * touch functionality only.
+ *
+ * s.a. A20 User Manual "1.15 TP" (Documentation/arm/sunxi/README)
+ * (looks like the description in the A20 User Manual v1.3 is better
+ * than the one in the A10 User Manual v.1.5)
  */
 
 #include <linux/err.h>
@@ -246,6 +250,9 @@  static int sun4i_ts_probe(struct platform_device *pdev)
 	int error;
 	u32 reg;
 	bool ts_attached;
+	/* optional device-tree properties, use old fixed values as default */
+	u32 tp_sensitive_adjust = 15;
+	u32 filter_type = 1;
 
 	ts = devm_kzalloc(dev, sizeof(struct sun4i_ts_data), GFP_KERNEL);
 	if (!ts)
@@ -313,14 +320,16 @@  static int sun4i_ts_probe(struct platform_device *pdev)
 	       ts->base + TP_CTRL0);
 
 	/*
-	 * sensitive_adjust = 15 : max, which is not all that sensitive,
+	 * tp_sensitive_adjust is an optional property
 	 * tp_mode = 0 : only x and y coordinates, as we don't use dual touch
 	 */
-	writel(TP_SENSITIVE_ADJUST(15) | TP_MODE_SELECT(0),
+	of_property_read_u32(np, "allwinner,tp-sensitive-adjust", &tp_sensitive_adjust);
+	writel(TP_SENSITIVE_ADJUST(tp_sensitive_adjust) | TP_MODE_SELECT(0),
 	       ts->base + TP_CTRL2);
 
-	/* Enable median filter, type 1 : 5/3 */
-	writel(FILTER_EN(1) | FILTER_TYPE(1), ts->base + TP_CTRL3);
+	/* Enable median and averaging filter, optional property for filter type */
+	of_property_read_u32(np, "allwinner,filter-type", &filter_type);
+	writel(FILTER_EN(1) | FILTER_TYPE(filter_type), ts->base + TP_CTRL3);
 
 	/* Enable temperature measurement, period 1953 (2 seconds) */
 	writel(TEMP_ENABLE(1) | TEMP_PERIOD(1953), ts->base + TP_TPR);