Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1692312 > unrolled thread

[PATCH v2 0/7] New bind/unbingd uevents

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/7] New bind/unbingd uevents Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-07-20 02:30 +0200
    [PATCH v2 6/7] Input: synaptics_rmi4 - use devm_device_add_group() for attributes in F01 Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-07-20 02:30 +0200
      Re: [PATCH v2 6/7] Input: synaptics_rmi4 - use  devm_device_add_group() for attributes in F01 Guenter Roeck <linux@roeck-us.net> - 2017-07-20 05:30 +0200
    [PATCH v2 4/7] driver core: add devm_device_add_group() and friends Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-07-20 02:30 +0200
      Re: [PATCH v2 4/7] driver core: add devm_device_add_group() and  friends Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-20 07:20 +0200
        Re: [PATCH v2 4/7] driver core: add devm_device_add_group() and friends Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-07-20 10:20 +0200
          Re: [PATCH v2 4/7] driver core: add devm_device_add_group() and  friends Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-20 10:30 +0200
            Re: [PATCH v2 4/7] driver core: add devm_device_add_group() and friends Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-07-20 18:00 +0200
              Re: [PATCH v2 4/7] driver core: add devm_device_add_group() and  friends Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-22 12:10 +0200

#1692312 — [PATCH v2 0/7] New bind/unbingd uevents

Hi Greg,

Here is the v2 of bind/unbind uevent patch series.  The new bind/unbind
will allow triggering firmware update through udev, and the new device
sysfs API will cut down on some boilerplate code in drivers.

As you requested, I moved the new functions to device.h, in the process I
exported existing device_add_groups() and device_remove_groups() and called
the new functions devm_device_{add|remove}_group[s]() to get away from the
sysfs "rawness".

Below is also a patch to systemd to stop dropping the new attributes
(why they think they need to inspect and discard the data they do not
understand is beyond me).

Thanks,
Dmitry

V2:
- made device_{add|remove}_groups() public
- added device_{add|remove}_group() helpers
- the new devm APIs are moved into device.h and "sysfs" suffix dropped
- added 3 patches showing use in the drivers

V1: initial [re]post

Dmitry Torokhov (7):
  driver core: emit uevents when device is bound to a driver
  driver core: make device_{add|remove}_groups() public
  driver core: add device_{add|remove}_group() helpers
  driver core: add devm_device_add_group() and friends
  Input: gpio_keys - use devm_device_add_group() for attributes
  Input: synaptics_rmi4 - use devm_device_add_group() for attributes in F01
  Input: axp20x-pek - switch to using devm_device_add_group()

 drivers/base/base.h                |   5 --
 drivers/base/core.c                | 132 +++++++++++++++++++++++++++++++++++++
 drivers/base/dd.c                  |   4 ++
 drivers/input/keyboard/gpio_keys.c |  16 +----
 drivers/input/misc/axp20x-pek.c    |  18 +----
 drivers/input/rmi4/rmi_f01.c       |  11 +---
 include/linux/device.h             |  30 +++++++++
 include/linux/kobject.h            |   2 +
 lib/kobject_uevent.c               |   2 +
 9 files changed, 176 insertions(+), 44 deletions(-)

-- >8 --

From 6d10e621578dffcca0ad785e4a73196aa25350f6 Mon Sep 17 00:00:00 2001
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: Mon, 17 Jul 2017 20:10:17 -0700
Subject: [PATCH] Add handling for bind/unbind actions

Newer kernels will emit uevents with "bind" and "unbind" actions. These
uevents will be issued when driver is bound to or unbound from a device.
"Bind" events are helpful when device requires a firmware to operate
properly, and driver is unable to create a child device before firmware
is properly loaded.

For some reason systemd validates actions and drops the ones it does not
know, instead of passing them on through as old udev did, so we need to
explicitly teach it about them.
---
 src/libsystemd/sd-device/device-internal.h | 2 ++
 src/libsystemd/sd-device/device-private.c  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/src/libsystemd/sd-device/device-internal.h b/src/libsystemd/sd-device/device-internal.h
index f4783deef..0505a2730 100644
--- a/src/libsystemd/sd-device/device-internal.h
+++ b/src/libsystemd/sd-device/device-internal.h
@@ -104,6 +104,8 @@ typedef enum DeviceAction {
         DEVICE_ACTION_MOVE,
         DEVICE_ACTION_ONLINE,
         DEVICE_ACTION_OFFLINE,
+        DEVICE_ACTION_BIND,
+        DEVICE_ACTION_UNBIND,
         _DEVICE_ACTION_MAX,
         _DEVICE_ACTION_INVALID = -1,
 } DeviceAction;
diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c
index b4cd676c1..8839c3266 100644
--- a/src/libsystemd/sd-device/device-private.c
+++ b/src/libsystemd/sd-device/device-private.c
@@ -466,6 +466,8 @@ static const char* const device_action_table[_DEVICE_ACTION_MAX] = {
         [DEVICE_ACTION_MOVE] = "move",
         [DEVICE_ACTION_ONLINE] = "online",
         [DEVICE_ACTION_OFFLINE] = "offline",
+        [DEVICE_ACTION_BIND] = "bind",
+        [DEVICE_ACTION_UNBIND] = "unbind",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(device_action, DeviceAction);

[toc] | [next] | [standalone]

#1692313 — [PATCH v2 6/7] Input: synaptics_rmi4 - use devm_device_add_group() for attributes in F01

Now that we have proper managed API to create device attributes, let's
start using it instead of the manual unwinding.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/rmi4/rmi_f01.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index aa1aabfdbe7c..ae966e333a2f 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -570,18 +570,14 @@ static int rmi_f01_probe(struct rmi_function *fn)
 
 	dev_set_drvdata(&fn->dev, f01);
 
-	error = sysfs_create_group(&fn->rmi_dev->dev.kobj, &rmi_f01_attr_group);
+	error = devm_device_add_group(&fn->rmi_dev->dev, &rmi_f01_attr_group);
 	if (error)
-		dev_warn(&fn->dev, "Failed to create sysfs group: %d\n", error);
+		dev_warn(&fn->dev,
+			 "Failed to create attribute group: %d\n", error);
 
 	return 0;
 }
 
-static void rmi_f01_remove(struct rmi_function *fn)
-{
-	sysfs_remove_group(&fn->rmi_dev->dev.kobj, &rmi_f01_attr_group);
-}
-
 static int rmi_f01_config(struct rmi_function *fn)
 {
 	struct f01_data *f01 = dev_get_drvdata(&fn->dev);
@@ -721,7 +717,6 @@ struct rmi_function_handler rmi_f01_handler = {
 	},
 	.func		= 0x01,
 	.probe		= rmi_f01_probe,
-	.remove		= rmi_f01_remove,
 	.config		= rmi_f01_config,
 	.attention	= rmi_f01_attention,
 	.suspend	= rmi_f01_suspend,
-- 
2.14.0.rc0.284.gd933b75aa4-goog

[toc] | [prev] | [next] | [standalone]

#1692370 — Re: [PATCH v2 6/7] Input: synaptics_rmi4 - use devm_device_add_group() for attributes in F01

On 07/19/2017 05:24 PM, Dmitry Torokhov wrote:
> Now that we have proper managed API to create device attributes, let's
> start using it instead of the manual unwinding.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/input/rmi4/rmi_f01.c | 11 +++--------
>   1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index aa1aabfdbe7c..ae966e333a2f 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -570,18 +570,14 @@ static int rmi_f01_probe(struct rmi_function *fn)
>   
>   	dev_set_drvdata(&fn->dev, f01);
>   
> -	error = sysfs_create_group(&fn->rmi_dev->dev.kobj, &rmi_f01_attr_group);
> +	error = devm_device_add_group(&fn->rmi_dev->dev, &rmi_f01_attr_group);
>   	if (error)
> -		dev_warn(&fn->dev, "Failed to create sysfs group: %d\n", error);
> +		dev_warn(&fn->dev,
> +			 "Failed to create attribute group: %d\n", error);
>   
>   	return 0;
>   }
>   
> -static void rmi_f01_remove(struct rmi_function *fn)
> -{
> -	sysfs_remove_group(&fn->rmi_dev->dev.kobj, &rmi_f01_attr_group);
> -}
> -
>   static int rmi_f01_config(struct rmi_function *fn)
>   {
>   	struct f01_data *f01 = dev_get_drvdata(&fn->dev);
> @@ -721,7 +717,6 @@ struct rmi_function_handler rmi_f01_handler = {
>   	},
>   	.func		= 0x01,
>   	.probe		= rmi_f01_probe,
> -	.remove		= rmi_f01_remove,
>   	.config		= rmi_f01_config,
>   	.attention	= rmi_f01_attention,
>   	.suspend	= rmi_f01_suspend,
> 

[toc] | [prev] | [next] | [standalone]

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

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);
+	if (error) {
+		devres_free(devres);
+		return error;
+	}
+
+	devres->group = grp;
+	devres_add(dev, devres);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_device_add_group);
+
+/**
+ * devm_device_remove_group: remove a managed group from a device
+ * @dev:	device to remove the group from
+ * @grp:	group to remove
+ *
+ * This function removes a group of attributes from a device. The attributes
+ * previously have to have been created for this group, otherwise it will fail.
+ */
+void devm_device_remove_group(struct device *dev,
+			      const struct attribute_group *grp)
+{
+	WARN_ON(devres_release(dev, devm_attr_group_remove,
+			       devm_attr_group_match,
+			       /* cast away const */ (void *)grp));
+}
+EXPORT_SYMBOL_GPL(devm_device_remove_group);
+
+/**
+ * devm_device_add_groups - create a bunch of managed attribute groups
+ * @dev:	The device to create the group for
+ * @groups:	The attribute groups to create, NULL terminated
+ *
+ * This function creates a bunch of managed attribute groups.  If an error
+ * occurs when creating a group, all previously created groups will be
+ * removed, unwinding everything back to the original state when this
+ * function was called.  It will explicitly warn and error if any of the
+ * attribute files being created already exist.
+ *
+ * Returns 0 on success or error code from sysfs_create_group on failure.
+ */
+int devm_device_add_groups(struct device *dev,
+			   const struct attribute_group **groups)
+{
+	union device_attr_group_devres *devres;
+	int error;
+
+	devres = devres_alloc(devm_attr_groups_remove,
+			      sizeof(*devres), GFP_KERNEL);
+	if (!devres)
+		return -ENOMEM;
+
+	error = sysfs_create_groups(&dev->kobj, groups);
+	if (error) {
+		devres_free(devres);
+		return error;
+	}
+
+	devres->groups = groups;
+	devres_add(dev, devres);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_device_add_groups);
+
+/**
+ * devm_device_remove_groups - remove a list of managed groups
+ *
+ * @dev:	The device for the groups to be removed from
+ * @groups:	NULL terminated list of groups to be removed
+ *
+ * If groups is not NULL, remove the specified groups from the device.
+ */
+void devm_device_remove_groups(struct device *dev,
+			       const struct attribute_group **groups)
+{
+	WARN_ON(devres_release(dev, devm_attr_groups_remove,
+			       devm_attr_group_match,
+			       /* cast away const */ (void *)groups));
+}
+EXPORT_SYMBOL_GPL(devm_device_remove_groups);
+
 static int device_add_attrs(struct device *dev)
 {
 	struct class *class = dev->class;
diff --git a/include/linux/device.h b/include/linux/device.h
index 7698a513b35e..f52288c24734 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1221,6 +1221,15 @@ static inline void device_remove_group(struct device *dev,
 	return device_remove_groups(dev, groups);
 }
 
+extern int __must_check devm_device_add_groups(struct device *dev,
+					const struct attribute_group **groups);
+extern void devm_device_remove_groups(struct device *dev,
+				      const struct attribute_group **groups);
+extern int __must_check devm_device_add_group(struct device *dev,
+					const struct attribute_group *grp);
+extern void devm_device_remove_group(struct device *dev,
+				     const struct attribute_group *grp);
+
 /*
  * Platform "fixup" functions - allow the platform to have their say
  * about devices and actions that the general device layer doesn't
-- 
2.14.0.rc0.284.gd933b75aa4-goog

[toc] | [prev] | [next] | [standalone]

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

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.

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 :)

greg k-h

[toc] | [prev] | [next] | [standalone]

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

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?


Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

On July 20, 2017 1:20:09 AM PDT, Greg Kroah-Hartman <gregkh@linuxfoundation.org> 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
><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?

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

[toc] | [prev] | [next] | [standalone]

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

On Thu, Jul 20, 2017 at 08:50:26AM -0700, Dmitry Torokhov wrote:
> On July 20, 2017 1:20:09 AM PDT, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> >On Thu, Jul 20, 2017 at 01:12:56AM -0700, Dmitry Torokhov wrote:
> >> 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.

I've now created this, it's based off of 4.12 and can be found at:
	 git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/ bind_unbind

If there are any problems with it, please let me know.

I've also pulled this branch into my driver_core-next branch so that I
can base stuff off of it as well.  Heck, I might go fix up some USB
drivers now too, so it might end up there...

thanks,

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web