[PATCH v2 0/3] ads7846: fix support for ADS7845

[PATCH v2 0/3] ads7846: fix support for ADS7845

[Luca Ellero]

ADS7845 support is buggy in this driver.
These patches fix various issues to get it work properly.

Changes for v2:
 - add missing period in patch 0001 message
 - elaborate comment in patch 0002
 
Luca Ellero (3):
  ads7846: don't report pressure for ads7845
  ads7846: always set last command to PWRDOWN
  ads7846: don't check penirq immediately for 7845

 drivers/input/touchscreen/ads7846.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] ads7846: don't report pressure for ads7845

[Luca Ellero]

From: Luca Ellero <l.ellero@asem.it>

ADS7845 doesn't support pressure.
This patch avoids the following error reported by libinput-list-devices:
"ADS7845 Touchscreen: kernel bug: device has min == max on ABS_PRESSURE".

Signed-off-by: Luca Ellero <l.ellero@asem.it>
---
Changes for v2:
 - add missing period in commit message

 drivers/input/touchscreen/ads7846.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index bed68a68f330..24605c40d039 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -1316,8 +1316,9 @@ static int ads7846_probe(struct spi_device *spi)
 			pdata->y_min ? : 0,
 			pdata->y_max ? : MAX_12BIT,
 			0, 0);
-	input_set_abs_params(input_dev, ABS_PRESSURE,
-			pdata->pressure_min, pdata->pressure_max, 0, 0);
+	if (ts->model != 7845)
+		input_set_abs_params(input_dev, ABS_PRESSURE,
+				pdata->pressure_min, pdata->pressure_max, 0, 0);
 
 	/*
 	 * Parse common framework properties. Must be done here to ensure the
-- 
2.25.1

Re: [PATCH v2 1/3] ads7846: don't report pressure for ads7845

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 732 bytes --]

On Thu, Jul 21, 2022 at 10:34:56AM +0200, Luca Ellero wrote:
> From: Luca Ellero <l.ellero@asem.it>
> 
> ADS7845 doesn't support pressure.
> This patch avoids the following error reported by libinput-list-devices:
> "ADS7845 Touchscreen: kernel bug: device has min == max on ABS_PRESSURE".
> 
> Signed-off-by: Luca Ellero <l.ellero@asem.it>

I don't know how picky Dmitry is, but some maintainers would ask you to
either add a Signed-off-by with the email address you sent this patch
from, or make sure to send it from the asem.it address.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/3] ads7846: don't report pressure for ads7845

[Luca Ellero]

Il 21/07/2022 22:48, Uwe Kleine-König ha scritto:
> On Thu, Jul 21, 2022 at 10:34:56AM +0200, Luca Ellero wrote:
>> From: Luca Ellero <l.ellero@asem.it>
>>
>> ADS7845 doesn't support pressure.
>> This patch avoids the following error reported by libinput-list-devices:
>> "ADS7845 Touchscreen: kernel bug: device has min == max on ABS_PRESSURE".
>>
>> Signed-off-by: Luca Ellero <l.ellero@asem.it>
> 
> I don't know how picky Dmitry is, but some maintainers would ask you to
> either add a Signed-off-by with the email address you sent this patch
> from, or make sure to send it from the asem.it address.
> 
> Best regards
> Uwe
> 
Hi Uwe,
thank you for your comment.
I have some problems with our SMTP mail server, but if needed I can
resend the patches with the proper email address.
Regards
Luca

-- 
Luca Ellero

E-mail: luca.ellero@brickedbrain.com
Internet: www.brickedbrain.com

Re: [PATCH v2 1/3] ads7846: don't report pressure for ads7845

[Andy Shevchenko]

On Thu, Jul 21, 2022 at 10:48:35PM +0200, Uwe Kleine-König wrote:
> On Thu, Jul 21, 2022 at 10:34:56AM +0200, Luca Ellero wrote:
> > From: Luca Ellero <l.ellero@asem.it>
> > 
> > ADS7845 doesn't support pressure.
> > This patch avoids the following error reported by libinput-list-devices:
> > "ADS7845 Touchscreen: kernel bug: device has min == max on ABS_PRESSURE".
> > 
> > Signed-off-by: Luca Ellero <l.ellero@asem.it>
> 
> I don't know how picky Dmitry is, but some maintainers would ask you to
> either add a Signed-off-by with the email address you sent this patch
> from, or make sure to send it from the asem.it address.

It's fine to be sent from different email of the same person.
But like you said, some maintainers are really picky and that
may block contribution (I have been seen a case, where actually
allowing to do as above unblocks contribution since person can't
re-sent properly the change from another address by some reasons).


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] ads7846: don't report pressure for ads7845

[Luca Ellero]

On 29/07/2022 15:50, Andy Shevchenko wrote:
> On Thu, Jul 21, 2022 at 10:48:35PM +0200, Uwe Kleine-König wrote:
>> On Thu, Jul 21, 2022 at 10:34:56AM +0200, Luca Ellero wrote:
>>> From: Luca Ellero <l.ellero@asem.it>
>>>
>>> ADS7845 doesn't support pressure.
>>> This patch avoids the following error reported by libinput-list-devices:
>>> "ADS7845 Touchscreen: kernel bug: device has min == max on ABS_PRESSURE".
>>>
>>> Signed-off-by: Luca Ellero <l.ellero@asem.it>
>>
>> I don't know how picky Dmitry is, but some maintainers would ask you to
>> either add a Signed-off-by with the email address you sent this patch
>> from, or make sure to send it from the asem.it address.
> 
> It's fine to be sent from different email of the same person.
> But like you said, some maintainers are really picky and that
> may block contribution (I have been seen a case, where actually
> allowing to do as above unblocks contribution since person can't
> re-sent properly the change from another address by some reasons).
> 
> 
Hi Andy,
thank you for your reply.
Ok, I will resend from the proper email.
Regards
Luca


-- 
Luca Ellero

E-mail: luca.ellero@brickedbrain.com
Internet: www.brickedbrain.com

[PATCH v2 2/3] ads7846: always set last command to PWRDOWN

[Luca Ellero]

From: Luca Ellero <l.ellero@asem.it>

Controllers that report pressure (e.g. ADS7846) use 5 commands and the
correct sequence is READ_X, READ_Y, READ_Z1, READ_Z2, PWRDOWN.

Controllers that don't report pressure (e.g. ADS7845/ADS7843) use only 3
commands and the correct sequence should be READ_X, READ_Y, PWRDOWN. But
the sequence sent was incorrect: READ_X, READ_Y, READ_Z1.

This patch fixes it.

Signed-off-by: Luca Ellero <l.ellero@asem.it>
---
Changes for v2:
 - elaborate commit message

 drivers/input/touchscreen/ads7846.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 24605c40d039..9e15cdf6faa0 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -1066,6 +1066,9 @@ static int ads7846_setup_spi_msg(struct ads7846 *ts,
 		struct ads7846_buf_layout *l = &packet->l[cmd_idx];
 		unsigned int max_count;
 
+		if (cmd_idx == packet->cmds - 1)
+			cmd_idx = ADS7846_PWDOWN;
+
 		if (ads7846_cmd_need_settle(cmd_idx))
 			max_count = packet->count + packet->count_skip;
 		else
@@ -1102,7 +1105,12 @@ static int ads7846_setup_spi_msg(struct ads7846 *ts,
 
 	for (cmd_idx = 0; cmd_idx < packet->cmds; cmd_idx++) {
 		struct ads7846_buf_layout *l = &packet->l[cmd_idx];
-		u8 cmd = ads7846_get_cmd(cmd_idx, vref);
+		u8 cmd;
+
+		if (cmd_idx == packet->cmds - 1)
+			cmd_idx = ADS7846_PWDOWN;
+
+		cmd = ads7846_get_cmd(cmd_idx, vref);
 
 		for (b = 0; b < l->count; b++)
 			packet->tx[l->offset + b].cmd = cmd;
-- 
2.25.1

[PATCH v2 3/3] ads7846: don't check penirq immediately for 7845

[Luca Ellero]

From: Luca Ellero <l.ellero@asem.it>

To discard false readings, one should use "ti,penirq-recheck-delay-usecs".
Checking get_pendown_state() at the beginning, most of the time fails
causing malfunctioning.

Signed-off-by: Luca Ellero <l.ellero@asem.it>
---
 drivers/input/touchscreen/ads7846.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 9e15cdf6faa0..122d3a13b7c5 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -843,14 +843,8 @@ static void ads7846_report_state(struct ads7846 *ts)
 	if (x == MAX_12BIT)
 		x = 0;
 
-	if (ts->model == 7843) {
+	if (ts->model == 7843 || ts->model == 7845) {
 		Rt = ts->pressure_max / 2;
-	} else if (ts->model == 7845) {
-		if (get_pendown_state(ts))
-			Rt = ts->pressure_max / 2;
-		else
-			Rt = 0;
-		dev_vdbg(&ts->spi->dev, "x/y: %d/%d, PD %d\n", x, y, Rt);
 	} else if (likely(x && z1)) {
 		/* compute touch pressure resistance using equation #2 */
 		Rt = z2;
-- 
2.25.1

Re: [PATCH v2 0/3] ads7846: fix support for ADS7845

[Andy Shevchenko]

On Thu, Jul 21, 2022 at 10:34:55AM +0200, Luca Ellero wrote:
> ADS7845 support is buggy in this driver.
> These patches fix various issues to get it work properly.
> 
> Changes for v2:
>  - add missing period in patch 0001 message
>  - elaborate comment in patch 0002

All three looks good to me,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Luca Ellero (3):
>   ads7846: don't report pressure for ads7845
>   ads7846: always set last command to PWRDOWN
>   ads7846: don't check penirq immediately for 7845
> 
>  drivers/input/touchscreen/ads7846.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> -- 
> 2.25.1
> 

-- 
With Best Regards,
Andy Shevchenko