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

Groups > linux.kernel > #1363629 > unrolled thread

Re: [PATCH] Input: Do not add SYN_REPORT in between a single packet data

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

  Re: [PATCH] Input: Do not add SYN_REPORT in between a single packet data Aniroop Mathur <aniroop.mathur@gmail.com> - 2016-03-23 20:40 +0100
    Re: [PATCH] Input: Do not add SYN_REPORT in between a single packet data Aniroop Mathur <aniroop.mathur@gmail.com> - 2016-03-30 19:20 +0200

#1363629 — Re: [PATCH] Input: Do not add SYN_REPORT in between a single packet data

Hello Mr. Torokhov / Mr. Henry,

On Wed, Mar 16, 2016 at 11:54 PM, Aniroop Mathur
<aniroop.mathur@gmail.com> wrote:
> Hello Mr. Torokhov,
>
> Could you kindly help to update about this patch?
>

So is this patch concluded? Are you applying it?

Thanks,
Aniroop Mathur

> Thank you,
> Aniroop Mathur
>
>
> On Fri, Mar 11, 2016 at 12:26 AM, Aniroop Mathur
> <aniroop.mathur@gmail.com> wrote:
>> Hi Henrik,
>>
>> On Thu, Mar 10, 2016 at 7:15 PM, Henrik Rydberg <rydberg@bitmath.org> wrote:
>>> Hi Dmitry,
>>>
>>>>> diff --git a/drivers/input/input.c b/drivers/input/input.c
>>>>> index 8806059..262ef77 100644
>>>>> --- a/drivers/input/input.c
>>>>> +++ b/drivers/input/input.c
>>>>> @@ -401,8 +401,7 @@ static void input_handle_event(struct input_dev *dev,
>>>>>                 if (dev->num_vals >= 2)
>>>>>                         input_pass_values(dev, dev->vals, dev->num_vals);
>>>>>                 dev->num_vals = 0;
>>>>> -       } else if (dev->num_vals >= dev->max_vals - 2) {
>>>>> -               dev->vals[dev->num_vals++] = input_value_sync;
>>>>> +       } else if (dev->num_vals >= dev->max_vals - 1) {
>>>>>                 input_pass_values(dev, dev->vals, dev->num_vals);
>>>>>                 dev->num_vals = 0;
>>>>>         }
>>>>
>>>> This makes sense to me. Henrik?
>>>
>>> I went through the commits that made these changes, and I cannot see any strong
>>> reason to keep it. However, this code path only triggers if no SYN events are
>>> seen, as in a driver that fails to emit them and consequently fills up the
>>> buffer. In other words, this change would only affect a device that is already,
>>> to some degree, broken.
>>>
>>> So, the question to Aniroop is: do you see this problem in practise, and in that
>>> case, for what driver?
>>>
>>
>> Nope. So far I have not dealt with any such driver.
>> I made this change because it is breaking protocol of SYN_REPORT event code.
>>
>> Further from the code, I could deduce that max_vals is just an estimation of
>> packet_size and it does not guarantee that packet_size is same as max_vals.
>> So real packet_size can be more than max_vals value and hence we could not
>> insert SYN_REPORT until packet ends really.
>> Further, if we consider that there exists a driver or will exist in future
>> which sets capability of x event code according to which max_value comes out to
>> y and the real packet size is z i.e. driver wants to send same event codes
>> again in the same packet, so input event reader would be expecting SYN_REPORT
>> after z events but due to current code SYN_REPORT will get inserted
>> automatically after y events, which is a wrong behaviour.
>>
>> Thanks,
>> Aniroop Mathur
>>
>>> Henrik
>>>

[toc] | [next] | [standalone]

#1367476

Hello Mr. Henrik,

So do you have any further comments on this patch please?
It is pending for more than 20 days. It would be really appreciating
if you could help out to conclude it as soon as possible.

Regards,
Aniroop Mathur

On Thu, Mar 24, 2016 at 1:05 AM, Aniroop Mathur
<aniroop.mathur@gmail.com> wrote:
> Hello Mr. Torokhov / Mr. Henry,
>
> On Wed, Mar 16, 2016 at 11:54 PM, Aniroop Mathur
> <aniroop.mathur@gmail.com> wrote:
>> Hello Mr. Torokhov,
>>
>> Could you kindly help to update about this patch?
>>
>
> So is this patch concluded? Are you applying it?
>
> Thanks,
> Aniroop Mathur
>
>> Thank you,
>> Aniroop Mathur
>>
>>
>> On Fri, Mar 11, 2016 at 12:26 AM, Aniroop Mathur
>> <aniroop.mathur@gmail.com> wrote:
>>> Hi Henrik,
>>>
>>> On Thu, Mar 10, 2016 at 7:15 PM, Henrik Rydberg <rydberg@bitmath.org> wrote:
>>>> Hi Dmitry,
>>>>
>>>>>> diff --git a/drivers/input/input.c b/drivers/input/input.c
>>>>>> index 8806059..262ef77 100644
>>>>>> --- a/drivers/input/input.c
>>>>>> +++ b/drivers/input/input.c
>>>>>> @@ -401,8 +401,7 @@ static void input_handle_event(struct input_dev *dev,
>>>>>>                 if (dev->num_vals >= 2)
>>>>>>                         input_pass_values(dev, dev->vals, dev->num_vals);
>>>>>>                 dev->num_vals = 0;
>>>>>> -       } else if (dev->num_vals >= dev->max_vals - 2) {
>>>>>> -               dev->vals[dev->num_vals++] = input_value_sync;
>>>>>> +       } else if (dev->num_vals >= dev->max_vals - 1) {
>>>>>>                 input_pass_values(dev, dev->vals, dev->num_vals);
>>>>>>                 dev->num_vals = 0;
>>>>>>         }
>>>>>
>>>>> This makes sense to me. Henrik?
>>>>
>>>> I went through the commits that made these changes, and I cannot see any strong
>>>> reason to keep it. However, this code path only triggers if no SYN events are
>>>> seen, as in a driver that fails to emit them and consequently fills up the
>>>> buffer. In other words, this change would only affect a device that is already,
>>>> to some degree, broken.
>>>>
>>>> So, the question to Aniroop is: do you see this problem in practise, and in that
>>>> case, for what driver?
>>>>
>>>
>>> Nope. So far I have not dealt with any such driver.
>>> I made this change because it is breaking protocol of SYN_REPORT event code.
>>>
>>> Further from the code, I could deduce that max_vals is just an estimation of
>>> packet_size and it does not guarantee that packet_size is same as max_vals.
>>> So real packet_size can be more than max_vals value and hence we could not
>>> insert SYN_REPORT until packet ends really.
>>> Further, if we consider that there exists a driver or will exist in future
>>> which sets capability of x event code according to which max_value comes out to
>>> y and the real packet size is z i.e. driver wants to send same event codes
>>> again in the same packet, so input event reader would be expecting SYN_REPORT
>>> after z events but due to current code SYN_REPORT will get inserted
>>> automatically after y events, which is a wrong behaviour.
>>>
>>> Thanks,
>>> Aniroop Mathur
>>>
>>>> Henrik
>>>>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web