New bind/unbingd uevents

From 6d10e621578dffcca0ad785e4a73196aa25350f6 Mon Sep 17 00:00:00 2001

There are certain touch controllers that may come up in either normal (application) or boot mode, depending on whether firmware/configuration is corrupted when they are powered on. In boot mode the kernel does not create input device instance (because it does not necessarily know the characteristics of the input device in question).

Another number of controllers does not store firmware in a non-volatile memory, and they similarly need to have firmware loaded before input device instance is created. There are also other types of devices with similar behavior.

There is a desire to be able to trigger firmware loading via udev, but it has to happen only when driver is bound to a physical device (i2c or spi). These udev actions can not use ADD events, as those happen too early, so we are introducing BIND and UNBIND events that are emitted at the right moment.

Also, many drivers create additional driver-specific device attributes when binding to the device, to provide userspace with additional controls. The new events allow userspace to adjust these driver-specific attributes without worrying that they are not there yet.

Signed-off-by Dmitry Torokhov <[email protected]>

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Dan Williams
Sent: Friday, September 29, 2017 3:37 PM
To: Dmitry Torokhov <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>; Tejun Heo <[email protected]>; Linux Kernel Mailing List <linux- [email protected]>; Guenter Roeck <[email protected]>; Ruhl, Michael J <[email protected]>
Subject: Re: [PATCH v2 1/7] driver core: emit uevents when device is bound to a driver

Quoted 2 lines · Dmitry Torokhov

There are certain touch controllers that may come up in either normal (application) or boot mode, depending on whether firmware/configuration

is

Quoted 1 line

corrupted when they are powered on. In boot mode the kernel does not

create

Quoted 5 lines

input device instance (because it does not necessarily know the characteristics of the input device in question).

Another number of controllers does not store firmware in a non-volatile memory, and they similarly need to have firmware loaded before input

device

Quoted 6 lines

instance is created. There are also other types of devices with similar behavior.

There is a desire to be able to trigger firmware loading via udev, but it has to happen only when driver is bound to a physical device (i2c or spi). These udev actions can not use ADD events, as those happen too early, so

we

Quoted 9 lines

are introducing BIND and UNBIND events that are emitted at the right moment.

Also, many drivers create additional driver-specific device attributes when binding to the device, to provide userspace with additional controls. The new events allow userspace to adjust these driver-specific attributes without worrying that they are not there yet.

Signed-off-by Dmitry Torokhov <[email protected]>

Hi Dmitry,

Mike (cc'd) reports a regression with this change:

---

Previously, if I did:

# rmmod hfi1

The driver would be removed.

With 4.14.0-rc2+, when I remove the driver, the PCI bus is automatically re-probed and the driver re-loaded.

---

A bisect points to commit 1455cf8dbfd0 "driver core: emit uevents when device is bound to a driver". I'm sending this because I have this mail in my archive, but I'll let Mike follow up with any other details.

Quoted 57 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Dan Williams
Sent: Friday, September 29, 2017 3:37 PM
To: Dmitry Torokhov <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>; Tejun Heo <[email protected]>; Linux Kernel Mailing List <linux- [email protected]>; Guenter Roeck <[email protected]>; Ruhl, Michael J <[email protected]>
Subject: Re: [PATCH v2 1/7] driver core: emit uevents when device is bound to a driver

Quoted 2 lines · Dmitry Torokhov

There are certain touch controllers that may come up in either normal (application) or boot mode, depending on whether firmware/configuration

is

Quoted 1 line

corrupted when they are powered on. In boot mode the kernel does not

create

Quoted 5 lines

input device instance (because it does not necessarily know the characteristics of the input device in question).

Another number of controllers does not store firmware in a non-volatile memory, and they similarly need to have firmware loaded before input

device

Quoted 6 lines

instance is created. There are also other types of devices with similar behavior.

There is a desire to be able to trigger firmware loading via udev, but it has to happen only when driver is bound to a physical device (i2c or spi). These udev actions can not use ADD events, as those happen too early, so

we

Quoted 9 lines

are introducing BIND and UNBIND events that are emitted at the right moment.

Also, many drivers create additional driver-specific device attributes when binding to the device, to provide userspace with additional controls. The new events allow userspace to adjust these driver-specific attributes without worrying that they are not there yet.

Signed-off-by Dmitry Torokhov <[email protected]>

Hi Dmitry,

Mike (cc'd) reports a regression with this change:

---

Previously, if I did:

# rmmod hfi1

The driver would be removed.

With 4.14.0-rc2+, when I remove the driver, the PCI bus is automatically re-probed and the driver re-loaded.

Quoted 6 lines

---

A bisect points to commit 1455cf8dbfd0 "driver core: emit uevents when device is bound to a driver". I'm sending this because I have this mail in my archive, but I'll let Mike follow up with any other details.

My test environment is RedHat 7.3 GA + 4.14.0-rc2 kernel.

Blacklisting the driver keeps it from being autoloaded, but this didn't seem correct.

With the 4.13.x branch this did not occur

Quoted 66 lines · Michael J

Quoted 57 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Dan Williams
Sent: Friday, September 29, 2017 3:37 PM
To: Dmitry Torokhov <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>; Tejun Heo <[email protected]>; Linux Kernel Mailing List <linux- [email protected]>; Guenter Roeck <[email protected]>; Ruhl, Michael J <[email protected]>
Subject: Re: [PATCH v2 1/7] driver core: emit uevents when device is bound to a driver

Quoted 2 lines · Dmitry Torokhov

There are certain touch controllers that may come up in either normal (application) or boot mode, depending on whether firmware/configuration

is

Quoted 1 line

corrupted when they are powered on. In boot mode the kernel does not

create

Quoted 5 lines

input device instance (because it does not necessarily know the characteristics of the input device in question).

Another number of controllers does not store firmware in a non-volatile memory, and they similarly need to have firmware loaded before input

device

Quoted 6 lines

instance is created. There are also other types of devices with similar behavior.

There is a desire to be able to trigger firmware loading via udev, but it has to happen only when driver is bound to a physical device (i2c or spi). These udev actions can not use ADD events, as those happen too early, so

we

Quoted 9 lines

are introducing BIND and UNBIND events that are emitted at the right moment.

Also, many drivers create additional driver-specific device attributes when binding to the device, to provide userspace with additional controls. The new events allow userspace to adjust these driver-specific attributes without worrying that they are not there yet.

Signed-off-by Dmitry Torokhov <[email protected]>

Hi Dmitry,

Mike (cc'd) reports a regression with this change:

---

Previously, if I did:

# rmmod hfi1

The driver would be removed.

With 4.14.0-rc2+, when I remove the driver, the PCI bus is automatically re-probed and the driver re-loaded.

Quoted 6 lines

---

A bisect points to commit 1455cf8dbfd0 "driver core: emit uevents when device is bound to a driver". I'm sending this because I have this mail in my archive, but I'll let Mike follow up with any other details.

My test environment is RedHat 7.3 GA + 4.14.0-rc2 kernel.

Quoted 3 lines

Blacklisting the driver keeps it from being autoloaded, but this didn't seem correct.

With the 4.13.x branch this did not occur

Yeah, udev is being stupid. Either change ACTION=="remove" to ACTION!="add" in /lib/udev/rules.d/80-drivers.rules or pick this patch:

https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=driver-core-linus&id=6878e7de6af726de47f9f3bec649c3f49e786586

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 <[email protected]>

---
 drivers/base/core.c    | 130 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h |   9 ++++
 2 files changed, 139 insertions(+)

drivers/base/core.c +39 -0

index 14f8cf5c8b05..09723532725d 100644
@@ -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)

Quoted 3 lines · Dmitry Torokhov

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

Quoted 4 lines

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

Quoted 1 line

providing a wrapper to device_add_group() and use it with

devm_add_action()

Quoted 5 lines

or devm_add_action_or_reset().

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/base/core.c    | 130

+++++++++++++++++++++++++++++++++++++++++++++++++

Quoted 8 lines

 include/linux/device.h |   9 ++++
 2 files changed, 139 insertions(+)

drivers/base/core.c +0 -0

index 14f8cf5c8b05..09723532725d 100644

drivers/base/core.c void device_remove_groups(struct device *dev, · line 1043

Quoted 9 lines

drivers/base/core.c +6 -0

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

Quoted 14 lines

drivers/base/core.c +14 -0

+{
+	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)

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 10 lines

drivers/base/core.c +10 -0

+{
+	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

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 4 lines

drivers/base/core.c +4 -0

+ * @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

drivers/base/core.c void device_remove_groups(struct device *dev, · line 1072

Quoted 1 line

drivers/base/core.c +1 -0

+ * warn and error if any of the attribute files being created

already exist.

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 4 lines

drivers/base/core.c +4 -0

+ *
+ * Returns 0 on success or error code on failure.
+ */
+int devm_device_add_group(struct device *dev, const struct

attribute_group *grp)

drivers/base/core.c void device_remove_groups(struct device *dev, · line 1086

Quoted 10 lines

drivers/base/core.c +10 -0

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

Quoted 86 lines · Greg Kroah-Hartman

Quoted 3 lines · Dmitry Torokhov

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

Quoted 4 lines

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

Quoted 1 line

providing a wrapper to device_add_group() and use it with

devm_add_action()

Quoted 5 lines

or devm_add_action_or_reset().

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/base/core.c    | 130

+++++++++++++++++++++++++++++++++++++++++++++++++

Quoted 8 lines

 include/linux/device.h |   9 ++++
 2 files changed, 139 insertions(+)

drivers/base/core.c +0 -0

index 14f8cf5c8b05..09723532725d 100644

drivers/base/core.c void device_remove_groups(struct device *dev, · line 1043

Quoted 9 lines

drivers/base/core.c +6 -0

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

Quoted 14 lines

drivers/base/core.c +14 -0

+{
+	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)

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 10 lines

drivers/base/core.c +10 -0

+{
+	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

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 4 lines

drivers/base/core.c +4 -0

+ * @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

drivers/base/core.c void device_remove_groups(struct device *dev, · line 1072

Quoted 1 line

drivers/base/core.c +1 -0

+ * warn and error if any of the attribute files being created

already exist.

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 4 lines

drivers/base/core.c +4 -0

+ *
+ * Returns 0 on success or error code on failure.
+ */
+int devm_device_add_group(struct device *dev, const struct

attribute_group *grp)

drivers/base/core.c void device_remove_groups(struct device *dev, · line 1086

Quoted 10 lines

drivers/base/core.c +10 -0

+{
+	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?

Quoted 1 line

Same with below I think as well.

Right.

Quoted 1 line

Quoted 4 lines

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?

Quoted 1 line · Dmitry Torokhov

On July 19, 2017 10:10:18 PM PDT, Greg Kroah-Hartman

Quoted 2 lines · <[email protected]> wrote:

Quoted 2 lines · Dmitry Torokhov

Quoted 1 line

Many drivers create additional driver-specific device attributes

when

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

binding to the device, and providing managed version of device_create_group() will simplify unbinding and error handling

in

Quoted 4 lines

Quoted 4 lines

probe

Quoted 3 lines

path for such drivers.

Without managed version driver writers either have to mix manual

and

Quoted 10 lines

Quoted 10 lines

Quoted 1 line

managed resources, which is prone to errors, or open-code this

function by

Quoted 1 line

providing a wrapper to device_add_group() and use it with

devm_add_action()

Quoted 5 lines

or devm_add_action_or_reset().

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/base/core.c    | 130

+++++++++++++++++++++++++++++++++++++++++++++++++

Quoted 8 lines

Quoted 8 lines

drivers/base/core.c void device_remove_groups(struct device

Quoted 8 lines

 include/linux/device.h |   9 ++++
 2 files changed, 139 insertions(+)

drivers/base/core.c +0 -0

index 14f8cf5c8b05..09723532725d 100644
@@ -1035,6 +1035,136 @@ void device_remove_groups(struct device

*dev,

Quoted 9 lines

Quoted 9 lines

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 9 lines

drivers/base/core.c +6 -0

 }
 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

Quoted 15 lines

Quoted 15 lines

*data)

Quoted 4 lines

drivers/base/core.c +4 -0

+{
+	return ((union device_attr_group_devres *)res)->group == data;
+}
+

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 10 lines

drivers/base/core.c +10 -0

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

Quoted 36 lines

Quoted 36 lines

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 10 lines

drivers/base/core.c +10 -0

+{
+	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

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 4 lines

drivers/base/core.c +4 -0

+ * @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

drivers/base/core.c void device_remove_groups(struct device · line 1072

Quoted 1 line

drivers/base/core.c +1 -0

+ * warn and error if any of the attribute files being created

already exist.

drivers/base/core.c void device_remove_groups(struct device *dev,

Quoted 4 lines

drivers/base/core.c +4 -0

+ *
+ * Returns 0 on success or error code on failure.
+ */
+int devm_device_add_group(struct device *dev, const struct

attribute_group *grp)

drivers/base/core.c void device_remove_groups(struct device *dev, · line 1086

Quoted 10 lines

drivers/base/core.c +10 -0

+{
+	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?

Quoted 8 lines

Quoted 1 line

Same with below I think as well.

Right.

Quoted 4 lines

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

Quoted 2 lines

Quoted 1 line

me I was wrong :)

Quoted 3 lines

:)

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?

Quoted 6 lines · Greg Kroah-Hartman

Quoted 1 line · Dmitry Torokhov

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.

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 <[email protected]>

---
  drivers/input/keyboard/gpio_keys.c | 16 ++--------------
  1 file changed, 2 insertions(+), 14 deletions(-)

drivers/input/keyboard/gpio_keys.c +2 -14

index f52812db91bc..e9f0ebf3267a 100644
@@ -827,7 +827,7 @@ static int gpio_keys_probe(struct platform_device *pdev)
 
  	fwnode_handle_put(child);
 
-	error = sysfs_create_group(&dev->kobj, &gpio_keys_attr_group);
+	error = devm_device_add_group(dev, &gpio_keys_attr_group);
  	if (error) {
  		dev_err(dev, "Unable to export keys/switches, error: %d\n",
  			error);

@@ -838,22 +838,11 @@ static int gpio_keys_probe(struct platform_device *pdev)
  	if (error) {
  		dev_err(dev, "Unable to register input device, error: %d\n",
  			error);
-		goto err_remove_group;
+		return error;
  	}
 
  	device_init_wakeup(dev, wakeup);
 
-	return 0;
-
-err_remove_group:
-	sysfs_remove_group(&dev->kobj, &gpio_keys_attr_group);
-	return error;
-}
-
-static int gpio_keys_remove(struct platform_device *pdev)
-{
-	sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group);
-
  	return 0;
  }
 

@@ -912,7 +901,6 @@ static SIMPLE_DEV_PM_OPS(gpio_keys_pm_ops, gpio_keys_suspend, gpio_keys_resume);
 
  static struct platform_driver gpio_keys_device_driver = {
  	.probe		= gpio_keys_probe,
-	.remove		= gpio_keys_remove,
  	.driver		= {
  		.name	= "gpio-keys",
  		.pm	= &gpio_keys_pm_ops,

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 <[email protected]>

---
  drivers/input/rmi4/rmi_f01.c | 11 +++--------
  1 file changed, 3 insertions(+), 8 deletions(-)

drivers/input/rmi4/rmi_f01.c +3 -8

index aa1aabfdbe7c..ae966e333a2f 100644
@@ -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,

Now that we have proper managed API to create device attributes, let's use it instead of installing a custom devm action.

Signed-off-by Dmitry Torokhov <[email protected]>

---
  drivers/input/misc/axp20x-pek.c | 18 +-----------------
  1 file changed, 1 insertion(+), 17 deletions(-)

drivers/input/misc/axp20x-pek.c +1 -17

index 38c79ebff033..cfeb0e943de6 100644
@@ -182,13 +182,6 @@ static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
  	return IRQ_HANDLED;
  }
 
-static void axp20x_remove_sysfs_group(void *_data)
-{
-	struct device *dev = _data;
-
-	sysfs_remove_group(&dev->kobj, &axp20x_attribute_group);
-}
-
  static int axp20x_pek_probe_input_device(struct axp20x_pek *axp20x_pek,
  					 struct platform_device *pdev)
  {

@@ -313,22 +306,13 @@ static int axp20x_pek_probe(struct platform_device *pdev)
  			return error;
  	}
 
-	error = sysfs_create_group(&pdev->dev.kobj, &axp20x_attribute_group);
+	error = devm_device_add_group(&pdev->dev, &axp20x_attribute_group);
  	if (error) {
  		dev_err(&pdev->dev, "Failed to create sysfs attributes: %d\n",
  			error);
  		return error;
  	}
 
-	error = devm_add_action(&pdev->dev,
-				axp20x_remove_sysfs_group, &pdev->dev);
-	if (error) {
-		axp20x_remove_sysfs_group(&pdev->dev);
-		dev_err(&pdev->dev, "Failed to add sysfs cleanup action: %d\n",
-			error);
-		return error;
-	}
-
  	platform_set_drvdata(pdev, axp20x_pek);
 
  	return 0;