Re: [PATCH v2 3/3] [media] rc: remove change_protocol in rc-ir-raw.c

[zhangfei <zhangfei.gao@linaro.org>]

On 08/27/2014 07:34 PM, Mauro Carvalho Chehab wrote:

>>>> With commit 4924a311a62f ("[media] rc-core: rename ir-raw.c"),
>>>> empty change_protocol was introduced.
>>>
>>> No. This was introduced on this changeset:
>>>
>>> commit da6e162d6a4607362f8478c715c797d84d449f8b
>>> Author: David Härdeman <david@hardeman.nu>
>>> Date:   Thu Apr 3 20:32:16 2014 -0300
>>>
>>>       [media] rc-core: simplify sysfs code
>>>
>>>> As a result, rc_register_device will set dev->enabled_protocols
>>>> addording to rc_map->rc_type, which prevent using all protocols.
>>>
>>> I strongly suspect that this patch will break some things, as
>>> the new code seems to expect that this is always be set.
>>>
>>> See the code at store_protocols(): if this callback is not set,
>>> then it won't allow to disable a protocol.
>>>
>>> Also, this doesn't prevent using all protocols. You can still use
>>> "ir-keytable -p all" to enable all protocols (the "all" protocol
>>> type were introduced recently at the userspace tool).
>>>
>>>   From the way I see, setting the protocol when a table is loaded
>>> is not a bad thing, as:
>>> - if RC tables are loaded, the needed protocol to decode it is
>>>     already known;
>>> - by running just one IR decoder, the IR handling routine will
>>>     be faster and will consume less power;
>>> - on a real case scenario, it is a way more likely that just one
>>>     decoder will ever be needed by the end user.
>>>
>>> So, I think that this is just annoying for developers when are checking
>>> if all decoders are working, by sending keycodes from different IR types
>>> at the same time.
>>>
>>
>> Thanks Mauro for the kind explanation.
>>
>> ir-keytable seems also enalbe specific protocol
>> -p, --protocol=PROTOCOL
>>
>> Currently we use lirc user space decoder/keymap and only need
>> pulse-length information from kernel.
>
> Well, you can use ir-keytable to disable everything but lirc, not
> compile the other hardware decoders or directly write "lirc" to
> /sys/class/rc/rc0/protocols (see Documentation/ABI/testing/sysfs-class-rc).
>
> Anyway, I suggest you to use the hardware decoder instead of lirc,
> as the in-kernel decoders should be lighter than lirc and works pretty
> well, but this is, of course, your decision.
>
> Btw, it would make sense, IMHO, to have a way to setup LIRC daemon to
> enable LIRC output on a given remote controller, and, optionally,
> disabling the hardware decoders that are needlessly enabled.
>

Thanks Mauro

Double checked, both ir-keytable and /sys/class/rc/rc0/protocols can 
enable/disable protocols, which is much easier than dts passing 
information, and by default all protocols are disabled.

Thanks for this kind information.