diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 9fa93bb923d70f..6085eb8c8d8558 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -126,8 +126,8 @@ static int sdw_drv_probe(struct device *dev) if (slave->prop.use_domain_irq) sdw_irq_create_mapping(slave); - /* init the sysfs as we have properties now */ - ret = sdw_slave_sysfs_init(slave); + /* init the dynamic sysfs attributes we need */ + ret = sdw_slave_sysfs_dpn_init(slave); if (ret < 0) dev_warn(dev, "Slave sysfs init failed:%d\n", ret); @@ -221,6 +221,7 @@ int __sdw_register_driver(struct sdw_driver *drv, struct module *owner) drv->driver.probe = sdw_drv_probe; drv->driver.remove = sdw_drv_remove; drv->driver.shutdown = sdw_drv_shutdown; + drv->driver.dev_groups = sdw_attr_groups; return driver_register(&drv->driver); } diff --git a/drivers/soundwire/sysfs_local.h b/drivers/soundwire/sysfs_local.h index 7268bc24c5384f..fa048e112629a2 100644 --- a/drivers/soundwire/sysfs_local.h +++ b/drivers/soundwire/sysfs_local.h @@ -11,8 +11,10 @@ /* basic attributes to report status of Slave (attachment, dev_num) */ extern const struct attribute_group *sdw_slave_status_attr_groups[]; +/* attributes for all soundwire devices */ +extern const struct attribute_group *sdw_attr_groups[]; + /* additional device-managed properties reported after driver probe */ -int sdw_slave_sysfs_init(struct sdw_slave *slave); int sdw_slave_sysfs_dpn_init(struct sdw_slave *slave); #endif /* __SDW_SYSFS_LOCAL_H */ diff --git a/drivers/soundwire/sysfs_slave.c b/drivers/soundwire/sysfs_slave.c index 3210359cd94428..f4259710dd0f72 100644 --- a/drivers/soundwire/sysfs_slave.c +++ b/drivers/soundwire/sysfs_slave.c @@ -105,7 +105,10 @@ static struct attribute *slave_attrs[] = { &dev_attr_modalias.attr, NULL, }; -ATTRIBUTE_GROUPS(slave); + +static const struct attribute_group slave_attr_group = { + .attrs = slave_attrs, +}; static struct attribute *slave_dev_attrs[] = { &dev_attr_mipi_revision.attr, @@ -126,10 +129,6 @@ static struct attribute *slave_dev_attrs[] = { NULL, }; -/* - * we don't use ATTRIBUTES_GROUP here since we want to add a subdirectory - * for device-level properties - */ static const struct attribute_group sdw_slave_dev_attr_group = { .attrs = slave_dev_attrs, .name = "dev-properties", @@ -181,41 +180,38 @@ static struct attribute *dp0_attrs[] = { NULL, }; -/* - * we don't use ATTRIBUTES_GROUP here since we want to add a subdirectory - * for dp0-level properties - */ -static const struct attribute_group dp0_group = { - .attrs = dp0_attrs, - .name = "dp0", -}; - -int sdw_slave_sysfs_init(struct sdw_slave *slave) +static umode_t dp0_attr_visible(struct kobject *kobj, struct attribute *attr, + int n) { - int ret; + struct sdw_slave *slave = dev_to_sdw_dev(kobj_to_dev(kobj)); - ret = devm_device_add_groups(&slave->dev, slave_groups); - if (ret < 0) - return ret; + if (slave->prop.dp0_prop) + return attr->mode; + return 0; +} - ret = devm_device_add_group(&slave->dev, &sdw_slave_dev_attr_group); - if (ret < 0) - return ret; +static bool dp0_group_visible(struct kobject *kobj) +{ + struct sdw_slave *slave = dev_to_sdw_dev(kobj_to_dev(kobj)); - if (slave->prop.dp0_prop) { - ret = devm_device_add_group(&slave->dev, &dp0_group); - if (ret < 0) - return ret; - } + if (slave->prop.dp0_prop) + return true; + return false; +} +DEFINE_SYSFS_GROUP_VISIBLE(dp0); - if (slave->prop.source_ports || slave->prop.sink_ports) { - ret = sdw_slave_sysfs_dpn_init(slave); - if (ret < 0) - return ret; - } +static const struct attribute_group dp0_group = { + .attrs = dp0_attrs, + .is_visible = SYSFS_GROUP_VISIBLE(dp0), + .name = "dp0", +}; - return 0; -} +const struct attribute_group *sdw_attr_groups[] = { + &slave_attr_group, + &sdw_slave_dev_attr_group, + &dp0_group, + NULL, +}; /* * the status is shown in capital letters for UNATTACHED and RESERVED diff --git a/drivers/soundwire/sysfs_slave_dpn.c b/drivers/soundwire/sysfs_slave_dpn.c index c4b6543c09fd67..a3fb380ee51941 100644 --- a/drivers/soundwire/sysfs_slave_dpn.c +++ b/drivers/soundwire/sysfs_slave_dpn.c @@ -283,6 +283,9 @@ int sdw_slave_sysfs_dpn_init(struct sdw_slave *slave) int ret; int i; + if (!slave->prop.source_ports && !slave->prop.sink_ports) + return 0; + mask = slave->prop.source_ports; for_each_set_bit(i, &mask, 32) { ret = add_all_attributes(&slave->dev, i, 1); diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c index 138676463336c7..683c0b10990b79 100644 --- a/fs/sysfs/group.c +++ b/fs/sysfs/group.c @@ -31,6 +31,23 @@ static void remove_files(struct kernfs_node *parent, kernfs_remove_by_name(parent, (*bin_attr)->attr.name); } +static umode_t __first_visible(const struct attribute_group *grp, struct kobject *kobj) +{ + if (grp->attrs && grp->is_visible) { + pr_info("kobj: %s is_visible: %pS\n", kobj->name, + grp->is_visible); + return grp->is_visible(kobj, grp->attrs[0], 0); + } + + if (grp->bin_attrs && grp->is_bin_visible) { + pr_info("kobj: %s is_bin_visible: %pS\n", kobj->name, + grp->is_bin_visible); + return grp->is_bin_visible(kobj, grp->bin_attrs[0], 0); + } + + return 0; +} + static int create_files(struct kernfs_node *parent, struct kobject *kobj, kuid_t uid, kgid_t gid, const struct attribute_group *grp, int update) @@ -51,7 +68,10 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj, if (update) kernfs_remove_by_name(parent, (*attr)->name); if (grp->is_visible) { + pr_info("kobj: %s is_visible: %pS\n", + kobj->name, grp->is_visible); mode = grp->is_visible(kobj, *attr, i); + mode &= ~SYSFS_GROUP_INVISIBLE; if (!mode) continue; } @@ -80,7 +100,10 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj, kernfs_remove_by_name(parent, (*bin_attr)->attr.name); if (grp->is_bin_visible) { + pr_info("kobj: %s is_bin_visible: %pS\n", + kobj->name, grp->is_bin_visible); mode = grp->is_bin_visible(kobj, *bin_attr, i); + mode &= ~SYSFS_GROUP_INVISIBLE; if (!mode) continue; } @@ -127,16 +150,31 @@ static int internal_create_group(struct kobject *kobj, int update, kobject_get_ownership(kobj, &uid, &gid); if (grp->name) { + umode_t mode = __first_visible(grp, kobj); + + if (mode & SYSFS_GROUP_INVISIBLE) + mode = 0; + else + mode = S_IRWXU | S_IRUGO | S_IXUGO; + if (update) { kn = kernfs_find_and_get(kobj->sd, grp->name); if (!kn) { - pr_warn("Can't update unknown attr grp name: %s/%s\n", - kobj->name, grp->name); - return -EINVAL; + pr_debug("attr grp %s/%s not created yet\n", + kobj->name, grp->name); + /* may have been invisible prior to this update */ + update = 0; + } else if (!mode) { + sysfs_remove_group(kobj, grp); + kernfs_put(kn); + return 0; } - } else { - kn = kernfs_create_dir_ns(kobj->sd, grp->name, - S_IRWXU | S_IRUGO | S_IXUGO, + } + + if (!update) { + if (!mode) + return 0; + kn = kernfs_create_dir_ns(kobj->sd, grp->name, mode, uid, gid, kobj, NULL); if (IS_ERR(kn)) { if (PTR_ERR(kn) == -EEXIST) @@ -279,9 +317,8 @@ void sysfs_remove_group(struct kobject *kobj, if (grp->name) { kn = kernfs_find_and_get(parent, grp->name); if (!kn) { - WARN(!kn, KERN_WARNING - "sysfs group '%s' not found for kobject '%s'\n", - grp->name, kobject_name(kobj)); + pr_debug("sysfs group '%s' not found for kobject '%s'\n", + grp->name, kobject_name(kobj)); return; } } else { diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index b717a70219f651..a42642b277dd07 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -61,22 +61,32 @@ do { \ /** * struct attribute_group - data structure used to declare an attribute group. * @name: Optional: Attribute group name - * If specified, the attribute group will be created in - * a new subdirectory with this name. + * If specified, the attribute group will be created in a + * new subdirectory with this name. Additionally when a + * group is named, @is_visible and @is_bin_visible may + * return SYSFS_GROUP_INVISIBLE to control visibility of + * the directory itself. * @is_visible: Optional: Function to return permissions associated with an - * attribute of the group. Will be called repeatedly for each - * non-binary attribute in the group. Only read/write + * attribute of the group. Will be called repeatedly for + * each non-binary attribute in the group. Only read/write * permissions as well as SYSFS_PREALLOC are accepted. Must - * return 0 if an attribute is not visible. The returned value - * will replace static permissions defined in struct attribute. + * return 0 if an attribute is not visible. The returned + * value will replace static permissions defined in struct + * attribute. Use SYSFS_GROUP_VISIBLE() when assigning this + * callback to specify separate _group_visible() and + * _attr_visible() handlers. * @is_bin_visible: * Optional: Function to return permissions associated with a * binary attribute of the group. Will be called repeatedly * for each binary attribute in the group. Only read/write - * permissions as well as SYSFS_PREALLOC are accepted. Must - * return 0 if a binary attribute is not visible. The returned - * value will replace static permissions defined in - * struct bin_attribute. + * permissions as well as SYSFS_PREALLOC (and the + * visibility flags for named groups) are accepted. Must + * return 0 if a binary attribute is not visible. The + * returned value will replace static permissions defined + * in struct bin_attribute. If @is_visible is not set, Use + * SYSFS_GROUP_VISIBLE() when assigning this callback to + * specify separate _group_visible() and _attr_visible() + * handlers. * @attrs: Pointer to NULL terminated list of attributes. * @bin_attrs: Pointer to NULL terminated list of binary attributes. * Either attrs or bin_attrs or both must be provided. @@ -91,13 +101,42 @@ struct attribute_group { struct bin_attribute **bin_attrs; }; +#define SYSFS_PREALLOC 010000 +#define SYSFS_GROUP_INVISIBLE 020000 + +/* + * The first call to is_visible() in the create / update path may + * indicate visibility for the entire group + */ +#define DEFINE_SYSFS_GROUP_VISIBLE(name) \ + static inline umode_t sysfs_group_visible_##name( \ + struct kobject *kobj, struct attribute *attr, int n) \ + { \ + if (n == 0 && !name##_group_visible(kobj)) \ + return SYSFS_GROUP_INVISIBLE; \ + return name##_attr_visible(kobj, attr, n); \ + } + +/* + * Same as DEFINE_SYSFS_GROUP_VISIBLE, but for groups with only binary + * attributes + */ +#define DEFINE_SYSFS_BIN_GROUP_VISIBLE(name) \ + static inline umode_t sysfs_group_visible_##name( \ + struct kobject *kobj, struct bin_attribute *attr, int n) \ + { \ + if (n == 0 && !name##_group_visible(kobj)) \ + return SYSFS_GROUP_INVISIBLE; \ + return name##_attr_visible(kobj, attr, n); \ + } + +#define SYSFS_GROUP_VISIBLE(fn) sysfs_group_visible_##fn + /* * Use these macros to make defining attributes easier. * See include/linux/device.h for examples.. */ -#define SYSFS_PREALLOC 010000 - #define __ATTR(_name, _mode, _show, _store) { \ .attr = {.name = __stringify(_name), \ .mode = VERIFY_OCTAL_PERMISSIONS(_mode) }, \