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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






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@xxxxxxxxxxx>
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.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux