Re: [PATCH 1/5] rc-main: add generic scancode filtering

[James Hogan <james.hogan@imgtec.com>]

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

On 26/03/14 13:44, David Härdeman wrote:
> On 2014-03-26 08:08, Antti Seppälä wrote:
>> On 26 March 2014 01:21, David Härdeman <david@hardeman.nu> wrote:
>>> On Tue, Mar 25, 2014 at 09:12:11AM +0000, James Hogan wrote:
>>>> On Tuesday 25 March 2014 00:51:46 David Härdeman wrote:
>>>>> What's the purpose of providing the sw scancode filtering in the
>>>>> case where
>>>>> there's no hardware filtering support at all?
>>>>
>>>> Consistency is probably the main reason, but I'll admit it's not
>>>> perfectly
>>>> consistent between generic/hardware filtering (mostly thanks to NEC
>>>> scancode
>>>> complexities), and I have no particular objection to dropping it if
>>>> that isn't
>>>> considered a good enough reason.
>>>
>>> I'm kind of sceptical...and given how difficult it is to remove
>>> functionality that is in a released kernel...I think that particular
>>> part (i.e. the software filtering) should be removed until it has had
>>> further discussion...
> ...
>>> I don't understand. What's the purpose of a "software fallback" for
>>> scancode filtering? Antti?
>>>
>>
>> Well since the ImgTec patches will create a new sysfs interface for
>> the HW scancode filtering I figured that it would be nice for it to
>> also function on devices which lack the hardware filtering
>> capabilities. Especially since it's only three lines of code. :)
>>
>> Therefore I suggested the software fallback. At the time I had no clue
>> that there might be added complexities with nec scancodes.
> 
> It's not only NEC scancodes, the sw scancode filter is state that is
> changeable from user-space and which will require reader/writer
> synchronization during the RX path (which is the "hottest" path in
> rc-core). I've posted patches before which make the RX path lockless,
> this change makes complicates such changes.
> 
> Additionally, the provision of the sw fallback means that userspace has
> no idea if there is an actual hardware filter present or not, meaning
> that a userspace program that is aware of the scancode filter will
> always enable it.
> 
> So, I still think the SW part should be reverted, at least for now (i.e.
> the sysfs file should only be present if there is hardware support).

I've prepared a revert patch (which also reverts a part of a later patch
which takes SW filtering into account when updating the filter on a
protocol change). I'll double check it works right later and submit.

Note that this still leaves the sysfs nodes there though, but if
!s_filter then they read as 0 and only accept the value 0 to be written
(mask == 0 represents no filtering).

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]