Re: [PATCH] rc-core: add separate defines for protocol bitmaps and numbers
On Mon, Dec 17, 2012 at 03:15:27PM +, James Hogan wrote:
On 12 October 2012 00:11, David Härdeman da...@hardeman.nu 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.
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.
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 da...@hardeman.nu
---
@@ -38,7 +70,7 @@ struct rc_map {
unsigned intsize; /* Max number of entries */
unsigned intlen;/* Used number of entries */
unsigned intalloc; /* Size of *scan in bytes */
- u64 rc_type;
+ enum rc_typerc_type;
const char *name;
spinlock_t lock;
};
But store_protocols() sets dev-rc_map.rc_type to a bitmap. Am I
missing something?
That was fixed in later patches (by introducing a u64 enabled_protocols
member to struct rc_dev which is used by store_protocols() and
show_protocols()).
I'll split that part out to a separate patch and submit.
--
David Härdeman
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rc-core: add separate defines for protocol bitmaps and numbers
On 12 October 2012 00:11, David Härdeman da...@hardeman.nu 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.
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.
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 da...@hardeman.nu
---
@@ -38,7 +70,7 @@ struct rc_map {
unsigned intsize; /* Max number of entries */
unsigned intlen;/* Used number of entries */
unsigned intalloc; /* Size of *scan in bytes */
- u64 rc_type;
+ enum rc_typerc_type;
const char *name;
spinlock_t lock;
};
But store_protocols() sets dev-rc_map.rc_type to a bitmap. Am I
missing something?
Cheers
James
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rc-core: add separate defines for protocol bitmaps and numbers
On Thu, Oct 18, 2012 at 11:59:21PM +0200, David Härdeman wrote: 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. Right, thanks for elaborating. Makes sense. Sean -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rc-core: add separate defines for protocol bitmaps and numbers
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 da...@hardeman.nu -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
