Re: [PATCH v2 1/1] power: Emit changed uevent on wakeup_sysfs_add/remove
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: 2020-07-07 14:29:03
Also in:
linux-pm, lkml
On Mon, Jul 06, 2020 at 02:07:17PM -0700, Abhishek Pandit-Subedi wrote:
quoted hunk ↗ jump to hunk
Udev rules that depend on the power/wakeup attribute don't get triggered correctly if device_set_wakeup_capable is called after the device is created. This can happen for several reasons (driver sets wakeup after device is created, wakeup is changed on parent device, etc) and it seems reasonable to emit a changed event when adding or removing attributes on the device. Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> --- Changes in v2: - Add newline at end of bt_dev_err drivers/base/power/sysfs.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c index 24d25cf8ab1487..d57e8e7f175ebf 100644 --- a/drivers/base/power/sysfs.c +++ b/drivers/base/power/sysfs.c@@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* sysfs entries for device PM */ #include <linux/device.h> +#include <linux/kobject.h> #include <linux/string.h> #include <linux/export.h> #include <linux/pm_qos.h>@@ -739,12 +740,30 @@ int dpm_sysfs_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid) int wakeup_sysfs_add(struct device *dev) { - return sysfs_merge_group(&dev->kobj, &pm_wakeup_attr_group); + int ret = sysfs_merge_group(&dev->kobj, &pm_wakeup_attr_group); + + if (!ret) { + int tmp = kobject_uevent(&dev->kobj, KOBJ_CHANGE); + + if (tmp) + dev_err(dev, + "Error in uevent for wakeup_sysfs_add: %d\n", + tmp); + } + + return ret; }
Shouldn't the above function look like this instead to be simpler:
int wakeup_sysfs_add(struct device *dev)
{
int ret = sysfs_merge_group(&dev->kobj, &pm_wakeup_attr_group);
if (ret)
return ret;
return kobject_uevent(&dev->kobj, KOBJ_CHANGE);
}
void wakeup_sysfs_remove(struct device *dev)
{
+ int tmp;Use 'ret' like the above function had, to be consistent.
+ sysfs_unmerge_group(&dev->kobj, &pm_wakeup_attr_group); + + tmp = kobject_uevent(&dev->kobj, KOBJ_CHANGE); + if (tmp) + dev_err(dev, "Error in uevent for wakeup_sysfs_remove: %d\n",
nit, use __func__ to describe a function name, if you really want it. Why do you need to send a message for this error, will that really ever happen? thanks, greg k-h