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 <[email protected]>
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);
Many drivers create additional driver-specific device attributes when
binding to the device. To avoid them calling SYSFS API directly, let's
export these helpers.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/base/base.h | 5 -----
drivers/base/core.c | 2 ++
include/linux/device.h | 5 +++++
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index e19b1008e5fb..539432a14b5c 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -126,11 +126,6 @@ extern int driver_add_groups(struct device_driver *drv,
extern void driver_remove_groups(struct device_driver *drv,
const struct attribute_group **groups);
-extern int device_add_groups(struct device *dev,
- const struct attribute_group **groups);
-extern void device_remove_groups(struct device *dev,
- const struct attribute_group **groups);
-
extern char *make_class_name(const char *name, struct kobject *kobj);
extern int devres_release_all(struct device *dev);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index bbecaf9293be..14f8cf5c8b05 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1026,12 +1026,14 @@ int device_add_groups(struct device *dev, const struct attribute_group **groups)
{
return sysfs_create_groups(&dev->kobj, groups);
}
+EXPORT_SYMBOL_GPL(device_add_groups);
void device_remove_groups(struct device *dev,
const struct attribute_group **groups)
{
sysfs_remove_groups(&dev->kobj, groups);
}
+EXPORT_SYMBOL_GPL(device_remove_groups);
static int device_add_attrs(struct device *dev)
{
diff --git a/include/linux/device.h b/include/linux/device.h
index 9ef518af5515..10cf209a4e82 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1200,6 +1200,11 @@ struct device *device_create_with_groups(struct class *cls,
const char *fmt, ...);
extern void device_destroy(struct class *cls, dev_t devt);
+extern int __must_check device_add_groups(struct device *dev,
+ const struct attribute_group **groups);
+extern void device_remove_groups(struct device *dev,
+ const struct attribute_group **groups);
+
/*
* 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
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(+)
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
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(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index f52812db91bc..e9f0ebf3267a 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -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,
--
2.14.0.rc0.284.gd933b75aa4-goog
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(-)
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
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(-)
diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index 38c79ebff033..cfeb0e943de6 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -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;
--
2.14.0.rc0.284.gd933b75aa4-goog
We have helpers that work with NULL terminated array of groups, but many
drivers only create a single supplemental group, and do not want to declare
a group array. Let's provide them with helpers working with a single group.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
include/linux/device.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h
index 10cf209a4e82..7698a513b35e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1205,6 +1205,22 @@ extern int __must_check device_add_groups(struct device *dev,
extern void device_remove_groups(struct device *dev,
const struct attribute_group **groups);
+static inline int __must_check device_add_group(struct device *dev,
+ const struct attribute_group *grp)
+{
+ const struct attribute_group *groups[] = { grp, NULL };
+
+ return device_add_groups(dev, groups);
+}
+
+static inline void device_remove_group(struct device *dev,
+ const struct attribute_group *grp)
+{
+ const struct attribute_group *groups[] = { grp, NULL };
+
+ return device_remove_groups(dev, groups);
+}
+
/*
* 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
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]>
---
drivers/base/dd.c | 4 ++++
include/linux/kobject.h | 2 ++
lib/kobject_uevent.c | 2 ++
3 files changed, 8 insertions(+)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 4882f06d12df..c17fefc77345 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -259,6 +259,8 @@ static void driver_bound(struct device *dev)
if (dev->bus)
blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
BUS_NOTIFY_BOUND_DRIVER, dev);
+
+ kobject_uevent(&dev->kobj, KOBJ_BIND);
}
static int driver_sysfs_add(struct device *dev)
@@ -848,6 +850,8 @@ static void __device_release_driver(struct device *dev, struct device *parent)
blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
BUS_NOTIFY_UNBOUND_DRIVER,
dev);
+
+ kobject_uevent(&dev->kobj, KOBJ_UNBIND);
}
}
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index ca85cb80e99a..12f5ddccb97c 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -57,6 +57,8 @@ enum kobject_action {
KOBJ_MOVE,
KOBJ_ONLINE,
KOBJ_OFFLINE,
+ KOBJ_BIND,
+ KOBJ_UNBIND,
KOBJ_MAX
};
diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index 9a2b811966eb..4682e8545b5c 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -50,6 +50,8 @@ static const char *kobject_actions[] = {
[KOBJ_MOVE] = "move",
[KOBJ_ONLINE] = "online",
[KOBJ_OFFLINE] = "offline",
+ [KOBJ_BIND] = "bind",
+ [KOBJ_UNBIND] = "unbind",
};
/**
--
2.14.0.rc0.284.gd933b75aa4-goog
On 07/19/2017 05:24 PM, Dmitry Torokhov wrote:
> 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]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/input/misc/axp20x-pek.c | 18 +-----------------
> 1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index 38c79ebff033..cfeb0e943de6 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c
> @@ -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;
>
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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> 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,
>
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 <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/input/keyboard/gpio_keys.c | 16 ++--------------
> 1 file changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index f52812db91bc..e9f0ebf3267a 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -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,
>
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 <[email protected]>
> ---
> 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
On July 19, 2017 10:10:18 PM PDT, Greg Kroah-Hartman <[email protected]> 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 <[email protected]>
>> ---
>> 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
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 <[email protected]> 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 <[email protected]>
> >> ---
> >> 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
On July 20, 2017 1:20:09 AM PDT, Greg Kroah-Hartman <[email protected]> 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
><[email protected]> 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 <[email protected]>
>> >> ---
>> >> 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
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 <[email protected]> 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
On Wed, Jul 19, 2017 at 5:24 PM, Dmitry Torokhov
<[email protected]> wrote:
> 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]>
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.
> -----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
>
> On Wed, Jul 19, 2017 at 5:24 PM, Dmitry Torokhov
> <[email protected]> wrote:
> > 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]>
>
> 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.
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
Thanks,
Mike
On Fri, Sep 29, 2017 at 07:40:15PM +0000, Ruhl, Michael J wrote:
> > -----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
> >
> > On Wed, Jul 19, 2017 at 5:24 PM, Dmitry Torokhov
> > <[email protected]> wrote:
> > > 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]>
> >
> > 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.
>
> 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
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
Thanks.
--
Dmitry
On Fri, Sep 29, 2017 at 04:23:20PM -0700, Dmitry Torokhov wrote:
> On Fri, Sep 29, 2017 at 07:40:15PM +0000, Ruhl, Michael J wrote:
> > > -----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
> > >
> > > On Wed, Jul 19, 2017 at 5:24 PM, Dmitry Torokhov
> > > <[email protected]> wrote:
> > > > 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]>
> > >
> > > 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.
> >
> > 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
>
> 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
I have this fix queued up for Linus, sorry for the delay in getting it
to him, hope to do that today...
thanks,
greg k-h