Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1241437 > unrolled thread

[RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read fcooper@ti.com - 2015-10-07 14:30 +0200
    Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to  determine how much to read Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2015-10-15 01:40 +0200
      Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine  how much to read "Franklin S Cooper Jr." <fcooper@ti.com> - 2015-10-15 04:00 +0200
        Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to  determine how much to read Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2015-10-16 02:20 +0200
          Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine  how much to read Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2015-10-16 02:50 +0200
            Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine  how much to read "Franklin S Cooper Jr." <fcooper@ti.com> - 2015-10-16 03:00 +0200
              Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2015-10-16 03:00 +0200
          Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine  how much to read "Franklin S Cooper Jr." <fcooper@ti.com> - 2015-10-16 02:50 +0200
            Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine  how much to read "Franklin S Cooper Jr." <fcooper@ti.com> - 2015-10-16 02:50 +0200

#1241437 — [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read

From: Franklin S Cooper Jr <fcooper@ti.com>

Calculate the amount of data that needs to be read for the specified max
number of support points. If the maximum number of support points changes
then the amount that is read from the touch screen controller should
reflect this.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 7239c31..1e0ed6e 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -178,14 +178,16 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
 		cmd = 0xf9; /* tell the controller to send touch data */
 		offset = 5; /* where the actual touch data starts */
 		tplen = 4;  /* data comes in so called frames */
-		datalen = 26; /* how much bytes to listen for */
+
+		/* how many bytes to listen for */
+		datalen = tplen * MAX_SUPPORT_POINTS + offset + 1;
 		break;
 
 	case M09:
 		cmd = 0x02;
 		offset = 1;
 		tplen = 6;
-		datalen = 29;
+		datalen = tplen * MAX_SUPPORT_POINTS - cmd + 1;
 		break;
 
 	default:
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1247253 — Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read

On Wed, Oct 07, 2015 at 07:21:38AM -0500, fcooper@ti.com wrote:
> From: Franklin S Cooper Jr <fcooper@ti.com>
> 
> Calculate the amount of data that needs to be read for the specified max
> number of support points. If the maximum number of support points changes
> then the amount that is read from the touch screen controller should
> reflect this.
> 
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> ---
>  drivers/input/touchscreen/edt-ft5x06.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index 7239c31..1e0ed6e 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -178,14 +178,16 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
>  		cmd = 0xf9; /* tell the controller to send touch data */
>  		offset = 5; /* where the actual touch data starts */
>  		tplen = 4;  /* data comes in so called frames */
> -		datalen = 26; /* how much bytes to listen for */
> +
> +		/* how many bytes to listen for */
> +		datalen = tplen * MAX_SUPPORT_POINTS + offset + 1;
>  		break;
>  
>  	case M09:
>  		cmd = 0x02;
>  		offset = 1;
>  		tplen = 6;
> -		datalen = 29;
> +		datalen = tplen * MAX_SUPPORT_POINTS - cmd + 1;
>  		break;


Hmm, why would formulae for datalen be different depending on the
firmware? And I think original 29 it too low: we need 30 bytes for 5
contacts + 1 to account for offset.

I also wonder why we need extra 1 byte in M06 case.

Lothar?

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247313 — Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read

On 10/14/2015 06:39 PM, Dmitry Torokhov wrote:
> On Wed, Oct 07, 2015 at 07:21:38AM -0500, fcooper@ti.com wrote:
>> From: Franklin S Cooper Jr <fcooper@ti.com>
>>
>> Calculate the amount of data that needs to be read for the specified max
>> number of support points. If the maximum number of support points changes
>> then the amount that is read from the touch screen controller should
>> reflect this.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> ---
>>  drivers/input/touchscreen/edt-ft5x06.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
>> index 7239c31..1e0ed6e 100644
>> --- a/drivers/input/touchscreen/edt-ft5x06.c
>> +++ b/drivers/input/touchscreen/edt-ft5x06.c
>> @@ -178,14 +178,16 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
>>  		cmd = 0xf9; /* tell the controller to send touch data */
>>  		offset = 5; /* where the actual touch data starts */
>>  		tplen = 4;  /* data comes in so called frames */
>> -		datalen = 26; /* how much bytes to listen for */
>> +
>> +		/* how many bytes to listen for */
>> +		datalen = tplen * MAX_SUPPORT_POINTS + offset + 1;
>>  		break;
>>  
>>  	case M09:
>>  		cmd = 0x02;
>>  		offset = 1;
>>  		tplen = 6;
>> -		datalen = 29;
>> +		datalen = tplen * MAX_SUPPORT_POINTS - cmd + 1;
>>  		break;
>
> Hmm, why would formulae for datalen be different depending on the
> firmware? And I think original 29 it too low: we need 30 bytes for 5
> contacts + 1 to account for offset.
So based on the current ISR we don't care about the touch weight and
which are the last two registers for each touch point. So for the last
touchpoint we really don't need to read the extra two registers (-2).
We need +1 simply for the fact that we read the register at location
cmd.

So 6 * 5 - 2 + 1 which is how we get to 29. The formula looks slightly
different because the registers we are reading are very close to zero
so the math works out to equal the equation I used for M09.

M06 since tplen = 4 then all four registers are used in the ISR per touch
point. Plus the offset and plus 1 again to account for the fact we are reading
the cmd register. But once again it would be nice if someone can confirm this.
>
> I also wonder why we need extra 1 byte in M06 case.
>
> Lothar?
>
> Thanks.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248269 — Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read

On Wed, Oct 14, 2015 at 08:58:32PM -0500, Franklin S Cooper Jr. wrote:
> 
> 
> On 10/14/2015 06:39 PM, Dmitry Torokhov wrote:
> > On Wed, Oct 07, 2015 at 07:21:38AM -0500, fcooper@ti.com wrote:
> >> From: Franklin S Cooper Jr <fcooper@ti.com>
> >>
> >> Calculate the amount of data that needs to be read for the specified max
> >> number of support points. If the maximum number of support points changes
> >> then the amount that is read from the touch screen controller should
> >> reflect this.
> >>
> >> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> >> ---
> >>  drivers/input/touchscreen/edt-ft5x06.c | 6 ++++--
> >>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> >> index 7239c31..1e0ed6e 100644
> >> --- a/drivers/input/touchscreen/edt-ft5x06.c
> >> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> >> @@ -178,14 +178,16 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
> >>  		cmd = 0xf9; /* tell the controller to send touch data */
> >>  		offset = 5; /* where the actual touch data starts */
> >>  		tplen = 4;  /* data comes in so called frames */
> >> -		datalen = 26; /* how much bytes to listen for */
> >> +
> >> +		/* how many bytes to listen for */
> >> +		datalen = tplen * MAX_SUPPORT_POINTS + offset + 1;
> >>  		break;
> >>  
> >>  	case M09:
> >>  		cmd = 0x02;
> >>  		offset = 1;
> >>  		tplen = 6;
> >> -		datalen = 29;
> >> +		datalen = tplen * MAX_SUPPORT_POINTS - cmd + 1;
> >>  		break;
> >
> > Hmm, why would formulae for datalen be different depending on the
> > firmware? And I think original 29 it too low: we need 30 bytes for 5
> > contacts + 1 to account for offset.
> So based on the current ISR we don't care about the touch weight and
> which are the last two registers for each touch point. So for the last
> touchpoint we really don't need to read the extra two registers (-2).

This is really not obvious. I do not think we'd see any performance
degradation if we actually read the whole last touchpoint.

> We need +1 simply for the fact that we read the register at location
> cmd.

I am not sure I follow this. We do not reference anything past
rdbuf[(MAX_SUPPORT_POINTS - 1) * tplen + offset] and
our offset takes care of the start position, so why exactly we need the
+1? Ah, CRC is in the extra byte.

Can we unify the calculation to be:

	datalen = tplen * MAX_SUPPORT_POINTS + offset + crc_len;

By the way, what version of firmware you tested your changes with?

> 
> So 6 * 5 - 2 + 1 which is how we get to 29. The formula looks slightly
> different because the registers we are reading are very close to zero
> so the math works out to equal the equation I used for M09.
> 
> M06 since tplen = 4 then all four registers are used in the ISR per touch
> point. Plus the offset and plus 1 again to account for the fact we are reading
> the cmd register. But once again it would be nice if someone can confirm this.
> >
> > I also wonder why we need extra 1 byte in M06 case.
> >
> > Lothar?
> >
> > Thanks.
> >
> 

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248276 — Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read

On Thu, Oct 15, 2015 at 5:43 PM, Franklin S Cooper Jr. <fcooper@ti.com> wrote:
>
>
> On 10/15/2015 07:16 PM, Dmitry Torokhov wrote:
>> On Wed, Oct 14, 2015 at 08:58:32PM -0500, Franklin S Cooper Jr. wrote:
>>>
>>> On 10/14/2015 06:39 PM, Dmitry Torokhov wrote:
>>>> On Wed, Oct 07, 2015 at 07:21:38AM -0500, fcooper@ti.com wrote:
>>>>> From: Franklin S Cooper Jr <fcooper@ti.com>
>>>>>
>>>>> Calculate the amount of data that needs to be read for the specified max
>>>>> number of support points. If the maximum number of support points changes
>>>>> then the amount that is read from the touch screen controller should
>>>>> reflect this.
>>>>>
>>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>>> ---
>>>>>  drivers/input/touchscreen/edt-ft5x06.c | 6 ++++--
>>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
>>>>> index 7239c31..1e0ed6e 100644
>>>>> --- a/drivers/input/touchscreen/edt-ft5x06.c
>>>>> +++ b/drivers/input/touchscreen/edt-ft5x06.c
>>>>> @@ -178,14 +178,16 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
>>>>>            cmd = 0xf9; /* tell the controller to send touch data */
>>>>>            offset = 5; /* where the actual touch data starts */
>>>>>            tplen = 4;  /* data comes in so called frames */
>>>>> -          datalen = 26; /* how much bytes to listen for */
>>>>> +
>>>>> +          /* how many bytes to listen for */
>>>>> +          datalen = tplen * MAX_SUPPORT_POINTS + offset + 1;
>>>>>            break;
>>>>>
>>>>>    case M09:
>>>>>            cmd = 0x02;
>>>>>            offset = 1;
>>>>>            tplen = 6;
>>>>> -          datalen = 29;
>>>>> +          datalen = tplen * MAX_SUPPORT_POINTS - cmd + 1;
>>>>>            break;
>>>> Hmm, why would formulae for datalen be different depending on the
>>>> firmware? And I think original 29 it too low: we need 30 bytes for 5
>>>> contacts + 1 to account for offset.
>>> So based on the current ISR we don't care about the touch weight and
>>> which are the last two registers for each touch point. So for the last
>>> touchpoint we really don't need to read the extra two registers (-2).
>> This is really not obvious. I do not think we'd see any performance
>> degradation if we actually read the whole last touchpoint.
> Yeah that shouldn't be a problem. I'll fix that.
>>
>>> We need +1 simply for the fact that we read the register at location
>>> cmd.
>> I am not sure I follow this. We do not reference anything past
>> rdbuf[(MAX_SUPPORT_POINTS - 1) * tplen + offset] and
>> our offset takes care of the start position, so why exactly we need the
>> +1? Ah, CRC is in the extra byte.
> Sorry your right the +1 isn't needed.
>>
>> Can we unify the calculation to be:
>>
>>       datalen = tplen * MAX_SUPPORT_POINTS + offset + crc_len;
> Why do we need the crc_len? M06 is the only one that uses the CRC
> and the offset insures we are reading the necessary crc registers.

CRC is at buf[datalen - 1] position, so it is the last byte after last
contact. That is why we have +1 for M06. For M09 crc_len will be 0.

>
> Unless I'm missing something it would simply be:
>
> datalen = tplen * MAX_SUPPORT_POINTS + offset
>
>>
>> By the way, what version of firmware you tested your changes with?
>>
>>> So 6 * 5 - 2 + 1 which is how we get to 29. The formula looks slightly
>>> different because the registers we are reading are very close to zero
>>> so the math works out to equal the equation I used for M09.
>>>
>>> M06 since tplen = 4 then all four registers are used in the ISR per touch
>>> point. Plus the offset and plus 1 again to account for the fact we are reading
>>> the cmd register. But once again it would be nice if someone can confirm this.
>>>> I also wonder why we need extra 1 byte in M06 case.
>>>>
>>>> Lothar?
>>>>
>>>> Thanks.
>>>>
>> Thanks.
>>
>

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248279 — Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read

On 10/15/2015 07:47 PM, Dmitry Torokhov wrote:
> On Thu, Oct 15, 2015 at 5:43 PM, Franklin S Cooper Jr. <fcooper@ti.com> wrote:
>>
>> On 10/15/2015 07:16 PM, Dmitry Torokhov wrote:
>>> On Wed, Oct 14, 2015 at 08:58:32PM -0500, Franklin S Cooper Jr. wrote:
>>>> On 10/14/2015 06:39 PM, Dmitry Torokhov wrote:
>>>>> On Wed, Oct 07, 2015 at 07:21:38AM -0500, fcooper@ti.com wrote:
>>>>>> From: Franklin S Cooper Jr <fcooper@ti.com>
>>>>>>
>>>>>> Calculate the amount of data that needs to be read for the specified max
>>>>>> number of support points. If the maximum number of support points changes
>>>>>> then the amount that is read from the touch screen controller should
>>>>>> reflect this.
>>>>>>
>>>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>>>> ---
>>>>>>  drivers/input/touchscreen/edt-ft5x06.c | 6 ++++--
>>>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
>>>>>> index 7239c31..1e0ed6e 100644
>>>>>> --- a/drivers/input/touchscreen/edt-ft5x06.c
>>>>>> +++ b/drivers/input/touchscreen/edt-ft5x06.c
>>>>>> @@ -178,14 +178,16 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
>>>>>>            cmd = 0xf9; /* tell the controller to send touch data */
>>>>>>            offset = 5; /* where the actual touch data starts */
>>>>>>            tplen = 4;  /* data comes in so called frames */
>>>>>> -          datalen = 26; /* how much bytes to listen for */
>>>>>> +
>>>>>> +          /* how many bytes to listen for */
>>>>>> +          datalen = tplen * MAX_SUPPORT_POINTS + offset + 1;
>>>>>>            break;
>>>>>>
>>>>>>    case M09:
>>>>>>            cmd = 0x02;
>>>>>>            offset = 1;
>>>>>>            tplen = 6;
>>>>>> -          datalen = 29;
>>>>>> +          datalen = tplen * MAX_SUPPORT_POINTS - cmd + 1;
>>>>>>            break;
>>>>> Hmm, why would formulae for datalen be different depending on the
>>>>> firmware? And I think original 29 it too low: we need 30 bytes for 5
>>>>> contacts + 1 to account for offset.
>>>> So based on the current ISR we don't care about the touch weight and
>>>> which are the last two registers for each touch point. So for the last
>>>> touchpoint we really don't need to read the extra two registers (-2).
>>> This is really not obvious. I do not think we'd see any performance
>>> degradation if we actually read the whole last touchpoint.
>> Yeah that shouldn't be a problem. I'll fix that.
>>>> We need +1 simply for the fact that we read the register at location
>>>> cmd.
>>> I am not sure I follow this. We do not reference anything past
>>> rdbuf[(MAX_SUPPORT_POINTS - 1) * tplen + offset] and
>>> our offset takes care of the start position, so why exactly we need the
>>> +1? Ah, CRC is in the extra byte.
>> Sorry your right the +1 isn't needed.
>>> Can we unify the calculation to be:
>>>
>>>       datalen = tplen * MAX_SUPPORT_POINTS + offset + crc_len;
>> Why do we need the crc_len? M06 is the only one that uses the CRC
>> and the offset insures we are reading the necessary crc registers.
> CRC is at buf[datalen - 1] position, so it is the last byte after last
> contact. That is why we have +1 for M06. For M09 crc_len will be 0.
Ok I get it now.

I can submit a v2 patchset with these changes. Are you ok with this patchset
as a whole other than these changes or should I give you more time to review
the rest before sending out a v2 and remove the RFC.

>
>> Unless I'm missing something it would simply be:
>>
>> datalen = tplen * MAX_SUPPORT_POINTS + offset
>>
>>> By the way, what version of firmware you tested your changes with?
>>>
>>>> So 6 * 5 - 2 + 1 which is how we get to 29. The formula looks slightly
>>>> different because the registers we are reading are very close to zero
>>>> so the math works out to equal the equation I used for M09.
>>>>
>>>> M06 since tplen = 4 then all four registers are used in the ISR per touch
>>>> point. Plus the offset and plus 1 again to account for the fact we are reading
>>>> the cmd register. But once again it would be nice if someone can confirm this.
>>>>> I also wonder why we need extra 1 byte in M06 case.
>>>>>
>>>>> Lothar?
>>>>>
>>>>> Thanks.
>>>>>
>>> Thanks.
>>>
> Thanks.
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248281

On October 15, 2015 5:54:14 PM PDT, "Franklin S Cooper Jr." <fcooper@ti.com> wrote:
>
>
>
>On 10/15/2015 07:47 PM, Dmitry Torokhov wrote:
>> On Thu, Oct 15, 2015 at 5:43 PM, Franklin S Cooper Jr.
><fcooper@ti.com> wrote:
>>>
>>> On 10/15/2015 07:16 PM, Dmitry Torokhov wrote:
>>>> On Wed, Oct 14, 2015 at 08:58:32PM -0500, Franklin S Cooper Jr.
>wrote:
>>>>> On 10/14/2015 06:39 PM, Dmitry Torokhov wrote:
>>>>>> On Wed, Oct 07, 2015 at 07:21:38AM -0500, fcooper@ti.com wrote:
>>>>>>> From: Franklin S Cooper Jr <fcooper@ti.com>
>>>>>>>
>>>>>>> Calculate the amount of data that needs to be read for the
>specified max
>>>>>>> number of support points. If the maximum number of support
>points changes
>>>>>>> then the amount that is read from the touch screen controller
>should
>>>>>>> reflect this.
>>>>>>>
>>>>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>>>>> ---
>>>>>>>  drivers/input/touchscreen/edt-ft5x06.c | 6 ++++--
>>>>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/input/touchscreen/edt-ft5x06.c
>b/drivers/input/touchscreen/edt-ft5x06.c
>>>>>>> index 7239c31..1e0ed6e 100644
>>>>>>> --- a/drivers/input/touchscreen/edt-ft5x06.c
>>>>>>> +++ b/drivers/input/touchscreen/edt-ft5x06.c
>>>>>>> @@ -178,14 +178,16 @@ static irqreturn_t edt_ft5x06_ts_isr(int
>irq, void *dev_id)
>>>>>>>            cmd = 0xf9; /* tell the controller to send touch data
>*/
>>>>>>>            offset = 5; /* where the actual touch data starts */
>>>>>>>            tplen = 4;  /* data comes in so called frames */
>>>>>>> -          datalen = 26; /* how much bytes to listen for */
>>>>>>> +
>>>>>>> +          /* how many bytes to listen for */
>>>>>>> +          datalen = tplen * MAX_SUPPORT_POINTS + offset + 1;
>>>>>>>            break;
>>>>>>>
>>>>>>>    case M09:
>>>>>>>            cmd = 0x02;
>>>>>>>            offset = 1;
>>>>>>>            tplen = 6;
>>>>>>> -          datalen = 29;
>>>>>>> +          datalen = tplen * MAX_SUPPORT_POINTS - cmd + 1;
>>>>>>>            break;
>>>>>> Hmm, why would formulae for datalen be different depending on the
>>>>>> firmware? And I think original 29 it too low: we need 30 bytes
>for 5
>>>>>> contacts + 1 to account for offset.
>>>>> So based on the current ISR we don't care about the touch weight
>and
>>>>> which are the last two registers for each touch point. So for the
>last
>>>>> touchpoint we really don't need to read the extra two registers
>(-2).
>>>> This is really not obvious. I do not think we'd see any performance
>>>> degradation if we actually read the whole last touchpoint.
>>> Yeah that shouldn't be a problem. I'll fix that.
>>>>> We need +1 simply for the fact that we read the register at
>location
>>>>> cmd.
>>>> I am not sure I follow this. We do not reference anything past
>>>> rdbuf[(MAX_SUPPORT_POINTS - 1) * tplen + offset] and
>>>> our offset takes care of the start position, so why exactly we need
>the
>>>> +1? Ah, CRC is in the extra byte.
>>> Sorry your right the +1 isn't needed.
>>>> Can we unify the calculation to be:
>>>>
>>>>       datalen = tplen * MAX_SUPPORT_POINTS + offset + crc_len;
>>> Why do we need the crc_len? M06 is the only one that uses the CRC
>>> and the offset insures we are reading the necessary crc registers.
>> CRC is at buf[datalen - 1] position, so it is the last byte after
>last
>> contact. That is why we have +1 for M06. For M09 crc_len will be 0.
>Ok I get it now.
>
>I can submit a v2 patchset with these changes. Are you ok with this
>patchset
>as a whole other than these changes or should I give you more time to
>review
>the rest before sending out a v2 and remove the RFC.

No, I think I'm good with the direction you are going. Please resubmit and I should be able to apply it.


Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248277 — Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read

On 10/15/2015 07:16 PM, Dmitry Torokhov wrote:
> On Wed, Oct 14, 2015 at 08:58:32PM -0500, Franklin S Cooper Jr. wrote:
>>
>> On 10/14/2015 06:39 PM, Dmitry Torokhov wrote:
>>> On Wed, Oct 07, 2015 at 07:21:38AM -0500, fcooper@ti.com wrote:
>>>> From: Franklin S Cooper Jr <fcooper@ti.com>
>>>>
>>>> Calculate the amount of data that needs to be read for the specified max
>>>> number of support points. If the maximum number of support points changes
>>>> then the amount that is read from the touch screen controller should
>>>> reflect this.
>>>>
>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>> ---
>>>>  drivers/input/touchscreen/edt-ft5x06.c | 6 ++++--
>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
>>>> index 7239c31..1e0ed6e 100644
>>>> --- a/drivers/input/touchscreen/edt-ft5x06.c
>>>> +++ b/drivers/input/touchscreen/edt-ft5x06.c
>>>> @@ -178,14 +178,16 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
>>>>  		cmd = 0xf9; /* tell the controller to send touch data */
>>>>  		offset = 5; /* where the actual touch data starts */
>>>>  		tplen = 4;  /* data comes in so called frames */
>>>> -		datalen = 26; /* how much bytes to listen for */
>>>> +
>>>> +		/* how many bytes to listen for */
>>>> +		datalen = tplen * MAX_SUPPORT_POINTS + offset + 1;
>>>>  		break;
>>>>  
>>>>  	case M09:
>>>>  		cmd = 0x02;
>>>>  		offset = 1;
>>>>  		tplen = 6;
>>>> -		datalen = 29;
>>>> +		datalen = tplen * MAX_SUPPORT_POINTS - cmd + 1;
>>>>  		break;
>>> Hmm, why would formulae for datalen be different depending on the
>>> firmware? And I think original 29 it too low: we need 30 bytes for 5
>>> contacts + 1 to account for offset.
>> So based on the current ISR we don't care about the touch weight and
>> which are the last two registers for each touch point. So for the last
>> touchpoint we really don't need to read the extra two registers (-2).
> This is really not obvious. I do not think we'd see any performance
> degradation if we actually read the whole last touchpoint.
Yeah that shouldn't be a problem. I'll fix that.
>
>> We need +1 simply for the fact that we read the register at location
>> cmd.
> I am not sure I follow this. We do not reference anything past
> rdbuf[(MAX_SUPPORT_POINTS - 1) * tplen + offset] and
> our offset takes care of the start position, so why exactly we need the
> +1? Ah, CRC is in the extra byte.
Sorry your right the +1 isn't needed.
>
> Can we unify the calculation to be:
>
> 	datalen = tplen * MAX_SUPPORT_POINTS + offset + crc_len;
Why do we need the crc_len? M06 is the only one that uses the CRC
and the offset insures we are reading the necessary crc registers.

Unless I'm missing something it would simply be:

datalen = tplen * MAX_SUPPORT_POINTS + offset

>
> By the way, what version of firmware you tested your changes with?
>
>> So 6 * 5 - 2 + 1 which is how we get to 29. The formula looks slightly
>> different because the registers we are reading are very close to zero
>> so the math works out to equal the equation I used for M09.
>>
>> M06 since tplen = 4 then all four registers are used in the ISR per touch
>> point. Plus the offset and plus 1 again to account for the fact we are reading
>> the cmd register. But once again it would be nice if someone can confirm this.
>>> I also wonder why we need extra 1 byte in M06 case.
>>>
>>> Lothar?
>>>
>>> Thanks.
>>>
> Thanks.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248278 — Re: [RFC 1/4] Input: edt-ft5x06 - Use max support points to determine how much to read

On 10/15/2015 07:43 PM, Franklin S Cooper Jr. wrote:
>
> On 10/15/2015 07:16 PM, Dmitry Torokhov wrote:
>> On Wed, Oct 14, 2015 at 08:58:32PM -0500, Franklin S Cooper Jr. wrote:
>>> On 10/14/2015 06:39 PM, Dmitry Torokhov wrote:
>>>> On Wed, Oct 07, 2015 at 07:21:38AM -0500, fcooper@ti.com wrote:
>>>>> From: Franklin S Cooper Jr <fcooper@ti.com>
>>>>>
>>>>> Calculate the amount of data that needs to be read for the specified max
>>>>> number of support points. If the maximum number of support points changes
>>>>> then the amount that is read from the touch screen controller should
>>>>> reflect this.
>>>>>
>>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>>> ---
>>>>>  drivers/input/touchscreen/edt-ft5x06.c | 6 ++++--
>>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
>>>>> index 7239c31..1e0ed6e 100644
>>>>> --- a/drivers/input/touchscreen/edt-ft5x06.c
>>>>> +++ b/drivers/input/touchscreen/edt-ft5x06.c
>>>>> @@ -178,14 +178,16 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
>>>>>  		cmd = 0xf9; /* tell the controller to send touch data */
>>>>>  		offset = 5; /* where the actual touch data starts */
>>>>>  		tplen = 4;  /* data comes in so called frames */
>>>>> -		datalen = 26; /* how much bytes to listen for */
>>>>> +
>>>>> +		/* how many bytes to listen for */
>>>>> +		datalen = tplen * MAX_SUPPORT_POINTS + offset + 1;
>>>>>  		break;
>>>>>  
>>>>>  	case M09:
>>>>>  		cmd = 0x02;
>>>>>  		offset = 1;
>>>>>  		tplen = 6;
>>>>> -		datalen = 29;
>>>>> +		datalen = tplen * MAX_SUPPORT_POINTS - cmd + 1;
>>>>>  		break;
>>>> Hmm, why would formulae for datalen be different depending on the
>>>> firmware? And I think original 29 it too low: we need 30 bytes for 5
>>>> contacts + 1 to account for offset.
>>> So based on the current ISR we don't care about the touch weight and
>>> which are the last two registers for each touch point. So for the last
>>> touchpoint we really don't need to read the extra two registers (-2).
>> This is really not obvious. I do not think we'd see any performance
>> degradation if we actually read the whole last touchpoint.
> Yeah that shouldn't be a problem. I'll fix that.
>>> We need +1 simply for the fact that we read the register at location
>>> cmd.
>> I am not sure I follow this. We do not reference anything past
>> rdbuf[(MAX_SUPPORT_POINTS - 1) * tplen + offset] and
>> our offset takes care of the start position, so why exactly we need the
>> +1? Ah, CRC is in the extra byte.
> Sorry your right the +1 isn't needed.
>> Can we unify the calculation to be:
>>
>> 	datalen = tplen * MAX_SUPPORT_POINTS + offset + crc_len;
> Why do we need the crc_len? M06 is the only one that uses the CRC
> and the offset insures we are reading the necessary crc registers.
>
> Unless I'm missing something it would simply be:
>
> datalen = tplen * MAX_SUPPORT_POINTS + offset
>
>> By the way, what version of firmware you tested your changes with?
Forgot to add. I'm using the M09 version.
>>
>>> So 6 * 5 - 2 + 1 which is how we get to 29. The formula looks slightly
>>> different because the registers we are reading are very close to zero
>>> so the math works out to equal the equation I used for M09.
>>>
>>> M06 since tplen = 4 then all four registers are used in the ISR per touch
>>> point. Plus the offset and plus 1 again to account for the fact we are reading
>>> the cmd register. But once again it would be nice if someone can confirm this.
>>>> I also wonder why we need extra 1 byte in M06 case.
>>>>
>>>> Lothar?
>>>>
>>>> Thanks.
>>>>
>> Thanks.
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web