Re: [PATCH v2 4/7] driver core: add devm_device_add_group() and friends

From: Dmitry Torokhov
Date: Thu Jul 20 2017 - 04:13:24 EST


On July 19, 2017 10:10:18 PM PDT, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>On Wed, Jul 19, 2017 at 05:24:33PM -0700, Dmitry Torokhov wrote:
>> Many drivers create additional driver-specific device attributes when
>> binding to the device, and providing managed version of
>> device_create_group() will simplify unbinding and error handling in
>probe
>> path for such drivers.
>>
>> Without managed version driver writers either have to mix manual and
>> managed resources, which is prone to errors, or open-code this
>function by
>> providing a wrapper to device_add_group() and use it with
>devm_add_action()
>> or devm_add_action_or_reset().
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>> ---
>> drivers/base/core.c | 130
>+++++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/device.h | 9 ++++
>> 2 files changed, 139 insertions(+)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 14f8cf5c8b05..09723532725d 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -1035,6 +1035,136 @@ void device_remove_groups(struct device *dev,
>> }
>> EXPORT_SYMBOL_GPL(device_remove_groups);
>>
>> +union device_attr_group_devres {
>> + const struct attribute_group *group;
>> + const struct attribute_group **groups;
>> +};
>> +
>> +static int devm_attr_group_match(struct device *dev, void *res, void
>*data)
>> +{
>> + return ((union device_attr_group_devres *)res)->group == data;
>> +}
>> +
>> +static void devm_attr_group_remove(struct device *dev, void *res)
>> +{
>> + union device_attr_group_devres *devres = res;
>> + const struct attribute_group *group = devres->group;
>> +
>> + dev_dbg(dev, "%s: removing group %p\n", __func__, group);
>> + sysfs_remove_group(&dev->kobj, group);
>> +}
>> +
>> +static void devm_attr_groups_remove(struct device *dev, void *res)
>> +{
>> + union device_attr_group_devres *devres = res;
>> + const struct attribute_group **groups = devres->groups;
>> +
>> + dev_dbg(dev, "%s: removing groups %p\n", __func__, groups);
>> + sysfs_remove_groups(&dev->kobj, groups);
>> +}
>> +
>> +/**
>> + * devm_device_add_group - given a device, create a managed
>attribute group
>> + * @dev: The device to create the group for
>> + * @grp: The attribute group to create
>> + *
>> + * This function creates a group for the first time. It will
>explicitly
>> + * warn and error if any of the attribute files being created
>already exist.
>> + *
>> + * Returns 0 on success or error code on failure.
>> + */
>> +int devm_device_add_group(struct device *dev, const struct
>attribute_group *grp)
>> +{
>> + union device_attr_group_devres *devres;
>> + int error;
>> +
>> + devres = devres_alloc(devm_attr_group_remove,
>> + sizeof(*devres), GFP_KERNEL);
>> + if (!devres)
>> + return -ENOMEM;
>> +
>> + error = sysfs_create_group(&dev->kobj, grp);
>
>Minor nit, this can now call device_create_group(), right?
>
>Same with below I think as well.

Right.

>
>It's fine, these look great, I'll queue them up this afternoon...
>
>Thanks for persisting with these, and sorry it took so long to convince
>me I was wrong :)

:)

Any chance you could create an unmutable branch off 4.12 so I can start using it in input drivers?


Thanks.

--
Dmitry