diff mbox

[5/7] input: atmel_mxt_ts: only apply mxt_platform_data blen and threshold on mxt224, not mxt224e

Message ID 1360246668-2291-6-git-send-email-pmeerw@pmeerw.net (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Meerwald-Stadler Feb. 7, 2013, 2:17 p.m. UTC
From: Peter Meerwald <p.meerwald@bct-electronic.com>

on the mxt224e, the upper 4 bits of blen are gain and the threshold should probably be set in CTECONFIG

mxt_handle_pdata() is problematic as there is no way to NOT apply settings

Signed-off-by: Peter Meerwald <p.meerwald@bct-electronic.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Yufeng Shen Feb. 20, 2013, 6:29 p.m. UTC | #1
On Thu, Feb 7, 2013 at 9:17 AM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
> From: Peter Meerwald <p.meerwald@bct-electronic.com>
>
> on the mxt224e, the upper 4 bits of blen are gain and the threshold should probably be set in CTECONFIG
>
> mxt_handle_pdata() is problematic as there is no way to NOT apply settings
>
> Signed-off-by: Peter Meerwald <p.meerwald@bct-electronic.com>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 7fdd439..313b201 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -673,14 +673,6 @@ static void mxt_handle_pdata(struct mxt_data *data)
>         mxt_write_object(data, MXT_TOUCH_MULTI_T9, MXT_TOUCH_ORIENT,
>                         pdata->orient);
>
> -       /* Set touchscreen burst length */
> -       mxt_write_object(data, MXT_TOUCH_MULTI_T9,
> -                       MXT_TOUCH_BLEN, pdata->blen);
> -
> -       /* Set touchscreen threshold */
> -       mxt_write_object(data, MXT_TOUCH_MULTI_T9,
> -                       MXT_TOUCH_TCHTHR, pdata->threshold);
> -
>         /* Set touchscreen resolution */
>         mxt_write_object(data, MXT_TOUCH_MULTI_T9,
>                         MXT_TOUCH_XRANGE_LSB, (pdata->x_size - 1) & 0xff);
> @@ -693,6 +685,14 @@ static void mxt_handle_pdata(struct mxt_data *data)
>
>         /* Set touchscreen voltage */
>         if (pdata->voltage && data->info.family_id == MXT224_ID) {

As Benson noted in patch 5, what about the device that is neither 224
nor 224E ? burst length
and  threshold will no longer be applied.

> +               /* Set touchscreen burst length */
> +               mxt_write_object(data, MXT_TOUCH_MULTI_T9,
> +                               MXT_TOUCH_BLEN, pdata->blen);
> +
> +               /* Set touchscreen threshold */
> +               mxt_write_object(data, MXT_TOUCH_MULTI_T9,
> +                               MXT_TOUCH_TCHTHR, pdata->threshold);
> +
>                 if (pdata->voltage < MXT_VOLTAGE_DEFAULT) {
>                         voltage = (MXT_VOLTAGE_DEFAULT - pdata->voltage) /
>                                 MXT_VOLTAGE_STEP;
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benson Leung Feb. 20, 2013, 6:50 p.m. UTC | #2
Furthermore, this patch has the effect of only applying blen and
tchthr when voltage is present.

If this is a 224 device, but the platform data does not specify
voltage (say, we are ok with the existing value loaded from NVRAM),
then we won't apply blen and tchthr either.


On Wed, Feb 20, 2013 at 10:29 AM, Yufeng Shen <miletus@google.com> wrote:
> On Thu, Feb 7, 2013 at 9:17 AM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
>>         /* Set touchscreen voltage */
>>         if (pdata->voltage && data->info.family_id == MXT224_ID) {
>
> As Benson noted in patch 5, what about the device that is neither 224
> nor 224E ? burst length
> and  threshold will no longer be applied.
>
Peter Meerwald-Stadler Feb. 20, 2013, 10:33 p.m. UTC | #3
Hello,

> Furthermore, this patch has the effect of only applying blen and
> tchthr when voltage is present.
 
> If this is a 224 device, but the platform data does not specify
> voltage (say, we are ok with the existing value loaded from NVRAM),
> then we won't apply blen and tchthr either.

I think all those specific settings in handle_pdata() should be removed 
altogether:

burst length is only the upper 4 bits actually;
for the touchscreen threshold I think there are additional registers 
elsewhere which should be kept in sync

expect not using pdata at all, there is no way to NOT make these settings

I think there are other places (on the mxt224e) as well where the 
orientation has to be set, so pdata->orient setting 
MXT_TOUCH_MULTI_T9@MXT_TOUCH_ORIENT is only half of the story, you still 
need correct pdata->config and pdata->orient is basically useless

the idea of the patch was to preserve the behaviour on the mxt224 but 
disable at least those two dubious settings; clearly not perfect

> On Wed, Feb 20, 2013 at 10:29 AM, Yufeng Shen <miletus@google.com> wrote:
> > On Thu, Feb 7, 2013 at 9:17 AM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
> >>         /* Set touchscreen voltage */
> >>         if (pdata->voltage && data->info.family_id == MXT224_ID) {
> >
> > As Benson noted in patch 5, what about the device that is neither 224
> > nor 224E ? burst length
> > and  threshold will no longer be applied.

regards, p.
diff mbox

Patch

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 7fdd439..313b201 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -673,14 +673,6 @@  static void mxt_handle_pdata(struct mxt_data *data)
 	mxt_write_object(data, MXT_TOUCH_MULTI_T9, MXT_TOUCH_ORIENT,
 			pdata->orient);
 
-	/* Set touchscreen burst length */
-	mxt_write_object(data, MXT_TOUCH_MULTI_T9,
-			MXT_TOUCH_BLEN, pdata->blen);
-
-	/* Set touchscreen threshold */
-	mxt_write_object(data, MXT_TOUCH_MULTI_T9,
-			MXT_TOUCH_TCHTHR, pdata->threshold);
-
 	/* Set touchscreen resolution */
 	mxt_write_object(data, MXT_TOUCH_MULTI_T9,
 			MXT_TOUCH_XRANGE_LSB, (pdata->x_size - 1) & 0xff);
@@ -693,6 +685,14 @@  static void mxt_handle_pdata(struct mxt_data *data)
 
 	/* Set touchscreen voltage */
 	if (pdata->voltage && data->info.family_id == MXT224_ID) {
+		/* Set touchscreen burst length */
+		mxt_write_object(data, MXT_TOUCH_MULTI_T9,
+				MXT_TOUCH_BLEN, pdata->blen);
+
+		/* Set touchscreen threshold */
+		mxt_write_object(data, MXT_TOUCH_MULTI_T9,
+				MXT_TOUCH_TCHTHR, pdata->threshold);
+
 		if (pdata->voltage < MXT_VOLTAGE_DEFAULT) {
 			voltage = (MXT_VOLTAGE_DEFAULT - pdata->voltage) /
 				MXT_VOLTAGE_STEP;