* [PATCH] Input: ads7846 - correct the value get from SPI
@ 2015-05-22 7:02 Haibo Chen
2015-06-10 8:53 ` Chen Bough
2015-07-21 11:52 ` Igor Grinberg
0 siblings, 2 replies; 6+ messages in thread
From: Haibo Chen @ 2015-05-22 7:02 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: jg1.han, pramod.gurav, linux-input, haibo.chen
According to the touch controller SPEC, SPI return a 16 bit value,
only 12 bits are valid, they are bit[14-3].
The value of MISO and MOSI can be configed when SPI in idle mode.
Currently this touch driver consider the SPI bus set the MOSI and
MISO in low level when SPI bus in idle mode. So the bit[15] of the
value get from SPI bus is always 0. But when SPI bus congfig the
MOSI and MISO in high level when SPI in idle mode, the bit[15] of
the value get from SPI is always 1, if not mask this bit[15], we may
get the wrong value.
This patch mask the invalid bit, and make sure to return the correct
value nomatter how SPI bus configed.
Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
---
drivers/input/touchscreen/ads7846.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index e4eb8a6..91b335f 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -678,7 +678,7 @@ static int ads7846_get_value(struct ads7846 *ts, struct spi_message *m)
* adjust: on-wire is a must-ignore bit, a BE12 value, then
* padding; built from two 8 bit values written msb-first.
*/
- return be16_to_cpup((__be16 *)t->rx_buf) >> 3;
+ return (be16_to_cpup((__be16 *)t->rx_buf) & 0x7ff8) >> 3;
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] Input: ads7846 - correct the value get from SPI
2015-05-22 7:02 [PATCH] Input: ads7846 - correct the value get from SPI Haibo Chen
@ 2015-06-10 8:53 ` Chen Bough
2015-07-21 2:31 ` Chen Bough
2015-07-21 11:52 ` Igor Grinberg
1 sibling, 1 reply; 6+ messages in thread
From: Chen Bough @ 2015-06-10 8:53 UTC (permalink / raw)
To: Chen Bough, dmitry.torokhov@gmail.com
Cc: jg1.han@samsung.com, pramod.gurav@smartplayin.com,
linux-input@vger.kernel.org
Ping...
> -----Original Message-----
> From: Haibo Chen [mailto:haibo.chen@freescale.com]
> Sent: Friday, May 22, 2015 3:02 PM
> To: dmitry.torokhov@gmail.com
> Cc: jg1.han@samsung.com; pramod.gurav@smartplayin.com; linux-
> input@vger.kernel.org; Chen Haibo-B51421
> Subject: [PATCH] Input: ads7846 - correct the value get from SPI
>
> According to the touch controller SPEC, SPI return a 16 bit value, only
> 12 bits are valid, they are bit[14-3].
>
> The value of MISO and MOSI can be configed when SPI in idle mode.
> Currently this touch driver consider the SPI bus set the MOSI and MISO in
> low level when SPI bus in idle mode. So the bit[15] of the value get from
> SPI bus is always 0. But when SPI bus congfig the MOSI and MISO in high
> level when SPI in idle mode, the bit[15] of the value get from SPI is
> always 1, if not mask this bit[15], we may get the wrong value.
>
> This patch mask the invalid bit, and make sure to return the correct
> value nomatter how SPI bus configed.
>
> Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
> ---
> drivers/input/touchscreen/ads7846.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/ads7846.c
> b/drivers/input/touchscreen/ads7846.c
> index e4eb8a6..91b335f 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -678,7 +678,7 @@ static int ads7846_get_value(struct ads7846 *ts,
> struct spi_message *m)
> * adjust: on-wire is a must-ignore bit, a BE12 value, then
> * padding; built from two 8 bit values written msb-first.
> */
> - return be16_to_cpup((__be16 *)t->rx_buf) >> 3;
> + return (be16_to_cpup((__be16 *)t->rx_buf) & 0x7ff8) >> 3;
> }
> }
>
> --
> 1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] Input: ads7846 - correct the value get from SPI
2015-06-10 8:53 ` Chen Bough
@ 2015-07-21 2:31 ` Chen Bough
0 siblings, 0 replies; 6+ messages in thread
From: Chen Bough @ 2015-07-21 2:31 UTC (permalink / raw)
To: Chen Bough, dmitry.torokhov@gmail.com
Cc: jg1.han@samsung.com, pramod.gurav@smartplayin.com,
linux-input@vger.kernel.org
Hi
> -----Original Message-----
> From: Chen Haibo-B51421
> Sent: Wednesday, June 10, 2015 4:54 PM
> To: Chen Haibo-B51421; dmitry.torokhov@gmail.com
> Cc: jg1.han@samsung.com; pramod.gurav@smartplayin.com; linux-
> input@vger.kernel.org
> Subject: RE: [PATCH] Input: ads7846 - correct the value get from SPI
>
> Ping...
>
>
> > -----Original Message-----
> > From: Haibo Chen [mailto:haibo.chen@freescale.com]
> > Sent: Friday, May 22, 2015 3:02 PM
> > To: dmitry.torokhov@gmail.com
> > Cc: jg1.han@samsung.com; pramod.gurav@smartplayin.com; linux-
> > input@vger.kernel.org; Chen Haibo-B51421
> > Subject: [PATCH] Input: ads7846 - correct the value get from SPI
> >
> > According to the touch controller SPEC, SPI return a 16 bit value,
> > only
> > 12 bits are valid, they are bit[14-3].
> >
> > The value of MISO and MOSI can be configed when SPI in idle mode.
> > Currently this touch driver consider the SPI bus set the MOSI and MISO
> > in low level when SPI bus in idle mode. So the bit[15] of the value
> > get from SPI bus is always 0. But when SPI bus congfig the MOSI and
> > MISO in high level when SPI in idle mode, the bit[15] of the value get
> > from SPI is always 1, if not mask this bit[15], we may get the wrong
> value.
> >
> > This patch mask the invalid bit, and make sure to return the correct
> > value nomatter how SPI bus configed.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
> > ---
> > drivers/input/touchscreen/ads7846.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/touchscreen/ads7846.c
> > b/drivers/input/touchscreen/ads7846.c
> > index e4eb8a6..91b335f 100644
> > --- a/drivers/input/touchscreen/ads7846.c
> > +++ b/drivers/input/touchscreen/ads7846.c
> > @@ -678,7 +678,7 @@ static int ads7846_get_value(struct ads7846 *ts,
> > struct spi_message *m)
> > * adjust: on-wire is a must-ignore bit, a BE12 value, then
> > * padding; built from two 8 bit values written msb-first.
> > */
> > - return be16_to_cpup((__be16 *)t->rx_buf) >> 3;
> > + return (be16_to_cpup((__be16 *)t->rx_buf) & 0x7ff8) >> 3;
> > }
> > }
> >
> > --
> > 1.9.1
Will this patch be merged? Since more than 5 weeks passed, I did not see any comments about this patch.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Input: ads7846 - correct the value get from SPI
2015-05-22 7:02 [PATCH] Input: ads7846 - correct the value get from SPI Haibo Chen
2015-06-10 8:53 ` Chen Bough
@ 2015-07-21 11:52 ` Igor Grinberg
2015-07-23 8:28 ` Chen Bough
1 sibling, 1 reply; 6+ messages in thread
From: Igor Grinberg @ 2015-07-21 11:52 UTC (permalink / raw)
To: Haibo Chen, dmitry.torokhov; +Cc: jg1.han, pramod.gurav, linux-input
[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]
Hi,
On 05/22/15 10:02, Haibo Chen wrote:
> According to the touch controller SPEC, SPI return a 16 bit value,
> only 12 bits are valid, they are bit[14-3].
>
> The value of MISO and MOSI can be configed when SPI in idle mode.
> Currently this touch driver consider the SPI bus set the MOSI and
> MISO in low level when SPI bus in idle mode. So the bit[15] of the
> value get from SPI bus is always 0. But when SPI bus congfig the
> MOSI and MISO in high level when SPI in idle mode, the bit[15] of
> the value get from SPI is always 1, if not mask this bit[15], we may
> get the wrong value.
>
> This patch mask the invalid bit, and make sure to return the correct
> value nomatter how SPI bus configed.
>
> Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
> ---
> drivers/input/touchscreen/ads7846.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index e4eb8a6..91b335f 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -678,7 +678,7 @@ static int ads7846_get_value(struct ads7846 *ts, struct spi_message *m)
> * adjust: on-wire is a must-ignore bit, a BE12 value, then
> * padding; built from two 8 bit values written msb-first.
> */
> - return be16_to_cpup((__be16 *)t->rx_buf) >> 3;
> + return (be16_to_cpup((__be16 *)t->rx_buf) & 0x7ff8) >> 3;
IIRC, 7845 is the same in regard to SPI bus, so may be we should
also adjust the value in if statement, not just the else part.
We have the attached patch locally, so may you would like to consider
it or a mixture of both...
--
Regards,
Igor.
[-- Attachment #2: 0001-input-ads7846-enforce-ADC-output-width.patch --]
[-- Type: text/x-patch, Size: 1733 bytes --]
>From fda22bb445c3866a89ef1ec876516d60f0292f7f Mon Sep 17 00:00:00 2001
From: Andrey Gelman <andrey.gelman@compulab.co.il>
Date: Sun, 26 Oct 2014 17:25:43 +0200
Subject: [PATCH] input: ads7846: enforce ADC output width
Enforce ADC output is 12 bits width.
For no clear reason, on i.MX6-based CM-FX6, we have experienced a
13-th bit set to 1 and therefore all reported values were out of
range.
Signed-off-by: Andrey Gelman <andrey.gelman@compulab.co.il>
[grinberg@compulab.co.il: white space, comment and commit message
adjusted]
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
drivers/input/touchscreen/ads7846.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 5196861..655ae85 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -689,18 +689,22 @@ static int ads7846_no_filter(void *ads, int data_idx, int *val)
static int ads7846_get_value(struct ads7846 *ts, struct spi_message *m)
{
+ int value;
struct spi_transfer *t =
list_entry(m->transfers.prev, struct spi_transfer, transfer_list);
if (ts->model == 7845) {
- return be16_to_cpup((__be16 *)&(((char*)t->rx_buf)[1])) >> 3;
+ value = be16_to_cpup((__be16 *)&(((char*)t->rx_buf)[1])) >> 3;
} else {
/*
* adjust: on-wire is a must-ignore bit, a BE12 value, then
* padding; built from two 8 bit values written msb-first.
*/
- return be16_to_cpup((__be16 *)t->rx_buf) >> 3;
+ value = be16_to_cpup((__be16 *)t->rx_buf) >> 3;
}
+
+ /* enforce ADC output is 12 bits width */
+ return value & 0xfff;
}
static void ads7846_update_value(struct spi_message *m, int val)
--
2.3.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* RE: [PATCH] Input: ads7846 - correct the value get from SPI
2015-07-21 11:52 ` Igor Grinberg
@ 2015-07-23 8:28 ` Chen Bough
0 siblings, 0 replies; 6+ messages in thread
From: Chen Bough @ 2015-07-23 8:28 UTC (permalink / raw)
To: Igor Grinberg, dmitry.torokhov@gmail.com
Cc: jg1.han@samsung.com, pramod.gurav@smartplayin.com,
linux-input@vger.kernel.org
Hi Igor,
The patch you attached is okay for me. But I suggest to explain this issue more clearly.
Actually it is not a no clear reason. i.MX6 platform SPI driver set MOSI and MISO in high
level. You can refer to my patch log.
Best Regards
Haibo Chen
> -----Original Message-----
> From: Igor Grinberg [mailto:grinberg@compulab.co.il]
> Sent: Tuesday, July 21, 2015 7:53 PM
> To: Chen Haibo-B51421; dmitry.torokhov@gmail.com
> Cc: jg1.han@samsung.com; pramod.gurav@smartplayin.com; linux-
> input@vger.kernel.org
> Subject: Re: [PATCH] Input: ads7846 - correct the value get from SPI
>
> Hi,
>
> On 05/22/15 10:02, Haibo Chen wrote:
> > According to the touch controller SPEC, SPI return a 16 bit value,
> > only 12 bits are valid, they are bit[14-3].
> >
> > The value of MISO and MOSI can be configed when SPI in idle mode.
> > Currently this touch driver consider the SPI bus set the MOSI and MISO
> > in low level when SPI bus in idle mode. So the bit[15] of the value
> > get from SPI bus is always 0. But when SPI bus congfig the MOSI and
> > MISO in high level when SPI in idle mode, the bit[15] of the value get
> > from SPI is always 1, if not mask this bit[15], we may get the wrong
> > value.
> >
> > This patch mask the invalid bit, and make sure to return the correct
> > value nomatter how SPI bus configed.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
> > ---
> > drivers/input/touchscreen/ads7846.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/touchscreen/ads7846.c
> > b/drivers/input/touchscreen/ads7846.c
> > index e4eb8a6..91b335f 100644
> > --- a/drivers/input/touchscreen/ads7846.c
> > +++ b/drivers/input/touchscreen/ads7846.c
> > @@ -678,7 +678,7 @@ static int ads7846_get_value(struct ads7846 *ts,
> struct spi_message *m)
> > * adjust: on-wire is a must-ignore bit, a BE12 value, then
> > * padding; built from two 8 bit values written msb-first.
> > */
> > - return be16_to_cpup((__be16 *)t->rx_buf) >> 3;
> > + return (be16_to_cpup((__be16 *)t->rx_buf) & 0x7ff8) >> 3;
>
> IIRC, 7845 is the same in regard to SPI bus, so may be we should also
> adjust the value in if statement, not just the else part.
>
> We have the attached patch locally, so may you would like to consider it
> or a mixture of both...
>
>
> --
> Regards,
> Igor.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Input: ads7846 - correct the value get from SPI
@ 2015-05-22 6:36 Haibo Chen
0 siblings, 0 replies; 6+ messages in thread
From: Haibo Chen @ 2015-05-22 6:36 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: jg1.han, pramod.gurav, linux-input
According to the touch controller SPEC, SPI return a 16 bit value,
only 12 bits are valid, they are bit[14-3].
The value of MISO and MOSI can be configed when SPI in idle mode.
Currently this touch driver consider the SPI bus set the MOSI and
MISO in low level when SPI bus in idle mode. So the bit[15] of the
value get from SPI bus is always 0. But when SPI bus congfig the
MOSI and MISO in high level when SPI in idle mode, the bit[15] of
the value get from SPI is always 1, if not mask this bit[15], we may
get the wrong value.
This patch mask the invalid bit, and make sure to return the correct
value nomatter how SPI bus configed.
Signed-off-by: Haibo Chen <haibo.chen@freescale.com>
---
drivers/input/touchscreen/ads7846.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index e4eb8a6..91b335f 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -678,7 +678,7 @@ static int ads7846_get_value(struct ads7846 *ts, struct spi_message *m)
* adjust: on-wire is a must-ignore bit, a BE12 value, then
* padding; built from two 8 bit values written msb-first.
*/
- return be16_to_cpup((__be16 *)t->rx_buf) >> 3;
+ return (be16_to_cpup((__be16 *)t->rx_buf) & 0x7ff8) >> 3;
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-07-23 8:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-22 7:02 [PATCH] Input: ads7846 - correct the value get from SPI Haibo Chen
2015-06-10 8:53 ` Chen Bough
2015-07-21 2:31 ` Chen Bough
2015-07-21 11:52 ` Igor Grinberg
2015-07-23 8:28 ` Chen Bough
-- strict thread matches above, loose matches on Subject: below --
2015-05-22 6:36 Haibo Chen
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.