Date: Thu, 20 Jul 2017 10:20:09 +0200
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: htejun@gmail.com, linux-kernel@vger.kernel.org,
        Guenter Roeck <linux@roeck-us.net>
Subject: Re: [PATCH v2 4/7] driver core: add devm_device_add_group() and
 friends
Message-ID: <20170720082009.GB12571@kroah.com>
References: <20170720002436.29309-1-dmitry.torokhov@gmail.com>
 <20170720002436.29309-5-dmitry.torokhov@gmail.com>
 <20170720051018.GE7323@kroah.com>
 <BE1DA622-8285-4A3F-8885-4D08F3E42646@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <BE1DA622-8285-4A3F-8885-4D08F3E42646@gmail.com>
User-Agent: Mutt/1.8.3 (2017-05-23)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3609
Lines: 108

On Thu, Jul 20, 2017 at 01:12:56AM -0700, Dmitry Torokhov wrote:
> On July 19, 2017 10:10:18 PM PDT, Greg Kroah-Hartman <gregkh@linuxfoundation.org> 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@gmail.com>
> >> ---
> >>  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?

I'll be glad to, can it be off of 4.13-rc1?  Or do you need it off of
4.12?

thanks,

greg k-h