On Wed, Oct 17, 2012 at 05:18:56PM +0100, Sean Young wrote: >On Fri, Oct 12, 2012 at 01:11:54AM +0200, David Härdeman wrote: >> The RC_TYPE_* defines are currently used both where a single protocol is >> expected and where a bitmap of protocols is expected. This patch tries >> to separate the two in preparation for the following patches. > >I'm not sure why this is needed. I'm not sure I can explain it much better. Something like rc_keydown() or functions which add/remove entries to the keytable want a single protocol. Future userspace APIs would also benefit from numeric protocols (rather than bitmap ones). Keytables are smaller if they can use a small(ish) integer rather than a bitmap. Other functions or struct members (e.g. allowed_protos, enabled_protocols, etc) accept multiple protocols and need a bitmap. Using different types reduces the risk of programmer error. Using a protocol enum whereever possible also makes for a more future-proof user-space API as we don't need to worry about a sufficient number of bits being available (e.g. in structs used for ioctl() calls). The use of both a number and a corresponding bit is dalso one in e.g. the input subsystem as well (see all the references to set/clear bit when changing keytables for example). > >> The intended use is also clearer to anyone reading the code. Where a >> single protocol is expected, enum rc_type is used, where one or more >> protocol(s) are expected, something like u64 is used. > >Having two sets of #define and enums for the same information is not >necessarily clearer. Not only two set of define and enum, two different data types. To me it helps a lot to be able to tell from a function declaration whether it expects *a* protocol or protocols. >I don't like the name RC_BIT_* either; how about >RC_PROTO_*? I have no strong opinions here > >Sean > >> The patch has been rewritten so that the format of the sysfs "protocols" >> file is no longer altered (at the loss of some detail). The file itself >> should probably be deprecated in the future though. >> >> I missed some drivers when creating the last version of the patch because >> some weren't enabled in my .config. This patch passes an allmodyes build. >> >> Signed-off-by: David Härdeman <david@xxxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html