On Thu, Jul 11, 2013 at 01:58:29PM +0200, Oliver Schinagl wrote:
> On 11-07-13 02:36, Greg Kroah-Hartman wrote:
> >To make it easier for driver subsystems to work with attribute groups,
> >create the ATTRIBUTE_GROUPS macro to remove some of the repetitive
> >typing for the most common use for attribute groups.
> But binary groups are discriminated against :(
Yes, as they are "rarer" by far, as they should be. binary sysfs files
should almost never be used, as they are only "pass-through" files to
the hardware, so I want to see you do more work in order to use them, as
they should not be created lightly.
> The attached patch should help here.
Can you give me an example of using these macros? I seem to be lost in
them somehow, or maybe my morning coffee just hasn't kicked in...
> I suppose one more additional helper wouldn't be bad to have:
>
> #define ATTRIBUTE_(BIN_)GROUPS_R[O/W](_name(, _size)) \
> ATTRIBUTE_(BIN_)ATTR_R[O/W](_name, _size); \
> ATTRIBUTE_(BIN_)GROUPS(_name)
Would that ever be needed?
> >From 003ab7a74ff689daa6934e7bc50c498b2d35a1cc Mon Sep 17 00:00:00 2001
> From: Oliver Schinagl <oli...@schinagl.nl>
> Date: Thu, 11 Jul 2013 13:48:18 +0200
> Subject: [PATCH] sysfs: add more helper macro's for (bin_)attribute(_groups)
>
> With the recent changes to sysfs there's various helper macro's.
> However there's no RW, RO BIN_ helper macro's. This patch adds them.
>
> Additionally there are no BIN_ group helpers so there's that aswell
> Moreso, if both bin and normal attribute groups are used, there's a
> simple helper for that, though the naming code be better. _TXT_ for the
> show/store ones and neither TXT or BIN for both, but that would change
> things to extensivly.
>
> Finally there's also helpers for ATTRIBUTE_ATTRS.
>
> After this patch, create default attributes can be as easy as:
>
> ATTRIBUTE_(BIN_)ATTR_RO(name, SIZE);
> ATTRIBUTE_BIN_GROUPS(name);
>
> Signed-off-by: Oliver Schinagl <oli...@schinagl.nl>
> ---
> include/linux/sysfs.h | 96
> ++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 84 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 2c3b6a3..0ebed11 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -17,6 +17,7 @@
> #include <linux/list.h>
> #include <linux/lockdep.h>
> #include <linux/kobject_ns.h>
> +#include <linux/stat.h>
> #include <linux/atomic.h>
>
> struct kobject;
> @@ -94,15 +95,32 @@ struct attribute_group {
> #define __ATTR_IGNORE_LOCKDEP __ATTR
> #endif
>
> -#define ATTRIBUTE_GROUPS(name) \
> -static const struct attribute_group name##_group = { \
> - .attrs = name##_attrs, \
> +#define __ATTRIBUTE_GROUPS(_name) \
> +static const struct attribute_group *_name##_groups[] = { \
> + &_name##_group, \
> + NULL, \
> +}
> +
> +#define ATTRIBUTE_GROUPS(_name) \
> +static const struct attribute_group _name##_group = { \
> + .attrs = _name##_attrs, \
> }; \
> -static const struct attribute_group *name##_groups[] = { \
> - &name##_group, \
> +__ATTRIBUTE_GROUPS(_name)
> +
> +#define __ATTRIBUTE_ATTRS(_name) \
> +struct bin_attribute *_name##_attrs[] = { \
> + &_name##_attr, \
> NULL, \
> }
>
> +#define ATTRIBUTE_ATTR_RO(_name, _size) \
> +struct attribute _name##_attr = __ATTR_RO(_name, _size); \
> +__ATTRIBUTE_ATTRS(_name)
> +
> +#define ATTRIBUTE_ATTR_RW(_name, _size) \
> +struct attribute _name##_attr = __ATTR_RW(_name, _size); \
> +__ATTRIBUTE_ATTRS(_name)
What do these two help out with? "attribute attribute read-write" seems
a bit "clunky", don't you think? :)
> +
> #define attr_name(_attr) (_attr).attr.name
>
> struct file;
> @@ -132,15 +150,69 @@ struct bin_attribute {
> */
> #define sysfs_bin_attr_init(bin_attr) sysfs_attr_init(&(bin_attr)->attr)
>
> -/* macro to create static binary attributes easier */
> -#define BIN_ATTR(_name, _mode, _read, _write, _size) \
> -struct bin_attribute bin_attr_##_name = { \
> - .attr = {.name = __stringify(_name), .mode = _mode }, \
> - .read = _read, \
> - .write = _write, \
> - .size = _size, \
> +/* macros to create static binary attributes easier */
> +#define __BIN_ATTR(_name, _mode, _read, _write, _size) { \
> + .attr = { .name = __stringify(_name), .mode = _mode }, \
> + .read = _read, \
> + .write = _write, \
> + .size = _size, \
> +}
> +
> +#define __BIN_ATTR_RO(_name, _size) {
> \
> + .attr = { .name = __stringify(_name), .mode = S_IRUGO }, \
> + .read = _name##_read, \
> + .size = _size, \
> +}
> +
> +#define __BIN_ATTR_RW(_name, _size) __BIN_ATTR(_name,
> \
> + (S_IWUSR | S_IRUGO), _name##_read, \
> + _name##_write)
> +
> +#define __BIN_ATTR_NULL __ATTR_NULL
> +
> +#define BIN_ATTR(_name, _mode, _read, _write, _size) \
> +struct bin_attribute bin_attr_##_name = __BIN_ATTR(_name, _mode, _read,
> \
> + _write, _size)
> +
> +#define BIN_RO_ATTR(_name, _size) \
> +struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
> +
> +#define BIN_RW_ATTR(_name, _size) \
> +struct bin_attribute bin_attr_##_name = __BIN_ATTR_RW(_name, _size)
To be consistent, these shoudl be BIN_ATTR_RO and BIN_ATTR_RW, right?
These all look fine to me, thanks.
It's these that I'm confused about:
> +
> +#define __ATTRIBUTE_BIN_GROUPS(_name)
> \
> +static const struct attribute_group *_name##_bin_groups[] = {
> \
> + &_name##_bin_group, \
> + NULL, \
> }
>
> +#define ATTRIBUTE_BIN_GROUPS(_name) \
> +static const struct attribute_group _name##_bin_group = { \
> + .bin_attrs = _name##_bin_attrs, \
> +}; \
> +__ATTRIBUTE_BIN_GROUPS(_name)
> +
> +#define ATTRIBUTE_FULL_GROUPS(_name) \
> +static const struct attribute_group _name##_full_group = { \
> + .attrs = _name##_attrs, \
> + .bin_attrs = _name##_bin_attrs, \
> +}; \
> +__ATTRIBUTE_GROUPS(_name); __ATTRIBUTE_BIN_GROUPS(_name)
> +
> +#define __ATTRIBUTE_BIN_ATTRS(_name) \
> +struct bin_attribute *_name##_bin_attrs[] = {
> \
> + &_name##_bin_attr, \
> + NULL, \
> +}
> +
> +#define ATTRIBUTE_BIN_ATTR_RO(_name, _size) \
> +struct bin_attribute _name##_bin_attr = __BIN_ATTR_RO(_name, _size); \
> +__ATTRIBUTE_BIN_ATTRS(_name)
> +
> +#define ATTRIBUTE_BIN_ATTR_RW(_name, _size) \
> +struct bin_attribute _name##_bin_attr = __BIN_ATTR_RW(_name, _size); \
> +__ATTRIBUTE_BIN_ATTRS(_name)
Can you show me how these would be used in a real-world example?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
