Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965699AbdGTPue (ORCPT ); Thu, 20 Jul 2017 11:50:34 -0400 Received: from mail-pf0-f195.google.com ([209.85.192.195]:36433 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965659AbdGTPub (ORCPT ); Thu, 20 Jul 2017 11:50:31 -0400 Date: Thu, 20 Jul 2017 08:50:26 -0700 User-Agent: K-9 Mail for Android In-Reply-To: <20170720082009.GB12571@kroah.com> References: <20170720002436.29309-1-dmitry.torokhov@gmail.com> <20170720002436.29309-5-dmitry.torokhov@gmail.com> <20170720051018.GE7323@kroah.com> <20170720082009.GB12571@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Subject: Re: [PATCH v2 4/7] driver core: add devm_device_add_group() and friends To: Greg Kroah-Hartman CC: htejun@gmail.com, linux-kernel@vger.kernel.org, Guenter Roeck From: Dmitry Torokhov Message-ID: <15DD98EC-82FB-47F2-9A0E-899DB10F404C@gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v6KFoc7w001699 Content-Length: 3972 Lines: 122 On July 20, 2017 1:20:09 AM PDT, Greg Kroah-Hartman wrote: >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 > 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 >> >> --- >> >> 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? It's just my preference for topic branches to be off a stable[r] releases, unless patches do not apply cleanly or will lead to merge conflicts down the road. Thanks! -- Dmitry